[PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()

Bin Meng posted 54 patches 3 years, 4 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Greg Kurz <groug@kaod.org>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Gerd Hoffmann <kraxel@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, John Snow <jsnow@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Markus Armbruster <armbru@redhat.com>, Coiby Xu <Coiby.Xu@gmail.com>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>, Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>
There is a newer version of this series
[PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()
Posted by Bin Meng 3 years, 4 months ago
From: Bin Meng <bin.meng@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.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 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 269d622fe3..f0ac467903 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 sighandler_t sighandler_old;
 
 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)
-- 
2.34.1


Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()
Posted by Thomas Huth 3 years, 4 months ago
On 27/09/2022 13.06, Bin Meng wrote:
> From: Bin Meng <bin.meng@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.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> (no changes since v1)
> 
>   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 269d622fe3..f0ac467903 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 sighandler_t sighandler_old;

This seems to break compilation on NetBSD (which you can test via "make 
vm-build-netbsd" on a Linux KVM host):


../src/tests/qtest/libqtest.c:86:8: error: unknown type name 'sighandler_t'
  static sighandler_t sighandler_old;
         ^~~~~~~~~~~~

  Thomas


Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()
Posted by Bin Meng 3 years, 4 months ago
On Wed, Sep 28, 2022 at 5:43 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 27/09/2022 13.06, Bin Meng wrote:
> > From: Bin Meng <bin.meng@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.meng@windriver.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >
> > (no changes since v1)
> >
> >   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 269d622fe3..f0ac467903 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 sighandler_t sighandler_old;
>
> This seems to break compilation on NetBSD (which you can test via "make
> vm-build-netbsd" on a Linux KVM host):

Oops, so this means this test is not covered by GitLab CI ...

>
>
> ../src/tests/qtest/libqtest.c:86:8: error: unknown type name 'sighandler_t'
>   static sighandler_t sighandler_old;
>          ^~~~~~~~~~~~
>

Regards,
Bin
Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()
Posted by Thomas Huth 3 years, 4 months ago
On 28/09/2022 11.57, Bin Meng wrote:
> On Wed, Sep 28, 2022 at 5:43 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 27/09/2022 13.06, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@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.meng@windriver.com>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>    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 269d622fe3..f0ac467903 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 sighandler_t sighandler_old;
>>
>> This seems to break compilation on NetBSD (which you can test via "make
>> vm-build-netbsd" on a Linux KVM host):
> 
> Oops, so this means this test is not covered by GitLab CI ...

You can enable the NetBSD tests in the Gitlab-CI by setting up Cirrus-CI for 
your account. The information hides here:

  .gitlab-ci.d/cirrus/README.rst

... but since this is running NetBSD in a KVM-enabled container on Cirrus-CI 
via a script from Gitlab-CI, it's a little bit fragile, so it might be 
easier to simply type "make vm-build-netbsd" on your local KVM-capable Linux 
host.

  Thomas