From b0a90fbb0c1085646111f7945b9a5b4af0fd3c78 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 9 Sep 2012 14:43:06 +0200 Subject: [PATCH 1/4] Add printf-style warnings to strprintf() and OutputDebugStringF() This finds about ~150 potential problems with format characters on a 64 bit build. --- src/util.cpp | 15 ++++++++++++--- src/util.h | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index d1270348e0e..c9edc782c85 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -274,7 +274,7 @@ inline int OutputDebugStringF(const char* pszFormat, ...) return ret; } -string vstrprintf(const std::string &format, va_list ap) +string vstrprintf(const char *format, va_list ap) { char buffer[50000]; char* p = buffer; @@ -284,7 +284,7 @@ string vstrprintf(const std::string &format, va_list ap) { va_list arg_ptr; va_copy(arg_ptr, ap); - ret = _vsnprintf(p, limit, format.c_str(), arg_ptr); + ret = _vsnprintf(p, limit, format, arg_ptr); va_end(arg_ptr); if (ret >= 0 && ret < limit) break; @@ -301,7 +301,7 @@ string vstrprintf(const std::string &format, va_list ap) return str; } -string real_strprintf(const std::string &format, int dummy, ...) +string real_strprintf(const char *format, int dummy, ...) { va_list arg_ptr; va_start(arg_ptr, dummy); @@ -310,6 +310,15 @@ string real_strprintf(const std::string &format, int dummy, ...) return str; } +string real_strprintf(const std::string &format, int dummy, ...) +{ + va_list arg_ptr; + va_start(arg_ptr, dummy); + string str = vstrprintf(format.c_str(), arg_ptr); + va_end(arg_ptr); + return str; +} + bool error(const char *format, ...) { va_list arg_ptr; diff --git a/src/util.h b/src/util.h index 65923e68a32..b75c3bff4e9 100644 --- a/src/util.h +++ b/src/util.h @@ -41,7 +41,6 @@ static const int64 CENT = 1000000; #define UBEGIN(a) ((unsigned char*)&(a)) #define UEND(a) ((unsigned char*)&((&(a))[1])) #define ARRAYLEN(array) (sizeof(array)/sizeof((array)[0])) -#define printf OutputDebugStringF #ifndef PRI64d #if defined(_MSC_VER) || defined(__MSVCRT__) @@ -94,6 +93,15 @@ inline void Sleep(int64 n) } #endif +/* This GNU C extension enables the compiler to check the format string against the parameters provided. + * X is the number of the "format string" parameter, and Y is the number of the first variadic parameter. + * Parameters count from 1. + */ +#ifdef __GNUC__ +#define ATTR_WARN_PRINTF(X,Y) __attribute__((format(printf,X,Y))) +#else +#define ATTR_WARN_PRINTF(X,Y) +#endif @@ -121,16 +129,32 @@ extern bool fReopenDebugLog; void RandAddSeed(); void RandAddSeedPerfmon(); -int OutputDebugStringF(const char* pszFormat, ...); +int ATTR_WARN_PRINTF(1,2) OutputDebugStringF(const char* pszFormat, ...); int my_snprintf(char* buffer, size_t limit, const char* format, ...); -/* It is not allowed to use va_start with a pass-by-reference argument. - (C++ standard, 18.7, paragraph 3). Use a dummy argument to work around this, and use a - macro to keep similar semantics. +/* + Rationale for the real_strprintf / strprintf construction: + It is not allowed to use va_start with a pass-by-reference argument. + (C++ standard, 18.7, paragraph 3). Use a dummy argument to work around this, and use a + macro to keep similar semantics. */ + +/** Overload strprintf for char*, so that GCC format type warnings can be given */ +std::string ATTR_WARN_PRINTF(1,3) real_strprintf(const char *format, int dummy, ...); +/** Overload strprintf for std::string, to be able to use it with _ (translation). + * This will not support GCC format type warnings (-Wformat) so be careful. + */ std::string real_strprintf(const std::string &format, int dummy, ...); #define strprintf(format, ...) real_strprintf(format, 0, __VA_ARGS__) -std::string vstrprintf(const std::string &format, va_list ap); +std::string vstrprintf(const char *format, va_list ap); + +/* Redefine printf so that it directs output to debug.log + * + * Do this *after* defining the other printf-like functions, because otherwise the + * __attribute__((format(printf,X,Y))) gets expanded to __attribute__((format(OutputDebugStringF,X,Y))) + * which confuses gcc. + */ +#define printf OutputDebugStringF bool error(const char *format, ...); void LogException(std::exception* pex, const char* pszThread); From 963af6449f3fbbb3f9fd79547a33e4f3d5e3f0d0 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 9 Sep 2012 14:50:31 +0200 Subject: [PATCH 2/4] Cleanup some unused macros from util.h Encapsulate _snprintf/sprintf difference in implementation not header --- src/util.cpp | 4 ++++ src/util.h | 5 ----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index c9edc782c85..3a5770c91c6 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -284,7 +284,11 @@ string vstrprintf(const char *format, va_list ap) { va_list arg_ptr; va_copy(arg_ptr, ap); +#ifdef WIN32 ret = _vsnprintf(p, limit, format, arg_ptr); +#else + ret = vsnprintf(p, limit, format, arg_ptr); +#endif va_end(arg_ptr); if (ret >= 0 && ret < limit) break; diff --git a/src/util.h b/src/util.h index b75c3bff4e9..90df4efa3b6 100644 --- a/src/util.h +++ b/src/util.h @@ -79,11 +79,7 @@ T* alignup(T* p) #define S_IRUSR 0400 #define S_IWUSR 0200 #endif -#define unlink _unlink #else -#define _vsnprintf(a,b,c,d) vsnprintf(a,b,c,d) -#define strlwr(psz) to_lower(psz) -#define _strlwr(psz) to_lower(psz) #define MAX_PATH 1024 inline void Sleep(int64 n) { @@ -130,7 +126,6 @@ extern bool fReopenDebugLog; void RandAddSeed(); void RandAddSeedPerfmon(); int ATTR_WARN_PRINTF(1,2) OutputDebugStringF(const char* pszFormat, ...); -int my_snprintf(char* buffer, size_t limit, const char* format, ...); /* Rationale for the real_strprintf / strprintf construction: From ac4e7f6269429b27f32d290df7e0572814e60068 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 9 Sep 2012 14:52:07 +0200 Subject: [PATCH 3/4] HexStr: don't build a vector first Also const correctness for lookup tables in hex functions throughout the code. --- src/bignum.h | 2 +- src/uint256.h | 2 +- src/util.cpp | 2 +- src/util.h | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/bignum.h b/src/bignum.h index 9fea3f70fb6..96b1b2e6ae4 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -305,7 +305,7 @@ public: psz++; // hex string to bignum - static signed char phexdigit[256] = { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,1,2,3,4,5,6,7,8,9,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0 }; + static const signed char phexdigit[256] = { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,1,2,3,4,5,6,7,8,9,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0 }; *this = 0; while (isxdigit(*psz)) { diff --git a/src/uint256.h b/src/uint256.h index fc5ed265920..abd0b71e6fb 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -306,7 +306,7 @@ public: psz += 2; // hex string to uint - static unsigned char phexdigit[256] = { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,1,2,3,4,5,6,7,8,9,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0 }; + static const unsigned char phexdigit[256] = { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,1,2,3,4,5,6,7,8,9,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0 }; const char* pbegin = psz; while (phexdigit[(unsigned char)*psz] || *psz == '0') psz++; diff --git a/src/util.cpp b/src/util.cpp index 3a5770c91c6..a8bd8228e52 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -424,7 +424,7 @@ bool ParseMoney(const char* pszIn, int64& nRet) } -static signed char phexdigit[256] = +static const signed char phexdigit[256] = { -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, diff --git a/src/util.h b/src/util.h index 90df4efa3b6..4b0814c6d36 100644 --- a/src/util.h +++ b/src/util.h @@ -256,9 +256,9 @@ inline int64 abs64(int64 n) template std::string HexStr(const T itbegin, const T itend, bool fSpaces=false) { - std::vector rv; - static char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + std::string rv; + static const char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; rv.reserve((itend-itbegin)*3); for(T it = itbegin; it < itend; ++it) { @@ -269,7 +269,7 @@ std::string HexStr(const T itbegin, const T itend, bool fSpaces=false) rv.push_back(hexmap[val&15]); } - return std::string(rv.begin(), rv.end()); + return rv; } inline std::string HexStr(const std::vector& vch, bool fSpaces=false) From 3b3d9996181d9c93c81667f610e31212997fd184 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 9 Sep 2012 15:04:25 +0200 Subject: [PATCH 4/4] Add format characters for (s)size_t and ptrdiff_t --- src/util.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/util.h b/src/util.h index 4b0814c6d36..eb8f781c2cf 100644 --- a/src/util.h +++ b/src/util.h @@ -54,6 +54,17 @@ static const int64 CENT = 1000000; #endif #endif +/* Format characters for (s)size_t and ptrdiff_t */ +#if defined(_MSC_VER) || defined(__MSVCRT__) + #define PRIszx "%Ix" + #define PRIszu "%Iu" + #define PRIszd "%Id" +#else + #define PRIszx "%zx" + #define PRIszu "%zu" + #define PRIszd "%zd" +#endif + // This is needed because the foreach macro can't get over the comma in pair #define PAIRTYPE(t1, t2) std::pair