FAB-11215 Enable vetting of more print/printf fncs

Go vet attempts to detect missing format arguments or format strings in non-format contexts. However, the default list of functions does not include many of our logging functions (like Warningf, Debugf, etc.). This CR enables the additional vetting and fixes the errors that it catches. Change-Id: I4eefdf84d9349f2ae08da9bb0c9b829226219c44 Signed-off-by: Jason Yellick <[email protected]>
hyperledger · Jul 18, 2018 · 2a3ebd5 · 2a3ebd5
1 parent 3bad1ec
commit 2a3ebd5
Show file tree

Hide file tree

Showing 13 changed files with 20 additions and 19 deletions.
diff --git a/bccsp/pkcs11/impl.go b/bccsp/pkcs11/impl.go
@@ -114,7 +114,7 @@ func (csp *impl) KeyGen(opts bccsp.KeyGenOpts) (k bccsp.Key, err error) {
 	case *bccsp.ECDSAP384KeyGenOpts:
 		ski, pub, err := csp.generateECKey(oidNamedCurveP384, opts.Ephemeral())
 		if err != nil {
-			return nil, errors.Wrapf(err, "Failed generating ECDSA P384 key [%s]")
+			return nil, errors.Wrapf(err, "Failed generating ECDSA P384 key")
 		}
 
 		k = &ecdsaPrivateKey{ski, ecdsaPublicKey{ski, pub}}
@@ -350,7 +350,7 @@ func (csp *impl) KeyImport(raw interface{}, opts bccsp.KeyImportOpts) (k bccsp.K
 
 		lowLevelKey, err := utils.DERToPrivateKey(der)
 		if err != nil {
-			return nil, errors.Wrapf(err, "Failed converting PKIX to ECDSA public key [%s]")
+			return nil, errors.Wrapf(err, "Failed converting PKIX to ECDSA public key")
 		}
 
 		ecdsaSK, ok := lowLevelKey.(*ecdsa.PrivateKey)

diff --git a/bccsp/pkcs11/pkcs11.go b/bccsp/pkcs11/pkcs11.go
@@ -599,7 +599,7 @@ func (csp *impl) getSecretValue(ski []byte) []byte {
 		logger.Debugf("ListAttr: type %d/0x%x, length %d\n%s", a.Type, a.Type, len(a.Value), hex.Dump(a.Value))
 		return a.Value
 	}
-	logger.Warningf("No Key Value found!", err)
+	logger.Warningf("No Key Value found: %v", err)
 	return nil
 }
 

diff --git a/common/tools/configtxgen/encoder/encoder.go b/common/tools/configtxgen/encoder/encoder.go
@@ -295,7 +295,7 @@ func NewApplicationGroup(conf *genesisconfig.Application) (*cb.ConfigGroup, erro
 func NewApplicationOrgGroup(conf *genesisconfig.Organization) (*cb.ConfigGroup, error) {
 	mspConfig, err := msp.GetVerifyingMspConfig(conf.MSPDir, conf.ID, conf.MSPType)
 	if err != nil {
-		return nil, errors.Wrapf(err, "1 - Error loading MSP configuration for org %s: %s", conf.Name)
+		return nil, errors.Wrapf(err, "1 - Error loading MSP configuration for org %s", conf.Name)
 	}
 
 	applicationOrgGroup := cb.NewConfigGroup()

diff --git a/common/tools/configtxgen/localconfig/config.go b/common/tools/configtxgen/localconfig/config.go
@@ -363,7 +363,7 @@ func (oc *Orderer) completeInitialization() {
 	for {
 		switch {
 		case oc.OrdererType == "":
-			logger.Infof("Orderer.OrdererType unset, setting to %s", genesisDefaults.Orderer.OrdererType)
+			logger.Infof("Orderer.OrdererType unset, setting to %v", genesisDefaults.Orderer.OrdererType)
 			oc.OrdererType = genesisDefaults.Orderer.OrdererType
 		case oc.Addresses == nil:
 			logger.Infof("Orderer.Addresses unset, setting to %s", genesisDefaults.Orderer.Addresses)
@@ -372,13 +372,13 @@ func (oc *Orderer) completeInitialization() {
 			logger.Infof("Orderer.BatchTimeout unset, setting to %s", genesisDefaults.Orderer.BatchTimeout)
 			oc.BatchTimeout = genesisDefaults.Orderer.BatchTimeout
 		case oc.BatchSize.MaxMessageCount == 0:
-			logger.Infof("Orderer.BatchSize.MaxMessageCount unset, setting to %s", genesisDefaults.Orderer.BatchSize.MaxMessageCount)
+			logger.Infof("Orderer.BatchSize.MaxMessageCount unset, setting to %v", genesisDefaults.Orderer.BatchSize.MaxMessageCount)
 			oc.BatchSize.MaxMessageCount = genesisDefaults.Orderer.BatchSize.MaxMessageCount
 		case oc.BatchSize.AbsoluteMaxBytes == 0:
-			logger.Infof("Orderer.BatchSize.AbsoluteMaxBytes unset, setting to %s", genesisDefaults.Orderer.BatchSize.AbsoluteMaxBytes)
+			logger.Infof("Orderer.BatchSize.AbsoluteMaxBytes unset, setting to %v", genesisDefaults.Orderer.BatchSize.AbsoluteMaxBytes)
 			oc.BatchSize.AbsoluteMaxBytes = genesisDefaults.Orderer.BatchSize.AbsoluteMaxBytes
 		case oc.BatchSize.PreferredMaxBytes == 0:
-			logger.Infof("Orderer.BatchSize.PreferredMaxBytes unset, setting to %s", genesisDefaults.Orderer.BatchSize.PreferredMaxBytes)
+			logger.Infof("Orderer.BatchSize.PreferredMaxBytes unset, setting to %v", genesisDefaults.Orderer.BatchSize.PreferredMaxBytes)
 			oc.BatchSize.PreferredMaxBytes = genesisDefaults.Orderer.BatchSize.PreferredMaxBytes
 		case oc.Kafka.Brokers == nil:
 			logger.Infof("Orderer.Kafka.Brokers unset, setting to %v", genesisDefaults.Orderer.Kafka.Brokers)

diff --git a/core/deliverservice/blocksprovider/blocksprovider.go b/core/deliverservice/blocksprovider/blocksprovider.go
@@ -195,7 +195,7 @@ func (b *blocksProviderImpl) DeliverBlocks() {
 				b.gossip.Gossip(gossipMsg)
 			}
 		default:
-			logger.Warningf("[%s] Received unknown: ", b.chainID, t)
+			logger.Warningf("[%s] Received unknown: %v", b.chainID, t)
 			return
 		}
 	}

diff --git a/core/scc/sccproviderimpl.go b/core/scc/sccproviderimpl.go
@@ -37,7 +37,7 @@ func (p *Provider) RegisterSysCC(scc SelfDescribingSysCC) {
 	p.SysCCs = append(p.SysCCs, scc)
 	_, err := p.registerSysCC(scc)
 	if err != nil {
-		sysccLogger.Panic("Could not register system chaincode: %s", err)
+		sysccLogger.Panicf("Could not register system chaincode: %s", err)
 	}
 }
 

diff --git a/core/scc/sysccapi.go b/core/scc/sysccapi.go
@@ -128,7 +128,7 @@ func (p *Provider) registerSysCC(syscc SelfDescribingSysCC) (bool, error) {
 		}
 	}
 
-	sysccLogger.Infof("system chaincode %s(%s) registered", syscc.Name, syscc.Path)
+	sysccLogger.Infof("system chaincode %s(%s) registered", syscc.Name(), syscc.Path())
 	return true, err
 }
 

diff --git a/gossip/comm/comm_impl.go b/gossip/comm/comm_impl.go
@@ -457,7 +457,7 @@ func (c *commImpl) authenticateRemotePeer(stream stream, initiator bool) (*proto
 	}
 
 	if receivedMsg.PkiId == nil {
-		c.logger.Warning("%s didn't send a pkiID", remoteAddress)
+		c.logger.Warningf("%s didn't send a pkiID", remoteAddress)
 		return nil, fmt.Errorf("No PKI-ID")
 	}
 

diff --git a/gossip/comm/conn.go b/gossip/comm/conn.go
@@ -350,13 +350,13 @@ func (conn *connection) readFromStream(errChan chan error, msgChan chan *proto.S
 		}
 		if err != nil {
 			errChan <- err
-			conn.logger.Debugf("%v Got error, aborting: %v", err)
+			conn.logger.Debugf("Got error, aborting: %v", err)
 			return
 		}
 		msg, err := envelope.ToGossipMessage()
 		if err != nil {
 			errChan <- err
-			conn.logger.Warning("%v Got error, aborting: %v", err)
+			conn.logger.Warningf("Got error, aborting: %v", err)
 		}
 		msgChan <- msg
 	}

diff --git a/gossip/gossip/gossip_impl.go b/gossip/gossip/gossip_impl.go
@@ -411,7 +411,7 @@ func (g *gossipServiceImpl) forwardDiscoveryMsg(msg proto.ReceivedMessage) {
 // and also checks that the tag matches the message type
 func (g *gossipServiceImpl) validateMsg(msg proto.ReceivedMessage) bool {
 	if err := msg.GetGossipMessage().IsTagLegal(); err != nil {
-		g.logger.Warningf("Tag of %v isn't legal:", msg.GetGossipMessage(), errors.WithStack(err))
+		g.logger.Warningf("Tag of %v isn't legal: %v", msg.GetGossipMessage(), errors.WithStack(err))
 		return false
 	}
 

diff --git a/gossip/privdata/coordinator.go b/gossip/privdata/coordinator.go
@@ -307,8 +307,8 @@ func (c *coordinator) fetchFromPeers(blockSeq uint64, ownedRWsets map[rwSetKey][
 				missingPvtRWKey.collection == dig.Collection &&
 				missingPvtRWKey.txID == dig.TxId {
 				delete(privateInfo.missingKeys, missingPvtRWKey)
-				logger.Warningf("Missing [%s] key because was purged or will soon be purged, "+
-					"continue block commit without [%s] in private rwset", missingPvtRWKey, missingPvtRWKey)
+				logger.Warningf("Missing key because was purged or will soon be purged, "+
+					"continue block commit without [%+v] in private rwset", missingPvtRWKey)
 			}
 		}
 	}

diff --git a/orderer/common/localconfig/config.go b/orderer/common/localconfig/config.go
@@ -262,7 +262,7 @@ func (c *TopLevel) completeInitialization(configDir string) {
 			logger.Infof("General.ListenAddress unset, setting to %s", Defaults.General.ListenAddress)
 			c.General.ListenAddress = Defaults.General.ListenAddress
 		case c.General.ListenPort == 0:
-			logger.Infof("General.ListenPort unset, setting to %s", Defaults.General.ListenPort)
+			logger.Infof("General.ListenPort unset, setting to %v", Defaults.General.ListenPort)
 			c.General.ListenPort = Defaults.General.ListenPort
 
 		case c.General.LogLevel == "":

diff --git a/scripts/golinter.sh b/scripts/golinter.sh
@@ -33,7 +33,8 @@ if [ -n "$OUTPUT" ]; then
 fi
 
 echo "Checking with go vet"
-OUTPUT="$(go vet ./...)"
+PRINTFUNCS="Print,Printf,Info,Infof,Warning,Warningf,Error,Errorf,Critical,Criticalf,Sprint,Sprintf,Log,Logf,Panic,Panicf,Fatal,Fatalf,Notice,Noticef,Wrap,Wrapf,WithMessage"
+OUTPUT="$(go vet -printfuncs $PRINTFUNCS ./...)"
 if [ -n "$OUTPUT" ]; then
     echo "The following files contain go vet errors"
     echo $OUTPUT
-Original file line number
+Diff line change
@@ Expand Up @@
     		}
     	}
-    	sysccLogger.Infof("system chaincode %s(%s) registered", syscc.Name, syscc.Path)
+    	sysccLogger.Infof("system chaincode %s(%s) registered", syscc.Name(), syscc.Path())
     	return true, err
     }
@@ Expand Down @@