keystore GetKeys(): return result instead of writing to reference

Marko Bencun · Marko Bencun · commit 5cb3da04b888 · 2017-07-23T23:38:52.000+02:00
Issue: bitcoin#10905 By returning the result, a few useless lines can be removed. Return-value-optimization means there should be no copy.
diff --git a/src/keystore.h b/src/keystore.h
@@ -30,7 +30,7 @@ class CKeyStore
     //! Check whether a key corresponding to a given address is present in the store.
     virtual bool HaveKey(const CKeyID &address) const =0;
     virtual bool GetKey(const CKeyID &address, CKey& keyOut) const =0;
-    virtual void GetKeys(std::set<CKeyID> &setAddress) const =0;
+    virtual std::set<CKeyID> GetKeys() const =0;
     virtual bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const =0;
 
     //! Support for BIP 0013 : see https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki
@@ -71,18 +71,14 @@ class CBasicKeyStore : public CKeyStore
         }
         return result;
     }
-    void GetKeys(std::set<CKeyID> &setAddress) const override
+    std::set<CKeyID> GetKeys() const override
     {
-        setAddress.clear();
-        {
-            LOCK(cs_KeyStore);
-            KeyMap::const_iterator mi = mapKeys.begin();
-            while (mi != mapKeys.end())
-            {
-                setAddress.insert((*mi).first);
-                mi++;
-            }
+        LOCK(cs_KeyStore);
+        std::set<CKeyID> set_address;
+        for (const auto& mi : mapKeys) {
+            set_address.insert(mi.first);
         }
+        return set_address;
     }
     bool GetKey(const CKeyID &address, CKey &keyOut) const override
     {
diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h
@@ -16,13 +16,13 @@ const unsigned int WALLET_CRYPTO_IV_SIZE = 16;
 /**
  * Private key encryption is done based on a CMasterKey,
  * which holds a salt and random encryption key.
- * 
+ *
  * CMasterKeys are encrypted using AES-256-CBC using a key
  * derived using derivation method nDerivationMethod
  * (0 == EVP_sha512()) and derivation iterations nDeriveIterations.
  * vchOtherDerivationParameters is provided for alternative algorithms
  * which may require more parameters (such as scrypt).
- * 
+ *
  * Wallet Private Keys are then encrypted using AES-256-CBC
  * with the double-sha256 of the public key as the IV, and the
  * master key's key as the encryption key (see keystore.[ch]).
@@ -162,28 +162,25 @@ class CCryptoKeyStore : public CBasicKeyStore
     {
         {
             LOCK(cs_KeyStore);
-            if (!IsCrypted())
+            if (!IsCrypted()) {
                 return CBasicKeyStore::HaveKey(address);
+            }
             return mapCryptedKeys.count(address) > 0;
         }
         return false;
     }
     bool GetKey(const CKeyID &address, CKey& keyOut) const override;
     bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const override;
-    void GetKeys(std::set<CKeyID> &setAddress) const override
+    std::set<CKeyID> GetKeys() const override
     {
-        if (!IsCrypted())
-        {
-            CBasicKeyStore::GetKeys(setAddress);
-            return;
+        if (!IsCrypted()) {
+            return CBasicKeyStore::GetKeys();
         }
-        setAddress.clear();
-        CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin();
-        while (mi != mapCryptedKeys.end())
-        {
-            setAddress.insert((*mi).first);
-            mi++;
+        std::set<CKeyID> set_address;
+        for (const auto& mi : mapCryptedKeys) {
+            set_address.insert(mi.first);
         }
+        return set_address;
     }
 
     /**
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -3660,13 +3660,10 @@ void CWallet::GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) c
     // map in which we'll infer heights of other keys
     CBlockIndex *pindexMax = chainActive[std::max(0, chainActive.Height() - 144)]; // the tip can be reorganized; use a 144-block safety margin
     std::map<CKeyID, CBlockIndex*> mapKeyFirstBlock;
-    std::set<CKeyID> setKeys;
-    GetKeys(setKeys);
-    for (const CKeyID &keyid : setKeys) {
+    for (const CKeyID &keyid : GetKeys()) {
         if (mapKeyBirth.count(keyid) == 0)
             mapKeyFirstBlock[keyid] = pindexMax;
     }
-    setKeys.clear();
 
     // if there are no such keys, we're done
     if (mapKeyFirstBlock.empty())

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ class CKeyStore`
`30`	`30`	`//! Check whether a key corresponding to a given address is present in the store.`
`31`	`31`	`virtual bool HaveKey(const CKeyID &address) const =0;`
`32`	`32`	`virtual bool GetKey(const CKeyID &address, CKey& keyOut) const =0;`
`33`		`- virtual void GetKeys(std::set<CKeyID> &setAddress) const =0;`
	`33`	`+ virtual std::set<CKeyID> GetKeys() const =0;`
`34`	`34`	`virtual bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const =0;`
`35`	`35`
`36`	`36`	`//! Support for BIP 0013 : see https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki`
`@@ -71,18 +71,14 @@ class CBasicKeyStore : public CKeyStore`
`71`	`71`	`}`
`72`	`72`	`return result;`
`73`	`73`	`}`
`74`		`- void GetKeys(std::set<CKeyID> &setAddress) const override`
	`74`	`+ std::set<CKeyID> GetKeys() const override`
`75`	`75`	`{`
`76`		`- setAddress.clear();`
`77`		`- {`
`78`		`- LOCK(cs_KeyStore);`
`79`		`- KeyMap::const_iterator mi = mapKeys.begin();`
`80`		`- while (mi != mapKeys.end())`
`81`		`- {`
`82`		`- setAddress.insert((*mi).first);`
`83`		`- mi++;`
`84`		`- }`
	`76`	`+ LOCK(cs_KeyStore);`
	`77`	`+ std::set<CKeyID> set_address;`
	`78`	`+ for (const auto& mi : mapKeys) {`
	`79`	`+ set_address.insert(mi.first);`
`85`	`80`	`}`
	`81`	`+ return set_address;`
`86`	`82`	`}`
`87`	`83`	`bool GetKey(const CKeyID &address, CKey &keyOut) const override`
`88`	`84`	`{`