After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784000 - Improve strerror_r() detection
Improve strerror_r() detection
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
2.50.x
Other other
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-06-20 13:38 UTC by Igor Pashev
Modified: 2017-07-19 10:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix detection and usage of strerror_r() (1.04 KB, patch)
2017-06-20 13:38 UTC, Igor Pashev
committed Details | Review
meson: fix unit tests and "Invalid byte sequence in conversion input" (1.94 KB, patch)
2017-07-19 09:47 UTC, Tim-Philipp Müller
committed Details | Review

Description Igor Pashev 2017-06-20 13:38:42 UTC
Created attachment 354100 [details] [review]
Fix detection and usage of strerror_r()

Present code for strerror_r() looks like this:

--------------------- 8<-------------------------
#elif defined(HAVE_STRERROR_R)
      /* Match the condition in strerror_r(3) for glibc */
#  if defined(__GLIBC__) && !((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE)
      msg = strerror_r (errnum, buf, sizeof (buf));
#  else
      strerror_r (errnum, buf, sizeof (buf));
      msg = buf;
#  endif /* HAVE_STRERROR_R */

--------------------- >8-------------------------


This could be better as there are systems with have GNU version of strerror_r(), but don't run GNU libc (thus __GLIBC__ is not defined) [1].

Fortunately, there is an Autoconf macro for this case - AC_FUNC_STRERROR_R [2].

Attaching a patch making use of that Autoconf macro.



[1] https://www.osdyson.org
[2] https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Particular-Functions.html
Comment 1 Igor Pashev 2017-06-20 13:41:25 UTC
Although, return value of The XSI-compliant strerror_r() should not be ignored, because in case of insufficient storage, the buffer may contain garbage.
Comment 2 Philip Withnall 2017-06-20 13:54:58 UTC
Review of attachment 354100 [details] [review]:

This looks good, thanks.
Comment 3 Philip Withnall 2017-06-20 13:57:49 UTC
(In reply to Igor Pashev from comment #1)
> Although, return value of The XSI-compliant strerror_r() should not be
> ignored, because in case of insufficient storage, the buffer may contain
> garbage.

Given that the buffer is 1024B long, I think an error return from strerror_r() is vanishingly unlikely. Please attach a separate patch if you want to fix it though.
Comment 4 Philip Withnall 2017-06-20 14:25:08 UTC
Pushed to master, thanks.

Attachment 354100 [details] pushed as c8e268b - Fix detection and usage of strerror_r()
Comment 5 Tim-Philipp Müller 2017-07-19 08:27:30 UTC
Was this just for cleanliness ?

It causes failures all over the place with the newly-merged Meson build now, as it doesn't set STRERROR_R_CHAR_P yet.

Question is: do we want to emulate those checks that the autoconf macro does in meson.build, or should we maybe just revert to the previous code which works just fine in both cases?
Comment 6 Tim-Philipp Müller 2017-07-19 09:47:09 UTC
Created attachment 355923 [details] [review]
meson: fix unit tests and "Invalid byte sequence in conversion input"

Check if strerror_r returns a char * and define STRERROR_R_CHAR_P
if so, which is needed by g_strerror() since c8e268b

The C code used in the meson compile check is obviously heavily inspired by what the autoconf macro does (in functions.m4 which is GPL3 licensed). I think it's trivial enough to not be an issue, and it's hard to see how to do it differently, just wanted to mention it.
Comment 7 Emmanuele Bassi (:ebassi) 2017-07-19 10:12:07 UTC
Review of attachment 355923 [details] [review]:

ACK
Comment 8 Tim-Philipp Müller 2017-07-19 10:14:46 UTC
Comment on attachment 355923 [details] [review]
meson: fix unit tests and "Invalid byte sequence in conversion input"

Pushed as 2ac8079b