From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51573) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wjt4F-0008LB-Ge for qemu-devel@nongnu.org; Mon, 12 May 2014 12:27:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wjt4B-000202-3w for qemu-devel@nongnu.org; Mon, 12 May 2014 12:27:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44771) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wjt4A-0001zx-SU for qemu-devel@nongnu.org; Mon, 12 May 2014 12:27:03 -0400 Message-ID: <5370F653.6060305@redhat.com> Date: Mon, 12 May 2014 10:26:59 -0600 From: Eric Blake MIME-Version: 1.0 References: <1399881324-14859-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1399881324-14859-1-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9mgKQIAv91Nkp2kxolKAjuJcTDMDam6t7" Subject: Re: [Qemu-devel] [PATCH] rewamp/simplify option parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev , qemu-devel@nongnu.org Cc: Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9mgKQIAv91Nkp2kxolKAjuJcTDMDam6t7 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/12/2014 01:55 AM, Michael Tokarev wrote: s/rewamp/revamp/ in the subject > Main change is to allow get_opt_name() to accept > a set of delimiters (string) instead of a single > delimiter (char). This way it is easier to search > for the next (sub)option in an option string, so > other code using get_opt_name() can be simplified. >=20 > Signed-off-by: Michael Tokarev > --- > This is an old patch (from Jun 2013) which was > sitting in my local git repository since that. > I rebased it to current master (with 2 trivial > conflicts resolved). >=20 > FWIW. >=20 > include/qemu/option.h | 3 +- > util/qemu-option.c | 136 +++++++++++++++++++++++------------------= -------- > vl.c | 4 +- > 3 files changed, 67 insertions(+), 76 deletions(-) >=20 > @@ -97,24 +95,27 @@ int get_next_param_value(char *buf, int buf_size, > =20 > p =3D *pstr; > for(;;) { > - p =3D get_opt_name(option, sizeof(option), p, '=3D'); > - if (*p !=3D '=3D') > - break; > - p++; > - if (!strcmp(tag, option)) { > - *pstr =3D get_opt_value(buf, buf_size, p); > - if (**pstr =3D=3D ',') { > - (*pstr)++; > + if (*p =3D=3D '\0') { > + return 0; > + } > + p =3D get_opt_name(option, sizeof(option), p, "=3D,"); > + if (strcmp(tag, option) =3D=3D 0) { > + if (*p =3D=3D '=3D') { > + p =3D get_opt_value(buf, buf_size, p + 1); > + } else { > + buf[0] =3D '\0'; > } > - return strlen(buf); > + *pstr =3D *p =3D=3D ',' ? p + 1 : p; > + return 1; Does this botch handling of ',,' escaping that allows literal commas as part of an option value? I suspect there may be some corner cases you are altering. Based just on a quick read of 'qemu-kvm -help', I see: -name string1[,process=3Dstring2][,debug-threads=3Don|off] Which, if I'm interpreting correctly, means that pre-patch I can do: qemu-kvm -name foo,,bar /dev/null as a way to start a (pretty pointless) machine with the window title of "QEMU (foo,bar)". Reading the source code, I also found the implicit name of string1: qemu-kvm -name guest=3Ddebug-threads=3D /dev/null creates the window title "QEMU (debug-threads=3D)", and: qemu-kvm -name debug-threads,, /dev/null creates the window title "QEMU (debug-threads,)". That is, I have a way of specifying ANY name, including trailing '=3D' or ',', for my guest. I did not apply your patch, but off-hand, I'm guessing that your patch breaks at least one, if not multiple, of these cases. We need unit tests in place first. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --9mgKQIAv91Nkp2kxolKAjuJcTDMDam6t7 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/ iQEcBAEBCAAGBQJTcPZTAAoJEKeha0olJ0NqFeEIAIf1BzMoZm6Kq7WZRjBOHO5G feQZ/mPH+EV88RwcWJgFIGG05hcxvdZfdMMBcynS2Ma1FvGSgTRAZSrljTUr/68y 6iS37D5g+jCz/u2QaPyWNd+1jMMmrhsN4trkjc0SdRdlzmhg0Iw9QxljlOLoVP17 dRZWWmzE9v4VwEfss6y9B4Qr+2yOoMrmWwL0aHiCFz5/mnUc3qOJJNk1O6ZQPc0f T3KF445DyrO27+QW0U8fB787IrXlAHae9WdEzm5oVVpZJqxfW/9nph6G8P0jMJs0 Rd4/35UCLZvGt6bZ6jHgMohdXBBpQgerfr2u7ZfCar8H/7n53+6H1c5ybwyYsnc= =7/i5 -----END PGP SIGNATURE----- --9mgKQIAv91Nkp2kxolKAjuJcTDMDam6t7--