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....@gmail.com>
> >
> > The Linux man pages recommend this version of the function for portable
> > applications.
> >
> > Signed-off-by: Érico Rolim <erico....@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 <eric...@disroot.org>
> > +
> > + * dwfl_error.c (strerror_r): Always use the XSI-compliant version.
> > +
> > 2020-12-16 Dmitry V. Levin <l...@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
