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@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list