Re: [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 12/03/2013 01:33 PM, Igor Mammedov wrote:
>
>>>> Also, is it worth adding asserts and/or compiler annotations to require
>>>> that the Error **err argument of functions be non-NULL, to ensure that
>>>> callers are always passing either a valid destination or one of the
>>>> special addresses?  But doing so would probably require adding a special
>>>> address for error_ignore for callers that intend to discard an error in
>>>> cases where the return type of the function lets them know to proceed
>>>> with a fallback implementation (that is, cases where ignoring an error
>>>> makes sense).
>>>
>>> Right now, we use NULL as "ignore errors" argument.
>>>
>>> NULL gives us a chance to express "caller must not ignore errors" via
>>> some non-null annotation that gets fed to a static analyzer.
>>>
>>> I doubt that would be possible with a special error_ignore object.
>>>
>>> Anyway, this series is about "abort on error".  Let's keep "ignore
>>> errors" issues separate.
>> I'm sorry for hijacking thread, but that actually an issue that started an
>> original discussion.
>> Where void returning QOM API functions are used with NULL, without any chance
>> to detect that error happened. So abusing NULL errp in this functions
>> might lead to hard to find runtime errors.
>> I think Eric's suggestion was to enforce passing non NULL errp and let caller
>> to deal with error gracefully so that above mentioned misuse was impossible.
>> Why is ignoring errors from "void foo(...)" like API considered acceptable?
>
> Okay, so it sounds like consensus is that using NULL as the means to
> ignore errors is okay when there is an alternative way to detect error,
> but that for any function that returns void, adding an assert(errp)
> would be appropriate because the caller cannot safely ignore the
> failure.  It's not worth inventing an error_ignore special address, but
> for functions that have no way to report errors except via errp, then it
> IS worth enforcing that the caller is either prepared to handle the
> error or has passed &error_abort (or any other special addresses we add
> later).

No objection to asserting that the caller passed an error object when
the error object is the only way to signal failure.  You can't force
your callers to check for failure, but the assertion could help prevent
accidental misuse.

Assertions fire at run-time, though.

Asserting "argument not null" first thing in the function should enable
a sufficiently smart whole-program static checker to flag null
arguments.

But having such a static check right at compile-time would be much
better.  Could attribute nonnull do it?  If yes, do we still need the
assertion?