[ovs-dev] [PATCH 2/2] signals: Make signal_name() thread-safe.

Alex Wang alexw at nicira.com
Tue Jun 4 16:03:22 UTC 2013


Looks good to me. Thanks


On Wed, May 1, 2013 at 11:25 AM, Ben Pfaff <blp at nicira.com> wrote:

> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/fatal-signal.c |    4 +++-
>  lib/process.c      |   10 ++++++++--
>  lib/signals.c      |   29 +++++++++++++++--------------
>  lib/signals.h      |    4 +++-
>  4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 66c0445..8c66ef5 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -155,8 +155,10 @@ fatal_signal_run(void)
>
>      sig_nr = stored_sig_nr;
>      if (sig_nr != SIG_ATOMIC_MAX) {
> +        char namebuf[SIGNAL_NAME_BUFSIZE];
> +
>          VLOG_WARN("terminating with signal %d (%s)",
> -                  (int)sig_nr, signal_name(sig_nr));
> +                  (int)sig_nr, signal_name(sig_nr, namebuf, sizeof
> namebuf));
>          call_hooks(sig_nr);
>
>          /* Re-raise the signal with the default handling so that the
> program
> diff --git a/lib/process.c b/lib/process.c
> index 795a136..3c6eabd 100644
> --- a/lib/process.c
> +++ b/lib/process.c
> @@ -348,9 +348,15 @@ process_status_msg(int status)
>      if (WIFEXITED(status)) {
>          ds_put_format(&ds, "exit status %d", WEXITSTATUS(status));
>      } else if (WIFSIGNALED(status)) {
> -        ds_put_format(&ds, "killed (%s)", signal_name(WTERMSIG(status)));
> +        char namebuf[SIGNAL_NAME_BUFSIZE];
> +
> +        ds_put_format(&ds, "killed (%s)",
> +                      signal_name(WTERMSIG(status), namebuf, sizeof
> namebuf));
>      } else if (WIFSTOPPED(status)) {
> -        ds_put_format(&ds, "stopped (%s)", signal_name(WSTOPSIG(status)));
> +        char namebuf[SIGNAL_NAME_BUFSIZE];
> +
> +        ds_put_format(&ds, "stopped (%s)",
> +                      signal_name(WSTOPSIG(status), namebuf, sizeof
> namebuf));
>      } else {
>          ds_put_format(&ds, "terminated abnormally (%x)", status);
>      }
> diff --git a/lib/signals.c b/lib/signals.c
> index 09e0e09..b5d6d0b 100644
> --- a/lib/signals.c
> +++ b/lib/signals.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -137,37 +137,38 @@ signal_handler(int signr)
>      }
>  }
>
> -/* Returns the name of signal 'signum' as a string.  The string may be in
> a
> - * static buffer that is reused from one call to the next.
> +/* Returns the name of signal 'signum' as a string.  The return value is
> either
> + * a statically allocated constant string or the 'bufsize'-byte buffer
> + * 'namebuf'.  'bufsize' should be at least SIGNAL_NAME_BUFSIZE.
>   *
>   * The string is probably a (possibly multi-word) description of the
> signal
>   * (e.g. "Hangup") instead of just the stringified version of the macro
>   * (e.g. "SIGHUP"). */
>  const char *
> -signal_name(int signum)
> +signal_name(int signum, char *namebuf, size_t bufsize)
>  {
> -    const char *name = NULL;
> -
>  #if HAVE_DECL_SYS_SIGLIST
>      if (signum >= 0 && signum < ARRAY_SIZE(sys_siglist)) {
> -        name = sys_siglist[signum];
> +        const char *name = sys_siglist[signum];
> +        if (name) {
> +            return name;
> +        }
>      }
>  #endif
>
> -    if (!name) {
> -        static char buffer[7 + INT_STRLEN(int) + 1];
> -        sprintf(buffer, "signal %d", signum);
> -        name = buffer;
> -    }
> -    return name;
> +    snprintf(namebuf, bufsize, "signal %d", signum);
> +    return namebuf;
>  }
>
>  void
>  xsigaction(int signum, const struct sigaction *new, struct sigaction *old)
>  {
>      if (sigaction(signum, new, old)) {
> +        char namebuf[SIGNAL_NAME_BUFSIZE];
> +
>          VLOG_FATAL("sigaction(%s) failed (%s)",
> -                   signal_name(signum), strerror(errno));
> +                   signal_name(signum, namebuf, sizeof namebuf),
> +                   strerror(errno));
>      }
>  }
>
> diff --git a/lib/signals.h b/lib/signals.h
> index ac96b0f..10a56b8 100644
> --- a/lib/signals.h
> +++ b/lib/signals.h
> @@ -19,6 +19,7 @@
>
>  #include <signal.h>
>  #include <stdbool.h>
> +#include "type-props.h"
>
>  void signal_init(void);
>
> @@ -28,7 +29,8 @@ void signal_unregister(struct signal *);
>  bool signal_poll(struct signal *);
>  void signal_wait(struct signal *);
>
> -const char *signal_name(int signum);
> +enum { SIGNAL_NAME_BUFSIZE = 7 + INT_STRLEN(int) + 1 };
> +const char *signal_name(int signum, char *namebuf, size_t bufsize);
>
>  void xsigaction(int signum, const struct sigaction *, struct sigaction
> *old);
>  void xsigprocmask(int how, const sigset_t *, sigset_t *old);
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130604/fe9ac7a8/attachment-0003.html>


More information about the dev mailing list