6:36 p.m.
I stumbled into this one on work I'm doing with qemu blockinfo.
The DBus interaction code was tricky enough to need to be its own
patch; but I like the end result that clients reading a 'b' entry
from virDBus code can now pass a sane 'bool*' instead of a weird
'int*' variable.
Eric Blake (2):
virdbus: don't force users to pass int for bool values
maint: forbid 'int foo = true'
cfg.mk | 6 ++++++
src/conf/snapshot_conf.c | 2 +-
src/qemu/qemu_driver.c | 4 ++--
src/test/test_driver.c | 2 +-
src/util/vircgroup.c | 2 +-
src/util/virdbus.c | 25 ++++++++++++++-----------
src/util/virpci.c | 2 +-
src/util/virpolkit.c | 6 +++---
src/util/virutil.c | 2 +-
tests/virdbustest.c | 4 ++--
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 12 ++++++------
12 files changed, 39 insertions(+), 30 deletions(-)
--
1.9.3
6:36 p.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
Use of an 'int' to represent a 'bool' value is confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
[Have I ever mentioned that the assymetry between type promotions
of values passed through var-args on setting, vs. no promotions
when passing pointers through var-args for getting, makes life
awkward, not just for DBus interactions, but also for printf vs.
scanf? Then again, using scanf is often the wrong thing to do...]
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/util/virdbus.c | 25 ++++++++++++++-----------
src/util/virpolkit.c | 6 +++---
tests/virdbustest.c | 4 ++--
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 8abb247..f27fce7 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -846,9 +846,10 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
# undef SET_NEXT_VAL
-# define GET_NEXT_VAL(dbustype, vargtype, fmt) \
+# define GET_NEXT_VAL(dbustype, member, vargtype, fmt) \
do { \
dbustype *x; \
+ DBusBasicValue v; \
if (arrayref) { \
VIR_DEBUG("Use arrayref"); \
vargtype **xptrptr = arrayptr; \
@@ -857,9 +858,11 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
VIR_DEBUG("Expanded to %zu", *narrayptr); \
} else { \
- x = (dbustype *)va_arg(args, vargtype *); \
+ x = (dbustype *)&(v.member); \
} \
dbus_message_iter_get_basic(iter, x); \
+ if (!arrayref) \
+ *va_arg(args, vargtype *) = v.member; \
VIR_DEBUG("Read basic type '" #dbustype "' varg
'" #vargtype \
"' val '" fmt "'", (vargtype)*x);
\
} while (0)
@@ -946,39 +949,39 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
switch (*t) {
case DBUS_TYPE_BYTE:
- GET_NEXT_VAL(unsigned char, unsigned char, "%d");
+ GET_NEXT_VAL(unsigned char, byt, unsigned char, "%d");
break;
case DBUS_TYPE_BOOLEAN:
- GET_NEXT_VAL(dbus_bool_t, int, "%d");
+ GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
break;
case DBUS_TYPE_INT16:
- GET_NEXT_VAL(dbus_int16_t, short, "%d");
+ GET_NEXT_VAL(dbus_int16_t, i16, short, "%d");
break;
case DBUS_TYPE_UINT16:
- GET_NEXT_VAL(dbus_uint16_t, unsigned short, "%d");
+ GET_NEXT_VAL(dbus_uint16_t, u16, unsigned short, "%d");
break;
case DBUS_TYPE_INT32:
- GET_NEXT_VAL(dbus_uint32_t, int, "%d");
+ GET_NEXT_VAL(dbus_uint32_t, i32, int, "%d");
break;
case DBUS_TYPE_UINT32:
- GET_NEXT_VAL(dbus_uint32_t, unsigned int, "%u");
+ GET_NEXT_VAL(dbus_uint32_t, u32, unsigned int, "%u");
break;
case DBUS_TYPE_INT64:
- GET_NEXT_VAL(dbus_uint64_t, long long, "%lld");
+ GET_NEXT_VAL(dbus_uint64_t, i64, long long, "%lld");
break;
case DBUS_TYPE_UINT64:
- GET_NEXT_VAL(dbus_uint64_t, unsigned long long, "%llu");
+ GET_NEXT_VAL(dbus_uint64_t, u64, unsigned long long, "%llu");
break;
case DBUS_TYPE_DOUBLE:
- GET_NEXT_VAL(double, double, "%lf");
+ GET_NEXT_VAL(double, dbl, double, "%lf");
break;
case DBUS_TYPE_STRING:
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index f4ea736..ae7ec13 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -1,7 +1,7 @@
/*
* virpolkit.c: helpers for using polkit APIs
*
- * Copyright (C) 2013 Red Hat, Inc.
+ * Copyright (C) 2013-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -64,8 +64,8 @@ int virPolkitCheckAuth(const char *actionid,
DBusMessage *reply = NULL;
char **retdetails = NULL;
size_t nretdetails = 0;
- int is_authorized; /* var-args requires int not bool */
- int is_challenge; /* var-args requires int not bool */
+ bool is_authorized;
+ bool is_challenge;
bool is_dismissed = false;
size_t i;
int ret = -1;
diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index 002f904..98b4bf6 100644
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -55,7 +55,7 @@ static int testMessageSimple(const void *args ATTRIBUTE_UNUSED)
DBusMessage *msg = NULL;
int ret = -1;
unsigned char in_byte = 200, out_byte = 0;
- int in_bool = true, out_bool = false;
+ bool in_bool = true, out_bool = false;
short in_int16 = 0xfefe, out_int16 = 0;
unsigned short in_uint16 = 32000, out_uint16 = 0;
int in_int32 = 100000000, out_int32 = 0;
@@ -424,7 +424,7 @@ static int testMessageStruct(const void *args ATTRIBUTE_UNUSED)
DBusMessage *msg = NULL;
int ret = -1;
unsigned char in_byte = 200, out_byte = 0;
- int in_bool = true, out_bool = false;
+ bool in_bool = true, out_bool = false;
short in_int16 = 12000, out_int16 = 0;
unsigned short in_uint16 = 32000, out_uint16 = 0;
int in_int32 = 100000000, out_int32 = 0;
--
1.9.3
3:42 p.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On 11/17/2014 05:36 PM, Eric Blake wrote:
Use of an 'int' to represent a 'bool' value is
confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
[Have I ever mentioned that the assymetry between type promotions
of values passed through var-args on setting, vs. no promotions
when passing pointers through var-args for getting, makes life
awkward, not just for DBus interactions, but also for printf vs.
scanf? Then again, using scanf is often the wrong thing to do...]
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
-# define GET_NEXT_VAL(dbustype, vargtype, fmt)
\
+# define GET_NEXT_VAL(dbustype, member, vargtype, fmt) \
do { \
dbustype *x; \
+ DBusBasicValue v; \
Blech. DBusBasicValue wasn't defined in the older dbus-types.h from
1.1.2 as shipped on RHEL 5, causing a compilation failure there. I'm
working on a fix...
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:21 a.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On Wed, Nov 19, 2014 at 02:42:51PM -0700, Eric Blake wrote:
On 11/17/2014 05:36 PM, Eric Blake wrote:
> Use of an 'int' to represent a 'bool' value is confusing. Just
> because dbus made the mistake of cementing their 4-byte wire
> format of dbus_bool_t into their API doesn't mean we have to
> repeat the mistake. With a little bit of finesse, we can
> guarantee that we provide a large-enough value to the DBus
> code, while still copying only the relevant one-byte bool
> to the client code, and isolate the rest of our code base from
> the DBus stupidity.
>
> [Have I ever mentioned that the assymetry between type promotions
> of values passed through var-args on setting, vs. no promotions
> when passing pointers through var-args for getting, makes life
> awkward, not just for DBus interactions, but also for printf vs.
> scanf? Then again, using scanf is often the wrong thing to do...]
>
> * src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
> (virDBusMessageIterDecode): Adjust all clients.
> * src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
> * tests/virdbustest.c (testMessageSimple, testMessageStruct):
> Test new behavior.
>
> -# define GET_NEXT_VAL(dbustype, vargtype, fmt) \
> +# define GET_NEXT_VAL(dbustype, member, vargtype, fmt) \
> do { \
> dbustype *x; \
> + DBusBasicValue v; \
Blech. DBusBasicValue wasn't defined in the older dbus-types.h from
1.1.2 as shipped on RHEL 5, causing a compilation failure there. I'm
working on a fix...
Its just a dumb union, so we can either provide the definition ourselves,
or just define our of virDBusValue union and ignore DBusBasicValue
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:12 a.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
Hi Eric,
I think this change breaks build on FreeBSD:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka
'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
(CC is Clang.)
Thanks,
Conrad
5:17 p.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On 11/20/2014 08:12 AM, Conrad Meyer wrote:
Hi Eric,
I think this change breaks build on FreeBSD:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka
'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
Valid complaint, so I'll have to figure something out to avoid it. :(
Thanks for the heads up.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:16 p.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On 11/20/2014 04:17 PM, Eric Blake wrote:
On 11/20/2014 08:12 AM, Conrad Meyer wrote:
> Hi Eric,
>
> I think this change breaks build on FreeBSD:
>
> CC util/libvirt_util_la-virdbus.lo
> util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
> GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
> x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
Valid complaint, so I'll have to figure something out to avoid it. :(
Thanks for the heads up.
Thanks again for flagging this!
We had a pre-existing bug. Even something like "a&n" was broken on
encoding, because even though type 'n' (int16_t) promotes to a full
'int' when parsed via varargs, passing an array of shorts and then
dereferencing it via (int*) will read beyond array bounds. We had a
hole in our testsuite for never testing arrayref with sub-int types,
even before my commit added 'bool *' to the list of sub-int types that
we now need to support.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:43 a.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On Fri, 2014-11-21 at 17:16 -0700, Eric Blake wrote:
On 11/20/2014 04:17 PM, Eric Blake wrote:
> On 11/20/2014 08:12 AM, Conrad Meyer wrote:
>> Hi Eric,
>>
>> I think this change breaks build on FreeBSD:
>>
>> CC util/libvirt_util_la-virdbus.lo
>> util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t
*' (aka 'unsigned int *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
>> GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
>> x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
>
> Valid complaint, so I'll have to figure something out to avoid it. :(
> Thanks for the heads up.
Thanks again for flagging this!
We had a pre-existing bug. Even something like "a&n" was broken on
encoding, because even though type 'n' (int16_t) promotes to a full
'int' when parsed via varargs, passing an array of shorts and then
dereferencing it via (int*) will read beyond array bounds. We had a
hole in our testsuite for never testing arrayref with sub-int types,
even before my commit added 'bool *' to the list of sub-int types that
we now need to support.
I'm, guessing that this is the same underlying issue as:
util/virdbus.c: In function 'virDBusMessageIterDecode':
util/virdbus.c:956:346: error: cast increases required alignment of target type
[-Werror=cast-align]
which we are seeing in the Xen automated tests [0, 1] (on armhf only,
probably compiler dependent?).
Ian.
[0]
http://www.chiark.greenend.org.uk/~xensrcts/logs/31787/build-armhf-libvir...
[1] http://www.chiark.greenend.org.uk/~xensrcts/logs/31787/build-armhf-libvir...
10:21 a.m.
New subject: [libvirt] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On 11/24/2014 02:43 AM, Ian Campbell wrote:
>>>
>>> I think this change breaks build on FreeBSD:
>>>
>>> CC util/libvirt_util_la-virdbus.lo
>>> util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t
*' (aka 'unsigned int *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
>>> GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
>>> x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
>>
>> Valid complaint, so I'll have to figure something out to avoid it. :(
I'm, guessing that this is the same underlying issue as:
util/virdbus.c: In function 'virDBusMessageIterDecode':
util/virdbus.c:956:346: error: cast increases required alignment of target type
[-Werror=cast-align]
Yes.
which we are seeing in the Xen automated tests [0, 1] (on armhf only,
probably compiler dependent?).
So, do I have an ACK on my proposed fix yet? :)
https://www.redhat.com/archives/libvir-list/2014-November/msg00838.html
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:51 a.m.
New subject: [libvirt] [Xen-devel] [PATCH 1/2] virdbus: don't force users to pass int for bool values
On Mon, 2014-11-24 at 09:21 -0700, Eric Blake wrote:
On 11/24/2014 02:43 AM, Ian Campbell wrote:
>>>>
>>>> I think this change breaks build on FreeBSD:
>>>>
>>>> CC util/libvirt_util_la-virdbus.lo
>>>> util/virdbus.c:956:13: error: cast from 'bool *' to
'dbus_bool_t *' (aka 'unsigned int *') increases required alignment from 1
to 4 [-Werror,-Wcast-align]
>>>> GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
>>>> x = (dbustype *)(*xptrptr + (*narrayptr - 1));
\
>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error
generated.
>>>
>>> Valid complaint, so I'll have to figure something out to avoid it. :(
> I'm, guessing that this is the same underlying issue as:
> util/virdbus.c: In function 'virDBusMessageIterDecode':
> util/virdbus.c:956:346: error: cast increases required alignment of target
type [-Werror=cast-align]
Yes.
>
> which we are seeing in the Xen automated tests [0, 1] (on armhf only,
> probably compiler dependent?).
So, do I have an ACK on my proposed fix yet? :)
https://www.redhat.com/archives/libvir-list/2014-November/msg00838.html
Well, FWIW it looks good to me:
Acked-by: Ian Campbell <ian.campbell(a)citrix.com>
Ian.
6:36 p.m.
New subject: [libvirt] [PATCH 2/2] maint: forbid 'int foo = true'
I noticed this while working on qemuDomainGetBlockInfo. Assigning
a bool value to an int variable compiles fine, but raises red flags
on the maintenance front as it becomes too easy to assign -1 or 2
or any other non-bool value to the same variable.
* cfg.mk (sc_prohibit_int_assign_bool): New rule.
* src/conf/snapshot_conf.c (virDomainSnapshotRedefinePrep): Fix
offenders.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotCreateXML): Likewise.
* src/test/test_driver.c (testDomainSnapshotAlignDisks):
Likewise.
* src/util/vircgroup.c (virCgroupSupportsCpuBW): Likewise.
* src/util/virpci.c (virPCIDeviceBindToStub): Likewise.
* src/util/virutil.c (virIsCapableVport): Likewise.
* tools/virsh-domain-monitor.c (cmdDomMemStat): Likewise.
* tools/virsh-domain.c (cmdBlockResize, cmdScreenshot)
(cmdInjectNMI, cmdSendKey, cmdSendProcessSignal)
(cmdDetachInterface): Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
cfg.mk | 6 ++++++
src/conf/snapshot_conf.c | 2 +-
src/qemu/qemu_driver.c | 4 ++--
src/test/test_driver.c | 2 +-
src/util/vircgroup.c | 2 +-
src/util/virpci.c | 2 +-
src/util/virutil.c | 2 +-
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 12 ++++++------
9 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index f188645..c49f4f3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -584,6 +584,12 @@ sc_prohibit_loop_var_decl:
halt='declare loop iterators outside the for statement' \
$(_sc_search_regexp)
+# Use 'bool', not 'int', when assigning true or false
+sc_prohibit_int_assign_bool:
+ @prohibit='\<int\>.*= *(true|false)' \
+ halt='use bool type for boolean values' \
+ $(_sc_search_regexp)
+
# Many of the function names below came from this filter:
# git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]*
?\(.*[,;]$' \
# |sed 's/.*\.c- *//'|perl -pe 's/ ?\(.*//'|sort -u \
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 1f83b2c..c2caf33 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1182,7 +1182,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
virDomainSnapshotDefPtr def = *defptr;
int ret = -1;
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
- int align_match = true;
+ bool align_match = true;
char uuidstr[VIR_UUID_STRING_BUFLEN];
virDomainSnapshotObjPtr other;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a84fd47..74d1bdc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10933,7 +10933,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
struct stat sb;
int idx;
int format;
- int activeFail = false;
+ bool activeFail = false;
virQEMUDriverConfigPtr cfg = NULL;
char *alias = NULL;
char *buf = NULL;
@@ -13809,7 +13809,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
virDomainSnapshotObjPtr other = NULL;
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
- int align_match = true;
+ bool align_match = true;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d7844bd..4b7fea1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6835,7 +6835,7 @@ testDomainSnapshotAlignDisks(virDomainObjPtr vm,
unsigned int flags)
{
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
- int align_match = true;
+ bool align_match = true;
if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 9bbe694..166f4dc 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3908,7 +3908,7 @@ bool
virCgroupSupportsCpuBW(virCgroupPtr cgroup)
{
char *path = NULL;
- int ret = false;
+ bool ret = false;
if (!cgroup)
return false;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index f60d0fa..cd78212 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1182,7 +1182,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
const char *stubDriverName)
{
int result = -1;
- int reprobe = false;
+ bool reprobe = false;
char *stubDriverPath = NULL;
char *driverLink = NULL;
char *path = NULL; /* reused for different purposes */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 88c8baf..9c40317 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1995,7 +1995,7 @@ virIsCapableVport(const char *sysfs_prefix,
{
char *scsi_host_path = NULL;
char *fc_host_path = NULL;
- int ret = false;
+ bool ret = false;
if (virAsprintf(&fc_host_path,
"%s/host%d/%s",
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 4e434f8..259400f 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -298,7 +298,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
unsigned int nr_stats;
size_t i;
- int ret = false;
+ bool ret = false;
int rv = 0;
int period = -1;
bool config = vshCommandOptBool(cmd, "config");
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a7e9151..d48765a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2640,7 +2640,7 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
const char *path = NULL;
unsigned long long size = 0;
unsigned int flags = 0;
- int ret = false;
+ bool ret = false;
if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path)
< 0)
return false;
@@ -5163,7 +5163,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
virStreamPtr st = NULL;
unsigned int screen = 0;
unsigned int flags = 0; /* currently unused */
- int ret = false;
+ bool ret = false;
bool created = false;
bool generated = false;
char *mime = NULL;
@@ -7615,7 +7615,7 @@ static bool
cmdInjectNMI(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
- int ret = true;
+ bool ret = true;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -7678,7 +7678,7 @@ static bool
cmdSendKey(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
- int ret = false;
+ bool ret = false;
const char *codeset_option;
int codeset;
unsigned int holdtime = 0;
@@ -7809,7 +7809,7 @@ static bool
cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
- int ret = false;
+ bool ret = false;
const char *pidstr;
const char *signame;
long long pid_value;
@@ -10592,7 +10592,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
int diff_mac;
size_t i;
int ret;
- int functionReturn = false;
+ bool functionReturn = false;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
--
1.9.3
7:05 a.m.
On 18.11.2014 01:36, Eric Blake wrote:
I stumbled into this one on work I'm doing with qemu blockinfo.
The DBus interaction code was tricky enough to need to be its own
patch; but I like the end result that clients reading a 'b' entry
from virDBus code can now pass a sane 'bool*' instead of a weird
'int*' variable.
Eric Blake (2):
virdbus: don't force users to pass int for bool values
maint: forbid 'int foo = true'
cfg.mk | 6 ++++++
src/conf/snapshot_conf.c | 2 +-
src/qemu/qemu_driver.c | 4 ++--
src/test/test_driver.c | 2 +-
src/util/vircgroup.c | 2 +-
src/util/virdbus.c | 25 ++++++++++++++-----------
src/util/virpci.c | 2 +-
src/util/virpolkit.c | 6 +++---
src/util/virutil.c | 2 +-
tests/virdbustest.c | 4 ++--
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 12 ++++++------
12 files changed, 39 insertions(+), 30 deletions(-)
ACK to both.
Michal
10:06 a.m.
On 11/19/2014 06:05 AM, Michal Privoznik wrote:
On 18.11.2014 01:36, Eric Blake wrote:
> I stumbled into this one on work I'm doing with qemu blockinfo.
> The DBus interaction code was tricky enough to need to be its own
> patch; but I like the end result that clients reading a 'b' entry
> from virDBus code can now pass a sane 'bool*' instead of a weird
> 'int*' variable.
>
> Eric Blake (2):
> virdbus: don't force users to pass int for bool values
> maint: forbid 'int foo = true'
>
> cfg.mk | 6 ++++++
> src/conf/snapshot_conf.c | 2 +-
> src/qemu/qemu_driver.c | 4 ++--
> src/test/test_driver.c | 2 +-
> src/util/vircgroup.c | 2 +-
> src/util/virdbus.c | 25 ++++++++++++++-----------
> src/util/virpci.c | 2 +-
> src/util/virpolkit.c | 6 +++---
> src/util/virutil.c | 2 +-
> tests/virdbustest.c | 4 ++--
> tools/virsh-domain-monitor.c | 2 +-
> tools/virsh-domain.c | 12 ++++++------
> 12 files changed, 39 insertions(+), 30 deletions(-)
>
ACK to both.
Pushed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3875
days inactive
3882
days old
12 comments
5 participants
participants (5)
-
Conrad Meyer -
Daniel P. Berrange -
Eric Blake -
Ian Campbell -
Michal Privoznik