util: introduce negative_errno()

Imagine a constructor like this:

        int object_new(void **out) {
                void *my_object;
                int r;

                ...
                r = ioctl(...);
                if (r < 0)
                        return -errno;
                ...

                *out = my_object;
                return 0;
        }

We have a lot of those in systemd. If you now call those, gcc might inline
the call and optimize it. However, gcc cannot know that "errno" is
negative if "r" is. Therefore, a caller like this will produce warnings:

        r = object_new(&obj);
        if (r < 0)
                return r;

        obj->xyz = "foobar";

In case the ioctl in the constructor fails, gcc might assume "errno" is 0
and thus the error-handling is not triggered. Therefore, "obj" is
uninitialized, but accessed. Gcc will warn about that.

The new negative_errno() helper can be used to mitigate those warnings.
The helper is guaranteed to return a negative integer. Furthermore, it
spills out runtime warnings if "errno" is non-negative.

Instead of returning "-errno", you can use:
        return negative_errno();

gcc will no longer assume that this can return >=0, thus, it will not warn
about it.

Use this new helper in libsystemd-terminal to fix some grdev-drm warnings.

diff --git a/src/libsystemd-terminal/grdev-drm.c b/src/libsystemd-terminal/grdev-drm.c
index dba6db2691c836cf009a4cf6fb6fcb38ec0d1dec..48f8e9ce9643be80a2d91ac941032c9a97afe407 100644 (file)
--- a/src/libsystemd-terminal/grdev-drm.c
+++ b/src/libsystemd-terminal/grdev-drm.c
@@ -1393,7 +1393,7 @@ static int grdrm_fb_new(grdrm_fb **out, grdrm_card *card, const struct drm_mode_
 
         r = ioctl(card->fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_dumb);
         if (r < 0) {
-                r = -errno;
+                r = negative_errno();
                 log_debug("grdrm: %s: cannot create dumb buffer %" PRIu32 "x%" PRIu32": %m",
                           card->base.name, fb->base.width, fb->base.height);
                 return r;
@@ -1407,7 +1407,7 @@ static int grdrm_fb_new(grdrm_fb **out, grdrm_card *card, const struct drm_mode_
 
         r = ioctl(card->fd, DRM_IOCTL_MODE_MAP_DUMB, &map_dumb);
         if (r < 0) {
-                r = -errno;
+                r = negative_errno();
                 log_debug("grdrm: %s: cannot map dumb buffer %" PRIu32 "x%" PRIu32": %m",
                           card->base.name, fb->base.width, fb->base.height);
                 return r;
@@ -1415,7 +1415,7 @@ static int grdrm_fb_new(grdrm_fb **out, grdrm_card *card, const struct drm_mode_
 
         fb->base.maps[0] = mmap(0, fb->sizes[0], PROT_WRITE, MAP_SHARED, card->fd, map_dumb.offset);
         if (fb->base.maps[0] == MAP_FAILED) {
-                r = -errno;
+                r = negative_errno();
                 log_debug("grdrm: %s: cannot memory-map dumb buffer %" PRIu32 "x%" PRIu32": %m",
                           card->base.name, fb->base.width, fb->base.height);
                 return r;
@@ -1433,7 +1433,7 @@ static int grdrm_fb_new(grdrm_fb **out, grdrm_card *card, const struct drm_mode_
 
         r = ioctl(card->fd, DRM_IOCTL_MODE_ADDFB2, &add_fb);
         if (r < 0) {
-                r = -errno;
+                r = negative_errno();
                 log_debug("grdrm: %s: cannot add framebuffer %" PRIu32 "x%" PRIu32": %m",
                           card->base.name, fb->base.width, fb->base.height);
                 return r;

diff --git a/src/shared/util.h b/src/shared/util.h
index e405b02a8189760d4a9a773c160f330baba20bc7..af589b6708c79d04f7b917538a1c408935be3711 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -794,6 +794,15 @@ static inline void _reset_errno_(int *saved_errno) {
 
 #define PROTECT_ERRNO _cleanup_(_reset_errno_) __attribute__((unused)) int _saved_errno_ = errno
 
+static inline int negative_errno(void) {
+        /* This helper should be used to shut up gcc if you know 'errno' is
+         * negative. Instead of "return -errno;", use "return negative_errno();"
+         * It will suppress bogus gcc warnings in case it assumes 'errno' might
+         * be 0 and thus the caller's error-handling might not be triggered. */
+        assert_return(errno > 0, -EINVAL);
+        return -errno;
+}
+
 struct _umask_struct_ {
         mode_t mask;
         bool quit;