After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741653 - gnetworkmonitornm: Check if network-manager is running
gnetworkmonitornm: Check if network-manager is running
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-12-17 14:02 UTC by Iain Lane
Modified: 2015-01-13 11:14 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
gnetworkmonitornm: Check if network-manager is running (1.98 KB, patch)
2014-12-17 14:02 UTC, Iain Lane 		needs-work Details | Review
gnetworkmonitornm: Check if network-manager is running (1.69 KB, patch)
2015-01-11 20:49 UTC, Iain Lane 		needs-work Details | Review
gnetworkmonitornm: Check if network-manager is running (1.45 KB, patch)
2015-01-12 10:04 UTC, Iain Lane 		accepted-commit_now Details | Review

Description Iain Lane 2014-12-17 14:02:06 UTC 
There's a crash here. g_dbus_proxy_new_for_bus_sync returns a non-NULL
GDBusProxy if NM isn't running, or even if it's not installed, which we then
try to operate on.

(gdb) bt
    at /build/buildd/glib2.0-2.43.2/./gio/gnetworkmonitornm.c:260
    cancellable=0x0, error=0x0) at /build/buildd/glib2.0-2.43.2/./gio/ginitable.c:228
    first_property_name=first_property_name@entry=0x0) at /build/buildd/glib2.0-2.43.2/./gio/ginitable.c:146
    at /build/buildd/glib2.0-2.43.2/./gio/giomodule.c:755
    envvar=0x7ffff7b69578 "GIO_USE_NETWORK_MONITOR", verify_func=0x0) at /build/buildd/glib2.0-2.43.2/./gio/giomodule.c:857
    at /build/buildd/glib2.0-2.43.2/./glib/gtestutils.c:2120
    at /build/buildd/glib2.0-2.43.2/./glib/gtestutils.c:2131
Comment 1 Iain Lane 2014-12-17 14:02:08 UTC 
Created attachment 292900 [details] [review]
gnetworkmonitornm: Check if network-manager is running

We were asking for properties on NM's dbus interface, but if NM is not
running then there won't be any, so we were crashing. Check if the name
has an owner before doing anything to it. Also check for NULL when
retrieving cached property names, in case there aren't any.
Comment 2 Bastien Nocera 2015-01-10 14:24:42 UTC 
Review of attachment 292900 [details] [review]:

Please retest with a version that includes 327d35ed414b845e0199a11e8bcbb5296ad70c95.

::: gio/gnetworkmonitornm.c
@@ +221,3 @@
   props = g_dbus_proxy_get_cached_property_names (proxy);
+
+  if (!props)

Already done in commit 327d35ed414b845e0199a11e8bcbb5296ad70c95.
Comment 3 Iain Lane 2015-01-10 16:41:58 UTC 
That will fix the crash. The other part avoids making the call at all if it's not going to give us anything.
Comment 4 Bastien Nocera 2015-01-11 13:45:12 UTC 
Review of attachment 292900 [details] [review]:

Still needs work...

::: gio/gnetworkmonitornm.c
@@ +263,3 @@
+    name_owner = g_dbus_proxy_get_name_owner (proxy);
+
+  if (!proxy || !name_owner)

Leaking the proxy here if there's no name owner.
Comment 5 Iain Lane 2015-01-11 20:49:42 UTC 
Created attachment 294308 [details] [review]
gnetworkmonitornm: Check if network-manager is running

We were asking for properties on NM's dbus interface, but if NM is not
running then there won't be any, so we were crashing. Check if the name
has an owner before doing anything to it.
Comment 6 Bastien Nocera 2015-01-12 08:43:52 UTC 
Review of attachment 294308 [details] [review]:

::: gio/gnetworkmonitornm.c
@@ +260,3 @@
                                          error);
+
+  if (proxy)

if (!proxy)
  return FALSE;

name_owner = ...
Comment 7 Iain Lane 2015-01-12 10:04:11 UTC 
Since there are no words, I have to guess that I think you're asking for a style change..?

Like this?
Comment 8 Iain Lane 2015-01-12 10:04:46 UTC 
Created attachment 294331 [details] [review]
gnetworkmonitornm: Check if network-manager is running

We were asking for properties on NM's dbus interface, but if NM is not
running then there won't be any. Check if the name has an owner before
doing anything to it.
Comment 9 Bastien Nocera 2015-01-12 10:58:38 UTC 
Review of attachment 294331 [details] [review]:

Looks good.
Comment 10 Iain Lane 2015-01-12 11:04:28 UTC 
please push
Comment 11 Lars Karlitski 2015-01-13 11:14:20 UTC 
Attachment 294331 [details] pushed as f7be461 - gnetworkmonitornm: Check if network-manager is running