[PATCH v2 1/2] libdwfl: use XSI-compliant strerror_r.

[Érico Nogueira]

On Wed Dec 16, 2020 at 9:08 PM -03, Dmitry V. Levin wrote:
> On Wed, Dec 16, 2020 at 07:30:11PM -0300, Érico Nogueira via
> Elfutils-devel wrote:
> > From: Érico Rolim <erico.erc@gmail.com>
> > 
> > The Linux man pages recommend this version of the function for portable
> > applications.
> > 
> > Signed-off-by: Érico Rolim <erico.erc@gmail.com>
>
> I'd rather not do it this way because __xpg_strerror_r in glibc is a
> wrapper around GNU strerror_r which does this almost always unneeded
> string copying. Instead of penalizing GNU systems, I'd suggest checking
> at configure time what kind of strerror_r do we have, and choosing the
> code appropriately.

Fair enough. The GNU version of strerror_r does have a nicer API :)

>
> > ---
> > 
> > Only difference from the initial patch is that this includes the
> > Signed-off-by line.
> > 
> >  libdwfl/ChangeLog    |  4 ++++
> >  libdwfl/dwfl_error.c | 11 ++++++++++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> > index f9f6f01f..d22f9892 100644
> > --- a/libdwfl/ChangeLog
> > +++ b/libdwfl/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-12-16  Érico Nogueira  <ericonr@disroot.org>
> > +
> > +	* dwfl_error.c (strerror_r): Always use the XSI-compliant version.
> > +
> >  2020-12-16  Dmitry V. Levin  <ldv@altlinux.org>
> >  
> >  	* argp-std.c (_): Remove.
> > diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
> > index 7bcf61cc..e5db1217 100644
> > --- a/libdwfl/dwfl_error.c
> > +++ b/libdwfl/dwfl_error.c
> > @@ -30,6 +30,11 @@
> >  # include <config.h>
> >  #endif
> >  
> > +/* Guarantee that we get the XSI compliant strerror_r */
> > +#ifdef _GNU_SOURCE
> > +#undef _GNU_SOURCE
> > +#endif
> > +
> >  #include <assert.h>
> >  #include <libintl.h>
> >  #include <stdbool.h>
> > @@ -136,6 +141,8 @@ __libdwfl_seterrno (Dwfl_Error error)
> >    global_error = canonicalize (error);
> >  }
> >  
> > +/* To store the error message from strerror_r */
> > +static __thread char errormsg[128];
> >  
> >  const char *
> >  dwfl_errmsg (int error)
> > @@ -154,7 +161,9 @@ dwfl_errmsg (int error)
> >    switch (error &~ 0xffff)
> >      {
> >      case OTHER_ERROR (ERRNO):
> > -      return strerror_r (error & 0xffff, "bad", 0);
> > +      if (strerror_r (error & 0xffff, errormsg, sizeof errormsg))
> > +	return "strerror_r() failed()";
> > +      return errormsg;
> >      case OTHER_ERROR (LIBELF):
> >        return elf_errmsg (error & 0xffff);
> >      case OTHER_ERROR (LIBDW):
>
> What if we had something like this:
>
> static const char *
> errnomsg(int error)
> {
> static const char unknown[] = "unknown error";
>
> #ifdef HAVE_STRERROR_R_POSIX_SIGNATURE
> static __thread char msg[128];
> return strerror_r (error, msg, sizeof (msg)) ? unknown : msg;
> #else
> return strerror_r (error, unknown, 0);
> #endif
> }

Would you prefer this go in lib/ as a "general usage" function or do I
just leave it in libdwfl/ for now?

As a side note, the trick of passing a constant string and buflen=0 is
quite neat, and isn't obvious from reading the man page :)

>
> and use it in dwfl_errmsg instead of strerror_r?
>
>
> --
> ldv