* [PATCH] libdwfl: use GNU strerror_r only when available.
@ 2021-02-01 18:56 Érico Nogueira
2021-02-01 21:35 ` Mark Wielaard
0 siblings, 1 reply; 3+ messages in thread
From: Érico Nogueira @ 2021-02-01 18:56 UTC (permalink / raw)
To: elfutils-devel; +Cc: Érico Rolim
From: Érico Rolim <erico.erc@gmail.com>
Some C libraries don't provide the GNU version of strerror_r, only the
XSI-compliant one. We use the GNU version when available, since it fits
the code better, and otherwise use the XSI-compliant one.
Signed-off-by: Érico Rolim <erico.erc@gmail.com>
---
If possible, I'd like to get this patch in before the release, otherwise
it's an easy one to carry locally.
ChangeLog | 4 ++++
configure.ac | 12 ++++++++++++
libdwfl/ChangeLog | 4 ++++
libdwfl/dwfl_error.c | 17 ++++++++++++++++-
4 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 142caa27..c8f921a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-01 Érico Nogueira <ericonr@disroot.org>
+
+ * configure.ac: Check for GNU strerror_r.
+
2021-01-12 Dmitry V. Levin <ldv@altlinux.org>
* configure.ac [--enable-gcov]: Check for gcov, lcov, and genhtml.
diff --git a/configure.ac b/configure.ac
index 346ab800..baf6faf5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -428,6 +428,18 @@ AC_CHECK_DECLS([mempcpy],[],[],
AC_CHECK_FUNCS([process_vm_readv])
+AC_CACHE_CHECK([whether C library provides GNU strerror_r], ac_cv_gnu_strerror_r, [dnl
+old_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
+#define _GNU_SOURCE
+#include <string.h>
+char * (*s)(int, char*, size_t) = strerror_r;
+])], ac_cv_gnu_strerror_r=yes, ac_cv_gnu_strerror_r=no)
+CFLAGS="$old_CFLAGS"])
+AS_IF([test "x$ac_cv_gnu_strerror_r" = "xyes"],
+ [AC_DEFINE([HAVE_GNU_STRERROR_R], [1], [Defined if libc has GNU style strerror_r])])
+
AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
AC_DEFINE([USE_DEMANGLE], [1], [Defined if demangling is enabled])])
AM_CONDITIONAL(DEMANGLE, test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes")
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 5058f5f8..d107e78f 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-01 Érico Nogueira <ericonr@disroot.org>
+
+ * dwfl_error.c (strerror_r): Only use the GNU version when available.
+
2021-01-08 Timm Bäder <tbaeder@redhat.com>
* elf-from-memory.c (elf_from_remote_memory): Add for loop over
diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
index 7bcf61cc..ef2834b2 100644
--- a/libdwfl/dwfl_error.c
+++ b/libdwfl/dwfl_error.c
@@ -137,6 +137,21 @@ __libdwfl_seterrno (Dwfl_Error error)
}
+static const char *
+errnomsg(int error)
+{
+ /* Won't be changed by strerror_r, but not const so compiler doesn't throw warning */
+ static char unknown[] = "unknown error";
+
+#ifdef HAVE_GNU_STRERROR_R
+ return strerror_r (error, unknown, 0);
+#else
+ /* To store the error message from strerror_r in a thread-safe manner */
+ static __thread char msg[128];
+ return strerror_r (error, msg, sizeof (msg)) ? unknown : msg;
+#endif
+}
+
const char *
dwfl_errmsg (int error)
{
@@ -154,7 +169,7 @@ dwfl_errmsg (int error)
switch (error &~ 0xffff)
{
case OTHER_ERROR (ERRNO):
- return strerror_r (error & 0xffff, "bad", 0);
+ return errnomsg (error & 0xffff);
case OTHER_ERROR (LIBELF):
return elf_errmsg (error & 0xffff);
case OTHER_ERROR (LIBDW):
--
2.30.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libdwfl: use GNU strerror_r only when available.
2021-02-01 18:56 [PATCH] libdwfl: use GNU strerror_r only when available Érico Nogueira
@ 2021-02-01 21:35 ` Mark Wielaard
2021-02-01 23:38 ` Érico Nogueira
0 siblings, 1 reply; 3+ messages in thread
From: Mark Wielaard @ 2021-02-01 21:35 UTC (permalink / raw)
To: Érico Nogueira; +Cc: elfutils-devel, Érico Rolim
Hi Érico,
On Mon, Feb 01, 2021 at 03:56:26PM -0300, Érico Nogueira via Elfutils-devel wrote:
> Some C libraries don't provide the GNU version of strerror_r, only the
> XSI-compliant one. We use the GNU version when available, since it fits
> the code better, and otherwise use the XSI-compliant one.
Could you also mention this bug in the commit message?
https://sourceware.org/bugzilla/show_bug.cgi?id=21010
> If possible, I'd like to get this patch in before the release, otherwise
> it's an easy one to carry locally.
One other comment, but this looks good to go otherwise.
> diff --git a/configure.ac b/configure.ac
> index 346ab800..baf6faf5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -428,6 +428,18 @@ AC_CHECK_DECLS([mempcpy],[],[],
>
> AC_CHECK_FUNCS([process_vm_readv])
>
> +AC_CACHE_CHECK([whether C library provides GNU strerror_r], ac_cv_gnu_strerror_r, [dnl
> +old_CFLAGS="$CFLAGS"
> +CFLAGS="$CFLAGS -Werror"
> +AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
> +#define _GNU_SOURCE
> +#include <string.h>
> +char * (*s)(int, char*, size_t) = strerror_r;
> +])], ac_cv_gnu_strerror_r=yes, ac_cv_gnu_strerror_r=no)
> +CFLAGS="$old_CFLAGS"])
> +AS_IF([test "x$ac_cv_gnu_strerror_r" = "xyes"],
> + [AC_DEFINE([HAVE_GNU_STRERROR_R], [1], [Defined if libc has GNU style strerror_r])])
> +
autoconf comes with the following check:
- Macro: AC_FUNC_STRERROR_R
If strerror_r is available, define HAVE_STRERROR_R, and if it is
declared, define HAVE_DECL_STRERROR_R. If it returns a char *
message, define STRERROR_R_CHAR_P; otherwise it returns an int
error number. The Thread-Safe Functions option of Posix requires
strerror_r to return int, but many systems (including, for
example, version 2.2.4 of the GNU C Library) return a char * value
that is not necessarily equal to the buffer argument.
Can we use that instead?
> +static const char *
> +errnomsg(int error)
> +{
> + /* Won't be changed by strerror_r, but not const so compiler doesn't throw warning */
> + static char unknown[] = "unknown error";
> +
> +#ifdef HAVE_GNU_STRERROR_R
And use #ifdef STRERROR_R_CHAR_P here?
Thanks,
Mark
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libdwfl: use GNU strerror_r only when available.
2021-02-01 21:35 ` Mark Wielaard
@ 2021-02-01 23:38 ` Érico Nogueira
0 siblings, 0 replies; 3+ messages in thread
From: Érico Nogueira @ 2021-02-01 23:38 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel
Em 01/02/2021 18:35, Mark Wielaard escreveu:
> Hi Érico,
>
> On Mon, Feb 01, 2021 at 03:56:26PM -0300, Érico Nogueira via Elfutils-devel wrote:
>> Some C libraries don't provide the GNU version of strerror_r, only the
>> XSI-compliant one. We use the GNU version when available, since it fits
>> the code better, and otherwise use the XSI-compliant one.
>
> Could you also mention this bug in the commit message?
> https://sourceware.org/bugzilla/show_bug.cgi?id=21010
Will do!
>
>> If possible, I'd like to get this patch in before the release, otherwise
>> it's an easy one to carry locally.
>
> One other comment, but this looks good to go otherwise.
>
>> diff --git a/configure.ac b/configure.ac
>> index 346ab800..baf6faf5 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -428,6 +428,18 @@ AC_CHECK_DECLS([mempcpy],[],[],
>>
>> AC_CHECK_FUNCS([process_vm_readv])
>>
>> +AC_CACHE_CHECK([whether C library provides GNU strerror_r], ac_cv_gnu_strerror_r, [dnl
>> +old_CFLAGS="$CFLAGS"
>> +CFLAGS="$CFLAGS -Werror"
>> +AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
>> +#define _GNU_SOURCE
>> +#include <string.h>
>> +char * (*s)(int, char*, size_t) = strerror_r;
>> +])], ac_cv_gnu_strerror_r=yes, ac_cv_gnu_strerror_r=no)
>> +CFLAGS="$old_CFLAGS"])
>> +AS_IF([test "x$ac_cv_gnu_strerror_r" = "xyes"],
>> + [AC_DEFINE([HAVE_GNU_STRERROR_R], [1], [Defined if libc has GNU style strerror_r])])
>> +
>
> autoconf comes with the following check:
>
> - Macro: AC_FUNC_STRERROR_R
>
> If strerror_r is available, define HAVE_STRERROR_R, and if it is
> declared, define HAVE_DECL_STRERROR_R. If it returns a char *
> message, define STRERROR_R_CHAR_P; otherwise it returns an int
> error number. The Thread-Safe Functions option of Posix requires
> strerror_r to return int, but many systems (including, for
> example, version 2.2.4 of the GNU C Library) return a char * value
> that is not necessarily equal to the buffer argument.
>
> Can we use that instead?
Thank you for pointing me at it :)
I was a bit worried about rolling my own check here, glad to find out
there is a standard way of doing it.
>
>> +static const char *
>> +errnomsg(int error)
>> +{
>> + /* Won't be changed by strerror_r, but not const so compiler doesn't throw warning */
>> + static char unknown[] = "unknown error";
>> +
>> +#ifdef HAVE_GNU_STRERROR_R
>
> And use #ifdef STRERROR_R_CHAR_P here?
>
> Thanks,
>
> Mark
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-01 23:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 18:56 [PATCH] libdwfl: use GNU strerror_r only when available Érico Nogueira
2021-02-01 21:35 ` Mark Wielaard
2021-02-01 23:38 ` Érico Nogueira
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).