GNOME Bugzilla – Bug 697821
[info] Critical warnings when PackageKit isn't installed
Last modified: 2013-04-15 15:19:27 UTC
I'm playing with a XO 1.75 which doesn't come with PackageKit installed. When going to the Info panel, gnome-control-center spews a number of warnings (and accesses uninitalized memory in the end). This is with 3.6.3 on F18, but the code looks the same in master. (gnome-control-center:1772): GLib-CRITICAL **: g_variant_get_va: assertion `value != NULL' failed (gnome-control-center:1772): GLib-CRITICAL **: g_variant_unref: assertion `value != NULL' failed (gnome-control-center:1772): GLib-CRITICAL **: g_variant_get_va: assertion `value != NULL' failed (gnome-control-center:1772): GLib-CRITICAL **: g_variant_unref: assertion `value != NULL' failed (gnome-control-center:1772): GLib-CRITICAL **: g_variant_get_va: assertion `value != NULL' failed (gnome-control-center:1772): GLib-CRITICAL **: g_variant_unref: assertion `value != NULL' failed (gnome-control-center:1772): info-cc-panel-WARNING **: PackageKit version 5181896.1086529084.8 not supported
Created attachment 241277 [details] [review] info: Improve PackageKit version parsing when PK isn't installed
(In reply to comment #0) > I'm playing with a XO 1.75 which doesn't come with PackageKit installed. When > going to the Info panel, gnome-control-center spews a number of warnings (and > accesses uninitalized memory in the end). This is with 3.6.3 on F18, but the > code looks the same in master. So PackageKit isn't installed but self->priv->pk_proxy = g_dbus_proxy_new_for_bus_finish (res, &error); returns a valid proxy? That doesn't sound correct in the first place.
Yep, that seems to be the case. I am not a dbus expert and I'm not in a position to say whether this is a bug in gdbus or not. At least the current behaviour for g_dbus_proxy_new() and friends appears to be to return a valid proxy object, even if the name doesn't exist on the bus.
(In reply to comment #2) > (In reply to comment #0) > > I'm playing with a XO 1.75 which doesn't come with PackageKit installed. When > > going to the Info panel, gnome-control-center spews a number of warnings (and > > accesses uninitalized memory in the end). This is with 3.6.3 on F18, but the > > code looks the same in master. > > So PackageKit isn't installed but > > self->priv->pk_proxy = g_dbus_proxy_new_for_bus_finish (res, &error); > > returns a valid proxy? That doesn't sound correct in the first place. The proxy will be valid, but calls to it will fail. "However, if no name owner currently exists, then calls will be sent to the well-known name which may result in the message bus launching an owner (unless G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START is set)." The patch looks good to me.
Review of attachment 241277 [details] [review]: ::: panels/info/cc-info-panel.c @@ -1770,3 @@ } - v = g_dbus_proxy_get_cached_property (self->priv->pk_proxy, "VersionMajor"); I don't think that splitting out the pk version check is necessary, please make minimal changes to the existing code. Simply checking for any NULL retvals would be enough.
Review of attachment 241277 [details] [review]: Interesting behavior. Let's push it then.
Comment on attachment 241277 [details] [review] info: Improve PackageKit version parsing when PK isn't installed Still needs work...
ups, sorry, your previous comment had been 1 hour ago...
Yeah, it's definitely not necessary to split out the pk version check. If you want to backport this to stable branches then I very much agree that minimal changes it the way to go. I'll post a new patch.
Created attachment 241429 [details] [review] info: Improve error handling for PackageKit version check Fixes critical errors when PackageKit isn't installed.
Review of attachment 241429 [details] [review]: ::: panels/info/cc-info-panel.c @@ +1772,2 @@ v = g_dbus_proxy_get_cached_property (self->priv->pk_proxy, "VersionMajor"); + if (v != NULL) You need to bail instantly if it's NULL.
Created attachment 241564 [details] [review] info: Improve error handling for PackageKit version check Fixes critical errors when PackageKit isn't installed.
Review of attachment 241564 [details] [review]: Nitpicking. Feel free to commit after making your changes. ::: panels/info/cc-info-panel.c @@ +1746,3 @@ + GVariant *v; + + v = g_dbus_proxy_get_cached_property (pk_proxy, property); if (!v) return FALSE; etc. @@ +1781,3 @@ + { + g_warning ("Unable to get PackageKit version"); + g_clear_object (&self->priv->pk_proxy); Line feed here.
Attachment 241564 [details] pushed as 1c8609e - info: Improve error handling for PackageKit version check