[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2] virnetserver: handle sigaction correctly



At 04/20/2012 11:45 AM, Eric Blake Wrote:
> POSIX says that sa_sigaction is only safe to use if sa_flags
> includes SA_SIGINFO; conversely, sa_handler is only safe to
> use when flags excludes that bit.  Gnulib doesn't guarantee
> an implementation of SA_SIGINFO, but does guarantee that
> if SA_SIGINFO is undefined, we can safely define it to 0 as
> long as we don't dereference the 2nd or 3rd argument of
> any handler otherwise registered via sa_sigaction.
> 
> Based on a report by Wen Congyang.
> 
> * src/rpc/virnetserver.c (SA_SIGINFO): Stub for mingw.
> (virNetServerSignalHandler): Avoid bogus dereference.
> (virNetServerFatalSignal, virNetServerNew): Set flags properly.
> ---
> 
> v2: Simplify according to what gnulib gives us for mingw.
> 
>  src/rpc/virnetserver.c |   28 ++++++++++++++++++++--------
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 3965fc2..3f3989e 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -1,7 +1,7 @@
>  /*
>   * virnetserver.c: generic network RPC server
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -40,6 +40,10 @@
>  # include "virnetservermdns.h"
>  #endif
> 
> +#ifndef SA_SIGINFO
> +# define SA_SIGINFO 0
> +#endif
> +
>  #define VIR_FROM_THIS VIR_FROM_RPC
>  #define virNetError(code, ...)                                    \
>      virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,           \
> @@ -270,8 +274,9 @@ error:
>  }
> 
> 
> -static void virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
> -                                    void *context ATTRIBUTE_UNUSED)
> +static void
> +virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
> +                        void *context ATTRIBUTE_UNUSED)
>  {
>      struct sigaction sig_action;
>      int origerrno;
> @@ -286,6 +291,7 @@ static void virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED
>  #ifdef SIGUSR2
>      if (sig != SIGUSR2) {
>  #endif
> +        memset(&sig_action, 0, sizeof(sig_action));
>          sig_action.sa_handler = SIG_DFL;
>          sigaction(sig, &sig_action, NULL);
>          raise(sig);
> @@ -363,6 +369,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>       * debugging purposes or testing
>       */
>      sig_action.sa_sigaction = virNetServerFatalSignal;
> +    sig_action.sa_flags = SA_SIGINFO;
>      sigaction(SIGFPE, &sig_action, NULL);
>      sigaction(SIGSEGV, &sig_action, NULL);
>      sigaction(SIGILL, &sig_action, NULL);
> @@ -420,17 +427,24 @@ static sig_atomic_t sigErrors = 0;
>  static int sigLastErrno = 0;
>  static int sigWrite = -1;
> 
> -static void virNetServerSignalHandler(int sig, siginfo_t * siginfo,
> -                                      void* context ATTRIBUTE_UNUSED)
> +static void
> +virNetServerSignalHandler(int sig, siginfo_t * siginfo ATTRIBUTE_UNUSED,

Why siginfo has the attribute ATTRIBUTE_UNUSED?

> +                          void* context ATTRIBUTE_UNUSED)
>  {
>      int origerrno;
>      int r;
> +    siginfo_t tmp;
> +
> +    if (SA_SIGINFO)
> +        tmp = *siginfo;
> +    else
> +        memset(&tmp, 0, sizeof(tmp));
> 
>      /* set the sig num in the struct */
> -    siginfo->si_signo = sig;
> +    tmp->si_signo = sig;

s/->/./

> 
>      origerrno = errno;
> -    r = safewrite(sigWrite, siginfo, sizeof(*siginfo));
> +    r = safewrite(sigWrite, &tmp, sizeof(tmp));
>      if (r == -1) {
>          sigErrors++;
>          sigLastErrno = errno;
> @@ -533,9 +547,7 @@ int virNetServerAddSignalHandler(virNetServerPtr srv,
> 
>      memset(&sig_action, 0, sizeof(sig_action));
>      sig_action.sa_sigaction = virNetServerSignalHandler;
> -#ifdef SA_SIGINFO
>      sig_action.sa_flags = SA_SIGINFO;
> -#endif
>      sigemptyset(&sig_action.sa_mask);
> 
>      sigaction(signum, &sig_action, &sigdata->oldaction);

ACK with nit fixed.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]