All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored
Date: Thu, 29 Jun 2017 18:38:50 +0100	[thread overview]
Message-ID: <20170629173850.GI32167@redhat.com> (raw)
In-Reply-To: <20170629170939.GU12152@localhost.localdomain>

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.

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

  reply	other threads:[~2017-06-29 17:39 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-13 16:52 [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored Eduardo Habkost
2017-06-13 16:52 ` [Qemu-devel] [RFC 01/15] tests: Test cases for error API Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 02/15] error: New IGNORE_ERRORS macro Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 03/15] Add qapi/error.h includes on files that will need it Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 04/15] [coccinelle] Use IGNORE_ERRORS instead of NULL as errp argument Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 05/15] qapi: Use IGNORE_ERRORS instead of NULL on generated code Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 06/15] test-qapi-util: Use IGNORE_ERRORS instead of NULL Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 07/15] Manual changes to use " Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 08/15] error: New ERR_IS_* macros for checking Error** values Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 09/15] [coccinelle] Use ERR_IS_* macros Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 10/15] test-qapi-util: " Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 11/15] Manual changes to use " Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 12/15] error: Make IGNORED_ERRORS not a NULL pointer Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 13/15] rdma: Simplify var declaration to avoid confusing Coccinelle Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 14/15] [coccinelle] Eliminate unnecessary local_err/error_propagate() usage Eduardo Habkost
2017-06-13 16:53 ` [Qemu-devel] [RFC 15/15] [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to catch NULL errp arguments Eduardo Habkost
     [not found]   ` <20170615121407.GA2399@work-vm>
2017-06-17 19:33     ` Eduardo Habkost
2017-06-19  8:48       ` Dr. David Alan Gilbert
2017-06-19  9:43         ` Peter Maydell
2017-06-19 13:26           ` Eduardo Habkost
2017-06-27 20:12             ` Eric Blake
2017-06-27 21:31               ` Eduardo Habkost
2017-06-27 20:22 ` [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored Eric Blake
2017-06-28  9:05 ` Markus Armbruster
2017-06-28 17:41   ` Eduardo Habkost
2017-06-29  6:54     ` Markus Armbruster
2017-06-29 12:57       ` Eduardo Habkost
2017-06-30 11:40         ` Markus Armbruster
2017-07-01 14:20           ` Eduardo Habkost
2017-07-03 12:51             ` Markus Armbruster
2017-07-01 14:29           ` Eduardo Habkost
2017-07-03 13:21             ` Markus Armbruster
2017-07-03 13:47               ` Eduardo Habkost
2017-06-29 14:01     ` Daniel P. Berrange
2017-06-29 13:39   ` Paolo Bonzini
2017-06-29 14:18     ` Daniel P. Berrange
2017-06-29 17:09       ` Eduardo Habkost
2017-06-29 17:38         ` Daniel P. Berrange [this message]
2017-06-29 17:47           ` Eduardo Habkost
2017-06-29 18:04             ` Daniel P. Berrange
2017-06-29 14:14   ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170629173850.GI32167@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.