GNOME Bugzilla – Bug 744533
gioerror: Add an API to set a literal GError from errno
Last modified: 2018-05-24 17:26:35 UTC
I work on a lot of projects that end up calling into Unix APIs for a variety of reasons, and briding errno -> GError is too much typing/too verbose at each call site. I've been using gs_set_error_from_errno() in libgsystem, but the goal is to deprecate that. Because this is such a tiny API, and useful for the many GNOME and other projects which use GLib + Unix, let's include it here. We could convert *many* more callsites to a g_set_prefix_error_from_errno(), which libgsystem also has. But this is a first step.
Created attachment 296842 [details] [review] gioerror: Add an API to set a literal GError from errno
Review of attachment 296842 [details] [review]: Sure. This seems like a good idea. A few comments/ideas: - why not do only the prefixing version of this and have NULL mean "no prefix"? - would expect this consistent with "prefix" in the sense of g_prefix_error() - I'd use the name you have here for that version, except... - since this is GIOError, it should use a namespace that makes it clear that it is part of GIO - whether to take errno as a parameter or not is an interesting question - can we make this return FALSE? { g_auto(ghandle) fd = open ... ... if (read (fd, ..) < 0) return g_set_error_from_errno ("unable to read file: ", error); ... } although in that case, it might make sense to give it a slightly different name. All that taken together, it seems like this might be nice: /* returns TRUE if errno == 0 */ gboolean g_io_check_errno (/* gint errno, ??? */ const gchar *error_prefix, GError **error); ::: gio/gioerror.h @@ +28,3 @@ #include <glib.h> #include <gio/gioenums.h> +#include <errno.h> as a general rule, please try to avoid adding new system includes to glib public headers unless absolutely necessary -- this is part of the reason why things like gstdio.h, gi18n.h, etc. are separate headers. @@ +54,3 @@ + * Since: 2.44 + */ +static inline void Not sure why this is inline and in the header... I don't expect it to be used _that_ often, and when it is, it is going to be on the unexpected path...
Arguments against errno parameter: - don't need to #include <errno.h> - very slightly easier to use arguments for errno parameter: - it's possible to get errno values from other places and one more note: GIOError is not what one would typically expect to use to represent the error result of a lot of functions that report via errno. It is a bit higher level, and it doesn't contain all of the things found in errno (like EINTR). GFileError is typically a better fit for errno. It is a bit of a mistake that we have both of these, in my opinion... I still think this function makes sense to be defined in terms of GIO, though...
(In reply to Ryan Lortie (desrt) from comment #2) > gboolean > g_io_check_errno (/* gint errno, ??? */ > const gchar *error_prefix, > GError **error); You need the explicit errno parameter here, since error_prefix may be generated from a call to _(), which may change errno. Ideally, we'd also define a macro around g_io_check_errno() to do the errsv part for you (although maybe we can't do that portably if it has a return value...). (Also, "err_no", not "errno", since "errno" might be a macro.)
(In reply to Dan Winship from comment #4) > Ideally, we'd also define a macro around g_io_check_errno() to do the errsv > part for you (although maybe we can't do that portably if it has a return > value...). That's a good point. Guess I haven't hit the _() problem myself because I'm not translating errors in my programs yet. It's pretty unfortunate that parameter evaluation order is undefined in C, otherwise we could make err_no first.
_() is gettext() gettext(3) says: ERRORS errno is not modified.
curious... we go to great trouble everywhere else in gio under the assumption that that's not true...
I always felt that I was doing it because of g_strerror() and g_io_error_from_errno(), even doing it when I don't translate messages, but now that I think about it, neither of those functions modify errno either. In any case, I checked glibc and standalone gettext() and both of them go out of their way to save errno on entry and restore it on exit (which is not surprising, seeing as one is more or less a clone of the other). I'm not sure we can rely on this fact for other gettext() ?
fwiw, I think we should define our own g_gettext() that is an alias for gettext() on systems where we know errno is preserved and is a very small wrapper on other systems. _() then should be defined in terms of g_gettext(). I still consider the question of whether to pass errno as an explicit parameter or not to be interesting...
ah, we already wrap most of the *_() macros with g_* wrappers, which call _g_dgettext_should_translate(), which does not necessarily preserve errno. So we should probably fix that. > I still consider the question of whether to pass errno as an explicit > parameter or not to be interesting... I suppose perror() is the closest libc analog to this function, and it doesn't take an explicit errno. But I've always felt that perror() was kind of weird for exactly that reason...
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/992.