Skip to content

Commit

Permalink
Cleanup confusing variable assignments (ava-labs#2268)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored Nov 19, 2022
1 parent 4dcaaf3 commit 2808ee5
Show file tree
Hide file tree
Showing 26 changed files with 96 additions and 88 deletions.
4 changes: 2 additions & 2 deletions api/keystore/keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,12 @@ func (ks *keystore) DeleteUser(username, pw string) error {
defer it.Release()

for it.Next() {
if err = dataBatch.Delete(it.Key()); err != nil {
if err := dataBatch.Delete(it.Key()); err != nil {
return err
}
}

if err = it.Error(); err != nil {
if err := it.Error(); err != nil {
return err
}

Expand Down
3 changes: 2 additions & 1 deletion api/keystore/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ func TestServiceDeleteUser(t *testing.T) {
}

// deleted user details should be available to create user again.
if err = s.CreateUser(nil, &api.UserPass{Username: testUser, Password: password}, &api.EmptyReply{}); err != nil {
err := s.CreateUser(nil, &api.UserPass{Username: testUser, Password: password}, &api.EmptyReply{})
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
}
Expand Down
6 changes: 4 additions & 2 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ func getHTTPConfig(v *viper.Viper) (node.HTTPConfig, error) {
}
case v.IsSet(HTTPSKeyFileKey):
httpsKeyFilepath := GetExpandedArg(v, HTTPSKeyFileKey)
if httpsKey, err = os.ReadFile(filepath.Clean(httpsKeyFilepath)); err != nil {
httpsKey, err = os.ReadFile(filepath.Clean(httpsKeyFilepath))
if err != nil {
return node.HTTPConfig{}, err
}
}
Expand All @@ -237,7 +238,8 @@ func getHTTPConfig(v *viper.Viper) (node.HTTPConfig, error) {
}
case v.IsSet(HTTPSCertFileKey):
httpsCertFilepath := GetExpandedArg(v, HTTPSCertFileKey)
if httpsCert, err = os.ReadFile(filepath.Clean(httpsCertFilepath)); err != nil {
httpsCert, err = os.ReadFile(filepath.Clean(httpsCertFilepath))
if err != nil {
return node.HTTPConfig{}, err
}
}
Expand Down
2 changes: 1 addition & 1 deletion database/test_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestBatchPut(t *testing.T, db Database) {
t.Fatalf("Unexpected error on db.Delete: %s", err)
}

if batch = db.NewBatch(); batch == nil {
if batch := db.NewBatch(); batch == nil {
t.Fatalf("db.NewBatch returned nil")
} else if err := batch.Put(key, value); err != nil {
t.Fatalf("Unexpected error on batch.Put: %s", err)
Expand Down
2 changes: 1 addition & 1 deletion indexer/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (i *index) getContainerByIndexBytes(indexBytes []byte) (Container, error) {
return Container{}, fmt.Errorf("couldn't read from database: %w", err)
}
var container Container
if _, err = i.codec.Unmarshal(containerBytes, &container); err != nil {
if _, err := i.codec.Unmarshal(containerBytes, &container); err != nil {
return Container{}, fmt.Errorf("couldn't unmarshal container: %w", err)
}
return container, nil
Expand Down
4 changes: 2 additions & 2 deletions ipcs/socket/socket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ func TestSocketSendAndReceive(t *testing.T) {

// Test max message size
client.SetMaxMessageSize(msgLen)
if _, err = client.Recv(); err != nil {
if _, err := client.Recv(); err != nil {
t.Fatal("Failed to receive from socket:", err.Error())
}
client.SetMaxMessageSize(msgLen - 1)
if _, err = client.Recv(); err != ErrMessageTooLarge {
if _, err := client.Recv(); err != ErrMessageTooLarge {
t.Fatal("Should have received message too large error, got:", err)
}
}
Expand Down
32 changes: 19 additions & 13 deletions ipcs/socket/socket_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,19 @@ func listen(addr string) (net.Listener, error) {
return nil, err
}

// Try to listen on the socket. If that fails we check to see if it's a stale
// socket and remove it if it is. Then we try to listen one more time.
// Try to listen on the socket.
l, err := net.ListenUnix("unix", uAddr)
if err != nil {
if err = removeIfStaleUnixSocket(addr); err != nil {
return nil, err
}
if l, err = net.ListenUnix("unix", uAddr); err != nil {
return nil, err
}
if err == nil {
return l, nil
}
return l, err

// Check to see if the socket is stale and remove it if it is.
if err := removeIfStaleUnixSocket(addr); err != nil {
return nil, err
}

// Try listening again now that it shouldn't be stale.
return net.ListenUnix("unix", uAddr)
}

// Dial creates a new *Client connected to the given address over a Unix socket
Expand All @@ -59,13 +60,16 @@ func Dial(addr string) (*Client, error) {
// that is refusing connections
func removeIfStaleUnixSocket(socketPath string) error {
// Ensure it's a socket; if not return without an error
if st, err := os.Stat(socketPath); err != nil || st.Mode()&os.ModeType != os.ModeSocket {
st, err := os.Stat(socketPath)
if err != nil {
return nil
}
if st.Mode()&os.ModeType != os.ModeSocket {
return nil
}

// Try to connect
conn, err := net.DialTimeout("unix", socketPath, staleSocketTimeout)

switch {
// The connection was refused so this socket is stale; remove it
case isSyscallError(err, syscall.ECONNREFUSED):
Expand All @@ -74,6 +78,8 @@ func removeIfStaleUnixSocket(socketPath string) error {
// The socket is alive so close this connection and leave the socket alone
case err == nil:
return conn.Close()

default:
return nil
}
return nil
}
6 changes: 3 additions & 3 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,6 @@ func (n *Node) Initialize(
) error {
n.Log = logger
n.Config = config
var err error
n.ID = ids.NodeIDFromCert(n.Config.StakingTLSCert.Leaf)
n.LogFactory = logFactory
n.DoneShuttingDown.Add(1)
Expand All @@ -1251,11 +1250,12 @@ func (n *Node) Initialize(
zap.Reflect("config", n.Config),
)

if err = n.initBeacons(); err != nil { // Configure the beacons
if err := n.initBeacons(); err != nil { // Configure the beacons
return fmt.Errorf("problem initializing node beacons: %w", err)
}

// Set up tracer
var err error
n.tracer, err = trace.New(n.Config.TraceConfig)
if err != nil {
return fmt.Errorf("couldn't initialize tracer: %w", err)
Expand Down Expand Up @@ -1307,7 +1307,7 @@ func (n *Node) Initialize(
}
n.initCPUTargeter(&config.CPUTargeterConfig, primaryNetVdrs)
n.initDiskTargeter(&config.DiskTargeterConfig, primaryNetVdrs)
if err = n.initNetworking(primaryNetVdrs); err != nil { // Set up networking layer.
if err := n.initNetworking(primaryNetVdrs); err != nil { // Set up networking layer.
return fmt.Errorf("problem initializing networking: %w", err)
}

Expand Down
6 changes: 3 additions & 3 deletions snow/consensus/avalanche/poll/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ func TestCreateAndFinishPoll(t *testing.T) {
t.Fatalf("Should only have one active poll")
} else if results := s.Vote(1, vdr1, votes); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish a non-existent poll")
} else if results = s.Vote(0, vdr1, votes); len(results) > 0 {
} else if results := s.Vote(0, vdr1, votes); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish an ongoing poll")
} else if results = s.Vote(0, vdr1, votes); len(results) > 0 {
} else if results := s.Vote(0, vdr1, votes); len(results) > 0 {
t.Fatalf("Should have dropped a duplicated poll")
} else if results = s.Vote(0, vdr2, votes); len(results) == 0 {
} else if results := s.Vote(0, vdr2, votes); len(results) == 0 {
t.Fatalf("Should have finished the poll")
} else if len(results) != 1 {
t.Fatalf("Wrong number of results returned")
Expand Down
12 changes: 6 additions & 6 deletions snow/consensus/snowman/poll/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,11 @@ func TestCreateAndFinishSuccessfulPoll(t *testing.T) {
t.Fatalf("Should only have one active poll")
} else if results := s.Vote(1, vdr1, vtxID); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish a non-existent poll")
} else if results = s.Vote(0, vdr1, vtxID); len(results) > 0 {
} else if results := s.Vote(0, vdr1, vtxID); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish an ongoing poll")
} else if results = s.Vote(0, vdr1, vtxID); len(results) > 0 {
} else if results := s.Vote(0, vdr1, vtxID); len(results) > 0 {
t.Fatalf("Should have dropped a duplicated poll")
} else if results = s.Vote(0, vdr2, vtxID); len(results) == 0 {
} else if results := s.Vote(0, vdr2, vtxID); len(results) == 0 {
t.Fatalf("Should have finished the")
} else if len(results) != 1 {
t.Fatalf("Wrong number of results returned")
Expand Down Expand Up @@ -290,11 +290,11 @@ func TestCreateAndFinishFailedPoll(t *testing.T) {
t.Fatalf("Should only have one active poll")
} else if results := s.Drop(1, vdr1); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish a non-existent poll")
} else if results = s.Drop(0, vdr1); len(results) > 0 {
} else if results := s.Drop(0, vdr1); len(results) > 0 {
t.Fatalf("Shouldn't have been able to finish an ongoing poll")
} else if results = s.Drop(0, vdr1); len(results) > 0 {
} else if results := s.Drop(0, vdr1); len(results) > 0 {
t.Fatalf("Should have dropped a duplicated poll")
} else if results = s.Drop(0, vdr2); len(results) == 0 {
} else if results := s.Drop(0, vdr2); len(results) == 0 {
t.Fatalf("Should have finished the")
} else if list := results[0].List(); len(list) != 0 {
t.Fatalf("Wrong number of vertices returned")
Expand Down
14 changes: 5 additions & 9 deletions snow/engine/avalanche/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,13 @@ type state struct {
// Returns nil if it's not found.
// TODO this should return an error
func (s *state) Vertex(id ids.ID) vertex.StatelessVertex {
var (
vtx vertex.StatelessVertex
bytes []byte
err error
)

if vtxIntf, found := s.dbCache.Get(id); found {
vtx, _ = vtxIntf.(vertex.StatelessVertex)
vtx, _ := vtxIntf.(vertex.StatelessVertex)
return vtx
}

if bytes, err = s.db.Get(id[:]); err != nil {
bytes, err := s.db.Get(id[:])
if err != nil {
s.log.Verbo("failed to get vertex from database",
zap.Binary("key", id[:]),
zap.Error(err),
Expand All @@ -48,7 +43,8 @@ func (s *state) Vertex(id ids.ID) vertex.StatelessVertex {
return nil
}

if vtx, err = s.serializer.parseVertex(bytes); err != nil {
vtx, err := s.serializer.parseVertex(bytes)
if err != nil {
s.log.Error("failed parsing saved vertex",
zap.Binary("key", id[:]),
zap.Binary("vertex", bytes),
Expand Down
3 changes: 2 additions & 1 deletion snow/engine/snowman/block/batched_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ func GetAncestors(
ancestorsBytesLen := len(blk.Bytes()) + wrappers.IntLen // length, in bytes, of all elements of ancestors

for numFetched := 1; numFetched < maxBlocksNum && time.Since(startTime) < maxBlocksRetrivalTime; numFetched++ {
if blk, err = vm.GetBlock(ctx, blk.Parent()); err != nil {
blk, err = vm.GetBlock(ctx, blk.Parent())
if err != nil {
break
}
blkBytes := blk.Bytes()
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/snowman/syncer/state_syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ func (ss *stateSyncer) startup(ctx context.Context) error {
}

ss.frontierSeeders = validators.NewSet()
if err = ss.frontierSeeders.Set(beacons); err != nil {
if err := ss.frontierSeeders.Set(beacons); err != nil {
return err
}

Expand Down
7 changes: 4 additions & 3 deletions snow/engine/snowman/transitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,13 @@ func (t *Transitive) issueWithAncestors(ctx context.Context, blk snowman.Block)
// issue [blk] and its ancestors into consensus
status := blk.Status()
for status.Fetched() && !t.wasIssued(blk) {
if err := t.issue(ctx, blk); err != nil {
err := t.issue(ctx, blk)
if err != nil {
return false, err
}
blkID = blk.Parent()
var err error
if blk, err = t.GetBlock(ctx, blkID); err != nil {
blk, err = t.GetBlock(ctx, blkID)
if err != nil {
status = choices.Unknown
break
}
Expand Down
10 changes: 5 additions & 5 deletions vms/avm/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2437,7 +2437,7 @@ func TestImportExportKey(t *testing.T) {
PrivateKey: sk,
}
importReply := &api.JSONAddress{}
if err = s.ImportKey(nil, importArgs, importReply); err != nil {
if err := s.ImportKey(nil, importArgs, importReply); err != nil {
t.Fatal(err)
}

Expand All @@ -2453,7 +2453,7 @@ func TestImportExportKey(t *testing.T) {
Address: addrStr,
}
exportReply := &ExportKeyReply{}
if err = s.ExportKey(nil, exportArgs, exportReply); err != nil {
if err := s.ExportKey(nil, exportArgs, exportReply); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -2486,7 +2486,7 @@ func TestImportAVMKeyNoDuplicates(t *testing.T) {
PrivateKey: sk,
}
reply := api.JSONAddress{}
if err = s.ImportKey(nil, &args, &reply); err != nil {
if err := s.ImportKey(nil, &args, &reply); err != nil {
t.Fatal(err)
}

Expand All @@ -2500,7 +2500,7 @@ func TestImportAVMKeyNoDuplicates(t *testing.T) {
}

reply2 := api.JSONAddress{}
if err = s.ImportKey(nil, &args, &reply2); err != nil {
if err := s.ImportKey(nil, &args, &reply2); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -2644,7 +2644,7 @@ func TestSendMultiple(t *testing.T) {
t.Fatal("Transaction ID returned by SendMultiple does not match the transaction found in vm's pending transactions")
}

if _, err = vm.GetTx(context.Background(), reply.TxID); err != nil {
if _, err := vm.GetTx(context.Background(), reply.TxID); err != nil {
t.Fatalf("Failed to retrieve created transaction: %s", err)
}
})
Expand Down
6 changes: 3 additions & 3 deletions vms/avm/tx_semantic_verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestBaseTxSemanticVerifyUnauthorizedFx(t *testing.T) {
}
vm.batchTimeout = 0

if err = vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
if err := vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -645,7 +645,7 @@ func TestBaseTxSemanticVerifyPendingUnauthorizedFx(t *testing.T) {
}
vm.batchTimeout = 0

if err = vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
if err := vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -793,7 +793,7 @@ func TestBaseTxSemanticVerifyPendingInvalidSignature(t *testing.T) {
}
vm.batchTimeout = 0

if err = vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
if err := vm.SetState(context.Background(), snow.Bootstrapping); err != nil {
t.Fatal(err)
}

Expand Down
Loading

0 comments on commit 2808ee5

Please sign in to comment.