qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Cc: pbonzini@redhat.com,
	Peter Crosthwaite <peter.crosthwaite@xilinx.com>,
	qemu-devel@nongnu.org, afaerber@suse.de
Subject: Re: [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups
Date: Tue, 03 Dec 2013 13:43:02 -0700	[thread overview]
Message-ID: <529E4256.40807@redhat.com> (raw)
In-Reply-To: <20131203213348.3f4e345e@thinkpad>

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

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).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-12-03 20:43 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-03  5:49 [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups Peter Crosthwaite
2013-12-03  5:49 ` [Qemu-devel] [RFC PATCH v1 1/5] error: Add error_abort Peter Crosthwaite
2013-12-03  5:50 ` [Qemu-devel] [RFC PATCH v1 2/5] hw: Remove assert_no_error usages Peter Crosthwaite
2013-12-03  9:35   ` Markus Armbruster
2013-12-03 10:04     ` Peter Crosthwaite
2013-12-03  5:51 ` [Qemu-devel] [RFC PATCH v1 3/5] target-i386: Remove assert_no_error usage Peter Crosthwaite
2013-12-03  5:51 ` [Qemu-devel] [RFC PATCH v1 4/5] qemu-option: Remove qemu_opts_create_nofail Peter Crosthwaite
2013-12-03  9:42   ` Markus Armbruster
2013-12-03 10:17     ` Peter Crosthwaite
2013-12-03 10:44       ` Markus Armbruster
2013-12-04  6:45     ` Peter Crosthwaite
2013-12-03  5:52 ` [Qemu-devel] [RFC PATCH v1 5/5] qerror: Remove assert_no_error() Peter Crosthwaite
2013-12-03  9:44 ` [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups Markus Armbruster
2013-12-03 11:49   ` Igor Mammedov
2013-12-03 11:57   ` Paolo Bonzini
2013-12-03 12:03     ` Peter Crosthwaite
2013-12-03 12:58   ` Eric Blake
2013-12-03 13:53     ` Markus Armbruster
2013-12-03 20:33       ` Igor Mammedov
2013-12-03 20:43         ` Eric Blake [this message]
2013-12-04  9:11           ` Markus Armbruster
2013-12-04 14:46             ` Eric Blake
2013-12-05 10:37         ` Paolo Bonzini
2013-12-05 15:32           ` Igor Mammedov
2013-12-05 15:59             ` Paolo Bonzini

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=529E4256.40807@redhat.com \
    --to=eblake@redhat.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.crosthwaite@xilinx.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).