[libvirt] [PATCH] build: Embed dbus_message_unref in virdbus

14 Apr 2014

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

    

Martin Kletzander

Daniel P. Berrange

Martin Kletzander

Daniel P. Berrange

Martin Kletzander

Daniel P. Berrange

Martin Kletzander

tags

participants (2)