From 96e0d6774f47d3b7c05bcc55144c45f5605fc666 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 21 Nov 2014 17:03:34 -0700 Subject: [PATCH] dbus: fix arrays of bools Commit 2aa167ca tried to fix the DBus interaction code to allow callers to use native types instead of 4-byte bools. But in fixing the issue, I missed the case of an arrayref; Conrad Meyer shows the following valid complaint issued by clang: 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. But fixing that points out that we have NEVER supported arrayrefs of sub-int types (byte, i16, u16, and now bool). Again, while raw types promote, arrays do not; so the macros HAVE to deal with both size possibilities rather than assuming that an arrayref uses the same sizing as the promoted raw type. Obviously, our testsuite wasn't covering as much as it should have. * src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases. (SET_NEXT_VAL): Fix uses of sub-int arrays. * tests/virdbustest.c (testMessageArray, testMessageArrayRef): Test it. Signed-off-by: Eric Blake --- src/util/virdbus.c | 39 +++++++++++++++++++++------------------ tests/virdbustest.c | 29 +++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index f27fce7a7f..ee8732c233 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -574,11 +574,11 @@ virDBusIsAllowedRefType(const char *sig) } -# define SET_NEXT_VAL(dbustype, vargtype, sigtype, fmt) \ +# define SET_NEXT_VAL(dbustype, vargtype, arrtype, sigtype, fmt) \ do { \ dbustype x; \ if (arrayref) { \ - vargtype *valarray = arrayptr; \ + arrtype valarray = arrayptr; \ x = (dbustype)*valarray; \ valarray++; \ arrayptr = valarray; \ @@ -666,45 +666,48 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter, switch (*t) { case DBUS_TYPE_BYTE: - SET_NEXT_VAL(unsigned char, int, *t, "%d"); + SET_NEXT_VAL(unsigned char, int, unsigned char *, *t, "%d"); break; case DBUS_TYPE_BOOLEAN: - SET_NEXT_VAL(dbus_bool_t, int, *t, "%d"); + SET_NEXT_VAL(dbus_bool_t, int, bool *, *t, "%d"); break; case DBUS_TYPE_INT16: - SET_NEXT_VAL(dbus_int16_t, int, *t, "%d"); + SET_NEXT_VAL(dbus_int16_t, int, short *, *t, "%d"); break; case DBUS_TYPE_UINT16: - SET_NEXT_VAL(dbus_uint16_t, unsigned int, *t, "%d"); + SET_NEXT_VAL(dbus_uint16_t, unsigned int, unsigned short *, + *t, "%d"); break; case DBUS_TYPE_INT32: - SET_NEXT_VAL(dbus_int32_t, int, *t, "%d"); + SET_NEXT_VAL(dbus_int32_t, int, int *, *t, "%d"); break; case DBUS_TYPE_UINT32: - SET_NEXT_VAL(dbus_uint32_t, unsigned int, *t, "%u"); + SET_NEXT_VAL(dbus_uint32_t, unsigned int, unsigned int *, + *t, "%u"); break; case DBUS_TYPE_INT64: - SET_NEXT_VAL(dbus_int64_t, long long, *t, "%lld"); + SET_NEXT_VAL(dbus_int64_t, long long, long long *, *t, "%lld"); break; case DBUS_TYPE_UINT64: - SET_NEXT_VAL(dbus_uint64_t, unsigned long long, *t, "%llu"); + SET_NEXT_VAL(dbus_uint64_t, unsigned long long, + unsigned long long *, *t, "%llu"); break; case DBUS_TYPE_DOUBLE: - SET_NEXT_VAL(double, double, *t, "%lf"); + SET_NEXT_VAL(double, double, double *, *t, "%lf"); break; case DBUS_TYPE_STRING: case DBUS_TYPE_OBJECT_PATH: case DBUS_TYPE_SIGNATURE: - SET_NEXT_VAL(char *, char *, *t, "%s"); + SET_NEXT_VAL(char *, char *, char **, *t, "%s"); break; case DBUS_TYPE_ARRAY: @@ -848,23 +851,23 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter, # define GET_NEXT_VAL(dbustype, member, vargtype, fmt) \ do { \ - dbustype *x; \ DBusBasicValue v; \ + dbustype *x = (dbustype *)&v.member; \ + vargtype *y; \ if (arrayref) { \ VIR_DEBUG("Use arrayref"); \ vargtype **xptrptr = arrayptr; \ if (VIR_EXPAND_N(*xptrptr, *narrayptr, 1) < 0) \ goto cleanup; \ - x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \ + y = (*xptrptr + (*narrayptr - 1)); \ VIR_DEBUG("Expanded to %zu", *narrayptr); \ } else { \ - x = (dbustype *)&(v.member); \ + y = va_arg(args, vargtype *); \ } \ dbus_message_iter_get_basic(iter, x); \ - if (!arrayref) \ - *va_arg(args, vargtype *) = v.member; \ + *y = *x; \ VIR_DEBUG("Read basic type '" #dbustype "' varg '" #vargtype \ - "' val '" fmt "'", (vargtype)*x); \ + "' val '" fmt "'", (vargtype)*y); \ } while (0) diff --git a/tests/virdbustest.c b/tests/virdbustest.c index 98b4bf6679..4ec3c0ddbd 100644 --- a/tests/virdbustest.c +++ b/tests/virdbustest.c @@ -183,6 +183,7 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED) int in_int32a = 1000000000, out_int32a = 0; int in_int32b = 2000000000, out_int32b = 0; int in_int32c = -2000000000, out_int32c = 0; + bool in_bool[] = { true, false, true }, out_bool[] = { false, true, false}; const char *in_str2 = "World"; char *out_str1 = NULL, *out_str2 = NULL; @@ -195,18 +196,20 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED) } if (virDBusMessageEncode(msg, - "sais", + "saiabs", in_str1, 3, in_int32a, in_int32b, in_int32c, + 3, in_bool[0], in_bool[1], in_bool[2], in_str2) < 0) { VIR_DEBUG("Failed to encode arguments"); goto cleanup; } if (virDBusMessageDecode(msg, - "sais", + "saiabs", &out_str1, 3, &out_int32a, &out_int32b, &out_int32c, + 3, &out_bool[0], &out_bool[1], &out_bool[2], &out_str2) < 0) { VIR_DEBUG("Failed to decode arguments"); goto cleanup; @@ -217,6 +220,9 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED) VERIFY("int32a", in_int32a, out_int32a, "%d"); VERIFY("int32b", in_int32b, out_int32b, "%d"); VERIFY("int32c", in_int32c, out_int32c, "%d"); + VERIFY("bool[0]", in_bool[0], out_bool[0], "%d"); + VERIFY("bool[1]", in_bool[1], out_bool[1], "%d"); + VERIFY("bool[2]", in_bool[2], out_bool[2], "%d"); VERIFY_STR("str2", in_str2, out_str2, "%s"); ret = 0; @@ -329,6 +335,7 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) int in_int32[] = { 100000000, 2000000000, -2000000000 }; + bool in_bool[] = { true, false, true }; const char *in_strv1[] = { "Fishfood", }; @@ -337,6 +344,8 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) }; int *out_int32 = NULL; size_t out_nint32 = 0; + bool *out_bool = NULL; + size_t out_nbool = 0; char **out_strv1 = NULL; char **out_strv2 = NULL; size_t out_nstrv1 = 0; @@ -354,10 +363,11 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) } if (virDBusMessageEncode(msg, - "sa&sa&ia&ss", + "sa&sa&ia&ba&ss", in_str1, 1, in_strv1, 3, in_int32, + 3, in_bool, 2, in_strv2, in_str2) < 0) { VIR_DEBUG("Failed to encode arguments"); @@ -365,10 +375,11 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) } if (virDBusMessageDecode(msg, - "sa&sa&ia&ss", + "sa&sa&ia&ba&ss", &out_str1, &out_nstrv1, &out_strv1, &out_nint32, &out_int32, + &out_nbool, &out_bool, &out_nstrv2, &out_strv2, &out_str2) < 0) { VIR_DEBUG("Failed to decode arguments"); @@ -393,6 +404,15 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) VERIFY("int32b", in_int32[1], out_int32[1], "%d"); VERIFY("int32c", in_int32[2], out_int32[2], "%d"); + if (out_nbool != 3) { + fprintf(stderr, "Expected 3 bools, but got %zu\n", + out_nbool); + goto cleanup; + } + VERIFY("bool[0]", in_bool[0], out_bool[0], "%d"); + VERIFY("bool[1]", in_bool[1], out_bool[1], "%d"); + VERIFY("bool[2]", in_bool[2], out_bool[2], "%d"); + if (out_nstrv2 != 2) { fprintf(stderr, "Expected 2 strings, but got %zu\n", out_nstrv2); @@ -407,6 +427,7 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED) cleanup: VIR_FREE(out_int32); + VIR_FREE(out_bool); VIR_FREE(out_str1); VIR_FREE(out_str2); for (i = 0; i < out_nstrv1; i++) -- GitLab