6:36 p.m.
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