(ITS#8361) mdb_strerror() issues

[h.b.furuseth@usit.uio.no]

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