diff mbox series

lib: Fix unused parameter warning in lib/error.c

Message ID 20210908182109.2150134-1-ccross@google.com
State Superseded
Headers show
Series lib: Fix unused parameter warning in lib/error.c | expand

Commit Message

Colin Cross Sept. 8, 2021, 6:21 p.m. UTC
Mark the errnum parameter with __attribute__((unused)).

Signed-off-by: Colin Cross <ccross@google.com>
---
 lib/error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Wielaard Sept. 9, 2021, 10:09 a.m. UTC | #1
Hi Colin
(CC Saleem who introduced this new error replacement function),

On Wed, 2021-09-08 at 11:21 -0700, Colin Cross via Elfutils-devel
wrote:
> Mark the errnum parameter with __attribute__((unused)).

Thanks, that is interesting.
But I think this is an actual bug in the code.

Rereviewing the new replacement error function:

> #if !defined(HAVE_ERROR_H) && defined(HAVE_ERR_H)
> #include <stdarg.h>
> #include <stdlib.h>
> #include <err.h>
> 
> unsigned int error_message_count = 0;
> 
> void error(int status, int errnum, const char *format, ...) {
>   va_list argp;
> 
>   va_start(argp, format);
>   verr(status, format, argp);
>   va_end(argp);
> 
>   if (status)
>     exit(status);
>   ++error_message_count;
> }
> #endif

I see three issues with the code:

1) error is supposed to first flush stdout before printing to stderr.
   This is minor, but might make the error message appear differently
   when stdout and stderr are mixed.

2) errnum isn't actually used as you noticed.
   error is supposed to print strerror(errnum) if errnum is not zero.

   Instead verr print strerror(errno), even when errno is zero.

   So I think the code should use errnum (if it is not zero), save the
   current value of errno, assign errnum to errno, call verr and set
   errno back.

3) error ignores status if it is zero, but verr seems to call exit (0)
   meaning it terminates the program instead of simply printing an
   error and increasing error_message_count.

Could the error replacement function be rewritten to behave more
correctly?

Thanks,

Mark
diff mbox series

Patch

diff --git a/lib/error.c b/lib/error.c
index 75e964fd..651a0f3d 100644
--- a/lib/error.c
+++ b/lib/error.c
@@ -35,7 +35,7 @@ 
 
 unsigned int error_message_count = 0;
 
-void error(int status, int errnum, const char *format, ...) {
+void error(int status, int errnum __attribute__ ((unused)), const char *format, ...) {
   va_list argp;
 
   va_start(argp, format);