Re: [Qemu-devel] [RFC PATCH 1/7] error reporting: Introduce errnoval parameter to vreport

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On 04/26/2018 12:24 PM, Eric Blake wrote:

>> @@ -222,6 +224,11 @@ static void vreport(report_type type, const char *fmt, va_list ap)
>>      }
>>  
>>      error_vprintf(fmt, ap);
>> +
>> +    if (errnoval >= 0) {
>> +        error_printf(": %s", strerror(errnoval));
> 
> Off-by-one.  You do NOT want to print strerror(0) (that results in
> appending ": Success" or similar to what is supposed to be an error
> message).

> 
> Passing -1 to suppress the output feels awkward, especially if 0 can be
> used for the same purpose and would then let us use -errno and errno
> interchangeable by passing the absolute value to sterror.
> 

I'm also using, as my reference, the glibc 'man 3 error' function, which
specifically documents:

  if  errnum  is  nonzero,  a second colon and a space followed by the
string
       given by strerror(errnum).

making it a very versatile function for outputting messages to stderr
whether or not you also have an errno value to convert via strerror(),
by using 0 (not -1) as the sentinel value.

Consistency argues that even if we are going to invent our own qemu
functions, we're better off sticking to well-known paradigms already in
the wild (such as 0 to suppress appending strerror()) rather than making
readers learn yet another paradigm.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]