From 8685ac8ef521110b1777baaef566c5e2cd92cde2 Mon Sep 17 00:00:00 2001 From: Martin Hutchinson Date: Wed, 26 Apr 2023 12:20:47 +0100 Subject: [PATCH 1/2] Fixed all lint errcheck in number of TLDs This fixes all errors in client, cmd, experimental, integration, log, merkle, quota, and testonly. The storage top level directory still has a huge number, so this change does not modify the .golangci.yaml file to enable checks across the repo. --- client/rpcflags/rpcflags_test.go | 12 ++++++++-- cmd/createtree/main.go | 6 ++++- cmd/deletetree/main.go | 6 ++++- cmd/internal/serverutil/main.go | 26 +++++++++++++++++----- cmd/trillian_log_server/main.go | 20 +++++++++++++---- cmd/trillian_log_signer/main.go | 20 +++++++++++++---- cmd/updatetree/main.go | 6 ++++- experimental/batchmap/cmd/build/mapdemo.go | 7 +++++- integration/format/format.go | 6 ++++- integration/log_integration_test.go | 6 ++++- integration/quota/quota_test.go | 9 +++++--- integration/storagetest/logtests.go | 12 ++++++++-- log/operation_manager_test.go | 8 +++++-- log/sequencer_manager_test.go | 12 +++++++--- merkle/coniks/coniks.go | 16 +++++++++---- quota/crdbqm/common_test.go | 5 ++++- quota/crdbqm/crdb_quota.go | 7 +++++- quota/etcd/quotaapi/quota_server_test.go | 6 ++--- quota/mysqlqm/mysql_quota.go | 7 +++++- testonly/integration/etcd/etcd.go | 12 ++++++---- testonly/integration/logenv.go | 8 +++++-- testonly/mdm/mdm.go | 4 +++- testonly/mock_server.go | 8 +++---- 23 files changed, 176 insertions(+), 53 deletions(-) diff --git a/client/rpcflags/rpcflags_test.go b/client/rpcflags/rpcflags_test.go index 654bfb6ecd..675770bee5 100644 --- a/client/rpcflags/rpcflags_test.go +++ b/client/rpcflags/rpcflags_test.go @@ -52,7 +52,11 @@ func TestNewClientDialOptionsFromFlagsWithTLSCertFileNotSet(t *testing.T) { if err != nil { t.Errorf("failed to dial %v: %v", logEnv.Address, err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + t.Error(err) + } + }() adminClient := trillian.NewTrillianAdminClient(conn) if _, err = adminClient.ListTrees(context.Background(), &trillian.ListTreesRequest{}); err != nil { @@ -127,7 +131,11 @@ func TestNewClientDialOptionsFromFlagsWithTLSCertFileSet(t *testing.T) { if err != nil { t.Errorf("failed to dial %v: %v", logEnv.Address, err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + t.Error(err) + } + }() adminClient := trillian.NewTrillianAdminClient(conn) if _, err := adminClient.ListTrees(context.Background(), &trillian.ListTreesRequest{}); err != nil { diff --git a/cmd/createtree/main.go b/cmd/createtree/main.go index 4d1ba44257..1208341b74 100644 --- a/cmd/createtree/main.go +++ b/cmd/createtree/main.go @@ -77,7 +77,11 @@ func createTree(ctx context.Context) (*trillian.Tree, error) { if err != nil { return nil, fmt.Errorf("failed to dial %v: %v", *adminServerAddr, err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() adminClient := trillian.NewTrillianAdminClient(conn) logClient := trillian.NewTrillianLogClient(conn) diff --git a/cmd/deletetree/main.go b/cmd/deletetree/main.go index 9c59aed6b8..e6e8424649 100644 --- a/cmd/deletetree/main.go +++ b/cmd/deletetree/main.go @@ -49,7 +49,11 @@ func main() { if err != nil { klog.Exitf("Failed to dial %v: %v", *adminServerAddr, err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() a := trillian.NewTrillianAdminClient(conn) if !*undeleteTree { diff --git a/cmd/internal/serverutil/main.go b/cmd/internal/serverutil/main.go index 32c9b9eec4..a45b832d55 100644 --- a/cmd/internal/serverutil/main.go +++ b/cmd/internal/serverutil/main.go @@ -98,11 +98,15 @@ func (m *Main) healthz(rw http.ResponseWriter, req *http.Request) { defer cancel() if err := m.IsHealthy(ctx); err != nil { rw.WriteHeader(http.StatusServiceUnavailable) - rw.Write([]byte(err.Error())) + if _, err := rw.Write([]byte(err.Error())); err != nil { + klog.Errorf("Write(): %v", err) + } return } } - rw.Write([]byte("ok")) + if _, err := rw.Write([]byte("ok")); err != nil { + klog.Errorf("Write(): %v", err) + } } // Run starts the configured server. Blocks until the server exits. @@ -119,7 +123,11 @@ func (m *Main) Run(ctx context.Context) error { } defer srv.GracefulStop() - defer m.DBClose() + defer func() { + if err := m.DBClose(); err != nil { + klog.Errorf("DBClose(): %v", err) + } + }() if err := m.RegisterServerFn(srv, m.Registry); err != nil { return err @@ -278,15 +286,21 @@ func AnnounceSelf(ctx context.Context, client *clientv3.Client, etcdService, end klog.Exitf("Failed to create etcd manager: %v", err) } fullEndpoint := fmt.Sprintf("%s/%s", etcdService, endpoint) - em.AddEndpoint(ctx, fullEndpoint, endpoints.Endpoint{Addr: endpoint}) + if err := em.AddEndpoint(ctx, fullEndpoint, endpoints.Endpoint{Addr: endpoint}); err != nil { + klog.Exitf("Failed to add endpoint: %v", err) + } klog.Infof("Announcing our presence in %v", etcdService) return func() { // Use a background context because the original context may have been cancelled. klog.Infof("Removing our presence in %v", etcdService) ctx := context.Background() - em.DeleteEndpoint(ctx, fullEndpoint) - client.Revoke(ctx, leaseRsp.ID) + if err := em.DeleteEndpoint(ctx, fullEndpoint); err != nil { + klog.Exitf("Failed to delete endpoint: %v", err) + } + if _, err := client.Revoke(ctx, leaseRsp.ID); err != nil { + klog.Exitf("Failed to revoke lease: %v", err) + } } } diff --git a/cmd/trillian_log_server/main.go b/cmd/trillian_log_server/main.go index 745881c9c2..c1f2191455 100644 --- a/cmd/trillian_log_server/main.go +++ b/cmd/trillian_log_server/main.go @@ -117,7 +117,11 @@ func main() { if err != nil { klog.Exitf("Failed to get storage provider: %v", err) } - defer sp.Close() + defer func() { + if err := sp.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() var client *clientv3.Client if servers := *etcd.Servers; servers != "" { @@ -127,7 +131,11 @@ func main() { }); err != nil { klog.Exitf("Failed to connect to etcd at %v: %v", servers, err) } - defer client.Close() + defer func() { + if err := client.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() } // Announce our endpoints to etcd if so configured. @@ -154,7 +162,9 @@ func main() { // Enable CPU profile if requested. if *cpuProfile != "" { f := mustCreate(*cpuProfile) - pprof.StartCPUProfile(f) + if err := pprof.StartCPUProfile(f); err != nil { + klog.Exitf("StartCPUProfile(): %v", err) + } defer pprof.StopCPUProfile() } @@ -196,7 +206,9 @@ func main() { if *memProfile != "" { f := mustCreate(*memProfile) - pprof.WriteHeapProfile(f) + if err := pprof.WriteHeapProfile(f); err != nil { + klog.Exitf("WriteHeapProfile(): %v", err) + } } } diff --git a/cmd/trillian_log_signer/main.go b/cmd/trillian_log_signer/main.go index d9de2eb16e..273099bbd6 100644 --- a/cmd/trillian_log_signer/main.go +++ b/cmd/trillian_log_signer/main.go @@ -114,7 +114,11 @@ func main() { if err != nil { klog.Exitf("Failed to get storage provider: %v", err) } - defer sp.Close() + defer func() { + if err := sp.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() var client *clientv3.Client if servers := *etcd.Servers; servers != "" { @@ -124,7 +128,11 @@ func main() { }); err != nil { klog.Exitf("Failed to connect to etcd at %v: %v", servers, err) } - defer client.Close() + defer func() { + if err := client.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() } ctx, cancel := context.WithCancel(context.Background()) @@ -188,7 +196,9 @@ func main() { // Enable CPU profile if requested if *cpuProfile != "" { f := mustCreate(*cpuProfile) - pprof.StartCPUProfile(f) + if err := pprof.StartCPUProfile(f); err != nil { + klog.Exitf("StartCPUProfile(): %v", err) + } defer pprof.StopCPUProfile() } @@ -211,7 +221,9 @@ func main() { if *memProfile != "" { f := mustCreate(*memProfile) - pprof.WriteHeapProfile(f) + if err := pprof.WriteHeapProfile(f); err != nil { + klog.Exitf("WriteHeapProfile(): %v", err) + } } // Give things a few seconds to tidy up diff --git a/cmd/updatetree/main.go b/cmd/updatetree/main.go index 88e6b8e617..8a5e56f8b5 100644 --- a/cmd/updatetree/main.go +++ b/cmd/updatetree/main.go @@ -104,7 +104,11 @@ func updateTree(ctx context.Context) (*trillian.Tree, error) { if err != nil { return nil, fmt.Errorf("failed to dial %v: %v", *adminServerAddr, err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() client := trillian.NewTrillianAdminClient(conn) for { diff --git a/experimental/batchmap/cmd/build/mapdemo.go b/experimental/batchmap/cmd/build/mapdemo.go index 091b8a916b..65dd7abce4 100644 --- a/experimental/batchmap/cmd/build/mapdemo.go +++ b/experimental/batchmap/cmd/build/mapdemo.go @@ -130,7 +130,12 @@ func (fn *writeTileFn) ProcessElement(ctx context.Context, t *batchmap.Tile) err if err != nil { return err } - defer w.Close() + + defer func() { + if err := w.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() bs, err := json.Marshal(t) if err != nil { diff --git a/integration/format/format.go b/integration/format/format.go index 839fa2c604..a356f033ca 100644 --- a/integration/format/format.go +++ b/integration/format/format.go @@ -41,6 +41,7 @@ import ( "github.com/transparency-dev/merkle/rfc6962" "google.golang.org/protobuf/encoding/prototext" "google.golang.org/protobuf/types/known/durationpb" + "k8s.io/klog/v2" ) func run(treeSize, batchSize int, leafFormat string) (string, error) { @@ -171,7 +172,10 @@ func latestRevisions(ls storage.LogStorage, treeID int64, hasher merkle.LogHashe out := new(bytes.Buffer) for _, k := range keys { subtree := vMap[k].subtree - cache.PopulateLogTile(subtree, hasher) + if err := cache.PopulateLogTile(subtree, hasher); err != nil { + // TODO(mhutchinson): This error should be propagated. + klog.Errorf("PopulateLogTile(): %v", err) + } fmt.Fprintf(out, "%s\n", prototext.Format(subtree)) } return out.String(), nil diff --git a/integration/log_integration_test.go b/integration/log_integration_test.go index 1ba10863cb..bf2076eb8c 100644 --- a/integration/log_integration_test.go +++ b/integration/log_integration_test.go @@ -85,7 +85,11 @@ func TestLiveLogIntegration(t *testing.T) { if err != nil { t.Fatalf("Failed to connect to log server: %v", err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + t.Errorf("Close(): %v", err) + } + }() lc := trillian.NewTrillianLogClient(conn) if err := RunLogIntegration(lc, params); err != nil { diff --git a/integration/quota/quota_test.go b/integration/quota/quota_test.go index 477be695a0..e43a2e7fb8 100644 --- a/integration/quota/quota_test.go +++ b/integration/quota/quota_test.go @@ -43,6 +43,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" + "k8s.io/klog/v2" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" ) @@ -206,18 +207,20 @@ type testServer struct { func (s *testServer) close() { if s.conn != nil { - s.conn.Close() + _ = s.conn.Close() } if s.server != nil { s.server.GracefulStop() } if s.lis != nil { - s.lis.Close() + _ = s.lis.Close() } } func (s *testServer) serve() { - s.server.Serve(s.lis) + if err := s.server.Serve(s.lis); err != nil { + klog.Errorf("Serve(): %v", err) + } } // newTestServer returns a new testServer configured for integration tests. diff --git a/integration/storagetest/logtests.go b/integration/storagetest/logtests.go index 5c810e3b61..d2047218e8 100644 --- a/integration/storagetest/logtests.go +++ b/integration/storagetest/logtests.go @@ -117,7 +117,11 @@ func (*logTests) TestSnapshot(ctx context.Context, t *testing.T, s storage.LogSt tx, err := s.SnapshotForTree(ctx, test.tree) if err == storage.ErrTreeNeedsInit { - defer tx.Close() + defer func() { + if err := tx.Close(); err != nil { + t.Errorf("Close(): %v", err) + } + }() } if hasErr := err != nil; hasErr != test.wantErr { @@ -125,7 +129,11 @@ func (*logTests) TestSnapshot(ctx context.Context, t *testing.T, s storage.LogSt } else if hasErr { return } - defer tx.Close() + defer func() { + if err := tx.Close(); err != nil { + t.Errorf("Close(): %v", err) + } + }() _, err = tx.LatestSignedLogRoot(ctx) if err != nil { diff --git a/log/operation_manager_test.go b/log/operation_manager_test.go index 4ce6d5765b..cb62535a98 100644 --- a/log/operation_manager_test.go +++ b/log/operation_manager_test.go @@ -408,14 +408,18 @@ func TestMasterFor(t *testing.T) { lom := NewOperationManager(info, nil) // Check mastership twice, to give the election threads a chance to get started and report. - lom.masterFor(testCtx, firstIDs) + if _, err := lom.masterFor(testCtx, firstIDs); err != nil { + t.Error(err) + } time.Sleep(100 * time.Millisecond) logIDs, err := lom.masterFor(testCtx, firstIDs) if !reflect.DeepEqual(logIDs, test.want1) { t.Fatalf("masterFor(factory=%T)=%v,%v; want %v,_", test.factory, logIDs, err, test.want1) } // Now add extra IDs and re-check. - lom.masterFor(testCtx, allIDs) + if _, err := lom.masterFor(testCtx, allIDs); err != nil { + t.Error(err) + } time.Sleep(100 * time.Millisecond) logIDs, err = lom.masterFor(testCtx, allIDs) if !reflect.DeepEqual(logIDs, test.want2) { diff --git a/log/sequencer_manager_test.go b/log/sequencer_manager_test.go index 3c51649a15..4dec57b891 100644 --- a/log/sequencer_manager_test.go +++ b/log/sequencer_manager_test.go @@ -105,7 +105,9 @@ func TestSequencerManagerSingleLogNoLeaves(t *testing.T) { } sm := NewSequencerManager(registry, zeroDuration) - sm.ExecutePass(ctx, logID, createTestInfo(registry)) + if _, err := sm.ExecutePass(ctx, logID, createTestInfo(registry)); err != nil { + t.Error(err) + } } func TestSequencerManagerCachesSigners(t *testing.T) { @@ -181,7 +183,9 @@ func TestSequencerManagerSingleLogOneLeaf(t *testing.T) { } sm := NewSequencerManager(registry, zeroDuration) - sm.ExecutePass(ctx, logID, createTestInfo(registry)) + if _, err := sm.ExecutePass(ctx, logID, createTestInfo(registry)); err != nil { + t.Error(err) + } } // cmpMatcher is a custom gomock.Matcher that uses cmp.Equal combined with a @@ -224,7 +228,9 @@ func TestSequencerManagerGuardWindow(t *testing.T) { } sm := NewSequencerManager(registry, time.Second*5) - sm.ExecutePass(ctx, logID, createTestInfo(registry)) + if _, err := sm.ExecutePass(ctx, logID, createTestInfo(registry)); err != nil { + t.Error(err) + } } func createTestInfo(registry extension.Registry) *OperationInfo { diff --git a/merkle/coniks/coniks.go b/merkle/coniks/coniks.go index 8185dc7691..400ced8414 100644 --- a/merkle/coniks/coniks.go +++ b/merkle/coniks/coniks.go @@ -57,9 +57,13 @@ func (m *Hasher) HashEmpty(treeID int64, root node.ID) []byte { buf := bytes.NewBuffer(make([]byte, 0, 32)) h := m.New() buf.Write(emptyIdentifier) - binary.Write(buf, binary.BigEndian, uint64(treeID)) + if err := binary.Write(buf, binary.BigEndian, uint64(treeID)); err != nil { + klog.Errorf("binary.Write(): %v", err) + } m.writeMaskedNodeID(buf, root) - binary.Write(buf, binary.BigEndian, uint32(depth)) + if err := binary.Write(buf, binary.BigEndian, uint32(depth)); err != nil { + klog.Errorf("binary.Write(): %v", err) + } h.Write(buf.Bytes()) r := h.Sum(nil) if klog.V(5).Enabled() { @@ -75,9 +79,13 @@ func (m *Hasher) HashLeaf(treeID int64, id node.ID, leaf []byte) []byte { buf := bytes.NewBuffer(make([]byte, 0, 32+len(leaf))) h := m.New() buf.Write(leafIdentifier) - binary.Write(buf, binary.BigEndian, uint64(treeID)) + if err := binary.Write(buf, binary.BigEndian, uint64(treeID)); err != nil { + klog.Errorf("binary.Write(): %v", err) + } m.writeMaskedNodeID(buf, id) - binary.Write(buf, binary.BigEndian, uint32(depth)) + if err := binary.Write(buf, binary.BigEndian, uint32(depth)); err != nil { + klog.Errorf("binary.Write(): %v", err) + } buf.Write(leaf) h.Write(buf.Bytes()) p := h.Sum(nil) diff --git a/quota/crdbqm/common_test.go b/quota/crdbqm/common_test.go index 50eb0234a0..f2dc0d89a0 100644 --- a/quota/crdbqm/common_test.go +++ b/quota/crdbqm/common_test.go @@ -37,7 +37,10 @@ func TestMain(m *testing.M) { dburl.Path = "/" // Set the environment variable for the test server - os.Setenv(testdb.CockroachDBURIEnv, dburl.String()) + if err := os.Setenv(testdb.CockroachDBURIEnv, dburl.String()); err != nil { + klog.Errorf("Failed to set CockroachDBURIEnv: %v", err) + os.Exit(1) + } if !testdb.CockroachDBAvailable() { klog.Errorf("CockroachDB not available, skipping all CockroachDB storage tests") diff --git a/quota/crdbqm/crdb_quota.go b/quota/crdbqm/crdb_quota.go index a55e8940ab..d623c687d8 100644 --- a/quota/crdbqm/crdb_quota.go +++ b/quota/crdbqm/crdb_quota.go @@ -21,6 +21,7 @@ import ( "errors" "github.com/google/trillian/quota" + "k8s.io/klog/v2" ) const ( @@ -87,7 +88,11 @@ func (m *QuotaManager) countUnsequenced(ctx context.Context) (int, error) { if err != nil { return 0, err } - defer rows.Close() + defer func() { + if err := rows.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() if !rows.Next() { return 0, errors.New("cursor has no rows after quota limit determination query") } diff --git a/quota/etcd/quotaapi/quota_server_test.go b/quota/etcd/quotaapi/quota_server_test.go index ca545b1a8e..5a98a126e3 100644 --- a/quota/etcd/quotaapi/quota_server_test.go +++ b/quota/etcd/quotaapi/quota_server_test.go @@ -785,13 +785,13 @@ func startServer(etcdClient *clientv3.Client) (quotapb.QuotaClient, func(), erro cleanup := func() { if conn != nil { - conn.Close() + _ = conn.Close() } if s != nil { s.GracefulStop() } if lis != nil { - lis.Close() + _ = lis.Close() } } @@ -804,7 +804,7 @@ func startServer(etcdClient *clientv3.Client) (quotapb.QuotaClient, func(), erro s = grpc.NewServer(grpc.UnaryInterceptor(interceptor.ErrorWrapper)) quotapb.RegisterQuotaServer(s, NewServer(etcdClient)) - go s.Serve(lis) + go func() { _ = s.Serve(lis) }() conn, err = grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { diff --git a/quota/mysqlqm/mysql_quota.go b/quota/mysqlqm/mysql_quota.go index 45e4dcc4ca..ca55613909 100644 --- a/quota/mysqlqm/mysql_quota.go +++ b/quota/mysqlqm/mysql_quota.go @@ -22,6 +22,7 @@ import ( "fmt" "github.com/google/trillian/quota" + "k8s.io/klog/v2" ) const ( @@ -108,7 +109,11 @@ func countFromInformationSchema(ctx context.Context, db *sql.DB) (int, error) { if err != nil { return 0, err } - defer rows.Close() + defer func() { + if err := rows.Close(); err != nil { + klog.Errorf("Close(): %v", err) + } + }() if !rows.Next() { return 0, errors.New("cursor has no rows after information_schema query") } diff --git a/testonly/integration/etcd/etcd.go b/testonly/integration/etcd/etcd.go index 29dbffa498..62f0536d51 100644 --- a/testonly/integration/etcd/etcd.go +++ b/testonly/integration/etcd/etcd.go @@ -51,12 +51,12 @@ func StartEtcd() (e *embed.Etcd, c *clientv3.Client, cleanup func(), err error) cleanup = func() { if c != nil { - c.Close() + _ = c.Close() } if e != nil { e.Close() } - os.RemoveAll(dir) + _ = os.RemoveAll(dir) } for i := 0; i < MaxEtcdStartAttempts; i++ { @@ -100,13 +100,17 @@ func tryStartEtcd(dir string) (*embed.Etcd, error) { if err != nil { return nil, err } - p1.Close() + if err := p1.Close(); err != nil { + return nil, err + } p2, err := net.Listen("tcp", "localhost:0") if err != nil { return nil, err } - p2.Close() + if err := p2.Close(); err != nil { + return nil, err + } // OK to ignore err, it'll error below if parsing fails clientURL, _ := url.Parse("http://" + p1.Addr().String()) diff --git a/testonly/integration/logenv.go b/testonly/integration/logenv.go index 6806a9ecfe..eb3a583b89 100644 --- a/testonly/integration/logenv.go +++ b/testonly/integration/logenv.go @@ -99,7 +99,9 @@ func NewLogEnvWithGRPCOptions(ctx context.Context, numSequencers int, serverOpts ret, err := NewLogEnvWithRegistryAndGRPCOptions(ctx, numSequencers, registry, serverOpts, clientOpts) if err != nil { - db.Close() + if err := db.Close(); err != nil { + return nil, err + } return nil, err } ret.DB = db @@ -196,7 +198,9 @@ func (env *LogEnv) Close() { if env.sequencerCancel != nil { env.sequencerCancel() } - env.ClientConn.Close() + if err := env.ClientConn.Close(); err != nil { + klog.Errorf("ClientConn.Close(): %v", err) + } env.grpcServer.GracefulStop() env.pendingTasks.Wait() if env.dbDone != nil { diff --git a/testonly/mdm/mdm.go b/testonly/mdm/mdm.go index c057524e86..eb8dff40e1 100644 --- a/testonly/mdm/mdm.go +++ b/testonly/mdm/mdm.go @@ -132,7 +132,9 @@ func (m *MergeDelayMonitor) monitor(ctx context.Context, idx int) error { createNew = true } if createNew { - crand.Read(data) + if _, err := crand.Read(data); err != nil { + return fmt.Errorf("rand.Read(): %v", err) + } } // Add the leaf data and wait for its inclusion. diff --git a/testonly/mock_server.go b/testonly/mock_server.go index fb8345fe75..0a4a482a09 100644 --- a/testonly/mock_server.go +++ b/testonly/mock_server.go @@ -48,19 +48,19 @@ func NewMockServer(ctrl *gomock.Controller) (*MockServer, func(), error) { if err != nil { return nil, nil, err } - go grpcServer.Serve(lis) + go func() { _ = grpcServer.Serve(lis) }() cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { grpcServer.Stop() - lis.Close() + _ = lis.Close() return nil, nil, err } stopFn := func() { - cc.Close() + _ = cc.Close() grpcServer.Stop() - lis.Close() + _ = lis.Close() } return &MockServer{ From ef7117bb061e3131db4d2566181479519f97b3c8 Mon Sep 17 00:00:00 2001 From: Martin Hutchinson Date: Wed, 26 Apr 2023 13:23:27 +0100 Subject: [PATCH 2/2] Fixed PR comments and Close() error causing tests to fail --- integration/storagetest/logtests.go | 4 +--- merkle/coniks/coniks.go | 18 ++++++------------ testonly/mdm/mdm.go | 2 +- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/integration/storagetest/logtests.go b/integration/storagetest/logtests.go index d2047218e8..8dcaef20fb 100644 --- a/integration/storagetest/logtests.go +++ b/integration/storagetest/logtests.go @@ -130,9 +130,7 @@ func (*logTests) TestSnapshot(ctx context.Context, t *testing.T, s storage.LogSt return } defer func() { - if err := tx.Close(); err != nil { - t.Errorf("Close(): %v", err) - } + _ = tx.Close() }() _, err = tx.LatestSignedLogRoot(ctx) diff --git a/merkle/coniks/coniks.go b/merkle/coniks/coniks.go index 400ced8414..c634e02478 100644 --- a/merkle/coniks/coniks.go +++ b/merkle/coniks/coniks.go @@ -54,16 +54,13 @@ func (m *Hasher) EmptyRoot() []byte { func (m *Hasher) HashEmpty(treeID int64, root node.ID) []byte { depth := int(root.BitLen()) + // bytes.Buffer never returns errors so we can ignore them below. buf := bytes.NewBuffer(make([]byte, 0, 32)) h := m.New() buf.Write(emptyIdentifier) - if err := binary.Write(buf, binary.BigEndian, uint64(treeID)); err != nil { - klog.Errorf("binary.Write(): %v", err) - } + _ = binary.Write(buf, binary.BigEndian, uint64(treeID)) m.writeMaskedNodeID(buf, root) - if err := binary.Write(buf, binary.BigEndian, uint32(depth)); err != nil { - klog.Errorf("binary.Write(): %v", err) - } + _ = binary.Write(buf, binary.BigEndian, uint32(depth)) h.Write(buf.Bytes()) r := h.Sum(nil) if klog.V(5).Enabled() { @@ -76,16 +73,13 @@ func (m *Hasher) HashEmpty(treeID int64, root node.ID) []byte { // H(Identifier || treeID || depth || index || dataHash) func (m *Hasher) HashLeaf(treeID int64, id node.ID, leaf []byte) []byte { depth := int(id.BitLen()) + // bytes.Buffer never returns errors so we can ignore them below. buf := bytes.NewBuffer(make([]byte, 0, 32+len(leaf))) h := m.New() buf.Write(leafIdentifier) - if err := binary.Write(buf, binary.BigEndian, uint64(treeID)); err != nil { - klog.Errorf("binary.Write(): %v", err) - } + _ = binary.Write(buf, binary.BigEndian, uint64(treeID)) m.writeMaskedNodeID(buf, id) - if err := binary.Write(buf, binary.BigEndian, uint32(depth)); err != nil { - klog.Errorf("binary.Write(): %v", err) - } + _ = binary.Write(buf, binary.BigEndian, uint32(depth)) buf.Write(leaf) h.Write(buf.Bytes()) p := h.Sum(nil) diff --git a/testonly/mdm/mdm.go b/testonly/mdm/mdm.go index eb8dff40e1..77e859423a 100644 --- a/testonly/mdm/mdm.go +++ b/testonly/mdm/mdm.go @@ -133,7 +133,7 @@ func (m *MergeDelayMonitor) monitor(ctx context.Context, idx int) error { } if createNew { if _, err := crand.Read(data); err != nil { - return fmt.Errorf("rand.Read(): %v", err) + return fmt.Errorf("Read(): %v", err) } }