diff mbox series

lib: Make error.c more like error(3)

Message ID 20210910180716.1464674-1-ccross@google.com
State Committed
Headers show
Series lib: Make error.c more like error(3) | expand

Commit Message

Colin Cross Sept. 10, 2021, 6:07 p.m. UTC
Fix some issues with the error reimplementation to make it match
the specification for error(3).

Flush stdout before printing to stderr.  Also flush stderr afterwards,
which is not specified in the man page for error(3), but is what
bionic does.

error(3) prints strerror(errnum) if and only if errnum is nonzero,
but verr prints strerror(errno) unconditionaly.  When errnum is nonzero
copy it to errno and use verr, and when it is not set use verrx that
doesn't print errno.

error(3) only exits if status is nonzero, but verr exits uncondtionally.
Use vwarn/vwarnx when status is zero, which don't exit.

Signed-off-by: Colin Cross <ccross@google.com>
---
 lib/error.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Mark Wielaard Sept. 12, 2021, 9 p.m. UTC | #1
Hi Colin,

On Fri, Sep 10, 2021 at 11:07:16AM -0700, Colin Cross via Elfutils-devel wrote:
> Fix some issues with the error reimplementation to make it match
> the specification for error(3).
> 
> Flush stdout before printing to stderr.  Also flush stderr afterwards,
> which is not specified in the man page for error(3), but is what
> bionic does.
> 
> error(3) prints strerror(errnum) if and only if errnum is nonzero,
> but verr prints strerror(errno) unconditionaly.  When errnum is nonzero
> copy it to errno and use verr, and when it is not set use verrx that
> doesn't print errno.
> 
> error(3) only exits if status is nonzero, but verr exits uncondtionally.
> Use vwarn/vwarnx when status is zero, which don't exit.
> 
> Signed-off-by: Colin Cross <ccross@google.com>

This is really nice. Thanks.  I pushed it with a ChangeLog entry and
reformatted the patch to GNU style as attached.

Note that there are still some small differences between "real" error
and our error implementation, which don't seem to matter in practice,
but do cause some testsuite failures because of silly
formatting. e.g. run-stack-i.sh fails with:

-stack: tid 13654: shown max number of frames (6, use -n 0 for unlimited)
+/home/mark/src/elfutils/src/stack: tid 13654: shown max number of frames (6, use -n 0 for unlimited)

I have not tries to fix these. The testcases probably should be
slightly less picky about the exact error message format.

Cheers,

Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lib-Make-error.c-more-like-error-3.patch
Type: text/x-diff
Size: 2489 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/elfutils-devel/attachments/20210912/9a1a2ad1/attachment.bin>
diff mbox series

Patch

diff --git a/lib/error.c b/lib/error.c
index 75e964fd..dba046ef 100644
--- a/lib/error.c
+++ b/lib/error.c
@@ -29,7 +29,9 @@ 
 #include <config.h>
 
 #if !defined(HAVE_ERROR_H) && defined(HAVE_ERR_H)
+#include <errno.h>
 #include <stdarg.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <err.h>
 
@@ -37,13 +39,32 @@  unsigned int error_message_count = 0;
 
 void error(int status, int errnum, const char *format, ...) {
   va_list argp;
+  int saved_errno = errno;
+
+  fflush(stdout);
 
   va_start(argp, format);
-  verr(status, format, argp);
+  if (status) {
+    if (errnum) {
+      errno = errnum;
+      verr(status, format, argp);
+    } else {
+      verrx(status, format, argp);
+    }
+  } else {
+    if (errnum) {
+      errno = errnum;
+      vwarn(format, argp);
+    } else {
+      vwarnx(format, argp);
+    }
+  }
   va_end(argp);
 
-  if (status)
-    exit(status);
+  fflush(stderr);
+
   ++error_message_count;
+
+  errno = saved_errno;
 }
 #endif