From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46881) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCUIo-00054r-Pm for qemu-devel@nongnu.org; Mon, 13 Jun 2016 12:01:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCUIn-0002Qc-L6 for qemu-devel@nongnu.org; Mon, 13 Jun 2016 12:01:26 -0400 References: <1465589538-24998-1-git-send-email-ehabkost@redhat.com> <1465589538-24998-3-git-send-email-ehabkost@redhat.com> <87k2htwc14.fsf@dusky.pond.sub.org> <20160613155226.GH18662@thinpad.lan.raisama.net> From: Eric Blake Message-ID: <575ED8CC.6010601@redhat.com> Date: Mon, 13 Jun 2016 10:01:16 -0600 MIME-Version: 1.0 In-Reply-To: <20160613155226.GH18662@thinpad.lan.raisama.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SU0kTIM6OJeeA4t1i3vh3wQxtXEFgK9DB" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/3] error: Remove unnecessary local_err variables List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , Markus Armbruster Cc: kwolf@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com, borntraeger@de.ibm.com, cornelia.huck@de.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --SU0kTIM6OJeeA4t1i3vh3wQxtXEFgK9DB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/13/2016 09:52 AM, Eduardo Habkost wrote: >> >> There is an (ugly) difference between >> >> error_setg(&local_err, ...); >> error_propagate(errp, &local_err); >> >> and >> >> error_setg(errp, ...); >> >> The latter aborts when @errp already contains an error, the former doe= s >> nothing. >=20 > Why the difference? Couldn't we change that so that both are equivalent= ? Maybe, but I think it weakens our position. An abort() on an attempt to incorrectly set an error twice helps catch errors where we are throwing away a more useful first error message. The documentation for error_propagate() already mentioned that it was an exception to the rule.= >=20 >> >> Your transformation has the error_setg() or similar hidden in F2(). I= t >> can add aborts. >> >> I think it can be salvaged: we know that @errp must not contain an err= or >> on function entry. If @errp doesn't occur elsewhere in this function,= >> it cannot pick up an error on the way to the transformed spot. Can yo= u >> add that to your when constraints? >=20 > Do we really know that *errp is NULL on entry? Aren't we allowed to cal= l > functions with a non-NULL *errp? Except for error_propagate(), no. >=20 > See, e.g.: >=20 > void qmp_guest_suspend_disk(Error **errp) > { > Error *local_err =3D NULL; > GuestSuspendMode *mode =3D g_new(GuestSuspendMode, 1); >=20 > *mode =3D GUEST_SUSPEND_MODE_DISK; > check_suspend_mode(*mode, &local_err); > acquire_privilege(SE_SHUTDOWN_NAME, &local_err); > execute_async(do_suspend, mode, &local_err); That usage is a bug. A Coccinelle script to root out such buggy instances would be nice. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --SU0kTIM6OJeeA4t1i3vh3wQxtXEFgK9DB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXXtjMAAoJEKeha0olJ0NqdpcIAJrmQGBH96xAOrCsiGzPhYrD Mdbo8WCT6xu/yxguj//fiyr5JKDvU830nMSql8fn/OkkrTGyXcd7QWh22qWMe+Uy bCSd3hMuf6S2HC94NCi4EkpXDiP3IJSjjNVd528gSaA8fzaQzVmkeAAAymwsxTIQ q+4asS0Git7W+pdGH+taYMKMN7kZNQYqVGeIjL+5V6a0lcirO2q+plKJItO5eQiR +2jpF9PMRgHJbqX+SvkNsLh0FUPt27VUapIlKimggotPFN7cOIYdjmEy4DKkQC10 7Udc9cWjHKYmatQ064hdOk9p9LxbHzxl7DQIDu2a+w+kiK8RRkYTK/m2l7RIEAA= =1bX+ -----END PGP SIGNATURE----- --SU0kTIM6OJeeA4t1i3vh3wQxtXEFgK9DB--