[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qobject: Use 'bool' for qbool
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qobject: Use 'bool' for qbool |
Date: |
Sat, 16 May 2015 11:13:07 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
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