From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTYZ3-0000qi-Ml for qemu-devel@nongnu.org; Fri, 28 Mar 2014 11:19:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTYYy-00064I-OI for qemu-devel@nongnu.org; Fri, 28 Mar 2014 11:19:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTYYy-000649-Fj for qemu-devel@nongnu.org; Fri, 28 Mar 2014 11:19:20 -0400 Message-ID: <533592F3.3000503@redhat.com> Date: Fri, 28 Mar 2014 09:19:15 -0600 From: Eric Blake MIME-Version: 1.0 References: <1395925471-19841-1-git-send-email-akong@redhat.com> <1395925471-19841-4-git-send-email-akong@redhat.com> <87bnwqxti2.fsf@blackfin.pond.sub.org> In-Reply-To: <87bnwqxti2.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JDhCIAL1JeMnRkwq9STvtTqMkKaGkb0dk" Subject: Re: [Qemu-devel] [PATCH v5 for 2.0 3/3] abort QEMU if group name in option table doesn't match with defined option name List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Amos Kong Cc: pbonzini@redhat.com, libvirt-list@redhat.com, afaerber@suse.de, anthony@codemonkey.ws, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JDhCIAL1JeMnRkwq9STvtTqMkKaGkb0dk Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/28/2014 08:55 AM, Markus Armbruster wrote: > Amos Kong writes: >=20 >> All the options are defined in qemu-options.hx. If we can't find a >> matched option definition by group name of option table, then the >> group name doesn't match with defined option name, it's not allowed >> from 2.0 >> >> @@ -193,6 +215,12 @@ void qemu_add_opts(QemuOptsList *list) >> for (i =3D 0; i < entries; i++) { >> if (vm_config_groups[i] =3D=3D NULL) { >> vm_config_groups[i] =3D list; >> + if (!opt_is_defined(list->name)) { >> + error_report("Didn't find a matched option definition= , " >> + "group name (%s) of option table must ma= tch with " >> + "defined option name (Since 2.0)", list-= >name); >> + abort(); >> + } >> return; >> } >> } >=20 > Simple! Wish it was my idea ;) >=20 > Why not simply assert(opt_is_defined(list->name))? Indeed, using assert() would also solve the problem of the error message being awkward. >> =20 >> -#define HAS_ARG 0x0001 >> - >> - >> -static const QEMUOption qemu_options[] =3D { >> - { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL }, >> -#define QEMU_OPTIONS_GENERATE_OPTIONS >> -#include "qemu-options-wrapper.h" >> - { NULL }, >> -}; >> =20 >> +#undef HAS_ARG HAS_ARG is not very namespace clean. Prior to your patch, it was used only in a single file (where we know it doesn't collide). After your patch, it is now in a header used by multiple files. >> + >> static gpointer malloc_and_trace(gsize n_bytes) >> { >> void *ptr =3D malloc(n_bytes); >=20 > Undefining HAS_ARG here, where it hasn't done any harm, while letting i= t > pollute every other compilation unit including qemu-options.h makes no > sense. Maybe a better approach would be to create an enum in qemu-options.h of actual flag values: typedef enum { QEMU_OPT_HAS_ARG =3D 1, } QEMUOptionFlags; and use QEMU_OPT_HAS_ARG instead of HAS_ARG in vl.c. Additionally, you either have to s/HAS_ARG/QEMU_OPT_HAS_ARG/ throughout the .hx file, or you can take a shortcut in qemu-config.c: #define HAS_ARG QEMU_OPT_HAS_ARG const QEMUOption qemu_options[] =3D { { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL }, #define QEMU_OPTIONS_GENERATE_OPTIONS #include "qemu-options-wrapper.h" { NULL }, }; #undef HAS_ARG since that is the only place that includes the .hx file at a point where HAS_ARG has to be expanded to something useful. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JDhCIAL1JeMnRkwq9STvtTqMkKaGkb0dk 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/ iQEcBAEBCAAGBQJTNZLzAAoJEKeha0olJ0Nqx9oH/2lpYtOE+TL0sLjzRLckYIED ffTiGOa4cDf97IS2tqkq8U+3J2Z3Cno7lKxAze+7kzW6FkDmeUyiUJ4fPOEz+Zzb ZAFdu7YjxQI8smm3RqGI41uvydMG8LYgbHW1/Y/xrKM/414Qsmco1NxCVugNScYF wbr17kwWgGEheAd2G8GZtiphsrWoWm5hLAbt819d84ZwHj+MqQTqyCvfVbckpobb heAivXssqsqiLx5PJ+Bry31UgsgfCZFUuvyNaj6pNTg1tfJM5BfcYDt1AF/kWCHh 9aq7Tii3xabblptvLiHAmyEB52bEFn7cSo6KHuWUJU6YZMJUBpOSkvp+xS32XH0= =8ye+ -----END PGP SIGNATURE----- --JDhCIAL1JeMnRkwq9STvtTqMkKaGkb0dk--