dbus: Don't unref NULL messages

Apparently we are not the only ones with dumb free functions
because dbus_message_unref() does not accept NULL either. But if
I were to vote, this one is even more evil. Instead of returning
an error just like we do it immediately dereference any pointer
passed and thus crash you app. Well done DBus!

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x7f878ebda700 (LWP 31264)]
  0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
  (gdb) bt
  #0  0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
  #1  0x00007f87be3f004e in dbus_message_unref () from /usr/lib64/libdbus-1.so.3
  #2  0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
  #3  0x00007f879761bd4d in qemuConnectCgroup (driver=0x7f87600a32a0, vm=0x7f87600c7550) at qemu/qemu_cgroup.c:909
  #4  0x00007f87976386b7 in qemuProcessReconnect (opaque=0x7f87600db840) at qemu/qemu_process.c:3386
  #5  0x00007f87bf6edfff in virThreadHelper (data=0x7f87600d5580) at util/virthread.c:206
  #6  0x00007f87bb602334 in start_thread (arg=0x7f878ebda700) at pthread_create.c:333
  #7  0x00007f87bb3481bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
  (gdb) frame 2
  #2  0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
  228         dbus_message_unref(reply);
  (gdb) p reply
  $1 = (DBusMessage *) 0x0

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/libvirt_private.syms

@@ -1370,6 +1370,7 @@ virDBusHasSystemBus;
 virDBusMessageDecode;
 virDBusMessageEncode;
 virDBusMessageRead;
+virDBusMessageUnref;
 virDBusSetSharedBus;
 
 

src/rpc/virnetdaemon.c

@@ -374,7 +374,7 @@ virNetDaemonGotInhibitReply(DBusPendingCall *pending,
             VIR_FORCE_CLOSE(fd);
         }
     }
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
 
  cleanup:
     virObjectUnlock(dmn);
@@ -426,7 +426,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
                                      dmn, NULL);
         dmn->autoShutdownCallingInhibit = true;
     }
-    dbus_message_unref(message);
+    virDBusMessageUnref(message);
 }
 #endif
 

src/util/virdbus.c

@@ -1398,7 +1398,7 @@ int virDBusCreateMethodV(DBusMessage **call,
     }
 
     if (virDBusMessageEncodeArgs(*call, types, args) < 0) {
-        dbus_message_unref(*call);
+        virDBusMessageUnref(*call);
         *call = NULL;
         goto cleanup;
     }
@@ -1467,7 +1467,7 @@ int virDBusCreateReplyV(DBusMessage **reply,
     }
 
     if (virDBusMessageEncodeArgs(*reply, types, args) < 0) {
-        dbus_message_unref(*reply);
+        virDBusMessageUnref(*reply);
         *reply = NULL;
         goto cleanup;
     }
@@ -1586,7 +1586,7 @@ virDBusCall(DBusConnection *conn,
         if (ret == 0 && replyout)
             *replyout = reply;
         else
-            dbus_message_unref(reply);
+            virDBusMessageUnref(reply);
     }
     return ret;
 }
@@ -1650,8 +1650,7 @@ int virDBusCallMethod(DBusConnection *conn,
     ret = virDBusCall(conn, call, replyout, error);
 
  cleanup:
-    if (call)
-        dbus_message_unref(call);
+    virDBusMessageUnref(call);
     return ret;
 }
 
@@ -1727,7 +1726,7 @@ static int virDBusIsServiceInList(const char *listMethod, const char *name)
     }
 
  cleanup:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return ret;
 }
 
@@ -1763,7 +1762,8 @@ int virDBusIsServiceRegistered(const char *name)
 
 void virDBusMessageUnref(DBusMessage *msg)
 {
-    dbus_message_unref(msg);
+    if (msg)
+        dbus_message_unref(msg);
 }
 
 #else /* ! WITH_DBUS */

src/util/virdbus.h

@@ -28,7 +28,6 @@
 # else
 #  define DBusConnection void
 #  define DBusMessage void
-#  define dbus_message_unref(m) do {} while (0)
 # endif
 # include "internal.h"
 

src/util/virfirewall.c

@@ -822,8 +822,7 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
 
  cleanup:
     virResetError(&error);
-    if (reply)
-        dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return ret;
 }
 

src/util/virsystemd.c

@@ -225,7 +225,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
 
  cleanup:
     VIR_FREE(object);
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
 
     return name;
 }

tests/virdbustest.c

@@ -121,7 +121,7 @@ static int testMessageSimple(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_string);
     VIR_FREE(out_signature);
     VIR_FREE(out_objectpath);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -171,7 +171,7 @@ static int testMessageVariant(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     VIR_FREE(out_str1);
     VIR_FREE(out_str2);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -230,7 +230,7 @@ static int testMessageArray(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     VIR_FREE(out_str1);
     VIR_FREE(out_str2);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -274,7 +274,7 @@ static int testMessageEmptyArrayRef(const void *args ATTRIBUTE_UNUSED)
     ret = 0;
 
  cleanup:
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -323,7 +323,7 @@ static int testMessageSingleArrayRef(const void *args ATTRIBUTE_UNUSED)
  cleanup:
     if (out_strv1)
         VIR_FREE(out_strv1[0]);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -436,7 +436,7 @@ 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);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -511,7 +511,7 @@ static int testMessageStruct(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_string);
     VIR_FREE(out_signature);
     VIR_FREE(out_objectpath);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -581,7 +581,7 @@ static int testMessageDict(const void *args ATTRIBUTE_UNUSED)
     VIR_FREE(out_key1);
     VIR_FREE(out_key2);
     VIR_FREE(out_key3);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -652,7 +652,7 @@ static int testMessageDictRef(const void *args ATTRIBUTE_UNUSED)
         VIR_FREE(out_strv1[5]);
     }
     VIR_FREE(out_strv1);
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 
@@ -695,7 +695,7 @@ static int testMessageEmptyDictRef(const void *args ATTRIBUTE_UNUSED)
     ret = 0;
 
  cleanup:
-    dbus_message_unref(msg);
+    virDBusMessageUnref(msg);
     return ret;
 }
 

tests/virfirewalltest.c

@@ -179,8 +179,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    if (reply)
-        dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     reply = NULL;
     if (error && !dbus_error_is_set(error))
         dbus_set_error_const(error,

tests/virpolkittest.c

@@ -140,7 +140,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return NULL;
 }
 

tests/virsystemdtest.c

@@ -28,6 +28,7 @@
 # include <dbus/dbus.h>
 
 # include "virsystemd.h"
+# include "virdbus.h"
 # include "virlog.h"
 # include "virmock.h"
 # define VIR_FROM_THIS VIR_FROM_NONE
@@ -151,7 +152,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
     return reply;
 
  error:
-    dbus_message_unref(reply);
+    virDBusMessageUnref(reply);
     return NULL;
 }