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

[Tamar Christina <Tamar.Christina@arm.com>]

Sorry, also forwarding to mailing list.

For some reason client drops it from CC when I do reply-all..

> -----Original Message-----
> From: Tamar Christina
> Sent: Wednesday, July 8, 2020 12:16 PM
> To: Adhemerval Zanella <adhemerval.zanella@linaro.org>; Carlos O'Donell
> <carlos@redhat.com>
> Subject: RE: [PATCH v5 12/13] string: Add sigabbrev_np and sigdescr_np
> 
> Hi,
> 
> This commit seems to have broken the aarch64 build:
> 
>   sigdescr_np.c:24:1: error: conflicting types for ‘__sigdescr_np’
>    __sigdescr_np (int signum)
>    ^~~~~~~~~~~~~
> 
>   In file included from <command-line>:0:0:
>   ../include/string.h:57:20: note: previous declaration of ‘__sigdescr_np’ was
> here
>    libc_hidden_proto (__sigdescr_np)
>                       ^
>   ./../include/libc-symbols.h:605:33: note: in definition of macro
> ‘__hidden_proto’
>      extern thread __typeof (name) name __hidden_proto_hiddenattr (attrs);
>                                    ^~~~
> 
>   ./../include/libc-symbols.h:624:44: note: in expansion of macro
> ‘hidden_proto’
>    # define libc_hidden_proto(name, attrs...) hidden_proto (name, ##attrs)
>                                               ^~~~~~~~~~~~
>   ../include/string.h:57:1: note: in expansion of macro ‘libc_hidden_proto’
>    libc_hidden_proto (__sigdescr_np)
>    ^~~~~~~~~~~~~~~~~
> 
>   In file included from <command-line>:0:0:
>   ../o-iterator.mk:9: recipe for target '/home/teamcity-
> agent/work/fcc7c27539f88edf/build/string/sigdescr_np.o' failed
>   sigdescr_np.c:34:28: error: conflicting types for ‘sigdescr_np’
>    weak_alias (__sigdescr_np, sigdescr_np)
>                               ^
>   ./../include/libc-symbols.h:152:26: note: in definition of macro ‘_weak_alias’
>      extern __typeof (name) aliasname __attribute__ ((weak, alias (#name)))
> \
>                             ^~~~~~~~~
>   sigdescr_np.c:34:1: note: in expansion of macro ‘weak_alias’
>    weak_alias (__sigdescr_np, sigdescr_np)
>    ^~~~~~~~~~
>   In file included from ../include/string.h:63:0,
>                    from sigdescr_np.c:19:
>   ../string/string.h:469:20: note: previous declaration of ‘sigdescr_np’ was
> here
>    extern const char *sigdescr_np (int __sig) __THROW;
>                       ^~~~~~~~~~~
>   make[2]: *** [/home/teamcity-
> agent/work/fcc7c27539f88edf/build/string/sigdescr_np.o] Error 1
>   make[2]: *** Waiting for unfinished jobs....
>   sigabbrev_np.c:24:1: error: conflicting types for ‘sigabbrev_np’
>    sigabbrev_np (int signum)
>    ^~~~~~~~~~~~
>   In file included from ../include/string.h:63:0,
>                    from sigabbrev_np.c:19:
>   ../string/string.h:466:20: note: previous declaration of ‘sigabbrev_np’ was
> here
>    extern const char *sigabbrev_np (int __sig) __THROW;
>                       ^~~~~~~~~~~~
> 
> I'm afraid I don't know glibc internals well enough to know what the right fix
> is here...
> 
> Thanks,
> Tamar
> 
> > -----Original Message-----
> > From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of
> > Adhemerval Zanella via Libc-alpha
> > Sent: Friday, July 3, 2020 9:24 PM
> > To: Carlos O'Donell <carlos@redhat.com>; libc-alpha@sourceware.org
> > Subject: Re: [PATCH v5 12/13] string: Add sigabbrev_np and sigdescr_np
> >
> >
> >
> > 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.