Replace non-threadsafe strerror gmtime setlocale #4152
Conversation
| #ifdef STRERROR_R_CHAR_P /* GNU variant can return a pointer */ | ||
| s = strerror_r(err, buf, sizeof(buf)); | ||
| #else /* POSIX variant always returns message in buffer */ | ||
| (void) strerror_r(err, buf, sizeof(buf)); |
There was a problem hiding this comment.
Why can't we just always use this form?
There was a problem hiding this comment.
Should be safe as long as we put something like
#if defined(_GNU_SOURCE) || _POSIX_C_SOURCE < 200112L
# error "We might be getting GNU strerror_r here"
#endifThere was a problem hiding this comment.
Well I get the GNU form here by default...
It would be possible to define _POSIX_C_SOURCE in the configure script, but there is no telling what impact that will have on other functions. Detecting what form to use is the safer way.
There was a problem hiding this comment.
Ah, the documentation clarifies. The GNU version may return something else than buf... puke
There was a problem hiding this comment.
Exactly, in that case buf will remain empty :(
|
untested ACK |
| #ifdef WIN32 | ||
| std::string NetworkErrorString(int err) | ||
| { | ||
| char buf[256]; |
There was a problem hiding this comment.
Couldn't you write just char buf[256] = 0; and it would zero the first array element?
There was a problem hiding this comment.
But that version is more likely to confuse people. I prefer laanwj's more obvious version, even though it's one line longer.
There was a problem hiding this comment.
@Diapolo That would zero out the entire buffer, which is not necessary here (according to the documentation FormatMessage will always return a null-terminated string or nothing at all).
http://msdn.microsoft.com/en-us/library/windows/desktop/ms679351%28v=vs.85%29.aspx
I see though that we don't check the output of FormatMessageA - which can fail, itself. We should probably set a default message (like 'Unknown') in case it fails. Edit: fixed
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.
Make DateTimeStrFormat use boost::posix_time. Also re-enable the util_DateTimeStrFormat tests, as they are no longer platform specific.
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/3e8ac6af9a993e262d1160fb2e6e1e1f1d5d19f2 for binaries and test log. |
happens. This makes network troubleshooting more convenient
longer platform specific