Make sure non-aborting signal handlers save and restore errno
authorSimon McVittie <smcv@collabora.com>
Wed, 27 Sep 2017 12:56:34 +0000 (13:56 +0100)
committerSimon McVittie <smcv@collabora.com>
Wed, 27 Sep 2017 14:14:12 +0000 (15:14 +0100)
If an async signal interrupts some function, we can have this
anti-pattern:

    /* in normal code */
    result = some_syscall (); /* fails, e.g. errno = EINVAL */

        /* interrupted by async signal handler */
        write (...); /* fails, e.g. errno = ENOBUFS */

    /* back to normal code */
    if (errno == EINVAL) /* problem! it should be but it isn't */

The solution is for signal handlers to save and restore errno.

This is unnecessary for signal handlers that can't touch errno (like
the one in dbus-launch that just sets a flag), and for signal handlers
that never return (like the one in test-utils-glib for timeouts).

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103010
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
bus/main.c
dbus/dbus-spawn.c

index 0ede03f..b3fcddd 100644 (file)
@@ -67,6 +67,10 @@ typedef enum
 static void
 signal_handler (int sig)
 {
+  /* Signal handlers that might set errno must save and restore the errno
+   * that the interrupted function might have been relying on. */
+  int saved_errno = errno;
+
   switch (sig)
     {
     case SIGHUP:
@@ -134,6 +138,8 @@ signal_handler (int sig)
        * signal, but keep -Wswitch-default happy */
       break;
     }
+
+  errno = saved_errno;
 }
 #endif /* DBUS_UNIX */
 
index 3c721d6..8ab529a 100644 (file)
@@ -1138,11 +1138,17 @@ static int babysit_sigchld_pipe = -1;
 static void
 babysit_signal_handler (int signo)
 {
+  /* Signal handlers that might set errno must save and restore the errno
+   * that the interrupted function might have been relying on. */
+  int saved_errno = errno;
   char b = '\0';
+
  again:
   if (write (babysit_sigchld_pipe, &b, 1) <= 0) 
     if (errno == EINTR)
       goto again;
+
+  errno = saved_errno;
 }
 
 static void babysit (pid_t grandchild_pid,