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 702190 - [review] dcbw/up-down-cleanup
[review] dcbw/up-down-cleanup
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-06-13 15:33 UTC by Dan Williams
Modified: 2013-06-14 18:48 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Dan Williams 2013-06-13 15:33:50 UTC
Turns out the up/down/is-up functions weren't really used anymore, and we can clean up them nicely.  Only a couple device classes used them, and what the functions did was already done in other places (like in device state handlers), could be moved elsewhere (ethernet supplicant manager init, wifi periodic state handler) or was dead code (olpc-mesh).

If you see code getting removed (like the remove_all_nsps() or remove_all_aps()) and not moved somewhere else, chances are those functions are already called from the device state handler which should cover the cases where take_down() was used.

The second commit for WiMAX is a drive-by that I noticed.
Comment 1 Dan Winship 2013-06-13 16:26:01 UTC
You can use g_clear_object() to clear the supplicant_mgr in
NMDeviceEthernet:dispose().

In nm-device.c, would it make sense to just move the

	if (nm_device_get_act_request (self))
		nm_device_deactivate (self, reason);

into nm_device_hw_take_down()?

Also, maybe remove the _hw_ from the names now? (In a second patch)
Comment 2 Pavel Simerda 2013-06-14 07:48:24 UTC
Looks good.

Also if bond's is_available just calls is_up, that could be handled by NMDevice superclass as it looks more like a rule than an exception.
Comment 3 Dan Williams 2013-06-14 18:45:48 UTC
Merged.  And just because removing code is awesome:

10 files changed, 85 insertions(+), 257 deletions(-)
Comment 4 Dan Williams 2013-06-14 18:48:55 UTC
We should leave the is_up/is_available thing for another patch to be sure how it works with devices not backed by a netdev; may need to override the parent function there.