Skip to content

Commit

Permalink
Make golangci-lint pass. (#99)
Browse files Browse the repository at this point in the history
This commit fixes various errors (mostly about ignoring returned
values) reported by golangci-lint.

Tested the changes with "go test ./..." and also:

    # Terminal A
    $ docker-compose up
    # Terminal B
    $ docker exec -it trcgfr_traceroute-caller_1 apt-get update
    $ tree local
  • Loading branch information
Saied Kazemi authored Jun 7, 2021
1 parent ef61483 commit 1074d92
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 22 deletions.
4 changes: 3 additions & 1 deletion connectionlistener/connectionlistener.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ func (cl *connectionListener) Close(ctx context.Context, timestamp time.Time, uu
cl.mutex.Unlock()

if ok {
go cl.cache.Trace(conn)
go func() {
_, _ = cl.cache.Trace(conn)
}()
}
}

Expand Down
4 changes: 3 additions & 1 deletion connectionlistener/connectionlistener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func TestListener(t *testing.T) {
rtx.Must(srv.Listen(), "Could not listen")
srvCtx, srvCancel := context.WithCancel(context.Background())
defer srvCancel()
go srv.Serve(srvCtx)
go func() {
_ = srv.Serve(srvCtx)
}()

// Create a new connectionlistener with a fake tracer.
ctx, cancel := context.WithCancel(context.Background())
Expand Down
4 changes: 3 additions & 1 deletion connectionpoller/connectionpoller.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ func (c *connectionPoller) TraceClosedConnections() {
c.connectionPool = c.GetConnections()
for conn := range oldConn {
if _, hasConn := c.connectionPool[conn]; !hasConn {
go c.recentIPCache.Trace(conn)
go func() {
_, _ = c.recentIPCache.Trace(conn)
}()
}
}
}
Expand Down
1 change: 0 additions & 1 deletion docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,5 @@ services:
- -prometheusx.listen-address=:9992
- -outputPath=/local/traceroute
- -poll=false
- -ipservice.sock=/local/uuid-annotator.sock
- -tcpinfo.eventsocket=/local/tcpevents.sock
- -tracetool=scamper-daemon
2 changes: 1 addition & 1 deletion ipcache/ipcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (rc *RecentIPCache) Trace(conn connection.Connection) (string, error) {
if cached {
<-c.dataReady
if c.err == nil {
t.TraceFromCachedTrace(conn, time.Now(), c.data)
_ = t.TraceFromCachedTrace(conn, time.Now(), c.data)
return c.data, nil
}
t.DontTrace(conn, c.err)
Expand Down
4 changes: 2 additions & 2 deletions ipcache/ipcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestTrace(t *testing.T) {
if tt.cctest != 0 {
t.Errorf("Should have had zero calls to CreateCachedTest, not %d", tt.cctest)
}
testCache.Trace(conn1) // This should be cached
_, _ = testCache.Trace(conn1) // This should be cached
if tt.cctest != 1 {
t.Errorf("Should have had one call to CreateCachedTest, not %d", tt.cctest)
}
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestRecentIPCache(t *testing.T) {
defer cancel()
var tt testTracer
tmp := ipcache.New(ctx, &tt, 10*time.Millisecond, 1*time.Millisecond)
tmp.Trace(connection.Connection{
_, _ = tmp.Trace(connection.Connection{
RemoteIP: "1.2.3.4",
RemotePort: 5,
LocalIP: "6.7.8.9",
Expand Down
2 changes: 1 addition & 1 deletion parser/json.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// parser package handles parsing of scamper JSONL.
// Package parser handles parsing of scamper JSONL.
// The format of JSON can be found at
// https://www.caida.org/tools/measurement/scamper/.
// NB: It is not clear where at that URL the format can be found.
Expand Down
4 changes: 2 additions & 2 deletions tracer/paris_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestParis(t *testing.T) {
Cookie: "CDEF",
}
afterNow := time.Date(2003, 11, 9, 15, 58, 1, 0, time.UTC)
p.TraceFromCachedTrace(conn2, afterNow, out)
_ = p.TraceFromCachedTrace(conn2, afterNow, out)

contents, err = ioutil.ReadFile(dir + "/2003/11/09/20031109T15:58:01Z-UUID-" + prefix.UnsafeString() + "_000000000000CDEF.cached.paris")
rtx.Must(err, "Could not read file")
Expand All @@ -88,6 +88,6 @@ func TestParis(t *testing.T) {
if err == nil {
t.Error("You can't save data to /dev/null")
}
p.TraceFromCachedTrace(conn2, afterNow, out) // no crash == success
_ = p.TraceFromCachedTrace(conn2, afterNow, out) // no crash == success

}
8 changes: 6 additions & 2 deletions tracer/scamper.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ func (d *ScamperDaemon) MustStart(ctx context.Context) {
// if the user started those processes. This is also why we don't use
// CommandContext in the exec package - if the process isn't successfully
// killed by SIGKILL, then the code in that package doesn't work correctly.
command.Process.Signal(syscall.SIGKILL)
if err := command.Process.Signal(syscall.SIGKILL); err != nil {
log.Printf("failed to send SIGKILL to scamper daemon, error: %v\n", err)
}
}

// Trace starts a sc_attach connecting to the scamper process for each
Expand All @@ -186,7 +188,9 @@ func (d *ScamperDaemon) Trace(conn connection.Connection, t time.Time) (out stri
func (d *ScamperDaemon) TraceAll(connections []connection.Connection) {
for _, c := range connections {
log.Printf("PT start: %s %d", c.RemoteIP, c.RemotePort)
go d.Trace(c, time.Now())
go func(c connection.Connection) {
_, _ = d.Trace(c, time.Now())
}(c)
}
}

Expand Down
13 changes: 3 additions & 10 deletions tracer/scamper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/m-lab/go/prometheusx"
"github.com/m-lab/go/rtx"
"github.com/m-lab/traceroute-caller/connection"
"github.com/m-lab/traceroute-caller/ipcache"
"github.com/m-lab/uuid/prefix"
)

Expand Down Expand Up @@ -281,13 +280,13 @@ func TestCreateCacheTest(t *testing.T) {
{"type":"tracelb", "version":"0.1", "userid":0, "method":"icmp-echo", "src":"::ffff:180.87.97.101", "dst":"::ffff:1.47.236.62", "start":{"sec":1566691298, "usec":476221, "ftime":"2019-08-25 00:01:38"}, "probe_size":60, "firsthop":1, "attempts":3, "confidence":95, "tos":0, "gaplimit":3, "wait_timeout":5, "wait_probe":250, "probec":0, "probec_max":3000, "nodec":0, "linkc":0}
{"type":"cycle-stop", "list_name":"/tmp/scamperctrl:51811", "id":1, "hostname":"ndt-plh7v", "stop_time":1566691298}`

d.TraceFromCachedTrace(c, faketime, "Broken cached test")
_ = d.TraceFromCachedTrace(c, faketime, "Broken cached test")
_, errInvalidTest := ioutil.ReadFile(tempdir + "/2019/04/01/20190401T034551Z_" + prefix.UnsafeString() + "_0000000000000001.jsonl")
if errInvalidTest == nil {
t.Error("should fail to generate cached test")
}

d.TraceFromCachedTrace(c, faketime, cachedTest)
_ = d.TraceFromCachedTrace(c, faketime, cachedTest)

// Unmarshal the first line of the output file.
b, err := ioutil.ReadFile(tempdir + "/2019/04/01/20190401T034551Z_" + prefix.UnsafeString() + "_0000000000000001.jsonl")
Expand Down Expand Up @@ -351,7 +350,7 @@ func TestRecovery(t *testing.T) {

// Run both trace methods.
d.TraceAll([]connection.Connection{c})
d.Trace(c, time.Now())
_, _ = d.Trace(c, time.Now())
// If this doesn't crash, then the recovery process works!
}

Expand Down Expand Up @@ -381,9 +380,3 @@ func TestGetMetaline(t *testing.T) {
t.Error("Fail to generate meta ", meta)
}
}

//lint:ignore U1000 This performs a compile time check.
// If this successfully compiles, then ScamperDaemon implements the Tracer interface.
func assertScamperDaemonIsTracer(d *ScamperDaemon) {
func(t ipcache.Tracer) {}(d)
}

0 comments on commit 1074d92

Please sign in to comment.