From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGAHl-0006WG-Qe for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:46:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGAHg-0000Oa-PG for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:46:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGAHg-0000OF-Gu for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:46:08 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1JGk67E018065 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 19 Feb 2014 11:46:07 -0500 Message-ID: <5304DFCE.3040003@redhat.com> Date: Wed, 19 Feb 2014 09:46:06 -0700 From: Eric Blake MIME-Version: 1.0 References: <1392822778-4823-1-git-send-email-kwolf@redhat.com> <1392822778-4823-6-git-send-email-kwolf@redhat.com> In-Reply-To: <1392822778-4823-6-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qsnguX4Xxe4OjHqWGedCatbIl1VojIpmx" Subject: Re: [Qemu-devel] [PATCH 5/7] qemu-img create: Support multiple -o options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-devel@nongnu.org Cc: stefanha@redhat.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qsnguX4Xxe4OjHqWGedCatbIl1VojIpmx Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/19/2014 08:12 AM, Kevin Wolf wrote: > Multiple -o options has the same meaning as having a single option with= > all settings in the order of their respective -o options. Ah, this addresses (part of) the problem I raised about patch 1. But it feels a bit awkward to have the intermediate state. > case 'o': > if (is_help_option(optarg)) { > options_help =3D true; > } else if (!options) { > - options =3D optarg; > + options =3D g_strdup(optarg); > } else { > - error_report("-o cannot be used multiple times. Please= use a " > - "single -o option with comma-separated se= ttings " > - "instead."); > - return 1; > + options =3D g_strdup_printf("%s,%s", options, optarg);= In addition to the memleak that Fam pointed out, I still think you have the problem that you aren't detecting: -o backing_file=3D/path/to/foo,help as specifying options_help. I think you instead need to collect ALL -o options without special-casing options_help, then at the end, call contains_help_option which looks for "help" or "?" among all the comma-separated options. > =20 > + g_free(options); > return 0; > + > +fail: > + g_free(options); > + return 1; > } I'd also like if we started using EXIT_FAILURE instead of the magic number 1 (but that's probably a separate cleanup). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --qsnguX4Xxe4OjHqWGedCatbIl1VojIpmx 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/ iQEcBAEBCAAGBQJTBN/OAAoJEKeha0olJ0Nq5i4H/jRSTAzetBoKO8o/C4YzWlCo OsEuisfi15kikoy9Lccmevsgi3u2Jn8fv3gQlHHleD0E7HYkg15qgPKkT4aouFTD 59lCThp/QtJZ+iH6Q3vHQX+8waIoK6JgpNjR6E/+kjyRIFFR7APqSNybykcUZ06w 0XLJZt9HZ+UeDlzeKHdsYA6WMtSBS/oSsH0v0/FlZ8Ai4KOUy25j0zOtjf3zg553 p46JYlh+RrKUnmqi2vZukSMZS/L74mBzaWU7n0hwfQSToTmY2OnmLDDHC3WwWYYr GhVd/iVLBEXl1JzfeUJWPAldBjMaOaEqWhWqgd4U7a681Cw5+hUwbFXsIeSoDhY= =0i1O -----END PGP SIGNATURE----- --qsnguX4Xxe4OjHqWGedCatbIl1VojIpmx--