upleb.uk

Public git repos — served from a NIP-34 GRASP relay at git.upleb.uk

summaryrefslogtreecommitdiff
path: root/src/main.h
diff options
context:
space:
mode:
authorBitflash <btf01@proton.me>2026-07-30 10:38:12 -0300
committerGitHub <noreply@github.com>2026-07-30 10:38:12 -0300
commit2ed9aed2ea9e302b3b36b37806ec03f2cccaaed1 (patch)
treeab30eade9435c074ee08c70303b8a15c08770b4f /src/main.h
parentf30f45849ecd45be6a073b6b1d308e1c12b8b52b (diff)
build: check format strings, which nothing ever did (#44)
The segfault in issue #5 was a format string: `%I64d` is MSVC-only, glibc parses it as the GNU `I` flag with a width of 64 and an `int` conversion, the vararg run desynchronises, and the following `%s` dereferences an integer. It killed the node mid-sync at the first block carrying a real transaction, deterministically. GCC can catch this. It never got the chance, for two reasons that had to be fixed together: - `-w` sat in both makefiles and suppressed every diagnostic. It cannot be narrowed; it wins over any -W or -Werror= that follows, so it had to go. The four -Wno- entries replacing it are the categories this 2009-era tree actually produces (measured: 248 deprecated-declarations, 157 write-strings, 18 narrowing, 1 unused-result), silenced by name instead of wholesale. - Even with -Wformat on, nothing was checked. GCC only inspects format strings for functions it knows are printf-like, and every format string here goes through strprintf, error, my_snprintf, or OutputDebugStringF -- `printf` itself is #defined to the last of those. None carried a format attribute, so the entire tree was invisible to the check. They are annotated now. Annotating surfaced 77 warnings across 17 distinct sites, all fixed here. Most were size_t or int64 passed to %d, harmless on x86-64 but wrong. Two were not cosmetic: - CBlockIndex::ToString printed pprev/pnext with %08x, truncating both pointers to 32 bits, so the two fields that line exists to show were unreliable on every 64-bit build. - CInv::GetCommand threw "type=% unknown type", where the % ate the space as a flag and the u as its conversion. The message read "type= 3nknown type". PRId64 is now one spelling for every target. Its MSVCRT branch was what produced "I64d" in the first place, it is pointless against UCRT which takes the C99 forms, the rest of the tree already prints int64 with %lld on Windows, and its only user (i64tostr) is dead code. Collapsing it lets both builds share the gnu_printf archetype -- ms_printf would reject %zu, which this tree uses and UCRT supports, while waving through the %I64d that crashes. Verified on both platforms: clean build, and reintroducing the issue #5 specifier now fails compilation rather than shipping. main.h:410:50: error: unknown conversion type character 'I' in format main.h:410:70: error: format '%s' expects argument of type 'char*', but argument 3 has type 'long long int' That second line is the crash itself, caught at build time. Runtime checked on both: node syncs past the block from issue #5, and the repaired log lines read correctly -- before: CTxOut(nValue= 50.%08I64d, scriptPubKey=(null)) after: CTxOut(nValue=50.00000000, scriptPubKey=0xE1C7EF3E706002) Four '_WIN32_WINNT redefined' warnings remain on Windows. Silencing them means letting a system header pick the Windows API level instead of this tree, which is a separate decision from format checking.
Diffstat (limited to 'src/main.h')
-rw-r--r--src/main.h14
1 files changed, 8 insertions, 6 deletions
diff --git a/src/main.h b/src/main.h
index fde3a4e..7e5bdf2 100644
--- a/src/main.h
+++ b/src/main.h
@@ -650,11 +650,11 @@ public:
650 string ToString() const 650 string ToString() const
651 { 651 {
652 string str; 652 string str;
653 str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%d, vout.size=%d, nLockTime=%d)\n", 653 str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%d, vout.size=%d, nLockTime=%u)\n",
654 GetHash().ToString().substr(0,6).c_str(), 654 GetHash().ToString().substr(0,6).c_str(),
655 nVersion, 655 nVersion,
656 vin.size(), 656 (int)vin.size(),
657 vout.size(), 657 (int)vout.size(),
658 nLockTime); 658 nLockTime);
659 for (int i = 0; i < vin.size(); i++) 659 for (int i = 0; i < vin.size(); i++)
660 str += " " + vin[i].ToString() + "\n"; 660 str += " " + vin[i].ToString() + "\n";
@@ -1081,7 +1081,7 @@ public:
1081 hashPrevBlock.ToString().substr(0,14).c_str(), 1081 hashPrevBlock.ToString().substr(0,14).c_str(),
1082 hashMerkleRoot.ToString().substr(0,6).c_str(), 1082 hashMerkleRoot.ToString().substr(0,6).c_str(),
1083 nTime, nBits, nNonce, 1083 nTime, nBits, nNonce,
1084 vtx.size()); 1084 (int)vtx.size());
1085 for (int i = 0; i < vtx.size(); i++) 1085 for (int i = 0; i < vtx.size(); i++)
1086 { 1086 {
1087 printf(" "); 1087 printf(" ");
@@ -1223,8 +1223,10 @@ public:
1223 1223
1224 string ToString() const 1224 string ToString() const
1225 { 1225 {
1226 return strprintf("CBlockIndex(nprev=%08x, pnext=%08x, nFile=%d, nBlockPos=%-6d nHeight=%d, merkle=%s, hashBlock=%s)", 1226 // pprev/pnext are pointers: %08x truncated them to 32 bits on a 64-bit
1227 pprev, pnext, nFile, nBlockPos, nHeight, 1227 // build, so the two fields this line exists to show were unreliable.
1228 return strprintf("CBlockIndex(nprev=%p, pnext=%p, nFile=%d, nBlockPos=%-6d nHeight=%d, merkle=%s, hashBlock=%s)",
1229 (void*)pprev, (void*)pnext, nFile, nBlockPos, nHeight,
1228 hashMerkleRoot.ToString().substr(0,6).c_str(), 1230 hashMerkleRoot.ToString().substr(0,6).c_str(),
1229 GetBlockHash().ToString().substr(0,14).c_str()); 1231 GetBlockHash().ToString().substr(0,14).c_str());
1230 } 1232 }