From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLEU7-0000u0-HO for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:20:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLEU6-00074n-Ey for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:20:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLEU6-00074d-8C for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:20:26 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1AHKOkL007754 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 10 Feb 2015 12:20:25 -0500 Message-ID: <54DA3DD7.4050308@redhat.com> Date: Tue, 10 Feb 2015 10:20:23 -0700 From: Eric Blake MIME-Version: 1.0 References: <1423586055-4932-1-git-send-email-armbru@redhat.com> <1423586055-4932-2-git-send-email-armbru@redhat.com> In-Reply-To: <1423586055-4932-2-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eflw73SJKsoLmxXJpdKouFxvAhJWVIFsH" Subject: Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --eflw73SJKsoLmxXJpdKouFxvAhJWVIFsH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/10/2015 09:34 AM, Markus Armbruster wrote: > I've typed error_report("%s", error_get_pretty(ERR)) too many times > already, and I've fixed too many instances of qerror_report_err(ERR) > to error_report("%s", error_get_pretty(ERR)) as well. Capture the > pattern in a convenience function. >=20 > Since it's almost invariably followed by error_free(), stuff that into > the convenience function as well. >=20 > @@ -2234,8 +2225,7 @@ static int sd_snapshot_create(BlockDriverState *b= s, QEMUSnapshotInfo *sn_info) > =20 > ret =3D do_sd_create(s, &new_vid, 1, &local_err); > if (ret < 0) { > - error_report("%s", error_get_pretty(local_err));; > - error_free(local_err); > + error_report_err(local_err); > error_report("failed to create inode for snapshot. %s", > strerror(errno)); Pre-existing bug, so maybe worth a separate patch. This looks fishy: are we guaranteed that errno is unchanged by error_report_err()? On the surface, error_vreport() and friends do NOT try to preserve errno; maybe your new function should guarantee that errno is not clobbered? (in libvirt, we've explicitly made error-reporting convenience functions document that they do not clobber errno, because it is much easier for callers to report an error then return an errno value without having to save errno locally) > @@ -152,6 +152,12 @@ const char *error_get_pretty(Error *err) > return err->msg; > } > =20 > +void error_report_err(Error *err) > +{ > + error_report("%s", error_get_pretty(err)); > + error_free(err); > +} > + If it were me, I'd split this patch in two, one that introduces the new function (with no clients), and the other which is a strict Coccinelle touchup to use it, so that readers don't have to hunt for the meat of the change. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --eflw73SJKsoLmxXJpdKouFxvAhJWVIFsH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJU2j3XAAoJEKeha0olJ0Nq/0MIAJW3DsmwE6H9gk8ou2raB8MC jW9TLoS4/F4Qclww5IKKhfTbuWq1o4qvremO3sMs5No1nCeYFkPDL8z6/b4lRGkc i1VuvCyWZ6Wh88R9E9+QNy33ttR0svOHxbQLL0f86mOHYw0AiF3u8bOEPEPFgOrh Emw5jmsN+9eePlrEHe6k0eRDHUeUUcPWi5hlEHl8odgDBMygMb8EqG4IEzRPjjQN 7D5vo94NltrFl6wWPjvEhRJR+GtzjLOycwI7m60yrLB7d8XJUx9A8D3+6CdIAWQY SXqvTxM9Hhbezv8V9fus+Cg/bup8W90OGHP+0Ssu58LPBqjTCVQ97cykg0HNACE= =7fCR -----END PGP SIGNATURE----- --eflw73SJKsoLmxXJpdKouFxvAhJWVIFsH--