From a60838d09aed4d976e9343e8329d61afff204435 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 8 May 2014 14:15:19 +0200 Subject: [PATCH 1/2] Replace non-threadsafe strerror Log the name of the error as well as the error code if a network problem happens. This makes network troubleshooting more convenient. Use thread-safe strerror_r and the WIN32 equivalent FormatMessage. --- configure.ac | 2 ++ src/net.cpp | 24 ++++++++++++------------ src/netbase.cpp | 41 +++++++++++++++++++++++++++++++++++++---- src/netbase.h | 2 ++ 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/configure.ac b/configure.ac index 6a8afe6e44..3a8d33a5a0 100644 --- a/configure.ac +++ b/configure.ac @@ -303,6 +303,8 @@ INCLUDES="$INCLUDES $PTHREAD_CFLAGS" # they also need to be passed down to any subprojects. Pull the results out of # the cache and add them to CPPFLAGS. AC_SYS_LARGEFILE +# detect POSIX or GNU variant of strerror_r +AC_FUNC_STRERROR_R if test x$ac_cv_sys_file_offset_bits != x && test x$ac_cv_sys_file_offset_bits != xno && diff --git a/src/net.cpp b/src/net.cpp index 6bde1e7999..4cbd26a563 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -178,7 +178,7 @@ bool RecvLine(SOCKET hSocket, string& strLine) { // socket error int nErr = WSAGetLastError(); - LogPrint("net", "recv failed: %d\n", nErr); + LogPrint("net", "recv failed: %s\n", NetworkErrorString(nErr)); return false; } } @@ -489,10 +489,10 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest) #ifdef WIN32 u_long nOne = 1; if (ioctlsocket(hSocket, FIONBIO, &nOne) == SOCKET_ERROR) - LogPrintf("ConnectSocket() : ioctlsocket non-blocking setting failed, error %d\n", WSAGetLastError()); + LogPrintf("ConnectSocket() : ioctlsocket non-blocking setting failed, error %s\n", NetworkErrorString(WSAGetLastError())); #else if (fcntl(hSocket, F_SETFL, O_NONBLOCK) == SOCKET_ERROR) - LogPrintf("ConnectSocket() : fcntl non-blocking setting failed, error %d\n", errno); + LogPrintf("ConnectSocket() : fcntl non-blocking setting failed, error %s\n", NetworkErrorString(errno)); #endif // Add node @@ -736,7 +736,7 @@ void SocketSendData(CNode *pnode) int nErr = WSAGetLastError(); if (nErr != WSAEWOULDBLOCK && nErr != WSAEMSGSIZE && nErr != WSAEINTR && nErr != WSAEINPROGRESS) { - LogPrintf("socket send error %d\n", nErr); + LogPrintf("socket send error %s\n", NetworkErrorString(nErr)); pnode->CloseSocketDisconnect(); } } @@ -896,7 +896,7 @@ void ThreadSocketHandler() if (have_fds) { int nErr = WSAGetLastError(); - LogPrintf("socket select error %d\n", nErr); + LogPrintf("socket select error %s\n", NetworkErrorString(nErr)); for (unsigned int i = 0; i <= hSocketMax; i++) FD_SET(i, &fdsetRecv); } @@ -933,7 +933,7 @@ void ThreadSocketHandler() { int nErr = WSAGetLastError(); if (nErr != WSAEWOULDBLOCK) - LogPrintf("socket error accept failed: %d\n", nErr); + LogPrintf("socket error accept failed: %s\n", NetworkErrorString(nErr)); } else if (nInbound >= nMaxConnections - MAX_OUTBOUND_CONNECTIONS) { @@ -1007,7 +1007,7 @@ void ThreadSocketHandler() if (nErr != WSAEWOULDBLOCK && nErr != WSAEMSGSIZE && nErr != WSAEINTR && nErr != WSAEINPROGRESS) { if (!pnode->fDisconnect) - LogPrintf("socket recv error %d\n", nErr); + LogPrintf("socket recv error %s\n", NetworkErrorString(nErr)); pnode->CloseSocketDisconnect(); } } @@ -1585,7 +1585,7 @@ bool BindListenPort(const CService &addrBind, string& strError) SOCKET hListenSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP); if (hListenSocket == INVALID_SOCKET) { - strError = strprintf("Error: Couldn't open socket for incoming connections (socket returned error %d)", WSAGetLastError()); + strError = strprintf("Error: Couldn't open socket for incoming connections (socket returned error %s)", NetworkErrorString(WSAGetLastError())); LogPrintf("%s\n", strError); return false; } @@ -1609,7 +1609,7 @@ bool BindListenPort(const CService &addrBind, string& strError) if (fcntl(hListenSocket, F_SETFL, O_NONBLOCK) == SOCKET_ERROR) #endif { - strError = strprintf("Error: Couldn't set properties on socket for incoming connections (error %d)", WSAGetLastError()); + strError = strprintf("Error: Couldn't set properties on socket for incoming connections (error %s)", NetworkErrorString(WSAGetLastError())); LogPrintf("%s\n", strError); return false; } @@ -1638,7 +1638,7 @@ bool BindListenPort(const CService &addrBind, string& strError) if (nErr == WSAEADDRINUSE) strError = strprintf(_("Unable to bind to %s on this computer. Bitcoin Core is probably already running."), addrBind.ToString()); else - strError = strprintf(_("Unable to bind to %s on this computer (bind returned error %d, %s)"), addrBind.ToString(), nErr, strerror(nErr)); + strError = strprintf(_("Unable to bind to %s on this computer (bind returned error %s)"), addrBind.ToString(), NetworkErrorString(nErr)); LogPrintf("%s\n", strError); return false; } @@ -1647,7 +1647,7 @@ bool BindListenPort(const CService &addrBind, string& strError) // Listen for incoming connections if (listen(hListenSocket, SOMAXCONN) == SOCKET_ERROR) { - strError = strprintf(_("Error: Listening for incoming connections failed (listen returned error %d)"), WSAGetLastError()); + strError = strprintf(_("Error: Listening for incoming connections failed (listen returned error %s)"), NetworkErrorString(WSAGetLastError())); LogPrintf("%s\n", strError); return false; } @@ -1785,7 +1785,7 @@ public: BOOST_FOREACH(SOCKET hListenSocket, vhListenSocket) if (hListenSocket != INVALID_SOCKET) if (closesocket(hListenSocket) == SOCKET_ERROR) - LogPrintf("closesocket(hListenSocket) failed with error %d\n", WSAGetLastError()); + LogPrintf("closesocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAGetLastError())); // clean up some globals (to help leak detection) BOOST_FOREACH(CNode *pnode, vNodes) diff --git a/src/netbase.cpp b/src/netbase.cpp index 82a681281d..e24a0a195c 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -361,7 +361,7 @@ bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRe } if (nRet == SOCKET_ERROR) { - LogPrintf("select() for %s failed: %i\n", addrConnect.ToString(), WSAGetLastError()); + LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); closesocket(hSocket); return false; } @@ -372,13 +372,13 @@ bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRe if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, &nRet, &nRetSize) == SOCKET_ERROR) #endif { - LogPrintf("getsockopt() for %s failed: %i\n", addrConnect.ToString(), WSAGetLastError()); + LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); closesocket(hSocket); return false; } if (nRet != 0) { - LogPrintf("connect() to %s failed after select(): %s\n", addrConnect.ToString(), strerror(nRet)); + LogPrintf("connect() to %s failed after select(): %s\n", addrConnect.ToString(), NetworkErrorString(nRet)); closesocket(hSocket); return false; } @@ -389,7 +389,7 @@ bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRe else #endif { - LogPrintf("connect() to %s failed: %i\n", addrConnect.ToString(), WSAGetLastError()); + LogPrintf("connect() to %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); closesocket(hSocket); return false; } @@ -1237,3 +1237,36 @@ bool operator!=(const CSubNet& a, const CSubNet& b) { return !(a==b); } + +#ifdef WIN32 +std::string NetworkErrorString(int err) +{ + char buf[256]; + buf[0] = 0; + if(FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + buf, sizeof(buf), NULL)) + { + return strprintf("%s (%d)", buf, err); + } + else + { + return strprintf("Unknown error (%d)", err); + } +} +#else +std::string NetworkErrorString(int err) +{ + char buf[256]; + const char *s = buf; + buf[0] = 0; + /* Too bad there are two incompatible implementations of the + * thread-safe strerror. */ +#ifdef STRERROR_R_CHAR_P /* GNU variant can return a pointer outside the passed buffer */ + s = strerror_r(err, buf, sizeof(buf)); +#else /* POSIX variant always returns message in buffer */ + (void) strerror_r(err, buf, sizeof(buf)); +#endif + return strprintf("%s (%d)", s, err); +} +#endif diff --git a/src/netbase.h b/src/netbase.h index 118f866d6c..5fd8be4aca 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -179,5 +179,7 @@ bool Lookup(const char *pszName, std::vector& vAddr, int portDefault = bool LookupNumeric(const char *pszName, CService& addr, int portDefault = 0); bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, int nTimeout = nConnectTimeout); bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault = 0, int nTimeout = nConnectTimeout); +/** Return readable error string for a network error code */ +std::string NetworkErrorString(int err); #endif From 3e8ac6af9a993e262d1160fb2e6e1e1f1d5d19f2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 8 May 2014 18:01:10 +0200 Subject: [PATCH 2/2] Replace non-threadsafe gmtime and setlocale Make DateTimeStrFormat use boost::posix_time. Also re-enable the util_DateTimeStrFormat tests, as they are no longer platform specific. --- src/rpcprotocol.cpp | 10 +--------- src/test/util_tests.cpp | 4 +--- src/util.cpp | 12 ++++++++++++ src/util.h | 9 +-------- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/rpcprotocol.cpp b/src/rpcprotocol.cpp index 5cbaa535ab..2718f81783 100644 --- a/src/rpcprotocol.cpp +++ b/src/rpcprotocol.cpp @@ -51,15 +51,7 @@ string HTTPPost(const string& strMsg, const map& mapRequestHeader static string rfc1123Time() { - char buffer[64]; - time_t now; - time(&now); - struct tm* now_gmt = gmtime(&now); - string locale(setlocale(LC_TIME, NULL)); - setlocale(LC_TIME, "C"); // we want POSIX (aka "C") weekday/month strings - strftime(buffer, sizeof(buffer), "%a, %d %b %Y %H:%M:%S +0000", now_gmt); - setlocale(LC_TIME, locale.c_str()); - return string(buffer); + return DateTimeStrFormat("%a, %d %b %Y %H:%M:%S +0000", GetTime()); } string HTTPReply(int nStatus, const string& strMsg, bool keepalive) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3811569c0a..0e53a57593 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -108,13 +108,11 @@ BOOST_AUTO_TEST_CASE(util_HexStr) BOOST_AUTO_TEST_CASE(util_DateTimeStrFormat) { -/*These are platform-dependant and thus removed to avoid useless test failures BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 0), "1970-01-01 00:00:00"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 0x7FFFFFFF), "2038-01-19 03:14:07"); - // Formats used within Bitcoin BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 1317425777), "2011-09-30 23:36:17"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M", 1317425777), "2011-09-30 23:36"); -*/ + BOOST_CHECK_EQUAL(DateTimeStrFormat("%a, %d %b %Y %H:%M:%S +0000", 1317425777), "Fri, 30 Sep 2011 23:36:17 +0000"); } BOOST_AUTO_TEST_CASE(util_ParseParameters) diff --git a/src/util.cpp b/src/util.cpp index aa3adf89ec..f7ceb3e95c 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -14,6 +14,8 @@ #include +#include + #ifndef WIN32 // for posix_fallocate #ifdef __linux_ @@ -1400,3 +1402,13 @@ void SetupEnvironment() } #endif } + +std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime) +{ + // std::locale takes ownership of the pointer + std::locale loc(std::locale::classic(), new boost::posix_time::time_facet(pszFormat)); + std::stringstream ss; + ss.imbue(loc); + ss << boost::posix_time::from_time_t(nTime); + return ss.str(); +} diff --git a/src/util.h b/src/util.h index 97185073e0..9e899b5084 100644 --- a/src/util.h +++ b/src/util.h @@ -325,14 +325,7 @@ inline int64_t GetTimeMicros() boost::posix_time::ptime(boost::gregorian::date(1970,1,1))).total_microseconds(); } -inline std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime) -{ - time_t n = nTime; - struct tm* ptmTime = gmtime(&n); - char pszTime[200]; - strftime(pszTime, sizeof(pszTime), pszFormat, ptmTime); - return pszTime; -} +std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime); inline bool IsSwitchChar(char c) {