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: 2612 bytes --]

On 04/26/2018 12:32 PM, Ian Jackson wrote:

>>>   * Print a message to current monitor if we have one, else to stderr.
>>>   * @report_type is the type of message: error, warning or informational.
>>> + * If @errnoval is nonnegative it is fed to strerror and printed too.
>>
>> That implies 0 is fed to strerror(), which is not the case.
> 
> But, 0 might be fed to strerror.  This is not normally deliberate, but
> it does occor.  It is not unusual for people to write code which can
> feed 0 to strerror.  strerror then conventionally returns "Error 0".

No, POSIX requires:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strerror.html
"if the value of errnum is zero, the message string shall either be an
empty string or indicate that no error occurred;"

and at least in glibc, strerror(0) returns "Success", which looks dumb
in an error message.

> 
> This is why I chose -1 as the sentinel value meaning "there is no
> errno being passed".
> 
>> "If @errnoval is not zero, its absolute value is fed to strerror" to let
>> people pass in both EIO and -EIO for the same output (something that
>> strerror() can't do, but our wrapper can) - but I don't know if that
>> convenience is a good thing.
> 
> I think it is more important not to turn inintended situations where 0
> would previously have been passed to strerror, into situations where
> the errno value string simply vanishes.

Passing 0 as an errno value to indicate an error is almost always wrong.
But if you don't have an errno value to report, having the error string
disappear is preferable to outputting garbage or even assert()ing that
the error value was nonzero.

>>
>> 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 think no good can come of taking the absolute value.  The confusion
> resulting from -errnoval is bad enough already IMO.

You are correct that in the past, we've had code passing or returning
the wrong sign without realizing it.  So the question is whether, at
least for reporting purposes, it looks better to report "Operation not
permitted" instead of "Unknown error -1", or whether munging the error
message as a convenience for better output makes it harder for the
programmer to realize that they are returning the wrong sign up the
stack to the caller.

-- 
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 --]