From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSY2r-000677-47 for qemu-devel@nongnu.org; Tue, 25 Mar 2014 16:34:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WSY2m-0004cs-7i for qemu-devel@nongnu.org; Tue, 25 Mar 2014 16:34:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26588) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSY2l-0004ce-Vy for qemu-devel@nongnu.org; Tue, 25 Mar 2014 16:33:56 -0400 Message-ID: <5331E832.8070408@redhat.com> Date: Tue, 25 Mar 2014 14:33:54 -0600 From: Eric Blake MIME-Version: 1.0 References: <1395396763-26081-1-git-send-email-cyliu@suse.com> <1395396763-26081-7-git-send-email-cyliu@suse.com> In-Reply-To: <1395396763-26081-7-git-send-email-cyliu@suse.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ENlC4h3rEefovatVuoTPgD2mXk0SB5X2c" Subject: Re: [Qemu-devel] [PATCH v23 06/32] add qemu_opt_get_*_del functions for replace work List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu , qemu-devel@nongnu.org Cc: stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ENlC4h3rEefovatVuoTPgD2mXk0SB5X2c Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/21/2014 04:12 AM, Chunyan Liu wrote: > Add qemu_opt_get_del, qemu_opt_get_bool_del and qemu_opt_get_number_gel= s/gel/del/ Any reason you don't mention qemu_opt_get_size_del here? > to replace the same handling of QEMUOptionParamter (get and delete). s/Paramter/Parameter/ >=20 > get_*_del purpose: >=20 > In specific driver (like "qcow2"), it only handles expected options, th= en > delete them from option list. Leave the left options to be passed down > to 2nd driver (like "raw") to do 2nd handling. Awkward wording. I suggest replacing these two paragraphs with: Several drivers are coded to parse a known subset of options, then remove them for the list before handing all remaining options to a second driver for further option processing. get_*_del makes it easier to retrieve a known option (or its default) and remove it from the list all in one action. > +++ b/util/qemu-option.c > @@ -588,6 +588,29 @@ const char *qemu_opt_get(QemuOpts *opts, const cha= r *name) > return opt ? opt->str : NULL; > } > =20 > +char *qemu_opt_get_del(QemuOpts *opts, const char *name) It would help to add documentation to this function; particularly that the caller must use g_free() on the result (in contrast to qemu_opt_get where they must NOT use g_free). > + str =3D g_strdup(opt->str); We could play a game with transfer semantics for fewer mallocs, by writing this as: str =3D opt->str; opt->str =3D NULL; but not the end of the world if you keep it as-is. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --ENlC4h3rEefovatVuoTPgD2mXk0SB5X2c 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/ iQEcBAEBCAAGBQJTMegyAAoJEKeha0olJ0Nql+kH+wYmgi36BTJD16jL5MCLIoZK yhAH1XfhJmQNihlR5/0oDg/eTOqj7IOF91l+A4fNufLDJPa4j2yfaLX60a+CEwz9 TWyX5ChLEFO/3aLGh+FyFykrctVOhXGVFrYmU3+90rfsv6by5nz3GnpOKy59XopR 0aXNDzuRaxa89h8OtMmGyWl8XUVgizGUH1vA4S0cTEYjv+HQgySbqYHvlNuzDahj Au/PQVmMaiEc4VCLS+unutRS8rly3Gj/cRRl7Rv0BgEvp/eetq34lNA/B0PLUl0Q YIkEt0WmR8BWcI3NwRfla5npsMKWnqyf0RnMjvO1ZiHge1s53gbBhi0/kQkRNKs= =4I7e -----END PGP SIGNATURE----- --ENlC4h3rEefovatVuoTPgD2mXk0SB5X2c--