Re: [PATCH v5 12/13] string: Add sigabbrev_np and sigdescr_np

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 02/07/2020 18:13, Carlos O'Donell wrote:
> On 6/19/20 9:43 AM, Adhemerval Zanella wrote:
>> The sigabbrev_np returns the abbreviated signal name (i.g. "HUP" for
>> SIGHUP) while sigdescr_np returns string describing error number
> 
> s/returns string/returns the string/g
> s/ error/ the error/g

Ack.

> 
>> (i.g "Hangup" for SIGHUP).  Different than strsignal, sigdescr_np
>> does not attempt to translate the return description and both
>> functions return NULL for an invalid signal number.
>>
>> They should be used instead of sys_siglist or sys_sigabbrev and they
>> are both thread and async-signal safe.  They are added as GNU
>> extensions on string.h header (same as strsignal).
> 
> These are perfect functions to use in place of sys_siglist and
> sys_sigabbrev. Thus we now have functions to access the old data in
> a way that avoids exposing the data to COPY relocations (and the
> size encoded in that ABI). Thanks for fixing this up.
>  
>> Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
>> and s390x-linux-gnu.
> 
> OK for master if you accept the manual suggestions and commit message
> changes.

Done, I also extended a bit the release wiki with a simple example on
how to use these interfaces.

> 
> No regressions on x86_64 and i686.
> 
> Tested-by: Carlos O'Donell <carlos@redhat.com>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 

>>
>> diff --git a/NEWS b/NEWS
>> index ec39b6e056..f504772eb6 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -31,6 +31,16 @@ Major new features:
>>    pthread_attr_getsigmask_np have been added.  They allow applications
>>    to specify the signal mask of a thread created with pthread_create.
>>  
>> +* The functions sigabbrev_np and sigdescr_np have been added.  The
>> +  sigabbrev_np returns the abbreviated signal name (i.g. "HUP" for SIGHUP)
>> +  while sigdescr_np returns string describing signal number (i.g "Hangup"
> 
> s/returns string/returns a string/g
> s/ signal/ the signal/g

Ack.

> 
>> +  for SIGHUP).  Different than strsignal, sigdescr_np does not attempt
>> +  to translate the return description and both functions return NULL for
>> +  an invalid signal number.
>> +
>> +  They should be used instead of sys_siglist or sys_sigabbrev and they
>> +  are both thread and async-signal safe.  These functions are GNU extensions.
> 
> OK.
> 
>> +
>>  Deprecated and removed features, and other changes affecting compatibility:
>>  
>>  * The deprecated <sys/sysctl.h> header and the sysctl function have been
>> diff --git a/include/signal.h b/include/signal.h
>> index 3d6315b741..b4ee02d153 100644
>> --- a/include/signal.h
>> +++ b/include/signal.h
>> @@ -16,7 +16,8 @@ libc_hidden_proto (__libc_current_sigrtmin)
>>  libc_hidden_proto (__libc_current_sigrtmax)
>>  extern const char * const __sys_siglist[_NSIG];
>>  libc_hidden_proto (__sys_siglist)
>> -
>> +extern const char * const __sys_sigabbrev[_NSIG];
>> +libc_hidden_proto (__sys_sigabbrev)
> 
> OK.
> 
>>  
>>  /* Now define the internal interfaces.  */
>>  extern __sighandler_t __bsd_signal (int __sig, __sighandler_t __handler);
>> diff --git a/include/string.h b/include/string.h
>> index ce01ad8254..f4ce138622 100644
>> --- a/include/string.h
>> +++ b/include/string.h
>> @@ -53,6 +53,9 @@ extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
>>  
>>  extern char *__strerror_l (int __errnum, locale_t __loc);
>>  
>> +extern const char *__sigdescr_np (int __errnum);
>> +libc_hidden_proto (__sigdescr_np)
> 
> OK.
> 
>> +
>>  /* Get _STRING_ARCH_unaligned.  */
>>  #include <string_private.h>
>>  #endif
>> diff --git a/manual/signal.texi b/manual/signal.texi
>> index 34def1c06c..a19dff85f8 100644
>> --- a/manual/signal.texi
>> +++ b/manual/signal.texi
>> @@ -880,6 +880,30 @@ to @var{signum}.
>>  This function is a BSD feature, declared in the header file @file{signal.h}.
>>  @end deftypefun
>>  
>> +@deftypefun void sigdescr_np (int @var{signum})
>> +@standards{GNU, string.h}
>> +@safety{@prelim{}@mtsafe{@mtssigintr{}}@assafe{}@acsafe{}}
> 
> Use @mtsafe{}, because the function does not touch _sigintr at all.

Ack.

> 
>> +This function returns the message describing the signal @var{signum} or
>> +@code{NULL} for invalid signal number (i.g "Hangup" for @code{SIGHUP}).
> 
> s/i.g/e.g./g

Ack.

> 
>> +Different than @code{strsignal} the returned description is not translated.
>> +The message porints to a static storage whose lifetime is the whole lifetime
> 
> s/porints/points/g

ACk.

> 
>> +of the program.
>> +
>> +@pindex string.h
>> +This function is a GNU extension, declared in the header file @file{string.h}.
>> +@end deftypefun
>> +
>> +@deftypefun void sigabbrev_np (int @var{signum})
>> +@standards{GNU, string.h}
>> +@safety{@prelim{}@mtsafe{@mtssigintr{}}@assafe{}@acsafe{}}
> 
> Use @mtsafe{}, because the function does not touch _sigintr at all.

Ack.

> 
>> +This function returns the abbreviation describing the signal @var{signum} or
>> +@code{NULL} for invalid signal number.  The message porints to a static
> 
> s/porints/points/g

Ack.