Re: [LTP] [PATCH] lapi/signal.h: rm references to _SC_SIGRT_* and __SIGRT*

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> The old API's tst_sig.c uses_SC_SIGRT_MIN/MAX are not extant and a
>> quick internet search does not reveal much. It's assumed they meant
>> the same thing as tst_sigrt{min,max}_kern.
>> 
>> The fallback using __SIGRT{MIN,MAX} is also replaced with
>> tst_signal_is_reserved_rt. It says sigrtmin on HPPA is 37. However in
>> the kernel all arches have it set to 32.
>> 
>> In all cases __SIGRT{MIN,MAX} is replaces with constants we define as
>> we can not use __ variables. Two functions are introduced to make the
>> meaning clear when using these constants to check if a signal is
>> reserved and/or realtime.
>> 
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> Suggested-by: Petr Vorel <pvorel@suse.cz>
>> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
>> ---
>>  include/lapi/signal.h                         | 36 ++++++-------
>>  lib/tst_sig.c                                 | 51 +------------------
>>  testcases/kernel/syscalls/ptrace/ptrace05.c   |  2 +-
>>  .../syscalls/rt_sigsuspend/rt_sigsuspend01.c  |  2 +-
>>  testcases/kernel/syscalls/sighold/sighold02.c |  4 +-
>>  5 files changed, 23 insertions(+), 72 deletions(-)
>> 
>> diff --git a/include/lapi/signal.h b/include/lapi/signal.h
>> index 6f4a76881..aa185a5a6 100644
>> --- a/include/lapi/signal.h
>> +++ b/include/lapi/signal.h
>> @@ -1,24 +1,24 @@
>> -// SPDX-License-Identifier: GPL-2.0-or-later
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>
> The style with "//" seems to be more common in LTP. If we want to unify
> on the latter it should be done for all files and in a separate patch.
>
>>  /*
>> - * Copyright (c) 2019 Linaro Limited. All rights reserved.
>> - * Author: Daniel D??az <daniel.diaz@linaro.org>
>> + * Copyright (c) 2022 SUSE LLC
>>   */
>> -
>> -#ifndef LAPI_SIGNAL_H__
>> -#define LAPI_SIGNAL_H__
>
> Why have you deleted the guards?

Uff, I don't think I was paying attention when writing this patch!

>
>>  #include <signal.h>
>>  
>> -/*
>> - * Some libc implementations might differ in the definitions they include. This
>> - * covers those differences for all tests to successfully build.
>> - */
>> +const int tst_sigrtmin_kern = 32;
>> +const int tst_sigrtmax_kern = 64;
>
> I would go for macros here but I guess that either one works.
>

This at least is deliberate because constant variables work better with
some tooling such as clangd. I'm not sure why, but it can't rename
macros. Although thinking about it, I'm not sure if these should be
static...

-- 
Thank you,
Richard.

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