| Message ID | 1424356217-17766-1-git-send-email-thomas.wood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 19, 2015 at 02:30:17PM +0000, Thomas Wood wrote: > Signed-off-by: Thomas Wood <thomas.wood@intel.com> > --- > lib/igt_core.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 75b98f6..adfa597 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -1303,8 +1303,10 @@ static struct { > static igt_exit_handler_t exit_handler_fn[MAX_EXIT_HANDLERS]; > static bool exit_handler_disabled; > static sigset_t saved_sig_mask; > -static const int handled_signals[] = > - { SIGINT, SIGHUP, SIGTERM, SIGQUIT, SIGPIPE, SIGABRT, SIGSEGV, SIGBUS }; > +#define SIGDEF(x) { x, #x, sizeof(#x) - 1 } > +static const struct { int number; const char *name; size_t name_len; } handled_signals[] = > + { SIGDEF(SIGINT), SIGDEF(SIGHUP), SIGDEF(SIGTERM), SIGDEF(SIGQUIT), > + SIGDEF(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) }; > > static int install_sig_handler(int sig_num, sighandler_t handler) > { > @@ -1357,8 +1359,20 @@ static void igt_atexit_handler(void) > > static void fatal_sig_handler(int sig) > { > + int i; > + > restore_all_sig_handler(); > > + for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { > + if (handled_signals[i].number == sig) { > + write(STDERR_FILENO, "Received signal ", 16); > + write(STDERR_FILENO, handled_signals[i].name, > + handled_signals[i].name_len); > + write(STDERR_FILENO, ".\n", 2); > + break; strsignal() is apparently a neato linux glibc extension. With that simplification instead of the string table this lgtm. -Daneil > + } > + } > + > /* > * exit_handler_disabled is always false here, since when we set it > * we also block signals. > @@ -1415,7 +1429,7 @@ void igt_install_exit_handler(igt_exit_handler_t fn) > return; > > for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { > - if (install_sig_handler(handled_signals[i], > + if (install_sig_handler(handled_signals[i].number, > fatal_sig_handler)) > goto err; > } > @@ -1447,7 +1461,7 @@ void igt_disable_exit_handler(void) > > sigemptyset(&set); > for (i = 0; i < ARRAY_SIZE(handled_signals); i++) > - sigaddset(&set, handled_signals[i]); > + sigaddset(&set, handled_signals[i].number); > > if (sigprocmask(SIG_BLOCK, &set, &saved_sig_mask)) { > perror("sigprocmask"); > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 23 February 2015 at 23:54, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Feb 19, 2015 at 02:30:17PM +0000, Thomas Wood wrote: >> Signed-off-by: Thomas Wood <thomas.wood@intel.com> >> --- >> lib/igt_core.c | 22 ++++++++++++++++++---- >> 1 file changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c >> index 75b98f6..adfa597 100644 >> --- a/lib/igt_core.c >> +++ b/lib/igt_core.c >> @@ -1303,8 +1303,10 @@ static struct { >> static igt_exit_handler_t exit_handler_fn[MAX_EXIT_HANDLERS]; >> static bool exit_handler_disabled; >> static sigset_t saved_sig_mask; >> -static const int handled_signals[] = >> - { SIGINT, SIGHUP, SIGTERM, SIGQUIT, SIGPIPE, SIGABRT, SIGSEGV, SIGBUS }; >> +#define SIGDEF(x) { x, #x, sizeof(#x) - 1 } >> +static const struct { int number; const char *name; size_t name_len; } handled_signals[] = >> + { SIGDEF(SIGINT), SIGDEF(SIGHUP), SIGDEF(SIGTERM), SIGDEF(SIGQUIT), >> + SIGDEF(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) }; >> >> static int install_sig_handler(int sig_num, sighandler_t handler) >> { >> @@ -1357,8 +1359,20 @@ static void igt_atexit_handler(void) >> >> static void fatal_sig_handler(int sig) >> { >> + int i; >> + >> restore_all_sig_handler(); >> >> + for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { >> + if (handled_signals[i].number == sig) { >> + write(STDERR_FILENO, "Received signal ", 16); >> + write(STDERR_FILENO, handled_signals[i].name, >> + handled_signals[i].name_len); >> + write(STDERR_FILENO, ".\n", 2); >> + break; > > strsignal() is apparently a neato linux glibc extension. With that > simplification instead of the string table this lgtm. strsignal() returns a description string such as "Terminated" or "Segmentation Fault", rather than the signal name, so it doesn't quite fit in this context. The function is also not in the list of async-signal-safe functions. > -Daneil > >> + } >> + } >> + >> /* >> * exit_handler_disabled is always false here, since when we set it >> * we also block signals. >> @@ -1415,7 +1429,7 @@ void igt_install_exit_handler(igt_exit_handler_t fn) >> return; >> >> for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { >> - if (install_sig_handler(handled_signals[i], >> + if (install_sig_handler(handled_signals[i].number, >> fatal_sig_handler)) >> goto err; >> } >> @@ -1447,7 +1461,7 @@ void igt_disable_exit_handler(void) >> >> sigemptyset(&set); >> for (i = 0; i < ARRAY_SIZE(handled_signals); i++) >> - sigaddset(&set, handled_signals[i]); >> + sigaddset(&set, handled_signals[i].number); >> >> if (sigprocmask(SIG_BLOCK, &set, &saved_sig_mask)) { >> perror("sigprocmask"); >> -- >> 2.1.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Feb 24, 2015 at 09:42:04AM +0000, Thomas Wood wrote: > On 23 February 2015 at 23:54, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Feb 19, 2015 at 02:30:17PM +0000, Thomas Wood wrote: > >> Signed-off-by: Thomas Wood <thomas.wood@intel.com> > >> --- > >> lib/igt_core.c | 22 ++++++++++++++++++---- > >> 1 file changed, 18 insertions(+), 4 deletions(-) > >> > >> diff --git a/lib/igt_core.c b/lib/igt_core.c > >> index 75b98f6..adfa597 100644 > >> --- a/lib/igt_core.c > >> +++ b/lib/igt_core.c > >> @@ -1303,8 +1303,10 @@ static struct { > >> static igt_exit_handler_t exit_handler_fn[MAX_EXIT_HANDLERS]; > >> static bool exit_handler_disabled; > >> static sigset_t saved_sig_mask; > >> -static const int handled_signals[] = > >> - { SIGINT, SIGHUP, SIGTERM, SIGQUIT, SIGPIPE, SIGABRT, SIGSEGV, SIGBUS }; > >> +#define SIGDEF(x) { x, #x, sizeof(#x) - 1 } > >> +static const struct { int number; const char *name; size_t name_len; } handled_signals[] = > >> + { SIGDEF(SIGINT), SIGDEF(SIGHUP), SIGDEF(SIGTERM), SIGDEF(SIGQUIT), > >> + SIGDEF(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) }; > >> > >> static int install_sig_handler(int sig_num, sighandler_t handler) > >> { > >> @@ -1357,8 +1359,20 @@ static void igt_atexit_handler(void) > >> > >> static void fatal_sig_handler(int sig) > >> { > >> + int i; > >> + > >> restore_all_sig_handler(); > >> > >> + for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { > >> + if (handled_signals[i].number == sig) { > >> + write(STDERR_FILENO, "Received signal ", 16); > >> + write(STDERR_FILENO, handled_signals[i].name, > >> + handled_signals[i].name_len); > >> + write(STDERR_FILENO, ".\n", 2); > >> + break; > > > > strsignal() is apparently a neato linux glibc extension. With that > > simplification instead of the string table this lgtm. > > strsignal() returns a description string such as "Terminated" or > "Segmentation Fault", rather than the signal name, so it doesn't quite > fit in this context. The function is also not in the list of > async-signal-safe functions. It seems to just do an array lookup (at least the array is exposed too), so pretty sure it actually is signal-safe. But anyway if you add this explanation to the commit message for why we roll our own them I'm fine with it. I just wondered. -Daniel
diff --git a/lib/igt_core.c b/lib/igt_core.c index 75b98f6..adfa597 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -1303,8 +1303,10 @@ static struct { static igt_exit_handler_t exit_handler_fn[MAX_EXIT_HANDLERS]; static bool exit_handler_disabled; static sigset_t saved_sig_mask; -static const int handled_signals[] = - { SIGINT, SIGHUP, SIGTERM, SIGQUIT, SIGPIPE, SIGABRT, SIGSEGV, SIGBUS }; +#define SIGDEF(x) { x, #x, sizeof(#x) - 1 } +static const struct { int number; const char *name; size_t name_len; } handled_signals[] = + { SIGDEF(SIGINT), SIGDEF(SIGHUP), SIGDEF(SIGTERM), SIGDEF(SIGQUIT), + SIGDEF(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) }; static int install_sig_handler(int sig_num, sighandler_t handler) { @@ -1357,8 +1359,20 @@ static void igt_atexit_handler(void) static void fatal_sig_handler(int sig) { + int i; + restore_all_sig_handler(); + for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { + if (handled_signals[i].number == sig) { + write(STDERR_FILENO, "Received signal ", 16); + write(STDERR_FILENO, handled_signals[i].name, + handled_signals[i].name_len); + write(STDERR_FILENO, ".\n", 2); + break; + } + } + /* * exit_handler_disabled is always false here, since when we set it * we also block signals. @@ -1415,7 +1429,7 @@ void igt_install_exit_handler(igt_exit_handler_t fn) return; for (i = 0; i < ARRAY_SIZE(handled_signals); i++) { - if (install_sig_handler(handled_signals[i], + if (install_sig_handler(handled_signals[i].number, fatal_sig_handler)) goto err; } @@ -1447,7 +1461,7 @@ void igt_disable_exit_handler(void) sigemptyset(&set); for (i = 0; i < ARRAY_SIZE(handled_signals); i++) - sigaddset(&set, handled_signals[i]); + sigaddset(&set, handled_signals[i].number); if (sigprocmask(SIG_BLOCK, &set, &saved_sig_mask)) { perror("sigprocmask");
Signed-off-by: Thomas Wood <thomas.wood@intel.com> --- lib/igt_core.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)