From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38703) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGZSG-0000lr-Db for qemu-devel@nongnu.org; Thu, 20 Feb 2014 14:38:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGZSA-0001fT-IA for qemu-devel@nongnu.org; Thu, 20 Feb 2014 14:38:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58724) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGZSA-0001fH-A2 for qemu-devel@nongnu.org; Thu, 20 Feb 2014 14:38:38 -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 s1KJcbWA005173 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 20 Feb 2014 14:38:37 -0500 Message-ID: <530659BB.7010004@redhat.com> Date: Thu, 20 Feb 2014 12:38:35 -0700 From: Eric Blake MIME-Version: 1.0 References: <1392908243-8835-1-git-send-email-kwolf@redhat.com> <1392908243-8835-4-git-send-email-kwolf@redhat.com> <20140220193330.GE30664@localhost.localdomain> In-Reply-To: <20140220193330.GE30664@localhost.localdomain> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RsSV8smmfnEhuS1mkiK4XtA5BRNGVwFnM" Subject: Re: [Qemu-devel] [PATCH v2 3/6] qemu-img convert: Support multiple -o options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody , Kevin Wolf Cc: armbru@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --RsSV8smmfnEhuS1mkiK4XtA5BRNGVwFnM Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/20/2014 12:33 PM, Jeff Cody wrote: > On Thu, Feb 20, 2014 at 03:57:20PM +0100, Kevin Wolf wrote: >> Instead of ignoring all option values but the last one, multiple -o >> options now have the same meaning as having a single option with all >> settings in the order of their respective -o options. >> > There is a 'return -1' that is missed, that I think needs to be 'goto > out;' for cleanup. It is right after the call to > bdrv_parse_cache_flags() in the img_convert() function: >=20 > ret =3D bdrv_parse_cache_flags(cache, &flags); > if (ret < 0) { > error_report("Invalid cache option: %s", cache); > return -1; > } >=20 > Looks like this has been this way for a while, though. And this is an > instance (the only instance) where img_convert returns -1 instead of 0 > or 1. I'm not sure the implications if that changed, and became '1', > so we'd probably want to preserve the return value. No, exit status of 255 on an invalid cache option is a bug; fixing it to return EXIT_FAILURE (1) is indeed correct. [Hmm, this shows that I only reviewed the changed sections, rather than also looking for all other 'return' statements - thanks Jeff for the more thorough review] >=20 > Since this doesn't have anything to do with this patch (although > leaking 'options' now, in addition to the other stuff leaked), I'll go > ahead and give my reviewed-by, and we can fix this in another > follow-up patch: Agreed to that plan of attack (we've now racked up several good followups= ). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --RsSV8smmfnEhuS1mkiK4XtA5BRNGVwFnM 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/ iQEcBAEBCAAGBQJTBlm7AAoJEKeha0olJ0NqvxUIAIwjRfkDEi/1Sni4XpULhL7o B0Tl4J/yyF2vL2d88enK674SbsqtEulK6qVvRyLsAt3VvWrcWEg3KjC4iiYUHWWe 4CIutYa2uFH7SuY7nOEv3ixedcnNgIUfOHMv7qH1pbJbUYNR+me9Zgx6wnSrBvqc hJKuyLcghi26tUNGlWbiumIXcTq89gjS3AUUAFM82EFtDmH6CPUOrFVq7pv7X0Yz 6cyrQSsXC/zG9NpAVcQA7cIigupFheo8ekKJhhlyuFWneCRqjzcfhy+OGQOoUSng PEMuxr3nZmP9L8vUHfjsfts14gkJSBxV1TVBiPR/g0O9TVo9kPeWpMcX30fNgHY= =Kn90 -----END PGP SIGNATURE----- --RsSV8smmfnEhuS1mkiK4XtA5BRNGVwFnM--