[libvirt] [PATCH 0/2] use bool type for bool values
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
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@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
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
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 :|
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
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
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
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-libvirt/5... [1] http://www.chiark.greenend.org.uk/~xensrcts/logs/31787/build-armhf-libvirt/i...
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
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@citrix.com> Ian.
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@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
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
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
participants (5)
-
Conrad Meyer -
Daniel P. Berrange -
Eric Blake -
Ian Campbell -
Michal Privoznik