From 6dd8984fef8742ec6e5481040c159941c9d0a819 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 29 Jul 2021 19:28:32 -0700 Subject: [PATCH] Fix and clarify breaks from select cases. (#6781) Update those break statements inside case clauses that are intended to reach an enclosing for loop, so that they correctly exit the loop. The candidate files for this change were located using: % staticcheck -checks SA4011 ./... | cut -d: -f-2 This change is intended to preserve the intended semantics of the code, but since the code as-written did not have its intended effect, some behaviour may change. Specifically: Some loops may have run longer than they were supposed to, prior to this change. In one case I was not able to clearly determine the intended outcome. That case has been commented but otherwise left as-written. Fixes #6780. --- crypto/merkle/proof.go | 5 ++++- internal/p2p/peermanager.go | 2 +- state/indexer/block/kv/kv.go | 15 +++++++++------ state/indexer/tx/kv/kv.go | 18 +++++++++++------- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index 2994e8048..80b289d23 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -204,7 +204,10 @@ func (spn *ProofNode) FlattenAunts() [][]byte { case spn.Right != nil: innerHashes = append(innerHashes, spn.Right.Hash) default: - break + // FIXME(fromberger): Per the documentation above, exactly one of + // these fields should be set. If that is true, this should probably + // be a panic since it violates the invariant. If not, when can it + // be OK to have no siblings? Does this occur at the leaves? } spn = spn.Parent } diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go index fd6f96933..1e9afb38b 100644 --- a/internal/p2p/peermanager.go +++ b/internal/p2p/peermanager.go @@ -385,7 +385,7 @@ func (m *PeerManager) prunePeers() error { peerID := ranked[i].ID switch { case m.store.Size() <= int(m.options.MaxPeers): - break + return nil case m.dialing[peerID]: case m.connected[peerID]: default: diff --git a/state/indexer/block/kv/kv.go b/state/indexer/block/kv/kv.go index 1787be9ef..bc90eadf5 100644 --- a/state/indexer/block/kv/kv.go +++ b/state/indexer/block/kv/kv.go @@ -186,6 +186,7 @@ func (idx *BlockerIndexer) Search(ctx context.Context, q *query.Query) ([]int64, // fetch matching heights results = make([]int64, 0, len(filteredHeights)) +heights: for _, hBz := range filteredHeights { h := int64FromBytes(hBz) @@ -199,7 +200,7 @@ func (idx *BlockerIndexer) Search(ctx context.Context, q *query.Query) ([]int64, select { case <-ctx.Done(): - break + break heights default: } @@ -240,7 +241,7 @@ func (idx *BlockerIndexer) matchRange( } defer it.Close() -LOOP: +iter: for ; it.Valid(); it.Next() { var ( eventValue string @@ -260,7 +261,7 @@ LOOP: if _, ok := qr.AnyBound().(int64); ok { v, err := strconv.ParseInt(eventValue, 10, 64) if err != nil { - continue LOOP + continue iter } include := true @@ -279,7 +280,7 @@ LOOP: select { case <-ctx.Done(): - break + break iter default: } @@ -372,12 +373,13 @@ func (idx *BlockerIndexer) match( } defer it.Close() + iterExists: for ; it.Valid(); it.Next() { tmpHeights[string(it.Value())] = it.Value() select { case <-ctx.Done(): - break + break iterExists default: } @@ -399,6 +401,7 @@ func (idx *BlockerIndexer) match( } defer it.Close() + iterContains: for ; it.Valid(); it.Next() { eventValue, err := parseValueFromEventKey(it.Key()) if err != nil { @@ -411,7 +414,7 @@ func (idx *BlockerIndexer) match( select { case <-ctx.Done(): - break + break iterContains default: } diff --git a/state/indexer/tx/kv/kv.go b/state/indexer/tx/kv/kv.go index 5d310eea7..080dbce2c 100644 --- a/state/indexer/tx/kv/kv.go +++ b/state/indexer/tx/kv/kv.go @@ -219,6 +219,7 @@ func (txi *TxIndex) Search(ctx context.Context, q *query.Query) ([]*abci.TxResul } results := make([]*abci.TxResult, 0, len(filteredHashes)) +hashes: for _, h := range filteredHashes { res, err := txi.Get(h) if err != nil { @@ -229,7 +230,7 @@ func (txi *TxIndex) Search(ctx context.Context, q *query.Query) ([]*abci.TxResul // Potentially exit early. select { case <-ctx.Done(): - break + break hashes default: } } @@ -285,13 +286,14 @@ func (txi *TxIndex) match( } defer it.Close() + iterEqual: for ; it.Valid(); it.Next() { tmpHashes[string(it.Value())] = it.Value() // Potentially exit early. select { case <-ctx.Done(): - break + break iterEqual default: } } @@ -308,13 +310,14 @@ func (txi *TxIndex) match( } defer it.Close() + iterExists: for ; it.Valid(); it.Next() { tmpHashes[string(it.Value())] = it.Value() // Potentially exit early. select { case <-ctx.Done(): - break + break iterExists default: } } @@ -332,6 +335,7 @@ func (txi *TxIndex) match( } defer it.Close() + iterContains: for ; it.Valid(); it.Next() { value, err := parseValueFromKey(it.Key()) if err != nil { @@ -344,7 +348,7 @@ func (txi *TxIndex) match( // Potentially exit early. select { case <-ctx.Done(): - break + break iterContains default: } } @@ -412,7 +416,7 @@ func (txi *TxIndex) matchRange( } defer it.Close() -LOOP: +iter: for ; it.Valid(); it.Next() { value, err := parseValueFromKey(it.Key()) if err != nil { @@ -421,7 +425,7 @@ LOOP: if _, ok := qr.AnyBound().(int64); ok { v, err := strconv.ParseInt(value, 10, 64) if err != nil { - continue LOOP + continue iter } include := true @@ -448,7 +452,7 @@ LOOP: // Potentially exit early. select { case <-ctx.Done(): - break + break iter default: } }