6:41 a.m.
Adding dbus_message_unref() into virDBusMessageDecodeArgs() makes sure
that the message gets unref'd, thus making one more pure dbus call not
necessary. Even though we are calling a lot of dbus_* functions
outside virdbus (which should be fixed in the future IMHO), this patch
fixes only this one instance because it merely aims to fix a
build-breaker caused by improperly included dbus.h. The message
printed when failing (using --without-dbus) is:
util/virsystemd.c: In function 'virSystemdPMSupportTarget':
util/virsystemd.c:367:5: error: implicit declaration of function
'dbus_message_unref' [-Werror=implicit-function-declaration]
dbus_message_unref(message);
^
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
I checked that we're not unreffing the message twice anywhere, but I
wanted to check there is no crash if we do unref it twice. While
trying whether double unreffing the message breaks anything, I saw
that dbus_message_unref() has a refcount and it cleans structure when
the refcount reaches 0, the cleansing means it sets it to -1 as well.
OK, so it doesn't break anything, but the allocated structure is still
not free()'d. I ran virsystemdtest under valgrind and saw that the
message is really not free()'d.
But here comes the best part; out of 6 test functions, only 3 of them
are causing a leak and adding VIR_FREE(msg) to any test function other
than the last one (which gets rid of one leak out of three) causes a
segfault. I though dbus has it's own memory management or whatever,
but looking at the backtrace, the crash happens when we are allocating
DBusMessageIter *newiter one function later than the VIR_FREE(msg) is
done (and msg is local to that function, btw).
Even though this has nothing to do with the patch sent, I'd appreciate
any explanation from anyone more dbus-knowledgeable than me (most
probably anyone).
src/util/virdbus.c | 1 +
src/util/virsystemd.c | 3 +--
tests/virdbustest.c | 6 ------
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 0cd3858..aef1d34 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1112,6 +1112,7 @@ int virDBusMessageDecodeArgs(DBusMessage* msg,
}
ret = virDBusMessageIterDecode(&iter, types, args);
+ dbus_message_unref(msg);
cleanup:
return ret;
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index e9ca564..9f67ac0 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -1,7 +1,7 @@
/*
* virsystemd.c: helpers for using systemd 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
@@ -364,7 +364,6 @@ virSystemdPMSupportTarget(const char *methodName, bool *result)
ret = 0;
cleanup:
- dbus_message_unref(message);
VIR_FREE(response);
return ret;
diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index a798fbe..50578d9 100644
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -121,7 +121,6 @@ static int testMessageSimple(const void *args ATTRIBUTE_UNUSED)
VIR_FREE(out_string);
VIR_FREE(out_signature);
VIR_FREE(out_objectpath);
- dbus_message_unref(msg);
return ret;
}
@@ -171,7 +170,6 @@ static int testMessageVariant(const void *args ATTRIBUTE_UNUSED)
cleanup:
VIR_FREE(out_str1);
VIR_FREE(out_str2);
- dbus_message_unref(msg);
return ret;
}
@@ -224,7 +222,6 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED)
cleanup:
VIR_FREE(out_str1);
VIR_FREE(out_str2);
- dbus_message_unref(msg);
return ret;
}
@@ -322,7 +319,6 @@ static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED)
for (i = 0; i < out_nstrv2; i++)
VIR_FREE(out_strv2[i]);
VIR_FREE(out_strv2);
- dbus_message_unref(msg);
return ret;
}
@@ -397,7 +393,6 @@ static int testMessageStruct(const void *args ATTRIBUTE_UNUSED)
VIR_FREE(out_string);
VIR_FREE(out_signature);
VIR_FREE(out_objectpath);
- dbus_message_unref(msg);
return ret;
}
@@ -467,7 +462,6 @@ static int testMessageDict(const void *args ATTRIBUTE_UNUSED)
VIR_FREE(out_key1);
VIR_FREE(out_key2);
VIR_FREE(out_key3);
- dbus_message_unref(msg);
return ret;
}
--
1.9.2