[upower] Some dbus error handling fixes

Matthias Clasen mclasen at fedoraproject.org
Wed Aug 18 01:58:19 UTC 2010


commit 14952db5330c8444779b363e175c3fb437cb214a
Author: Matthias Clasen <mclasen at redhat.com>
Date:   Tue Aug 17 21:56:59 2010 -0400

    Some dbus error handling fixes

 upower-dbus-fixes.patch |  291 +++++++++++++++++++++++++++++++++++++++++++++++
 upower.spec             |   10 ++-
 2 files changed, 300 insertions(+), 1 deletions(-)
---
diff --git a/upower-dbus-fixes.patch b/upower-dbus-fixes.patch
new file mode 100644
index 0000000..2bad9b1
--- /dev/null
+++ b/upower-dbus-fixes.patch
@@ -0,0 +1,291 @@
+diff --git a/src/up-daemon.c b/src/up-daemon.c
+index 3fb952b..4884b64 100644
+--- a/src/up-daemon.c
++++ b/src/up-daemon.c
+@@ -323,6 +323,7 @@ up_daemon_about_to_sleep (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "Sleep has already been requested and is pending");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -376,6 +377,7 @@ up_daemon_deferred_sleep_cb (UpDaemonDeferredSleep *sleep)
+ 				     "Failed to spawn: %s, stdout:%s, stderr:%s", error_local->message, stdout, stderr);
+ 		g_error_free (error_local);
+ 		dbus_g_method_return_error (sleep->context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -470,6 +472,7 @@ up_daemon_suspend (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "No kernel support");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -477,7 +480,7 @@ up_daemon_suspend (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 	if (subject == NULL)
+ 		goto out;
+ 
+-	if (!up_polkit_check_auth (priv->polkit, subject, "org.freedesktop.upower.suspend", context))
++	if (!up_polkit_check_auth (priv->polkit, subject, "org.freedesktop.upower.suspend", NULL))
+ 		goto out;
+ 
+ 	/* already requested */
+@@ -486,6 +489,7 @@ up_daemon_suspend (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "Sleep has already been requested and is pending");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -507,13 +511,21 @@ up_daemon_suspend_allowed (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 	gboolean ret;
+ 	PolkitSubject *subject = NULL;
+ 	UpDaemonPrivate *priv = daemon->priv;
++        GError *error;
+ 
+ 	subject = up_polkit_get_subject (priv->polkit, context);
+ 	if (subject == NULL)
+ 		goto out;
+ 
+-	ret = up_polkit_is_allowed (priv->polkit, subject, "org.freedesktop.upower.suspend", context);
+-	dbus_g_method_return (context, ret);
++        error = NULL;
++	ret = up_polkit_is_allowed (priv->polkit, subject, "org.freedesktop.upower.suspend", &error);
++        if (error) {
++                dbus_g_method_return_error (context, error);
++                g_error_free (error);
++        }
++        else {
++	        dbus_g_method_return (context, ret);
++        }
+ 
+ out:
+ 	if (subject != NULL)
+@@ -563,6 +575,7 @@ up_daemon_hibernate (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "No kernel support");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -572,6 +585,7 @@ up_daemon_hibernate (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "Not enough swap space");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -582,6 +596,7 @@ up_daemon_hibernate (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "Swap space is encrypted, use AllowHibernateEncryptedSwap to override");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -598,6 +613,7 @@ up_daemon_hibernate (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 				     UP_DAEMON_ERROR_GENERAL,
+ 				     "Sleep has already been requested and is pending");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -619,13 +635,21 @@ up_daemon_hibernate_allowed (UpDaemon *daemon, DBusGMethodInvocation *context)
+ 	gboolean ret;
+ 	PolkitSubject *subject = NULL;
+ 	UpDaemonPrivate *priv = daemon->priv;
++        GError *error;
+ 
+ 	subject = up_polkit_get_subject (priv->polkit, context);
+ 	if (subject == NULL)
+ 		goto out;
+ 
+-	ret = up_polkit_is_allowed (priv->polkit, subject, "org.freedesktop.upower.hibernate", context);
+-	dbus_g_method_return (context, ret);
++        error = NULL;
++	ret = up_polkit_is_allowed (priv->polkit, subject, "org.freedesktop.upower.hibernate", &error);
++        if (error) {
++                dbus_g_method_return_error (context, error);
++                g_error_free (error);
++        }
++        else {
++	        dbus_g_method_return (context, ret);
++        }
+ 
+ out:
+ 	if (subject != NULL)
+diff --git a/src/up-polkit.c b/src/up-polkit.c
+index 9b86394..bd1e5e0 100644
+--- a/src/up-polkit.c
++++ b/src/up-polkit.c
+@@ -52,12 +52,19 @@ static gpointer up_polkit_object = NULL;
+ PolkitSubject *
+ up_polkit_get_subject (UpPolkit *polkit, DBusGMethodInvocation *context)
+ {
++        GError *error;
+ 	const gchar *sender;
+ 	PolkitSubject *subject;
+ 
+ 	sender = dbus_g_method_get_sender (context);
+ 	subject = polkit_system_bus_name_new (sender);
+ 
++        if (subject == NULL) {
++		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "failed to get PolicyKit subject");
++                dbus_g_method_return_error (context, error);
++		g_error_free (error);
++        }
++
+ 	return subject;
+ }
+ 
+@@ -79,9 +86,9 @@ up_polkit_check_auth (UpPolkit *polkit, PolkitSubject *subject, const gchar *act
+ 							    NULL, &error_local);
+ 	if (result == NULL) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "failed to check authorisation: %s", error_local->message);
+-		dbus_g_method_return_error (context, error);
++                dbus_g_method_return_error (context, error);
+ 		g_error_free (error_local);
+-		g_error_free (error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -90,8 +97,8 @@ up_polkit_check_auth (UpPolkit *polkit, PolkitSubject *subject, const gchar *act
+ 		ret = TRUE;
+ 	} else {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "not authorized");
+-		dbus_g_method_return_error (context, error);
+-		g_error_free (error);
++                dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 	}
+ out:
+ 	if (result != NULL)
+@@ -103,10 +110,9 @@ out:
+  * up_polkit_is_allowed:
+  **/
+ gboolean
+-up_polkit_is_allowed (UpPolkit *polkit, PolkitSubject *subject, const gchar *action_id, DBusGMethodInvocation *context)
++up_polkit_is_allowed (UpPolkit *polkit, PolkitSubject *subject, const gchar *action_id, GError **error)
+ {
+ 	gboolean ret = FALSE;
+-	GError *error;
+ 	GError *error_local = NULL;
+ 	PolkitAuthorizationResult *result;
+ 
+@@ -116,10 +122,8 @@ up_polkit_is_allowed (UpPolkit *polkit, PolkitSubject *subject, const gchar *act
+ 							    POLKIT_CHECK_AUTHORIZATION_FLAGS_NONE,
+ 							    NULL, &error_local);
+ 	if (result == NULL) {
+-		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "failed to check authorisation: %s", error_local->message);
+-		dbus_g_method_return_error (context, error);
++		g_set_error (error, UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "failed to check authorisation: %s", error_local->message);
+ 		g_error_free (error_local);
+-		g_error_free (error);
+ 		goto out;
+ 	}
+ 
+diff --git a/src/up-polkit.h b/src/up-polkit.h
+index acee70e..b9abd7e 100644
+--- a/src/up-polkit.h
++++ b/src/up-polkit.h
+@@ -56,11 +56,11 @@ PolkitSubject	*up_polkit_get_subject		(UpPolkit		*polkit,
+ gboolean	 up_polkit_check_auth		(UpPolkit		*polkit,
+ 						 PolkitSubject		*subject,
+ 						 const gchar		*action_id,
+-						 DBusGMethodInvocation	*context);
++						 DBusGMethodInvocation *context);
+ gboolean	 up_polkit_is_allowed		(UpPolkit		*polkit,
+ 						 PolkitSubject		*subject,
+ 						 const gchar		*action_id,
+-						 DBusGMethodInvocation	*context);
++						 GError               **error);
+ gboolean         up_polkit_get_uid		(UpPolkit              *polkit,
+                                                  PolkitSubject          *subject,
+                                                  uid_t                  *uid);
+diff --git a/src/up-qos.c b/src/up-qos.c
+index 0ce3eea..b36df3f 100644
+--- a/src/up-qos.c
++++ b/src/up-qos.c
+@@ -262,6 +262,7 @@ up_qos_request_latency (UpQos *qos, const gchar *type_text, gint value, gboolean
+ 	if (type == UP_QOS_KIND_UNKNOWN) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "type invalid: %s", type_text);
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -270,6 +271,7 @@ up_qos_request_latency (UpQos *qos, const gchar *type_text, gint value, gboolean
+ 	if (sender == NULL) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "no DBUS sender");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -291,6 +293,7 @@ up_qos_request_latency (UpQos *qos, const gchar *type_text, gint value, gboolean
+ 	if (!retval) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "cannot get UID");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -299,6 +302,7 @@ up_qos_request_latency (UpQos *qos, const gchar *type_text, gint value, gboolean
+ 	if (!retval) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "cannot get PID");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -307,6 +311,7 @@ up_qos_request_latency (UpQos *qos, const gchar *type_text, gint value, gboolean
+ 	if (cmdline == NULL) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "cannot get cmdline");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -359,6 +364,7 @@ up_qos_cancel_request (UpQos *qos, guint cookie, DBusGMethodInvocation *context)
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL,
+ 				     "Cannot find request for #%i", cookie);
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -367,6 +373,7 @@ up_qos_cancel_request (UpQos *qos, guint cookie, DBusGMethodInvocation *context)
+ 	if (sender == NULL) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "no DBUS sender");
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		goto out;
+ 	}
+ 
+@@ -388,6 +395,8 @@ up_qos_cancel_request (UpQos *qos, guint cookie, DBusGMethodInvocation *context)
+ 	/* TODO: if persistent remove from datadase */
+ 
+ 	g_signal_emit (qos, signals [REQUESTS_CHANGED], 0);
++
++	dbus_g_method_return (context, NULL);
+ out:
+ 	if (subject != NULL)
+ 		g_object_unref (subject);
+@@ -430,6 +439,7 @@ up_qos_set_minimum_latency (UpQos *qos, const gchar *type_text, gint value, DBus
+ 	if (type == UP_QOS_KIND_UNKNOWN) {
+ 		error = g_error_new (UP_DAEMON_ERROR, UP_DAEMON_ERROR_GENERAL, "type invalid: %s", type_text);
+ 		dbus_g_method_return_error (context, error);
++                g_error_free (error);
+ 		return;
+ 	}
+ 
diff --git a/upower.spec b/upower.spec
index e44229c..f421836 100644
--- a/upower.spec
+++ b/upower.spec
@@ -1,7 +1,7 @@
 Summary:        Power Management Service
 Name:           upower
 Version:        0.9.5
-Release:        5%{?dist}
+Release:        6%{?dist}
 License:        GPLv2+
 Group:          System Environment/Libraries
 URL:            http://hal.freedesktop.org/releases/
@@ -28,6 +28,10 @@ Requires:       gobject-introspection
 # Upstream: don't crash with new polkits.
 Patch0:    upower-0.9.6-ensure-gerror-is-init.patch
 
+# Don't return more or less than once from a dbus call
+# and don't leak errors all over the place
+Patch1:    upower-dbus-fixes.patch
+
 # Old project name
 Obsoletes: DeviceKit-power < 1:0.9.0-2
 
@@ -53,6 +57,7 @@ Headers and libraries for UPower.
 %prep
 %setup -q
 %patch0 -p1 -b .new-polkit
+%patch1 -p1 -b .dbus-fixes
 
 %build
 %configure \
@@ -112,6 +117,9 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/*.la
 %{_includedir}/libupower-glib/upower.h
 
 %changelog
+* Tue Aug 17 2010 Matthias Clasen <mclasen at redhat.com> - 0.9.5-6
+- Some fixes for dbus error handling
+
 * Tue Aug 10 2010 Richard Hughes <rhughes at redhat.com> - 0.9.5-5
 - Ensure we've initialized errors when calling into PolicyKit.
 - Resolves: #622830


More information about the scm-commits mailing list