From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46751) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNdRB-0006Yu-3G for qemu-devel@nongnu.org; Mon, 25 Jan 2016 04:27:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aNdR7-0005vf-SJ for qemu-devel@nongnu.org; Mon, 25 Jan 2016 04:27:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33017) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNdR7-0005vY-KM for qemu-devel@nongnu.org; Mon, 25 Jan 2016 04:27:49 -0500 From: Markus Armbruster References: <1453219845-30939-1-git-send-email-eblake@redhat.com> <1453219845-30939-27-git-send-email-eblake@redhat.com> <8737tpfmec.fsf@blackfin.pond.sub.org> <56A284FB.50502@redhat.com> Date: Mon, 25 Jan 2016 10:27:45 +0100 In-Reply-To: <56A284FB.50502@redhat.com> (Eric Blake's message of "Fri, 22 Jan 2016 12:37:31 -0700") Message-ID: <874me2c8ku.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v9 26/37] qapi: Simplify excess input reporting in input visitors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: marcandre.lureau@redhat.com, Laszlo Ersek , qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > On 01/22/2016 12:24 PM, Markus Armbruster wrote: >> Eric Blake writes: >>=20 >>> When reporting that an unvisited member remains at the end of an >>> input visit for a struct, we were using g_hash_table_find() >>> coupled with a callback function that always returns true, to >>> locate an arbitrary member of the hash table. But if all we >>> need is an arbitrary entry, we can get that from a single-use >>> iterator, without needing a tautological callback function. >>=20 >> Good idea. >>=20 >>> Suggested-by: Markus Armbruster >>=20 >> Whoops, it's even mine! I forgot... %-) >>=20 >>> Signed-off-by: Eric Blake >>> Reviewed-by: Marc-Andr=C3=A9 Lureau >>> >>> --- > >>> GQueue *any; >>> >>> if (--ov->depth > 0) { >>> @@ -174,8 +168,8 @@ opts_end_struct(Visitor *v, Error **errp) >>> } >>> >>> /* we should have processed all (distinct) QemuOpt instances */ >>> - any =3D g_hash_table_find(ov->unprocessed_opts, &ghr_true, NULL); >>> - if (any) { >>> + g_hash_table_iter_init(&iter, ov->unprocessed_opts); >>> + if (g_hash_table_iter_next(&iter, NULL, (void **)&any)) { >>=20 >> Is this cast kosher? > > You may have a point that it violates some corner of C99, but I'm not > the first such user: > > hw/i386/intel_iommu.c: while (g_hash_table_iter_next (&bus_it, NULL, > (void**)&vtd_bus)) { > qom/object.c: while (g_hash_table_iter_next(&iter, NULL, (gpointer > *)&prop)) { > > Conceptually, it seems fine - void* can be assigned to any pointer, and > 'any' qualifies as such a pointer. We are then taking the address of > that (or void**) to pass by reference. Yes, any pointer can be converted to and from void *. Doesn't mean the conversion results in the same bits. It does on machines with a single, uniform pointer format, i.e. any sane machine. (void **)iter converts iter, not *iter. *iter gets reinterpreted on dereference. You're right in that this kind of type punning already occurs in QEMU. Also in other programs. We can hope it's common enough to deter optimizers from screwing it up even on sane machines. > I suppose a stricter version would be: > > void *wrap; > GQueue *any; > if (g_hash_table_iter_next(&iter, NULL, &wrap)) { > any =3D wrap; > ... > > but is it worth the bother? Put another way, will a compiler ever do > the wrong thing to us because C99 might have some corner case? Laszlo, > what's your take? Perhaps Laszlo can confirm or refute my reading of the standard. >>> if (top_ht) { >>> - if (g_hash_table_size(top_ht)) { >>> - const char *key; >>> - g_hash_table_find(top_ht, always_true, &key); >>> + GHashTableIter iter; >>> + const char *key; >>> + >>> + g_hash_table_iter_init(&iter, top_ht); >>> + if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { >>=20 >> Is this cast kosher? > > Here, in addition to the above argument, we also needed to cast away cons= t.