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

[Dmitry V. Levin]

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.

> ---
> 
> 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
}

and use it in dwfl_errmsg instead of strerror_r?


-- 
ldv