GNOME Bugzilla – Bug 740320
[PATCH] cli: Watch for device as well as AC state changes when activating
Last modified: 2014-12-01 11:05:28 UTC
Created attachment 290915 [details] [review] [PATCH] cli: Watch for device as well as AC state changes when activating The signals might be delivered in no particular order and we need to wait for the device to reach stable state (whether it's successfully conntected or not) as well as the active connection to leave ACTIVATING state. --- clients/cli/devices.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/device-activation
I pushed a fixup to lr/device-activation. Shouldn't we take a ref to @device? Also, on timeout we don't properly unref. That is a bit ugly.
(In reply to comment #1) > I pushed a fixup to lr/device-activation. > > > Shouldn't we take a ref to @device? We should. Pushed and updated version, please take a look. > Also, on timeout we don't properly unref. That is a bit ugly. Nor do we unhook signals, which is even worse (the connection attempt can both succeed and time out at the same time in case they both finish the race and the same time and fire up in one main loop spin). I guess we do this in more places in the client and should be addressed separately.
(In reply to comment #2) > I guess we do this in more places in the client and should be addressed > separately. agreed. (maybe file a bug so we don't forget?). Otherwise the latest patch looks good to me
+ g_signal_handlers_disconnect_by_func (active, G_CALLBACK (active_state_cb), device); g_object_unref (active); + + g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_state_cb), active); + g_object_unref (device); + I'd move "g_object_unref (active);" at the end, otherwise you pass a dangling pointer to g_signal_handlers_disconnect_by_func() -- which is not really wrong, but ugly. Otherwise LGTM.
Works for me, though I would also say the same as thomas in his comment about g_object_unref (active).
Pushed, with the unref/signal disconnect order fixed.