[PATCH 1/5] lib: Don't unref message passed to virGDBusCallMethod{WithFD}()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The virGDBusCallMethod() and virGDBusCallMethodWithFD() are
simple wrappers over g_dbus_connection_call_sync() and
g_dbus_connection_call_with_unix_fd_list_sync() respectively. The
documentation to these function states that passed parameters
(@message in our case) is consumed for 'convenient' inline use of
g_variant_new() [1]. But that means we must not unref the message
afterwards. To make it explicit that the message is consumed the
signature of our wrappers is changed too.

1: https://developer.gnome.org/gio/stable/GDBusConnection.html#g-dbus-connection-call-sync

Reported-by: Cole Robinson <crobinso@xxxxxxxxxx>
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/rpc/virnetdaemon.c  |  4 ++--
 src/util/virfirewalld.c | 16 ++++++++--------
 src/util/virgdbus.c     | 29 ++++++++++++++++++++---------
 src/util/virgdbus.h     |  4 ++--
 src/util/virpolkit.c    |  4 ++--
 src/util/virsystemd.c   | 23 ++++++++---------------
 6 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 37a5662e04..b42feb7f1e 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -469,7 +469,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
 {
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(GUnixFDList) replyFD = NULL;
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     GDBusConnection *systemBus;
     int fd;
     int rc;
@@ -494,7 +494,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
                                   "/org/freedesktop/login1",
                                   "org.freedesktop.login1.Manager",
                                   "Inhibit",
-                                  message,
+                                  &message,
                                   NULL);
 
     if (rc < 0)
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index a94ac7c183..b00f3bf418 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -83,7 +83,7 @@ int
 virFirewallDGetVersion(unsigned long *version)
 {
     GDBusConnection *sysbus = virGDBusGetSystemBus();
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(GVariant) gvar = NULL;
     char *versionStr;
@@ -101,7 +101,7 @@ virFirewallDGetVersion(unsigned long *version)
                            "/org/fedoraproject/FirewallD1",
                            "org.freedesktop.DBus.Properties",
                            "Get",
-                           message) < 0)
+                           &message) < 0)
         return -1;
 
     g_variant_get(reply, "(v)", &gvar);
@@ -129,7 +129,7 @@ int
 virFirewallDGetBackend(void)
 {
     GDBusConnection *sysbus = virGDBusGetSystemBus();
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(GVariant) gvar = NULL;
     g_autoptr(virError) error = NULL;
@@ -154,7 +154,7 @@ virFirewallDGetBackend(void)
                            "/org/fedoraproject/FirewallD1/config",
                            "org.freedesktop.DBus.Properties",
                            "Get",
-                           message) < 0)
+                           &message) < 0)
         return -1;
 
     if (error->level == VIR_ERR_ERROR) {
@@ -273,7 +273,7 @@ virFirewallDApplyRule(virFirewallLayer layer,
 {
     const char *ipv = virFirewallLayerFirewallDTypeToString(layer);
     GDBusConnection *sysbus = virGDBusGetSystemBus();
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(virError) error = NULL;
 
@@ -304,7 +304,7 @@ virFirewallDApplyRule(virFirewallLayer layer,
                            "/org/fedoraproject/FirewallD1",
                            "org.fedoraproject.FirewallD1.direct",
                            "passthrough",
-                           message) < 0)
+                           &message) < 0)
         return -1;
 
     if (error->level == VIR_ERR_ERROR) {
@@ -353,7 +353,7 @@ virFirewallDInterfaceSetZone(const char *iface,
                              const char *zone)
 {
     GDBusConnection *sysbus = virGDBusGetSystemBus();
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
 
     if (!sysbus)
         return -1;
@@ -368,5 +368,5 @@ virFirewallDInterfaceSetZone(const char *iface,
                              "/org/fedoraproject/FirewallD1",
                              "org.fedoraproject.FirewallD1.zone",
                              "changeZoneOfInterface",
-                             message);
+                             &message);
 }
diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c
index 4360a6acff..856ad80a88 100644
--- a/src/util/virgdbus.c
+++ b/src/util/virgdbus.c
@@ -205,23 +205,26 @@ virGDBusCallMethod(GDBusConnection *conn,
                    const char *objectPath,
                    const char *ifaceName,
                    const char *method,
-                   GVariant *data)
+                   GVariant **data)
 {
     g_autoptr(GVariant) ret = NULL;
     g_autoptr(GError) gerror = NULL;
+    GVariant *parameters = NULL;
 
     if (error)
         memset(error, 0, sizeof(*error));
 
-    if (data)
-        g_variant_ref_sink(data);
+    if (data && *data) {
+        parameters = g_variant_ref_sink(*data);
+        *data = NULL;
+    }
 
     ret = g_dbus_connection_call_sync(conn,
                                       busName,
                                       objectPath,
                                       ifaceName,
                                       method,
-                                      data,
+                                      parameters,
                                       replyType,
                                       G_DBUS_CALL_FLAGS_NONE,
                                       VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS,
@@ -259,24 +262,27 @@ virGDBusCallMethodWithFD(GDBusConnection *conn,
                          const char *objectPath,
                          const char *ifaceName,
                          const char *method,
-                         GVariant *data,
+                         GVariant **data,
                          GUnixFDList *dataFD)
 {
     g_autoptr(GVariant) ret = NULL;
     g_autoptr(GError) gerror = NULL;
+    GVariant *parameters = NULL;
 
     if (error)
         memset(error, 0, sizeof(*error));
 
-    if (data)
-        g_variant_ref_sink(data);
+    if (data && *data) {
+        parameters = g_variant_ref_sink(*data);
+        *data = NULL;
+    }
 
     ret = g_dbus_connection_call_with_unix_fd_list_sync(conn,
                                                         busName,
                                                         objectPath,
                                                         ifaceName,
                                                         method,
-                                                        data,
+                                                        parameters,
                                                         replyType,
                                                         G_DBUS_CALL_FLAGS_NONE,
                                                         VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS,
@@ -317,9 +323,14 @@ virGDBusCallMethodWithFD(GDBusConnection *conn G_GNUC_UNUSED,
                          const char *objectPath G_GNUC_UNUSED,
                          const char *ifaceName G_GNUC_UNUSED,
                          const char *method G_GNUC_UNUSED,
-                         GVariant *data G_GNUC_UNUSED,
+                         GVariant **data,
                          GUnixFDList *dataFD G_GNUC_UNUSED)
 {
+    if (data && *data) {
+        g_variant_unref(*data);
+        *data = NULL;
+    }
+
     virReportSystemError(ENOSYS, "%s",
                          _("Unix file descriptors not supported on this platform"));
     return -1;
diff --git a/src/util/virgdbus.h b/src/util/virgdbus.h
index ca7073e27c..9c3955073c 100644
--- a/src/util/virgdbus.h
+++ b/src/util/virgdbus.h
@@ -51,7 +51,7 @@ virGDBusCallMethod(GDBusConnection *conn,
                    const char *objectPath,
                    const char *ifaceName,
                    const char *method,
-                   GVariant *data);
+                   GVariant **data);
 
 int
 virGDBusCallMethodWithFD(GDBusConnection *conn,
@@ -63,7 +63,7 @@ virGDBusCallMethodWithFD(GDBusConnection *conn,
                          const char *objectPath,
                          const char *ifaceName,
                          const char *method,
-                         GVariant *data,
+                         GVariant **data,
                          GUnixFDList *dataFD);
 
 int
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index aad924a065..e9fb4ec64f 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -67,7 +67,7 @@ int virPolkitCheckAuth(const char *actionid,
     GVariantBuilder builder;
     GVariant *gprocess = NULL;
     GVariant *gdetails = NULL;
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(GVariantIter) iter = NULL;
     char *retkey;
@@ -110,7 +110,7 @@ int virPolkitCheckAuth(const char *actionid,
                            "/org/freedesktop/PolicyKit1/Authority",
                            "org.freedesktop.PolicyKit1.Authority",
                            "CheckAuthorization",
-                           message) < 0)
+                           &message) < 0)
         return -1;
 
     g_variant_get(reply, "((bba{ss}))", &is_authorized, &is_challenge, &iter);
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8456085476..f849cbbf60 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -198,7 +198,7 @@ char *
 virSystemdGetMachineNameByPID(pid_t pid)
 {
     GDBusConnection *conn;
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(GVariant) reply = NULL;
     g_autoptr(GVariant) gvar = NULL;
     g_autofree char *object = NULL;
@@ -220,7 +220,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
                            "/org/freedesktop/machine1",
                            "org.freedesktop.machine1.Manager",
                            "GetMachineByPID",
-                           message) < 0)
+                           &message) < 0)
         return NULL;
 
     g_variant_get(reply, "(o)", &object);
@@ -231,7 +231,6 @@ virSystemdGetMachineNameByPID(pid_t pid)
     VIR_DEBUG("Domain with pid %lld has object path '%s'",
               (long long) pid, object);
 
-    g_variant_unref(message);
     message = g_variant_new("(ss)",
                             "org.freedesktop.machine1.Machine", "Name");
 
@@ -243,7 +242,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
                            object,
                            "org.freedesktop.DBus.Properties",
                            "Get",
-                           message) < 0)
+                           &message) < 0)
         return NULL;
 
     g_variant_get(reply, "(v)", &gvar);
@@ -393,9 +392,7 @@ int virSystemdCreateMachine(const char *name,
                                 "/org/freedesktop/machine1",
                                 "org.freedesktop.machine1.Manager",
                                 "CreateMachineWithNetwork",
-                                message);
-
-        g_variant_unref(message);
+                                &message);
 
         if (rc < 0)
             return -1;
@@ -440,9 +437,7 @@ int virSystemdCreateMachine(const char *name,
                                 "/org/freedesktop/machine1",
                                 "org.freedesktop.machine1.Manager",
                                 "CreateMachine",
-                                message);
-
-        g_variant_unref(message);
+                                &message);
 
         if (rc < 0)
             return -1;
@@ -468,9 +463,7 @@ int virSystemdCreateMachine(const char *name,
                                 "/org/freedesktop/systemd1",
                                 "org.freedesktop.systemd1.Manager",
                                 "SetUnitProperties",
-                                message);
-
-        g_variant_unref(message);
+                                &message);
 
         if (rc < 0)
             return -1;
@@ -483,7 +476,7 @@ int virSystemdTerminateMachine(const char *name)
 {
     int rc;
     GDBusConnection *conn;
-    g_autoptr(GVariant) message = NULL;
+    GVariant *message = NULL;
     g_autoptr(virError) error = NULL;
 
     if (!name)
@@ -519,7 +512,7 @@ int virSystemdTerminateMachine(const char *name)
                            "/org/freedesktop/machine1",
                            "org.freedesktop.machine1.Manager",
                            "TerminateMachine",
-                           message) < 0)
+                           &message) < 0)
         return -1;
 
     if (error->level == VIR_ERR_ERROR &&
-- 
2.26.2




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux