From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49634) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4b4h-0002b2-Hh for qemu-devel@nongnu.org; Mon, 15 Jun 2015 16:33:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4b4Z-0006o2-QH for qemu-devel@nongnu.org; Mon, 15 Jun 2015 16:33:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45748) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4b4Z-0006ns-IH for qemu-devel@nongnu.org; Mon, 15 Jun 2015 16:33:35 -0400 Message-ID: <557F3697.30005@redhat.com> Date: Mon, 15 Jun 2015 14:33:27 -0600 From: Eric Blake MIME-Version: 1.0 References: <1434205258-1932-1-git-send-email-armbru@redhat.com> <1434205258-1932-5-git-send-email-armbru@redhat.com> <20150615111833.0d5398fe@redhat.com> In-Reply-To: <20150615111833.0d5398fe@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QK6gsNTHvO3W8mhSvQq4ELkMXpFIQ48EI" Subject: Re: [Qemu-devel] [PATCH 04/11] qerror: Eliminate QERR_DEVICE_NOT_FOUND List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino , Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --QK6gsNTHvO3W8mhSvQq4ELkMXpFIQ48EI Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/15/2015 09:18 AM, Luiz Capitulino wrote: > On Sat, 13 Jun 2015 16:20:51 +0200 > Markus Armbruster wrote: >=20 >> Error classes other than ERROR_CLASS_GENERIC_ERROR should not be used >> in new code. Hiding them in QERR_ macros makes new uses hard to spot.= >> Fortunately, there's just one such macro left. Eliminate it with this= >> coccinelle semantic patch: >> >> @@ >> expression EP, E; >> @@ >> -error_set(EP, QERR_DEVICE_NOT_FOUND, E) >> +error_set(EP, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not foun= d", E) >=20 > This is a bit minor, but I think I'd have created a new function instea= d, > say error_set_enodev(). This avoids all the duplication. But I'm not as= king > you to change, as the patch is good and this can be done in the future = if > we so want. In fact, in both patch 4 and 5, I thought a similar thing - the remaining QERR_ macros that contain a % embedded in them are a bit unusual (when reading error_setg(), you have to go look up the QERR_ macro to see how many arguments you should really be passing). If I understand correctly, one of the reasons we went with QERR_ macros embedding % was to ensure consistent error messages among multiple call sites, in part if we want to enable error message translations (but that's a bigger project anyways, which you have argued that we may not even want). Having helper functions for the more common error messages can both reduce confusion (no hidden %) and duplication (callers don't need to repeat the same string). But I likewise agree with Luiz that it can be deferred to the future if desired. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --QK6gsNTHvO3W8mhSvQq4ELkMXpFIQ48EI 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/ iQEcBAEBCAAGBQJVfzaXAAoJEKeha0olJ0NqK6UIAIkywi2gw5U1RtIgmJTjym0I TJZsYdBMYCLbAsaHpXbQbEN3kbX5Kz65TkIjZP1eZhDKkb5DTDBnZlkpKkg21qqC k4SEpUDRki7OW0hhZDvq/BxPUPxVpNUDY3ToSVJX8+YHOyMNoF4Cf3D3YpMcQ4lk UNM7qcbYe3jTYXsh4ITaHm5XVwbB7voYuJpcOS+eC3eWanqntFDv0PLijZWG/Qya N00j+JsnL28xaigzsOHQkNPWmeQ0RecEQf2Yry4cdWq0k5XgHZPTia7vH5atntR9 COZAvHtadKhEN2kepU/5sTeGmlOEQ8AUF0TrmWajUOsMmGXv+nm7VHE8PDUYTUo= =G22G -----END PGP SIGNATURE----- --QK6gsNTHvO3W8mhSvQq4ELkMXpFIQ48EI--