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
signature.asc
Description: OpenPGP digital signature
