6:12 p.m.
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 <eblake(a)redhat.com>
---
Although this fixes a build-breaker, it is not trivial, and
therefore I will wait for a full review.
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 f27fce7..ee8732c 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 98b4bf6..4ec3c0d 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++)
--
1.9.3