(ITS#8361) mdb_strerror() issues
by 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