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

[Ben Pfaff]

Thank you.  I applied these two commits to master.

On Tue, Jun 04, 2013 at 09:03:22AM -0700, Alex Wang wrote:
> 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
> >