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

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 03 Dec 2013 14:53:06 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Eric Blake <eblake@redhat.com> writes:
> 
> > On 12/03/2013 02:44 AM, Markus Armbruster wrote:
> >> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
> >> 
> >>> Following our discussion RE self asserting API calls, here is a spin of
> >>> my proposal. This series obsoletes the need for _nofail variants for
> >>> Error ** accepting APIs. Is also greately reduces the verbosity of calls
> >>> sites that are currently asserting against errors.
> >>>
> >>> Patch 1 is the main event - addition of error_abort. The following
> >>> patches then cleanup uses of _nofail and assert_no_error().
> >>>
> >>> To give it a smoke test, I introduce a (critical) bug into QOM:
> >> [...]
> >>>  32 files changed, 100 insertions(+), 143 deletions(-)
> >> 
> >> I like it.  Nice diffstat, too.
> >> 
> >> There are some _nofail functions left, but none of them can use
> >> error_abort.
> >> 
> >
> > 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?


-- 
Regards,
  Igor