qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] checkpatch: should not use signal except for SI


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [PATCH] checkpatch: should not use signal except for SIG_DFL or SIG_IGN
Date: Tue, 4 Jul 2017 10:33:31 +0100
User-agent: Mutt/1.8.0 (2017-02-23)

On Tue, Jul 04, 2017 at 11:27:04AM +0200, Paolo Bonzini wrote:
> Using signal to establish a signal handler is not portable; on
> SysV systems, the signal handler would be reset to SIG_DFL after
> delivery, while BSD preserves the signal handler.  Daniel Berrange
> reported that (to complicate matters further) the signal system call
> has SysV behavior, but glibc signal() actually calls the sigaction
> system call to provide BSD behavior.
> 
> However, using signal() to set a signal's disposition to SIG_DFL
> or SIG_IGN is portable and is a relatively common occurrence in
> QEMU source code, so allow that.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  scripts/checkpatch.pl | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrange <address@hidden>

We currently only have 1 violation of this rule, and that
is in the TCG test suite, so pretty harmless.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 45027b9281..73efc927a9 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2473,6 +2473,10 @@ sub process {
>               if ($line =~ /\b(strto[^kd].*?)\s*\(/) {
>                       ERROR("consider using qemu_$1 in preference to $1\n" . 
> $herecurr);
>               }
> +# recommend sigaction over signal for portability, when establishing a 
> handler
> +             if ($line =~ /\bsignal\s*\(/ && !($line =~ /SIG_(?:IGN|DFL)/)) {
> +                     ERROR("use sigaction to establish signal handlers; 
> signal is not portable\n" . $herecurr);
> +             }
>  # check for module_init(), use category-specific init macros explicitly 
> please
>               if ($line =~ /^module_init\s*\(/) {
>                       ERROR("please use block_init(), type_init() etc. 
> instead of module_init()\n" . $herecurr);
> -- 
> 2.13.0
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]