Full_Name: Hallvard B Furuseth Version: LMDB 0.9.17 OS: Linux x86_64 URL: Submission from: (NULL) (81.191.45.31) Submitted by: hallvard
Unix issues:
mdb_strerror uses strerror(), which can sprintf(buf, "Error %d", err) to a thread-unsafe global buffer. In other cases, strerror hopefully returns a pointer to a constant string. This means we can make it hopefully-threadsafe on Unix by documenting "mdb_strerror() should only be used for codes returned by mdb_*() calls".
Windows issues:
mdb_strerror()'s doc says it extends strerror(). Wrong on Windows.
It terminates messages with CRLF. Maybe programs written by Windows users will expect this, but not those from Unix folks. Fix: Strip trailing CRLF, or include FORMAT_MESSAGE_MAX_WIDTH_MASK and then strip trailing space which gets appended instead. The FormatMessage doc says this mask ignores "regular" line breaks but stores "hard-coded" line breaks, I don't know what that means. (Can some system messages have CRLFs inside them?)
The pad[] hack breaks if the compiler rearranges locals vars. Fix: Join pad and buf in a struct/array, and drop the (va_list *)pad hack.
That hack must be documented. Or replaced, or made a global option. I note that OpenLDAP lutil_debug() does use more tha 4K stack like the mdb_strerror() comment says it shouldn't.
Maybe it'd be best to use a thread key by default, but if mdb_set_scarce() or something is called before any mdb_env_create() call, then it will use the current code. But - if mdb_strerror() is called before that again, it'll... init the key itself? Sigh.
Also we could add "char *mdb_strerror_r()" which *may* use the user-provided buffer, like the glibc version. Should also add a constant MDB_STRERROR_BUFSIZE as the recommended/minimum buffer size, instead of expecting the programmer to guess the value. That'll at least provide an officially sane alternative. Except - possibly this function would still use strerror() rather than strerror_r() on Unix. The Gnulib (not glibc) comments about strerror_r() portability/bugs are depressing, though hopefully most are outdated: https://www.gnu.org/software/gnulib/manual/html_node/strerror_005fr.html