Attachment 134503 Details for Bug 103010 – Make sure non-aborting signal handlers save and restore errno

[patch] Make sure non-aborting signal handlers save and restore errno

0003-Make-sure-non-aborting-signal-handlers-save-and-rest.patch (text/plain), 2.16 KB, created by Simon McVittie on 2017-09-27 13:35:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-09-27 13:35:33 UTC

Size: 2.16 KB

patch

obsolete

>From 78a64c8137593a80a6338babc850fc5f85d6743e Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Wed, 27 Sep 2017 13:56:34 +0100
>Subject: [PATCH 3/4] Make sure non-aborting signal handlers save and restore
> errno
>
>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).
>
>Signed-off-by: Simon McVittie <smcv@collabora.com>
>---
> bus/main.c        | 6 ++++++
> dbus/dbus-spawn.c | 6 ++++++
> 2 files changed, 12 insertions(+)
>
>diff --git a/bus/main.c b/bus/main.c
>index 0ede03f7..b3fcddd0 100644
>--- a/bus/main.c
>+++ b/bus/main.c
>@@ -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 */
> 
>diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
>index 3c721d6f..8ab529a4 100644
>--- a/dbus/dbus-spawn.c
>+++ b/dbus/dbus-spawn.c
>@@ -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,
>-- 
>2.14.2
>

Actions: View | Splinter Review

Attachments on bug 103010: 134503 | 134504

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.