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.

* 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>

diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 8abb2479219b87588a861465303930c9ef69bbe3..f27fce7a7ff1a959898aa166ad06ced5dfef0726 100644 (file)
--- 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 f4ea736d67fdc36f6d2646f92355e12c2c282a66..ae7ec132b229c61acb3ae3d5aa4a9d42d6562501 100644 (file)
--- 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 002f90430849d3d3e7556cf1fdaf6e9536bf23d1..98b4bf66795d631e6a445b706c436ee6b2d0249d 100644 (file)
--- 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;