Re: [LTP] [PATCH] sigrelse01: Check if signal 34 is available for musl compat

[Florian Schmaus via ltp <ltp@lists.linux.it>]

On 08/08/2025 16.04, Petr Vorel wrote:
> Hi Florian,

Hi Petr,

thanks for your review.

>> Do not select signal 34 when the test is run using musl. Signal 34 is
>> used internally by musl as SIGSYNCCALL. Consequently, musl's signal()
>> will return with an error status and errno set to EINVAL when trying
>> to setup a signal handler for signal 34, causing the sigrelse01 test
>> to fail.
> 
>> Since musl provides no preprocessor macro, we check for the
>> availability of signal 34 by attempting to setup a signal handler. If
>> signal() returns SIG_ERR with errno set to EINVAL then we assume the
>> signal is unavailable. Knowing signal 34 is available with glibc, we
>> perform this check only if __GLIBC__ is not defined.
> 
> ...
>> +++ b/testcases/kernel/syscalls/sigrelse/sigrelse01.c
> 
>> +#define _GNU_SOURCE
> 
> Unfortunately +#define _GNU_SOURCE causes test to hang, at least on glibc.

I don't think it hangs, but takes ages to complete. At least, that is 
what I found.


> And I see for musl it is necessary to get sighandler_t.
> Until you fix glibc with _GNU_SOURCE NACK.

I think I can get rid of the _GNU_SOURCE. IIRC it was just needed to use 
the sighandler_t type. Without _GNU_SOURCE, declaring the return value 
of signal() as void* should also work.


> But on glibc it also brought a warning, which means _GNU_SOURCE really switches
> something on:
> 
> sigrelse01.c: In function ‘child’:
> sigrelse01.c:397:33: warning: ‘sighold’ is deprecated: Use the sigprocmask function instead [-Wdeprecated-declarations]
>    397 |                                 if ((rv = sighold(sig)) != 0) {
>        |                                 ^~
> In file included from /usr/include/sys/wait.h:36,
>                   from sigrelse01.c:104:
> /usr/include/signal.h:355:12: note: declared here
>    355 | extern int sighold (int __sig) __THROW
>        |            ^~~~~~~
> sigrelse01.c:472:25: warning: ‘sigrelse’ is deprecated: Use the sigprocmask function instead [-Wdeprecated-declarations]
>    472 |                         if ((rv = sigrelse(sig)) != 0) {
>        |                         ^~
> /usr/include/signal.h:359:12: note: declared here
>    359 | extern int sigrelse (int __sig) __THROW
>        |            ^~~~~~~~
> sigrelse01.c: In function ‘timeout’:
> sigrelse01.c:675:25: warning: unused parameter ‘sig’ [-Wunused-parameter]
>    675 | static void timeout(int sig)
>        |                     ~~~~^~~
> 
> Also this is a very old test, which needs cleanup and rewrite to new LTP API
> (e.g. remove old unixes, e.g. VAX and get test more reliable). I suppose
> handling signals with LTP legacy API is broken.
> 
>> +
>>   #include <sys/types.h>
>>   #include <sys/wait.h>
>>   #include <errno.h>
>>   #include <fcntl.h>
>>   #include <signal.h>
>> +#include <stdbool.h>
> 
> nit: I would postpone this after conversion this to new LTP API.
> Script already uses legacy definitions
> #define TRUE 1
> #define FALSE 0
> and on a different place happily returns 0 or 1.
> Mixing that with <stdbool.h> makes even more mess.

Agreed.

I am going send a v2 that does not use _GNU_SOURCE nor stdbool.h after 
checking that it still fixes musl compat for us while not introducing a 
regression on glibc.

- Florian




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp