Re: [PATCH] lib: Fix unused parameter warning in lib/error.c

[Mark Wielaard <mark@klomp.org>]

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