refactor: use std:: prefix for std lib funcs #25172

We currently use a mix of std::mem*( and mem*( throughout the codebase. Consolidate to std::mem*(.

Follows up to a comment in #24962:

Maybe switch to std::memmove, which comes with the documentation:

Also includes a change to std::vsnprintf and header changes required to perform compilation.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #26177 (refactor / kernel: Move non-gArgs chainparams functionality to kernel by TheCharlatan)
• #26153 (Reduce wasted pseudorandom bytes in ChaCha20 + various improvements by sipa)
• #25903 (Use static member functions from class instead of instances by aureleoules)
• #25830 (refactor: Replace m_params with chainman.GetParams() by aureleoules)
• #25361 (BIP324: Cipher suite by dhruv)
• #25152 (refactor: Split util/system into exception, shell, and fs-specific files by Empact)
• #24994 (build: Build libbitcoinconsensus from its own convenience library by hebasto)
• #23561 (BIP324: Handshake prerequisites by dhruv)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

I believe some were missed:

<details> <summary>Git diff</summary>

diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index 31f8eadf9..cd124d157 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -92,7 +92,7 @@ void DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true)
     unsigned char pchMsgTmp[4];
     verifier >> pchMsgTmp;
     // ... verify the network matches ours
-    if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) {
+    if (std::memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) {
         throw std::runtime_error{"Invalid network magic number"};
     }
 
diff --git a/src/base58.cpp b/src/base58.cpp
index 11c1ce739..de9a82d46 100644
--- a/src/base58.cpp
+++ b/src/base58.cpp
@@ -150,7 +150,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input)
     }
     // re-calculate the checksum, ensure it matches the included 4-byte checksum
     uint256 hash = Hash(Span{vchRet}.first(vchRet.size() - 4));
-    if (memcmp(&hash, &vchRet[vchRet.size() - 4], 4) != 0) {
+    if (std::memcmp(&hash, &vchRet[vchRet.size() - 4], 4) != 0) {
         vchRet.clear();
         return false;
     }
diff --git a/src/net.cpp b/src/net.cpp
index 88f4cef5b..47ccede26 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -732,7 +732,7 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
     }
 
     // Check start string, network magic
-    if (memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
+    if (std::memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
         LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id);
         return -1;
     }
@@ -793,7 +793,7 @@ CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds
     RandAddEvent(ReadLE32(hash.begin()));
 
     // Check checksum and header message type string
-    if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
+    if (std::memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
         LogPrint(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n",
                  SanitizeString(msg.m_type), msg.m_message_size,
                  HexStr(Span{hash}.first(CMessageHeader::CHECKSUM_SIZE)),
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 17ab226a3..88a90e256 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -768,7 +768,7 @@ bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, c
 
         filein >> blk_start >> blk_size;
 
-        if (memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) {
+        if (std::memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) {
             return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(),
                          HexStr(blk_start),
                          HexStr(message_start));
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index c4d13d728..1cabf65ac 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1877,7 +1877,7 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
             exec_script = CScript(script_bytes.begin(), script_bytes.end());
             uint256 hash_exec_script;
             CSHA256().Write(exec_script.data(), exec_script.size()).Finalize(hash_exec_script.begin());
-            if (memcmp(hash_exec_script.begin(), program.data(), 32)) {
+            if (std::memcmp(hash_exec_script.begin(), program.data(), 32)) {
                 return set_error(serror, SCRIPT_ERR_WITNESS_PROGRAM_MISMATCH);
             }
             return ExecuteWitnessScript(stack, exec_script, flags, SigVersion::WITNESS_V0, checker, execdata, serror);
diff --git a/src/validation.cpp b/src/validation.cpp
index fefbe033e..b3fcba2cd 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3548,7 +3548,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat
                 return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__));
             }
             CHash256().Write(hashWitness).Write(block.vtx[0]->vin[0].scriptWitness.stack[0]).Finalize(hashWitness);
-            if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
+            if (std::memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
                 return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
             }
             fHaveWitness = true;
@@ -4294,7 +4294,7 @@ void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp)
                 blkdat.FindByte(m_params.MessageStart()[0]);
                 nRewind = blkdat.GetPos() + 1;
                 blkdat >> buf;
-                if (memcmp(buf, m_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {
+                if (std::memcmp(buf, m_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {
                     continue;
                 }
                 // read size

</details>

I believe some were missed:

Thanks. Have addressed more cases.

ACK 7749a28

No problem with this, I agree it's good to be consistent here, though not generally a fan of 'all-over-the-place' PRs.

I do think we need a way to enforce this to prevent reintroducing unqualified std functions, for example adapt one of the linters.

In the spirit of "Include what you use", we should also ensure we are including the appropriate headers for these functions, <cstring> for std::mem* and <cstdlib> for std::getenv.

Strong ACK https://github.com/bitcoin/bitcoin/pull/25172/commits/7749a284ebb796b31e24bca8ee0308049a752105

Concept ACK.

Was #25172 (comment) addressed?

Was #25172 (comment) addressed?

Not yet, but I am going to address this via clang-tidy plugin, rather than a python linter. Although there a few steps before that will be available.

Looks like Clang is moving in the direction of adding warnings for unqualified std function usage, beginning with std::move and std::forward. Starting with LLVM 15 there will be an -Wunqualified-std-cast-call warning. See https://reviews.llvm.org/D119670.

ACK 8dd3c4aa74c064ae8a926220480ce55eba2517f3, I have reviewed the code and it looks OK, I agree it can be merged.

glozow removed the Waiting for author label 21 hours ago

Well, if we want the changes, I'd still prefer to enforce them. If we don't, then we could close this pull.

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}