GNOME Bugzilla – Bug 667685
[PATCH] Use GDBus
Last modified: 2012-01-12 15:47:56 UTC
Created attachment 205014 [details] [review] Use GDBus instead of dbus-glib Use GDBus instead of dbus-glib
Review of attachment 205014 [details] [review]: Looks good to me, fwiw
Review of attachment 205014 [details] [review]: Apart from the error handling, looks fine to me. ::: panels/display/cc-display-panel.c @@ +2260,1 @@ g_assert (self->priv->proxy != NULL); /* that call does not fail unless we pass bogus names */ That's not true anymore, and we should have a good error message here. Passing G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START might alleviate that problem.
Created attachment 205103 [details] [review] Use GDBus instead of dbus-glib Don't crash out if the dbus proxy fails to resolve. Note that I intentionally didn't improve the error handling in the code as the rest of the code seems to be like that and it's probably better resolved in another patch.
(In reply to comment #3) > Created an attachment (id=205103) [details] [review] > Use GDBus instead of dbus-glib > > Don't crash out if the dbus proxy fails to resolve. Note that I intentionally > didn't improve the error handling in the code as the rest of the code seems to > be like that and it's probably better resolved in another patch. That's still not equivalent to what was there before. We would have got an error message from the "ApplyConfiguration" call if the proxy didn't correspond to any running instance, and got an error message to that effect. Something like those removed lines would have done (obviously change the mention of the session bus to something else): - if (self->priv->connection == NULL) { - error_message (self, _("Could not get session bus while applying display configuration"), error->message); - g_error_free (error); - return; - }
Created attachment 205105 [details] [review] Use GDBus instead of dbus-glib Ah, I see what you mean. This one should solve that then.
Review of attachment 205105 [details] [review]: Small change, I'll commit that now. ::: panels/display/cc-display-panel.c @@ +2259,3 @@ + NULL, + &error); + "org.gnome.SettingsDaemon", Should be: if (self->priv->proxy == NULL) { error_message (self, _("Failed to apply configuration: %s"), error->message); g_error_free (error); return; }
With the aforementioned fixes.