On 5/27/23 13:53, Bruno Haible wrote:
+# define error(status, ...) \
+ ((error)(0, __VA_ARGS__), (status) ? exit (status) : (void)0)
We can do a bit better than that by using 'unreachable ()' instead of
'exit (status)', and passing 'status' (instead of 0) to the underlying
error function. This saves a function call and should still pacify GCC.
Also, it's better to not evaluate 'status' twice. Not that I think
'status' should have side effects or even that it does have side effects
in any Gnulib-using code, just that it's more hygienic in case some
caller foolishly puts side effects there.
I installed the attached further patches into Gnulib to try to address
these issues.From 8b21ff255996a518244a5635e36ea58e21f818be Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Tue, 30 May 2023 12:49:20 -0700
Subject: [PATCH 1/2] =?UTF-8?q?error:=20don=E2=80=99t=20evaluate=20status?=
=?UTF-8?q?=20arg=20twice?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This avoids potential issues if the first argument has a side effect.
* lib/error.in.h (__gl_error_call): New macro, which evaluates its
status arg only once, by using a statement expression if GNU C -
the only platform we need to worry about pacifying - and by simply
calling ‘error’ otherwise.
(error, error_at_line): Use it.
---
ChangeLog | 10 ++++++++++
lib/error.in.h | 20 ++++++++++++++++----
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 91a6135f8a..f70eeef893 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2023-05-30 Paul Eggert <egg...@cs.ucla.edu>
+
+ error: don’t evaluate status arg twice
+ This avoids potential issues if the first argument has a side effect.
+ * lib/error.in.h (__gl_error_call): New macro, which evaluates its
+ status arg only once, by using a statement expression if GNU C -
+ the only platform we need to worry about pacifying - and by simply
+ calling ‘error’ otherwise.
+ (error, error_at_line): Use it.
+
2023-05-28 Bruno Haible <br...@clisp.org>
warnings, manywarnings: Assume autoconf >= 2.64.
diff --git a/lib/error.in.h b/lib/error.in.h
index 70fb132133..a5f49e4143 100644
--- a/lib/error.in.h
+++ b/lib/error.in.h
@@ -49,6 +49,18 @@
# define _GL_ATTRIBUTE_SPEC_PRINTF_ERROR _GL_ATTRIBUTE_SPEC_PRINTF_SYSTEM
#endif
+#ifdef __GNUC__
+# define __gl_error_call(function, status, ...) \
+ ({ \
+ int const __errstatus = status; \
+ (function) (0, __VA_ARGS__); \
+ __errstatus != 0 ? exit (__errstatus) : (void) 0; \
+ })
+#else
+# define __gl_error_call(function, status, ...) \
+ (function) (status, __VA_ARGS__)
+#endif
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -69,7 +81,7 @@ _GL_CXXALIAS_RPL (error, void,
# ifndef _GL_NO_INLINE_ERROR
# undef error
# define error(status, ...) \
- ((rpl_error)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0)
+ __gl_error_call (rpl_error, status, __VA_ARGS)
# endif
#else
# if ! @HAVE_ERROR@
@@ -81,7 +93,7 @@ _GL_CXXALIAS_SYS (error, void,
(int __status, int __errnum, const char *__format, ...));
# ifndef _GL_NO_INLINE_ERROR
# define error(status, ...) \
- ((error)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0)
+ __gl_error_call (error, status, __VA_ARGS__)
# endif
#endif
#if __GLIBC__ >= 2
@@ -105,7 +117,7 @@ _GL_CXXALIAS_RPL (error_at_line, void,
# ifndef _GL_NO_INLINE_ERROR
# undef error_at_line
# define error_at_line(status, ...) \
- ((rpl_error_at_line)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0)
+ __gl_error_call (rpl_error_at_line, status, __VA_ARGS__)
# endif
#else
# if ! @HAVE_ERROR_AT_LINE@
@@ -119,7 +131,7 @@ _GL_CXXALIAS_SYS (error_at_line, void,
unsigned int __lineno, const char *__format, ...));
# ifndef _GL_NO_INLINE_ERROR
# define error_at_line(status, ...) \
- ((error_at_line)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0)
+ __gl_error_call (error_at_line, status, __VA_ARGS__)
# endif
#endif
_GL_CXXALIASWARN (error_at_line);
--
2.40.1
From 47811d1ac24e38e8842dc37e2e3cd3b4120338ad Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Tue, 30 May 2023 14:38:41 -0700
Subject: [PATCH 2/2] =?UTF-8?q?error:=20don=E2=80=99t=20call=20=E2=80=98ex?=
=?UTF-8?q?it=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Let the underlying functions call ‘exit’, instead of having the
Gnulib replacement macros do it. Use ‘unreachable’ to tell the
compiler that those functions exit when the status is nonzero.
This saves a function call.
* lib/error.in.h: Include stddef.h, not stdlib.h.
(__gl_error_call): Rely on the function to exit, using
‘unreachable’ to tell the compiler that the function does not return.
* modules/error (Depends-on): Add stddef.
---
ChangeLog | 10 ++++++++++
lib/error.in.h | 10 +++++-----
modules/error | 1 +
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f70eeef893..cd427e9819 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
2023-05-30 Paul Eggert <egg...@cs.ucla.edu>
+ error: don’t call ‘exit’
+ Let the underlying functions call ‘exit’, instead of having the
+ Gnulib replacement macros do it. Use ‘unreachable’ to tell the
+ compiler that those functions exit when the status is nonzero.
+ This saves a function call.
+ * lib/error.in.h: Include stddef.h, not stdlib.h.
+ (__gl_error_call): Rely on the function to exit, using
+ ‘unreachable’ to tell the compiler that the function does not return.
+ * modules/error (Depends-on): Add stddef.
+
error: don’t evaluate status arg twice
This avoids potential issues if the first argument has a side effect.
* lib/error.in.h (__gl_error_call): New macro, which evaluates its
diff --git a/lib/error.in.h b/lib/error.in.h
index a5f49e4143..ecb19818c8 100644
--- a/lib/error.in.h
+++ b/lib/error.in.h
@@ -35,12 +35,12 @@
#error "Please include config.h first."
#endif
+/* Get 'unreachable'. */
+#include <stddef.h>
+
/* Get _GL_ATTRIBUTE_SPEC_PRINTF_STANDARD, _GL_ATTRIBUTE_SPEC_PRINTF_SYSTEM. */
#include <stdio.h>
-/* Get exit(). */
-#include <stdlib.h>
-
/* The definitions of _GL_FUNCDECL_RPL etc. are copied here. */
#if GNULIB_VFPRINTF_POSIX
@@ -53,8 +53,8 @@
# define __gl_error_call(function, status, ...) \
({ \
int const __errstatus = status; \
- (function) (0, __VA_ARGS__); \
- __errstatus != 0 ? exit (__errstatus) : (void) 0; \
+ (function) (__errstatus, __VA_ARGS__); \
+ __errstatus != 0 ? unreachable () : (void) 0; \
})
#else
# define __gl_error_call(function, status, ...) \
diff --git a/modules/error b/modules/error
index 63317f1821..88d567674a 100644
--- a/modules/error
+++ b/modules/error
@@ -7,6 +7,7 @@ m4/error.m4
Depends-on:
error-h
+stddef
stdio
getprogname
strerror [test $COMPILE_ERROR_C = 1]
--
2.40.1
