Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored

[Daniel P. Berrange]

On Thu, Jun 29, 2017 at 02:47:34PM -0300, Eduardo Habkost wrote:
> On Thu, Jun 29, 2017 at 06:38:50PM +0100, Daniel P. Berrange wrote:
> > On Thu, Jun 29, 2017 at 02:09:39PM -0300, Eduardo Habkost wrote:
> > > On Thu, Jun 29, 2017 at 03:18:05PM +0100, Daniel P. Berrange wrote:
> > > > On Thu, Jun 29, 2017 at 03:39:58PM +0200, Paolo Bonzini wrote:
> > > > > On 28/06/2017 11:05, Markus Armbruster wrote:
> > > > > > If foo() additionally returned an indication of success, you could 
> > > > > > write
> > > > > > 
> > > > > >       if (!foo(arg, errp)) {    // assuming foo() returns a bool
> > > > > >           handle the error...
> > > > > >       }
> > > > > > 
> > > > > > Nicely concise.
> > > > > > 
> > > > > > For what it's worth, this is how GLib wants GError to be used.  We
> > > > > > deviated from it, and it has turned out to be a self-inflicted 
> > > > > > wound.
> > > > > > 
> > > > > 
> > > > > I find Eduardo's proposal better.  With GLib's way it's easy to 
> > > > > confuse
> > > > > functions that return 0/-1, 0/-errno, TRUE/FALSE, FALSE/TRUE or
> > > > > NULL/non-NULL.
> > > > 
> > > > NB, glib basically standardizes on just FALSE/TRUE and NULL/non-NULL,
> > > > avoiding anything returning -1, or errno values, so in their usage
> > > > there isn't really any confusion.
> > > > 
> > > > QEMU of course has lots of pre-existing code, but we could at least
> > > > declare a preferred approach, and work towards it.
> > > > 
> > > > Having the return value indicate error is slightly shorter, and it
> > > > avoids all the blackmagic with special Error values in Eduardo's
> > > > series. Most usefully, it lets you use __attribute__(return_check)
> > > > to get compile time checking of callers who forget to check for
> > > > failure.
> > > 
> > > I agree it's better when the return value is obvious, but I still think
> > > the Error value magic in IGNORE_ERRORS/ERR_IS_SET is preferable to the
> > > existing 700+ error_propagate() calls in the tree.
> > 
> > Both approaches would let us kill the error_propagate() calls. I think
> > we're all agreed those would be better off gone, no matter which
> > approach is used.
> 
> Changing the return value of the 1500+ void functions that return errors
> will take a very long while.  I would like to get rid of the
> error_propagate() calls before that, even if the long term plan is to
> eventually get rid of ERR_IS_SET.

Ah, I see what you mean, that's fine.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|