LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Packages Projects Releases 32 Wiki Activity

Merge #13815: util: Add [[nodiscard]] to all {Decode,Parse}[...](...) functions returning bool

Browse source 
9cc0230cfc Add NODISCARD to all {Decode,Parse}[...](...) functions returning bool. Sort includes. (practicalswift)
579497e77a tests: Explicitly ignore the return value of DecodeBase58(...) (practicalswift)
145fe95ec7 tests: Check return value of ParseParameters(...) (practicalswift)
7c5bc2a523 miner: Default to DEFAULT_BLOCK_MIN_TX_FEE if unable to parse -blockmintxfee (practicalswift)

Pull request description:

  Changes in this PR:
  * ~~Add linter to make sure the return value of `Parse[...](...)` is checked~~
  * Add `__attribute__((warn_unused_result))` to all `{Decode,Parse}[...](...)` functions returning `bool`
  * Fix violations

  Context:
  * #13712: `wallet: Fix non-determinism in ParseHDKeypath(...). Avoid using an uninitialized variable in path calculation.` would have been prevented by this

Tree-SHA512: 41a97899f2d5a26584235fa02b1ebfb4faacd81ea97e927022955a658fa7e15d07a1443b4b7635151a43259a1adf8f2f4de3c1c75d7b5f09f0d5496463a1dae6
This commit is contained in:
MarcoFalke 2018-11-15 14:37:13 -05:00
parent e74649e951 9cc0230cfc
commit 384967f311
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
15 changed files with 84 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/Makefile.am
View file

@ -95,6 +95,7 @@ endif
BITCOIN_CORE_H = \
addrdb.h \
addrman.h \
attributes.h \
base58.h \
bech32.h \
bloom.h \

22
src/attributes.h Normal file
View file

@ -0,0 +1,22 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_ATTRIBUTES_H
#define BITCOIN_ATTRIBUTES_H
#if defined(__has_cpp_attribute)
# if __has_cpp_attribute(nodiscard)
# define NODISCARD [[nodiscard]]
# endif
#endif
#ifndef NODISCARD
# if defined(_MSC_VER) && _MSC_VER >= 1700
# define NODISCARD _Check_return_
# else
# define NODISCARD __attribute__((warn_unused_result))
# endif
#endif
#endif // BITCOIN_ATTRIBUTES_H

10
src/base58.h
View file

@ -14,6 +14,8 @@
#ifndef BITCOIN_BASE58_H
#define BITCOIN_BASE58_H
#include <attributes.h>
#include <string>
#include <vector>
@ -33,13 +35,13 @@ std::string EncodeBase58(const std::vector<unsigned char>& vch);
* return true if decoding is successful.
* psz cannot be nullptr.
*/
bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet);
NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet);
/**
* Decode a base58-encoded string (str) into a byte vector (vchRet).
* return true if decoding is successful.
*/
bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet);
NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet);
/**
* Encode a byte vector into a base58-encoded string, including checksum
@ -50,12 +52,12 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn);
* Decode a base58-encoded string (psz) that includes a checksum into a byte
* vector (vchRet), return true if decoding is successful
*/
bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet);
NODISCARD bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet);
/**
* Decode a base58-encoded string (str) that includes a checksum into a byte
* vector (vchRet), return true if decoding is successful
*/
bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet);
NODISCARD bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet);
#endif // BITCOIN_BASE58_H

2
src/bench/base58.cpp
View file

@ -49,7 +49,7 @@ static void Base58Decode(benchmark::State& state)
const char* addr = "17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem";
std::vector<unsigned char> vch;
while (state.KeepRunning()) {
DecodeBase58(addr, vch);
(void) DecodeBase58(addr, vch);
}
}

7
src/core_io.h
View file

@ -6,6 +6,7 @@
#define BITCOIN_CORE_IO_H
#include <amount.h>
#include <attributes.h>
#include <string>
#include <vector>
@ -22,8 +23,8 @@ class UniValue;
// core_read.cpp
CScript ParseScript(const std::string& s);
std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false);
bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true);
bool DecodeHexBlk(CBlock&, const std::string& strHexBlk);
NODISCARD bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true);
NODISCARD bool DecodeHexBlk(CBlock&, const std::string& strHexBlk);
bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
/**
@ -36,7 +37,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
*/
bool ParseHashStr(const std::string& strHex, uint256& result);
std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName);
bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error);
NODISCARD bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error);
int ParseSighashString(const UniValue& sighash);
// core_write.cpp

5
src/miner.cpp
View file

@ -70,9 +70,8 @@ static BlockAssembler::Options DefaultOptions()
// If -blockmaxweight is not given, limit to DEFAULT_BLOCK_MAX_WEIGHT
BlockAssembler::Options options;
options.nBlockMaxWeight = gArgs.GetArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
if (gArgs.IsArgSet("-blockmintxfee")) {
CAmount n = 0;
ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n);
CAmount n = 0;
if (gArgs.IsArgSet("-blockmintxfee") && ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n)) {
options.blockMinFeeRate = CFeeRate(n);
} else {
options.blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE);

3
src/outputtype.h
View file

@ -6,6 +6,7 @@
#ifndef BITCOIN_OUTPUTTYPE_H
#define BITCOIN_OUTPUTTYPE_H
#include <attributes.h>
#include <keystore.h>
#include <script/standard.h>
@ -26,7 +27,7 @@ enum class OutputType {
CHANGE_AUTO,
};
bool ParseOutputType(const std::string& str, OutputType& output_type);
NODISCARD bool ParseOutputType(const std::string& str, OutputType& output_type);
const std::string& FormatOutputType(OutputType type);
/**

5
src/rest.cpp
View file

@ -3,20 +3,21 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <attributes.h>
#include <chain.h>
#include <chainparams.h>
#include <core_io.h>
#include <httpserver.h>
#include <index/txindex.h>
#include <primitives/block.h>
#include <primitives/transaction.h>
#include <validation.h>
#include <httpserver.h>
#include <rpc/blockchain.h>
#include <rpc/server.h>
#include <streams.h>
#include <sync.h>
#include <txmempool.h>
#include <util/strencodings.h>
#include <validation.h>
#include <version.h>
#include <boost/algorithm/string.hpp>

2
src/script/descriptor.cpp
View file

@ -478,7 +478,7 @@ std::vector<Span<const char>> Split(const Span<const char>& sp, char sep)
}
/** Parse a key path, being passed a split list of elements (the first element is ignored). */
bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out)
NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out)
{
for (size_t i = 1; i < split.size(); ++i) {
Span<const char> elem = split[i];

2
src/test/getarg_tests.cpp
View file

@ -28,7 +28,7 @@ static void ResetArgs(const std::string& strArg)
vecChar.push_back(s.c_str());
std::string error;
gArgs.ParseParameters(vecChar.size(), vecChar.data(), error);
BOOST_CHECK(gArgs.ParseParameters(vecChar.size(), vecChar.data(), error));
}
static void SetupArgs(const std::vector<std::string>& args)

44
src/test/util_tests.cpp
View file

@ -210,13 +210,13 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
std::string error;
testArgs.SetupArgs(4, avail_args);
testArgs.ParseParameters(0, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(0, (char**)argv_test, error));
BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
testArgs.ParseParameters(1, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error));
BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
testArgs.ParseParameters(7, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error));
// expectation: -ignored is ignored (program name argument),
// -a, -b and -ccc end up in map, -d ignored because it is after
// a non-option argument (non-GNU option parsing)
@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
"ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
std::string error;
testArgs.SetupArgs(6, avail_args);
testArgs.ParseParameters(7, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error));
// Each letter should be set.
for (const char opt : "abcdef")
@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"};
testArgs.SetupArgs(2, avail_args);
std::string error;
testArgs.ParseParameters(4, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(4, (char**)argv_test, error));
// This was passed twice, second one overrides the negative setting.
BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
@ -290,7 +290,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
// Config test
const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n";
testArgs.ParseParameters(1, (char**)argv_test, error);
BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error));
testArgs.ReadConfigString(conf_test);
// This was passed twice, second one overrides the negative setting,
@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
// Combined test
const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"};
const char *combo_test_conf = "foo=1\nnobar=1\n";
testArgs.ParseParameters(3, (char**)combo_test_args, error);
BOOST_CHECK(testArgs.ParseParameters(3, (char**)combo_test_args, error));
testArgs.ReadConfigString(combo_test_conf);
// Command line overrides, but doesn't erase old setting
@ -557,38 +557,38 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1";
std::string error;
test_args.ParseParameters(0, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error));
BOOST_CHECK_EQUAL(test_args.GetChainName(), "main");
test_args.ParseParameters(2, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error));
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(2, (char**)argv_regtest, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error));
BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest");
test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error));
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(3, (char**)argv_both, error);
BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error));
BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
test_args.ParseParameters(0, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(2, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(2, (char**)argv_regtest, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(3, (char**)argv_both, error);
BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
@ -596,23 +596,23 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
// [test] regtest=1 potentially relevant) doesn't break things
test_args.SelectConfigNetwork("test");
test_args.ParseParameters(0, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(2, (char**)argv_testnet, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(2, (char**)argv_regtest, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
test_args.ParseParameters(2, (char**)argv_test_no_reg, error);
BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_test_no_reg, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
test_args.ParseParameters(3, (char**)argv_both, error);
BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error));
test_args.ReadConfigString(testnetconf);
BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
}

11
src/util/moneystr.h
View file

@ -9,16 +9,17 @@
#ifndef BITCOIN_UTIL_MONEYSTR_H
#define BITCOIN_UTIL_MONEYSTR_H
#include <stdint.h>
#include <string>
#include <amount.h>
#include <attributes.h>
#include <cstdint>
#include <string>
/* Do not use these functions to represent or parse monetary amounts to or from
* JSON but use AmountFromValue and ValueFromAmount for that.
*/
std::string FormatMoney(const CAmount& n);
bool ParseMoney(const std::string& str, CAmount& nRet);
bool ParseMoney(const char* pszIn, CAmount& nRet);
NODISCARD bool ParseMoney(const std::string& str, CAmount& nRet);
NODISCARD bool ParseMoney(const char* pszIn, CAmount& nRet);
#endif // BITCOIN_UTIL_MONEYSTR_H

2
src/util/strencodings.cpp
View file

@ -263,7 +263,7 @@ std::string DecodeBase32(const std::string& str)
return std::string((const char*)vchRet.data(), vchRet.size());
}
static bool ParsePrechecks(const std::string& str)
NODISCARD static bool ParsePrechecks(const std::string& str)
{
if (str.empty()) // No empty string allowed
return false;

18
src/util/strencodings.h
View file

@ -9,7 +9,9 @@
#ifndef BITCOIN_UTIL_STRENCODINGS_H
#define BITCOIN_UTIL_STRENCODINGS_H
#include <stdint.h>
#include <attributes.h>
#include <cstdint>
#include <string>
#include <vector>
@ -92,35 +94,35 @@ constexpr inline bool IsSpace(char c) noexcept {
* @returns true if the entire string could be parsed as valid integer,
* false if not the entire string could be parsed or when overflow or underflow occurred.
*/
bool ParseInt32(const std::string& str, int32_t *out);
NODISCARD bool ParseInt32(const std::string& str, int32_t *out);
/**
* Convert string to signed 64-bit integer with strict parse error feedback.
* @returns true if the entire string could be parsed as valid integer,
* false if not the entire string could be parsed or when overflow or underflow occurred.
*/
bool ParseInt64(const std::string& str, int64_t *out);
NODISCARD bool ParseInt64(const std::string& str, int64_t *out);
/**
* Convert decimal string to unsigned 32-bit integer with strict parse error feedback.
* @returns true if the entire string could be parsed as valid integer,
* false if not the entire string could be parsed or when overflow or underflow occurred.
*/
bool ParseUInt32(const std::string& str, uint32_t *out);
NODISCARD bool ParseUInt32(const std::string& str, uint32_t *out);
/**
* Convert decimal string to unsigned 64-bit integer with strict parse error feedback.
* @returns true if the entire string could be parsed as valid integer,
* false if not the entire string could be parsed or when overflow or underflow occurred.
*/
bool ParseUInt64(const std::string& str, uint64_t *out);
NODISCARD bool ParseUInt64(const std::string& str, uint64_t *out);
/**
* Convert string to double with strict parse error feedback.
* @returns true if the entire string could be parsed as valid double,
* false if not the entire string could be parsed or when overflow or underflow occurred.
*/
bool ParseDouble(const std::string& str, double *out);
NODISCARD bool ParseDouble(const std::string& str, double *out);
template<typename T>
std::string HexStr(const T itbegin, const T itend, bool fSpaces=false)
@ -173,7 +175,7 @@ bool TimingResistantEqual(const T& a, const T& b)
* @returns true on success, false on error.
* @note The result must be in the range (-10^18,10^18), otherwise an overflow error will trigger.
*/
bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out);
NODISCARD bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out);
/** Convert from one power-of-2 number base to another. */
template<int frombits, int tobits, bool pad, typename O, typename I>
@ -200,7 +202,7 @@ bool ConvertBits(const O& outfn, I it, I end) {
}
/** Parse an HD keypaths like "m/7/0'/2000". */
bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath);
NODISCARD bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath);
/**
* Converts the given character to its lowercase equivalent.

3
src/util/system.h
View file

@ -14,6 +14,7 @@
#include <config/bitcoin-config.h>
#endif
#include <attributes.h>
#include <compat.h>
#include <fs.h>
#include <logging.h>
@ -159,7 +160,7 @@ public:
*/
void SelectConfigNetwork(const std::string& network);
bool ParseParameters(int argc, const char* const argv[], std::string& error);
NODISCARD bool ParseParameters(int argc, const char* const argv[], std::string& error);
bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false);
/**