[PATCH 1/5] lib: Don't unref message passed to virGDBusCallMethod{WithFD}()
Michal Privoznik
mprivozn at redhat.com
Fri Oct 2 09:22:06 UTC 2020
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 at redhat.com>
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
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
More information about the libvir-list
mailing list