qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Eduardo Habkost <ehabkost@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 15:14:21 +0100	[thread overview]
Message-ID: <20170629141421.GF32167@redhat.com> (raw)
In-Reply-To: <87o9t8qy7d.fsf@dusky.pond.sub.org>

On Wed, Jun 28, 2017 at 11:05:26AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Rationale
> > ---------
> >
> > I'm often bothered by the fact that we can't write the following:
> >
> >     foo(arg, errp);
> >     if (*errp) {
> >         handle the error...
> >         error_propagate(errp, err);
> >     }
> >
> > because errp can be NULL.
> 
> 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.

FWIW, I took that approach for the io & crypto layers - except
returning 0 vs -1, rather than true/false, precisely because
it lead to simpler code by not having to use local errors and
propagation.

For added benefit you can potentially then also annotate the
method as  __attribute__(return_check)  to force the caller
to include a check for error conditions, which is often a
useful compile time protection to have.

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

      parent reply	other threads:[~2017-06-29 14:14 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
2017-06-29 17:47           ` Eduardo Habkost
2017-06-29 18:04             ` Daniel P. Berrange
2017-06-29 14:14   ` Daniel P. Berrange [this message]

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=20170629141421.GF32167@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=mdroth@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).