From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34351) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSWwm-0001IJ-9I for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:23:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WSWwh-0007r2-S4 for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:23:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10674) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSWwh-0007qL-JG for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:23:35 -0400 Message-ID: <5331D7B4.2070101@redhat.com> Date: Tue, 25 Mar 2014 13:23:32 -0600 From: Eric Blake MIME-Version: 1.0 References: <1395396763-26081-1-git-send-email-cyliu@suse.com> <1395396763-26081-5-git-send-email-cyliu@suse.com> In-Reply-To: <1395396763-26081-5-git-send-email-cyliu@suse.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Tmss9O1MLhvcDAiajoQolGAfcNHOBeA8v" Subject: Re: [Qemu-devel] [PATCH v23 04/32] change opt->name and opt->str from (const char *) to (char *) 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) --Tmss9O1MLhvcDAiajoQolGAfcNHOBeA8v Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/21/2014 04:12 AM, Chunyan Liu wrote: Your subject says "what", but your commit message lacks a "why". Without a good reason, it's hard to see what this patch is good for. May I suggest: qemu_opt_del() already assumes that all QemuOpt instances contain malloc'd name and value; but it had to cast away const because opts_start_struct() was doing its own thing and using static storage instead. By using the correct type and malloced strings everywhere, the usage of this struct becomes clearer. > Signed-off-by: Chunyan Liu > --- > include/qemu/option_int.h | 4 ++-- > qapi/opts-visitor.c | 10 +++++++--- > util/qemu-option.c | 4 ++-- > 3 files changed, 11 insertions(+), 7 deletions(-) >=20 > diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h > index 8212fa4..db9ed91 100644 > --- a/include/qemu/option_int.h > +++ b/include/qemu/option_int.h > @@ -30,8 +30,8 @@ > #include "qemu/error-report.h" > =20 > struct QemuOpt { > - const char *name; > - const char *str; > + char *name; > + char *str; While touching this, is it worth trimming the extra spacing before '*'? It's not like you are preserving alignment or anything. But as that's cosmetic, and as the code itself is correct, I'm fine if you add this when you expand your commit message: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Tmss9O1MLhvcDAiajoQolGAfcNHOBeA8v 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/ iQEcBAEBCAAGBQJTMde0AAoJEKeha0olJ0NqwnEIAKV0lHUmVzwUlYTy+pUM99Xr 8/A9eKyCA4LMTbLGLobv77+5yQk++AKC4mwyhl2XJOFZHGxKHGPN3lf3Bib2oHiv Z+MyFIoDQ8i8TKewWp8tsVCsmnc3QwCcnRMs1tJeqZFDapeBbzCVm3uktBgsFVwI JJuMxxKOUsT411m28UYeaS5obDFhoTv2uIl+mUnOqrKaUdXMEBlAUiYfaceFqLSs xpZ3Trf8ySRySBv+qcNwC1UNuz6OAaIwo18tsZxUfzIwkdQpCrPr6SVzLEtEn6GM rIlJhhM27geQYcf8YWTy6NejEwpNTo13PgeGMOcpAXVVfPNUbNxQKPhdBHEo0nk= =8PYO -----END PGP SIGNATURE----- --Tmss9O1MLhvcDAiajoQolGAfcNHOBeA8v--