On Tue, Oct 11, 2022 at 10:14 PM Thomas Huth <th...@redhat.com> wrote:
>
> On 06/10/2022 17.19, Bin Meng wrote:
> > From: Bin Meng <bin.m...@windriver.com>
> >
> > At present the codes uses sigaction() to install signal handler with
> > a flag SA_RESETHAND. Such usage can be covered by the signal() API
> > that is a simplified interface to the general sigaction() facility.
> >
> > Update to use signal() to install the signal handler, as it is
> > available on Windows which we are going to support.
> >
> > Signed-off-by: Bin Meng <bin.m...@windriver.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> >
> > Changes in v5:
> > - Replace sighandler_t with its actual definition, since it is not
> > available on BSD hosts
> >
> > tests/qtest/libqtest.c | 14 +++-----------
> > 1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> > index 8228262938..54e5f64f20 100644
> > --- a/tests/qtest/libqtest.c
> > +++ b/tests/qtest/libqtest.c
> > @@ -66,7 +66,7 @@ struct QTestState
> > };
> >
> > static GHookList abrt_hooks;
> > -static struct sigaction sigact_old;
> > +static void (*sighandler_old)(int);
> >
> > static int qtest_query_target_endianness(QTestState *s);
> >
> > @@ -179,20 +179,12 @@ static void sigabrt_handler(int signo)
> >
> > static void setup_sigabrt_handler(void)
> > {
> > - struct sigaction sigact;
> > -
> > - /* Catch SIGABRT to clean up on g_assert() failure */
> > - sigact = (struct sigaction){
> > - .sa_handler = sigabrt_handler,
> > - .sa_flags = SA_RESETHAND,
> > - };
> > - sigemptyset(&sigact.sa_mask);
> > - sigaction(SIGABRT, &sigact, &sigact_old);
> > + sighandler_old = signal(SIGABRT, sigabrt_handler);
> > }
> >
> > static void cleanup_sigabrt_handler(void)
> > {
> > - sigaction(SIGABRT, &sigact_old, NULL);
> > + signal(SIGABRT, sighandler_old);
> > }
> >
> > static bool hook_list_is_empty(GHookList *hook_list)
>
> Hmm, did you notice the error from checkpatch.pl ?
>
> ERROR: use sigaction to establish signal handlers; signal is not portable
>
> ... rationale is given in the commit description here:
>
> https://gitlab.com/qemu-project/qemu/-/commit/e8c2091d4c4dd
Yes, I noticed this checkpatch warning.
>
> ... but since we likely don't care about continuing running after the first
> signal has been delivered, I guess it's ok here to use signal() instead of
> sigaction?
>
I think so. I mentioned in the commit message that the code is using
SA_RESETHAND for sigaction, and such usage can be replaced with
signal().
Regards,
Bin
