From 14952db5330c8444779b363e175c3fb437cb214a Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 17 Aug 2010 21:56:59 -0400 Subject: [PATCH] Some dbus error handling fixes --- upower-dbus-fixes.patch | 291 ++++++++++++++++++++++++++++++++++++++++ upower.spec | 10 +- 2 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 upower-dbus-fixes.patch 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 - 0.9.5-6 +- Some fixes for dbus error handling + * Tue Aug 10 2010 Richard Hughes - 0.9.5-5 - Ensure we've initialized errors when calling into PolicyKit. - Resolves: #622830