| Message ID | 20220824094029.1634519-30-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qtest: Enable running qtest on Windows | expand |
Hi On Wed, Aug 24, 2022 at 2:47 PM Bin Meng <bmeng.cn@gmail.com> 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 > avaiable on Windows which we are going to support. > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > --- > > 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 1b24a4f1f7..70d7578740 100644 > --- a/tests/qtest/libqtest.c > +++ b/tests/qtest/libqtest.c > @@ -68,7 +68,7 @@ struct QTestState > QTestState *global_qtest; > > static GHookList abrt_hooks; > -static struct sigaction sigact_old; > +static sighandler_t sighandler_old; > > static int qtest_query_target_endianness(QTestState *s); > > @@ -181,20 +181,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) > -- > We should keep the sigaction() version for !WIN32, it has notoriously less issues, more modern etc. signal() only on win32. Although in this particular usage, I don't think that makes much difference...
On Wed, Aug 31, 2022 at 10:16 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Wed, Aug 24, 2022 at 2:47 PM Bin Meng <bmeng.cn@gmail.com> 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 >> avaiable on Windows which we are going to support. >> >> Signed-off-by: Bin Meng <bin.meng@windriver.com> >> --- >> >> 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 1b24a4f1f7..70d7578740 100644 >> --- a/tests/qtest/libqtest.c >> +++ b/tests/qtest/libqtest.c >> @@ -68,7 +68,7 @@ struct QTestState >> QTestState *global_qtest; >> >> static GHookList abrt_hooks; >> -static struct sigaction sigact_old; >> +static sighandler_t sighandler_old; >> >> static int qtest_query_target_endianness(QTestState *s); >> >> @@ -181,20 +181,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) >> -- > > > > We should keep the sigaction() version for !WIN32, it has notoriously less issues, more modern etc. signal() only on win32. > > Although in this particular usage, I don't think that makes much difference... Yes, as I mentioned in the commit message, the codes uses sigaction() to install signal handler with a flag SA_RESETHAND, and such can be replaced by the signal() API. It is still a supported API in POSIX so we should be safe to use it to simplify the code paths. Unless you are strongly against this? Regards, Bin
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 1b24a4f1f7..70d7578740 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -68,7 +68,7 @@ struct QTestState QTestState *global_qtest; static GHookList abrt_hooks; -static struct sigaction sigact_old; +static sighandler_t sighandler_old; static int qtest_query_target_endianness(QTestState *s); @@ -181,20 +181,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)