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

[Eric Blake]

On 05/16/2015 07:30 AM, Andreas Färber wrote:
> Am 16.05.2015 um 00:24 schrieb Eric Blake:
>> 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.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---

>>  /**
>> - * qbool_from_int(): Create a new QBool from an int
>> + * qbool_from_bool(): Create a new QBool from a bool
>>   *
>>   * Return strong reference.
> 
> Can you fix the syntax as follow-up please?
> 
> /**
>  * qbool_from_bool:
>  * @value: ...
>  *
>  * Desc...
>  *
>  * Returns: ...
>  */

Sure, I can do that over all the qboject files, as a new patch.

>> @@ -662,7 +662,7 @@ static void check_native_list(QObject *qobj,
>>              tmp = qlist_peek(qlist);
>>              g_assert(tmp);
>>              qvalue = qobject_to_qbool(tmp);
>> -            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 
>> 0);
>> +            g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
>>              qobject_decref(qlist_pop(qlist));
>>          }
>>          break;
> [snip]
> 
> I notice that we're inconsistent in using g_assert() vs.
> g_assert_cmpint(). Given that GLib has a weird GBoolean, should we add a
> macro qtest_assert_cmpbool() instead as follow-up?

We aren't even touching GBoolean (qbool_get_bool now returns 'bool', not
GBoolean; and bool promotes just fine to int under C rules), so I don't
see the point to making any further changes here.  Or are you proposing
that the new macro would do something like 'expecting "true" but got
"false"' instead of g_assert_cmpint() collapsing things to 0 and 1?

> 
> That said,
> 
> Reviewed-by: Andreas Färber <address@hidden>
> 
> Regards,
> Andreas
> 

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

signature.asc
Description: OpenPGP digital signature