Re: [Qemu-devel] [PATCH 1/2] qobject: Use 'bool' for qbool

[Eric Blake]

On 06/11/2015 11:35 PM, Markus Armbruster wrote:
> Patch looks good to me, but it made me wonder about something.  Please
> find the question inline.
> 
> Eric Blake <address@hidden> writes:
> 
>> We require a C99 compiler, so let's use 'bool' instead of 'int'
>> when dealing with boolean values.  There are few enough clients
>> to fix them all in one pass.
>>

>> @@ -593,7 +593,7 @@ static QObject *parse_escape(JSONParserContext *ctxt, 
>> va_list *ap)
>>      if (token_is_escape(token, "%p")) {
>>          obj = va_arg(*ap, QObject *);
>>      } else if (token_is_escape(token, "%i")) {
>> -        obj = QOBJECT(qbool_from_int(va_arg(*ap, int)));
>> +        obj = QOBJECT(qbool_from_bool(va_arg(*ap, int)));
> 
> Funny: JSON_ESCAPE "%i" gets an int, but maps it to bool.  See also
> patch to check-qjson.c below.
> 
> Is this feature actually used anywhere other than the tests?
> 

When using va_arg(), you have to use the type argument that things
promote to when called through var-args (that is, va_arg(*ap, bool)
would be a compiler warning).

I don't know if anyone besides the testsuite is using %i; but I've
already mentioned in another thread [1] that the correlation between...

>>
>> -    obj = qobject_from_jsonf("%i", true);
>> +    /* Test that non-zero values other than 1 get collapsed to true */
>> +    obj = qobject_from_jsonf("%i", 2);
>>      g_assert(obj != NULL);
>>      g_assert(qobject_type(obj) == QTYPE_QBOOL);
> 
> These are test test cases for JSON_ESCAPE "%i".

...qobject_from_json in one file to the implementation of %i in another
was very hard to trace when writing this patch, as well as the idea of
getting rid of the remaining 2 out of only 3 clients of %p [1].  So it's
already on my table of ideas to do a followup patch that adds
documentation, and audits all clients to see what can be pruned.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg04660.html


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature