GNOME Bugzilla – Bug 733821
g_strerror() uses strerror(3) instead of strerror_r(3)
Last modified: 2017-07-29 21:39:30 UTC
g_strerror() uses strerror(3) instead of strerror_r(3) making it not thread-safe. This might cause unexpected consequences as it is assumed to be thread safe. This also goes against the advice in the threading docs suggesting people to use said function. "C library functions that return data in statically allocated buffers, such as strtok() or strerror(). For many of these, there are thread-safe variants with a _r suffix, or you can look at corresponding GLib APIs (like g_strsplit() or g_strerror())." [1] [1] https://developer.gnome.org/glib/stable/glib-Threads.html
g_strerror() is probably older than strerror_r()... Anyway, patches welcome. You'd have to include a test for strerror_r in configure.ac as well.
Created attachment 281875 [details] [review] Patch to enable strerror_r(3) where available
Any updates about this?
See #754788
Review of attachment 281875 [details] [review]: strerror_r() was introduced to g_strerror() in 2015 in 36fac0849ceabafb9e2a15045230833e7dbc9e9d.
There are various places in GLib which still use strerror() directly, though, which should be ported to g_strerror(). Patch coming.
Created attachment 354103 [details] [review] gio: Use g_strerror() instead of strerror() This marginally improves thread safety, and marginally improves consistency. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 354103 [details] [review]: LGTM.
Attachment 354103 [details] pushed as 18f8b77 - gio: Use g_strerror() instead of strerror()
Noticed the commit in git, and saw that in many places this code uses errno directly, like g_set_error(...., _("foo..."), ..., g_strerror(errno)) where it really needs to do the int errsv = errno; g_set_error(...., g_strerror(errsv)) dance instead, to preserve the actual error, not any errno that the _() call may set.
(In reply to Christian Persch from comment #10) > Noticed the commit in git, and saw that in many places this code uses errno > directly, like > > g_set_error(...., _("foo..."), ..., g_strerror(errno)) > > where it really needs to do the > > int errsv = errno; > g_set_error(...., g_strerror(errsv)) > > dance instead, to preserve the actual error, not any errno that the _() call > may set. True. Do you want to put together a patch?
See also https://git.gnome.org/browse/libglnx/tree/glnx-errors.h?id=caa51ac24ffcdffcb610bc6ccc9da964d4be74ee#n97
(In reply to Philip Withnall from comment #11) > True. Do you want to put together a patch? I don't, but I've now filed this as bug 785577 so it doesn't get lost.