Re: [PATCH 1/5] progress.c: replace signal() with sigaction()

From: Chris Packham <judge.packham@gmail.com>
Date: May 28, 2014, 08:07 UTC
Message-ID: <5385994A.5040507@gmail.com>
In-Reply-To: <1401257655-6043-2-git-send-email-jmmahler@gmail.com>

On 28/05/14 18:14, Jeremiah Mahler wrote:

Show 5 quoted lines

> From signal(2)
> 
>   The behavior of signal() varies across UNIX versions, and has also var‐
>   ied historically across different versions of Linux.   Avoid  its  use:
>   use sigaction(2) instead.  See Portability below.

Minor nit. The last sentence applies to the man page you're quoting and doesn't really make sense when viewed in the context of this commit message. Same applies to other patches in this series.

Show 20 quoted lines

> 
> Replaced signal() with sigaction() in progress.c
> 
> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
> ---
>  progress.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/progress.c b/progress.c
> index 261314e..24df263 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -66,8 +66,12 @@ static void set_progress_signal(void)
>  static void clear_progress_signal(void)
>  {
>  	struct itimerval v = {{0,},};
> +	struct sigaction sa;
> +
> +	memset(&sa, 0, sizeof(sa));
> +	sa.sa_handler = SIG_IGN;

A C99 initialiser here would save the call to memset. Unfortunately Documentation/CodingGuidelines is fairly clear on not using C99 initialisers, given the fact we're now at git 2.0 maybe it's time to revisit this policy?

Show 7 quoted lines

>  	setitimer(ITIMER_REAL, &v, NULL);
> -	signal(SIGALRM, SIG_IGN);
> +	sigaction(SIGALRM, &sa, 0);
>  	progress_update = 0;
>  }
>  
>