[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 26/37] qapi: Simplify excess input reporting
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v9 26/37] qapi: Simplify excess input reporting in input visitors |
Date: |
Fri, 22 Jan 2016 12:37:31 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 01/22/2016 12:24 PM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> 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.
>
> Good idea.
>
>> Suggested-by: Markus Armbruster <address@hidden>
>
> Whoops, it's even mine! I forgot... %-)
>
>> Signed-off-by: Eric Blake <address@hidden>
>> Reviewed-by: Marc-André Lureau <address@hidden>
>>
>> ---
>> 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 = 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)) {
>
> 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.
I suppose a stricter version would be:
void *wrap;
GQueue *any;
if (g_hash_table_iter_next(&iter, NULL, &wrap)) {
any = 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?
>> 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)) {
>
> Is this cast kosher?
Here, in addition to the above argument, we also needed to cast away const.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 0/3] qapi-visit: Unify struct and union visit, Markus Armbruster, 2016/01/27
[Qemu-devel] [PATCH 3/3] qapi-visit: Unify struct and union visit, Markus Armbruster, 2016/01/27
[Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around visit_end_union(), Markus Armbruster, 2016/01/27
Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around visit_end_union(), Eric Blake, 2016/01/27
Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around visit_end_union(), Markus Armbruster, 2016/01/27
[Qemu-devel] [PATCH v9 18/37] qapi: Drop unused error argument for list and implicit struct, Eric Blake, 2016/01/19