Merge #20429: refactor: replace (sizeof(a)/sizeof(a[0])) with C++17 std::size

e829c9afbf refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c846 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968 refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)

Pull request description:

  This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size)  (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.

  As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
  * all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
  * `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).

ACKs for top commit:
  practicalswift:
    cr ACK e829c9afbf: patch looks correct
  fanquake:
    ACK e829c9afbf
  MarcoFalke:
    review ACK e829c9afbf 🌩

Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
This commit is contained in:
MarcoFalke 2021-02-18 07:53:27 +01:00
commit cd66d8b1d8
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
15 changed files with 41 additions and 51 deletions

View File

@ -52,7 +52,7 @@ static const int8_t mapBase58[256] = {
int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up.
std::vector<unsigned char> b256(size);
// Process the characters.
static_assert(sizeof(mapBase58)/sizeof(mapBase58[0]) == 256, "mapBase58.size() should be 256"); // guarantee not out of range
static_assert(std::size(mapBase58) == 256, "mapBase58.size() should be 256"); // guarantee not out of range
while (*psz && !IsSpace(*psz)) {
// Decode base58 character
int carry = mapBase58[(uint8_t)*psz];

View File

@ -8,7 +8,7 @@ namespace benchmark {
namespace data {
#include <bench/data/block413567.raw.h>
const std::vector<uint8_t> block413567{block413567_raw, block413567_raw + sizeof(block413567_raw) / sizeof(block413567_raw[0])};
const std::vector<uint8_t> block413567{std::begin(block413567_raw), std::end(block413567_raw)};
} // namespace data
} // namespace benchmark

View File

@ -9,7 +9,6 @@
#include <consensus/merkle.h>
#include <hash.h> // for signet block challenge hash
#include <util/system.h>
#include <util/strencodings.h>
#include <versionbitsinfo.h>
#include <assert.h>
@ -135,7 +134,7 @@ public:
bech32_hrp = "bc";
vFixedSeeds = std::vector<SeedSpec6>(pnSeed6_main, pnSeed6_main + ARRAYLEN(pnSeed6_main));
vFixedSeeds = std::vector<SeedSpec6>(std::begin(pnSeed6_main), std::end(pnSeed6_main));
fDefaultConsistencyChecks = false;
fRequireStandard = true;
@ -240,7 +239,7 @@ public:
bech32_hrp = "tb";
vFixedSeeds = std::vector<SeedSpec6>(pnSeed6_test, pnSeed6_test + ARRAYLEN(pnSeed6_test));
vFixedSeeds = std::vector<SeedSpec6>(std::begin(pnSeed6_test), std::end(pnSeed6_test));
fDefaultConsistencyChecks = false;
fRequireStandard = false;

View File

@ -5,7 +5,6 @@
#include <protocol.h>
#include <util/strencodings.h>
#include <util/system.h>
static std::atomic<bool> g_initial_block_download_completed(false);
@ -86,7 +85,7 @@ const static std::string allNetMessageTypes[] = {
NetMsgType::CFCHECKPT,
NetMsgType::WTXIDRELAY,
};
const static std::vector<std::string> allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes+ARRAYLEN(allNetMessageTypes));
const static std::vector<std::string> allNetMessageTypesVec(std::begin(allNetMessageTypes), std::end(allNetMessageTypes));
CMessageHeader::CMessageHeader()
{

View File

@ -22,7 +22,6 @@ static const struct {
{"signet", QAPP_APP_NAME_SIGNET, 35, 15},
{"regtest", QAPP_APP_NAME_REGTEST, 160, 30},
};
static const unsigned network_styles_count = sizeof(network_styles)/sizeof(*network_styles);
// titleAddText needs to be const char* for tr()
NetworkStyle::NetworkStyle(const QString &_appName, const int iconColorHueShift, const int iconColorSaturationReduction, const char *_titleAddText):
@ -81,14 +80,12 @@ NetworkStyle::NetworkStyle(const QString &_appName, const int iconColorHueShift,
const NetworkStyle* NetworkStyle::instantiate(const std::string& networkId)
{
std::string titleAddText = networkId == CBaseChainParams::MAIN ? "" : strprintf("[%s]", networkId);
for (unsigned x=0; x<network_styles_count; ++x)
{
if (networkId == network_styles[x].networkId)
{
for (const auto& network_style : network_styles) {
if (networkId == network_style.networkId) {
return new NetworkStyle(
network_styles[x].appName,
network_styles[x].iconColorHueShift,
network_styles[x].iconColorSaturationReduction,
network_style.appName,
network_style.iconColorHueShift,
network_style.iconColorSaturationReduction,
titleAddText.c_str());
}
}

View File

@ -23,7 +23,6 @@ static const struct {
/* Other: linux, unix, ... */
{"other", true, true, false}
};
static const unsigned platform_styles_count = sizeof(platform_styles)/sizeof(*platform_styles);
namespace {
/* Local functions for colorizing single-color images */
@ -121,15 +120,13 @@ QIcon PlatformStyle::TextColorIcon(const QIcon& icon) const
const PlatformStyle *PlatformStyle::instantiate(const QString &platformId)
{
for (unsigned x=0; x<platform_styles_count; ++x)
{
if (platformId == platform_styles[x].platformId)
{
for (const auto& platform_style : platform_styles) {
if (platformId == platform_style.platformId) {
return new PlatformStyle(
platform_styles[x].platformId,
platform_styles[x].imagesOnButtons,
platform_styles[x].colorizeIcons,
platform_styles[x].useExtraSpacing);
platform_style.platformId,
platform_style.imagesOnButtons,
platform_style.colorizeIcons,
platform_style.useExtraSpacing);
}
}
return nullptr;

View File

@ -38,7 +38,6 @@
#include <sys/random.h>
#endif
#ifdef HAVE_SYSCTL_ARND
#include <util/strencodings.h> // for ARRAYLEN
#include <sys/sysctl.h>
#endif
@ -333,7 +332,7 @@ void GetOSRand(unsigned char *ent32)
int have = 0;
do {
size_t len = NUM_OS_RANDOM_BYTES - have;
if (sysctl(name, ARRAYLEN(name), ent32 + have, &len, nullptr, 0) != 0) {
if (sysctl(name, std::size(name), ent32 + have, &len, nullptr, 0) != 0) {
RandFailure();
}
have += len;

View File

@ -19,7 +19,6 @@
#include <txmempool.h>
#include <util/check.h>
#include <util/ref.h>
#include <util/strencodings.h>
#include <validation.h>
#include <version.h>
@ -117,9 +116,10 @@ static RetFormat ParseDataFormat(std::string& param, const std::string& strReq)
param = strReq.substr(0, pos);
const std::string suff(strReq, pos + 1);
for (unsigned int i = 0; i < ARRAYLEN(rf_names); i++)
if (suff == rf_names[i].name)
return rf_names[i].rf;
for (const auto& rf_name : rf_names) {
if (suff == rf_name.name)
return rf_name.rf;
}
/* If no suffix is found, return original string. */
param = strReq;
@ -129,12 +129,13 @@ static RetFormat ParseDataFormat(std::string& param, const std::string& strReq)
static std::string AvailableDataFormatsString()
{
std::string formats;
for (unsigned int i = 0; i < ARRAYLEN(rf_names); i++)
if (strlen(rf_names[i].name) > 0) {
for (const auto& rf_name : rf_names) {
if (strlen(rf_name.name) > 0) {
formats.append(".");
formats.append(rf_names[i].name);
formats.append(rf_name.name);
formats.append(", ");
}
}
if (formats.length() > 0)
return formats.substr(0, formats.length() - 2);
@ -695,6 +696,7 @@ void InterruptREST()
void StopREST()
{
for (unsigned int i = 0; i < ARRAYLEN(uri_prefixes); i++)
UnregisterHTTPHandler(uri_prefixes[i].prefix, false);
for (const auto& up : uri_prefixes) {
UnregisterHTTPHandler(up.prefix, false);
}
}

View File

@ -17,7 +17,7 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
static const std::string vstrIn[] = {"","f","fo","foo","foob","fooba","foobar"};
static const std::string vstrOut[] = {"","my======","mzxq====","mzxw6===","mzxw6yq=","mzxw6ytb","mzxw6ytboi======"};
static const std::string vstrOutNoPadding[] = {"","my","mzxq","mzxw6","mzxw6yq","mzxw6ytb","mzxw6ytboi"};
for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++)
for (unsigned int i=0; i<std::size(vstrIn); i++)
{
std::string strEnc = EncodeBase32(vstrIn[i]);
BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);

View File

@ -16,7 +16,7 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
{
static const std::string vstrIn[] = {"","f","fo","foo","foob","fooba","foobar"};
static const std::string vstrOut[] = {"","Zg==","Zm8=","Zm9v","Zm9vYg==","Zm9vYmE=","Zm9vYmFy"};
for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++)
for (unsigned int i=0; i<std::size(vstrIn); i++)
{
std::string strEnc = EncodeBase64(vstrIn[i]);
BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);

View File

@ -107,14 +107,14 @@ BOOST_AUTO_TEST_CASE(siphash)
// Check test vectors from spec, one byte at a time
CSipHasher hasher2(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
for (uint8_t x=0; x<ARRAYLEN(siphash_4_2_testvec); ++x)
for (uint8_t x=0; x<std::size(siphash_4_2_testvec); ++x)
{
BOOST_CHECK_EQUAL(hasher2.Finalize(), siphash_4_2_testvec[x]);
hasher2.Write(&x, 1);
}
// Check test vectors from spec, eight bytes at a time
CSipHasher hasher3(0x0706050403020100ULL, 0x0F0E0D0C0B0A0908ULL);
for (uint8_t x=0; x<ARRAYLEN(siphash_4_2_testvec); x+=8)
for (uint8_t x=0; x<std::size(siphash_4_2_testvec); x+=8)
{
BOOST_CHECK_EQUAL(hasher3.Finalize(), siphash_4_2_testvec[x]);
hasher3.Write(uint64_t(x)|(uint64_t(x+1)<<8)|(uint64_t(x+2)<<16)|(uint64_t(x+3)<<24)|

View File

@ -219,11 +219,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
// We can't make transactions until we have inputs
// Therefore, load 110 blocks :)
static_assert(sizeof(blockinfo) / sizeof(*blockinfo) == 110, "Should have 110 blocks to import");
static_assert(std::size(blockinfo) == 110, "Should have 110 blocks to import");
int baseheight = 0;
std::vector<CTransactionRef> txFirst;
for (unsigned int i = 0; i < sizeof(blockinfo)/sizeof(*blockinfo); ++i)
{
for (const auto& bi : blockinfo) {
CBlock *pblock = &pblocktemplate->block; // pointer for convenience
{
LOCK(cs_main);
@ -232,7 +231,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
CMutableTransaction txCoinbase(*pblock->vtx[0]);
txCoinbase.nVersion = 1;
txCoinbase.vin[0].scriptSig = CScript();
txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce);
txCoinbase.vin[0].scriptSig.push_back(bi.extranonce);
txCoinbase.vin[0].scriptSig.push_back(::ChainActive().Height());
txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this)
txCoinbase.vout[0].scriptPubKey = CScript();
@ -242,7 +241,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
if (txFirst.size() < 4)
txFirst.push_back(pblock->vtx[0]);
pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
pblock->nNonce = blockinfo[i].nonce;
pblock->nNonce = bi.nonce;
}
std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr));

View File

@ -164,9 +164,9 @@ static void RunOperators(const int64_t& num1, const int64_t& num2)
BOOST_AUTO_TEST_CASE(creation)
{
for(size_t i = 0; i < sizeof(values) / sizeof(values[0]); ++i)
for(size_t i = 0; i < std::size(values); ++i)
{
for(size_t j = 0; j < sizeof(offsets) / sizeof(offsets[0]); ++j)
for(size_t j = 0; j < std::size(offsets); ++j)
{
RunCreate(values[i]);
RunCreate(values[i] + offsets[j]);
@ -177,9 +177,9 @@ BOOST_AUTO_TEST_CASE(creation)
BOOST_AUTO_TEST_CASE(operators)
{
for(size_t i = 0; i < sizeof(values) / sizeof(values[0]); ++i)
for(size_t i = 0; i < std::size(values); ++i)
{
for(size_t j = 0; j < sizeof(offsets) / sizeof(offsets[0]); ++j)
for(size_t j = 0; j < std::size(offsets); ++j)
{
RunOperators(values[i], values[i]);
RunOperators(values[i], -values[i]);

View File

@ -88,7 +88,7 @@ void static RandomScript(CScript &script) {
script = CScript();
int ops = (InsecureRandRange(10));
for (int i=0; i<ops; i++)
script << oplist[InsecureRandRange(sizeof(oplist)/sizeof(oplist[0]))];
script << oplist[InsecureRandRange(std::size(oplist))];
}
void static RandomTransaction(CMutableTransaction &tx, bool fSingle) {

View File

@ -17,8 +17,6 @@
#include <string>
#include <vector>
#define ARRAYLEN(array) (sizeof(array)/sizeof((array)[0]))
/** Used by SanitizeString() */
enum SafeChars
{