Re: [Qemu-devel] [PULL v2 00/14] QMP and QObject patches

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 10/28/2015 09:07 AM, Peter Maydell wrote:
>
>>>       glib: add compatibility interface for g_hash_table_add()
>>>       monitor: Turn monitor_qapi_event_state[] into a hash table
>
>> 
>>   CC    qga/commands-posix.o
>> In file included from /Users/pm215/src/qemu-for-merges/qga/main.c:25:
>> In file included from
>> /Users/pm215/src/qemu-for-merges/include/qapi/qmp/json-parser.h:17:
>> In file included from 
>> /Users/pm215/src/qemu-for-merges/include/qemu-common.h:25:
>> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:171:12: error:
>> returning 'void' from a function with incompatible result type
>> 'gboolean' (aka 'int')
>>     return g_hash_table_replace(hash_table, key, key)
>>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Looks like g_hash_table_replace was originally a 'void' return
>> and switched to 'gboolean' return at some later date:
>> 
>> https://github.com/GNOME/glib/commit/910191597a6c2e5d5d460e9ce9efb4f47d9cc63c

The 80s are calling: cavalier API break.  The GLib maintainers should
know better.

Anyway, this change is in 2.39.2.  The replacement function is for
<2.32.  Therefore, g_hash_table_replace() will always return void then.

> This patch series isn't using the return value of g_hash_table_add, so
> our glib replacement could be changed to return void.

Yes.  Matches GLib's function before 2.39.2.

> On the other hand, would it be better to proactively retrofit the return
> type into ALL of the g_hash_table_* functions that were swapped to
> return a value, so that future uses of the functions with qemu can make
> use of the modern contract, even though this series doesn't use it?

I think we should add glib-compat cruft only when we actually need it.