From 640736ff65059bbe29ae7fa273015d849c78092a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Mar 2018 20:06:38 +0100 Subject: [PATCH 01/10] core/dbus: cache variants for D-Bus exported properties GVariant is immutable and can nicely be shared and cached. Cache the property variants. This makes getting the properties faster, at the expense of using some extra memory. Tested with https://tratt.net/laurie/src/multitime/ $ multitime -n 200 -s 0 bash -c 'echo -n .; exec busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null' # Mean Std.Dev. Min Median Max # real(before) 0.013+/-0.0000 0.001 0.012 0.013 0.019 # real(after) 0.013+/-0.0000 0.002 0.011 0.012 0.034 $ multitime -n 100 -s 0 bash -c 'for i in {1..5}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null & done; wait; echo -n .' # Mean Std.Dev. Min Median Max # real(before) 0.040+/-0.0000 0.002 0.037 0.040 0.049 # real(after) 0.037+/-0.0000 0.002 0.034 0.036 0.045 $ multitime -n 30 -s 0 bash -c 'for i in {1..100}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null & done; wait; echo -n .' # Mean Std.Dev. Min Median Max # real(before) 0.704+/-0.0000 0.016 0.687 0.701 0.766 # real(after) 0.639+/-0.0000 0.013 0.622 0.637 0.687 $ multitime -n 200 -s 0 bash -c 'echo -n .; exec nmcli &>/dev/null' # Mean Std.Dev. Min Median Max # real(before) 0.092+/-0.0000 0.005 0.081 0.092 0.119 # real(after) 0.092+/-0.0000 0.005 0.080 0.091 0.123 $ multitime -n 100 -s 0 bash -c 'for i in {1..5}; do nmcli &>/dev/null & done; wait; echo -n .' # Mean Std.Dev. Min Median Max # real(before) 0.436+/-0.0000 0.043 0.375 0.424 0.600 # real(after) 0.413+/-0.0000 0.022 0.380 0.410 0.558 $ multitime -n 20 -s 0 bash -c 'for i in {1..100}; do nmcli &>/dev/null & done; wait; echo -n .' # Mean Std.Dev. Min Median Max # real(before) 8.796+/-0.1070 0.291 8.073 8.818 9.247 # real(after) 8.736+/-0.0893 0.243 8.017 8.780 9.101 The time savings are small, but that is because caching mostly speeds up the GetManagedObjects calls, and that is only a small part of the entire nmcli call from client side. --- src/nm-dbus-manager.c | 79 +++++++++++++++++++++++++++++++------------ src/nm-dbus-utils.c | 7 ++-- src/nm-dbus-utils.h | 3 +- 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/src/nm-dbus-manager.c b/src/nm-dbus-manager.c index e0977b1fc9..88fd9a40fd 100644 --- a/src/nm-dbus-manager.c +++ b/src/nm-dbus-manager.c @@ -45,12 +45,17 @@ /*****************************************************************************/ +typedef struct { + GVariant *value; +} PropertyCacheData; + typedef struct { CList registration_lst; NMDBusObject *obj; NMDBusObjectClass *klass; guint info_idx; guint registration_id; + PropertyCacheData property_cache[]; } RegistrationData; /* we require that @path is the first member of NMDBusManagerData @@ -807,7 +812,8 @@ dbus_vtable_method_call (GDBusConnection *connection, nm_assert (nm_streq (property_interface, interface_info->parent.name)); property_info = (const NMDBusPropertyInfoExtended *) nm_dbus_utils_interface_info_lookup_property (&interface_info->parent, - property_name); + property_name, + NULL); if ( !property_info || !NM_FLAGS_HAS (property_info->parent.flags, G_DBUS_PROPERTY_INFO_FLAGS_WRITABLE)) g_return_if_reached (); @@ -854,6 +860,33 @@ dbus_vtable_method_call (GDBusConnection *connection, parameters); } +static GVariant * +_obj_get_property (RegistrationData *reg_data, + guint property_idx, + gboolean refetch) +{ + const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); + const NMDBusPropertyInfoExtended *property_info; + GVariant *value; + + property_info = (const NMDBusPropertyInfoExtended *) (interface_info->parent.properties[property_idx]); + + if (refetch) + nm_clear_g_variant (®_data->property_cache[property_idx].value); + else { + value = reg_data->property_cache[property_idx].value; + if (value) + goto out; + } + + value = nm_dbus_utils_get_property (G_OBJECT (reg_data->obj), + property_info->parent.signature, + property_info->property_name); + reg_data->property_cache[property_idx].value = value; +out: + return g_variant_ref (value); +} + static GVariant * dbus_vtable_get_property (GDBusConnection *connection, const char *sender, @@ -865,16 +898,14 @@ dbus_vtable_get_property (GDBusConnection *connection, { RegistrationData *reg_data = user_data; const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); - const NMDBusPropertyInfoExtended *property_info; + guint property_idx; - property_info = (const NMDBusPropertyInfoExtended *) nm_dbus_utils_interface_info_lookup_property (&interface_info->parent, - property_name); - if (!property_info) + if (!nm_dbus_utils_interface_info_lookup_property (&interface_info->parent, + property_name, + &property_idx)) g_return_val_if_reached (NULL); - return nm_dbus_utils_get_property (G_OBJECT (reg_data->obj), - property_info->parent.signature, - property_info->property_name); + return _obj_get_property (reg_data, property_idx, FALSE); } static const GDBusInterfaceVTable dbus_vtable = { @@ -930,8 +961,9 @@ _obj_register (NMDBusManager *self, RegistrationData *reg_data; gs_free_error GError *error = NULL; guint registration_id; + guint prop_len = NM_PTRARRAY_LEN (interface_info->parent.properties); - reg_data = g_slice_new (RegistrationData); + reg_data = g_malloc0 (sizeof (RegistrationData) + (sizeof (PropertyCacheData) * prop_len)); registration_id = g_dbus_connection_register_object (priv->connection, obj->internal.path, @@ -997,14 +1029,23 @@ _obj_unregister (NMDBusManager *self, g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); while ((reg_data = c_list_last_entry (&obj->internal.registration_lst_head, RegistrationData, registration_lst))) { + const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); + guint i; + g_variant_builder_add (&builder, "s", - _reg_data_get_interface_info (reg_data)->parent.name); + interface_info->parent.name); c_list_unlink_stale (®_data->registration_lst); if (!g_dbus_connection_unregister_object (priv->connection, reg_data->registration_id)) nm_assert_not_reached (); + + if (interface_info->parent.properties) { + for (i = 0; interface_info->parent.properties[i]; i++) + nm_clear_g_variant (®_data->property_cache[i].value); + } + g_type_class_unref (reg_data->klass); - g_slice_free (RegistrationData, reg_data); + g_free (reg_data); } g_dbus_connection_emit_signal (priv->connection, @@ -1142,9 +1183,7 @@ _nm_dbus_manager_obj_notify (NMDBusObject *obj, if (!nm_streq (property_info->property_name, pspec->name)) continue; - value = nm_dbus_utils_get_property (G_OBJECT (obj), - property_info->parent.signature, - property_info->property_name); + value = _obj_get_property (reg_data, i, TRUE); if ( property_info->include_in_legacy_property_changed && any_legacy_signals) { @@ -1277,9 +1316,10 @@ _nm_dbus_manager_obj_emit_signal (NMDBusObject *obj, static GVariantBuilder * _obj_collect_properties_per_interface (NMDBusObject *obj, - const NMDBusInterfaceInfoExtended *interface_info, + RegistrationData *reg_data, GVariantBuilder *builder) { + const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); guint i; g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sv}")); @@ -1288,9 +1328,7 @@ _obj_collect_properties_per_interface (NMDBusObject *obj, const NMDBusPropertyInfoExtended *property_info = (const NMDBusPropertyInfoExtended *) interface_info->parent.properties[i]; gs_unref_variant GVariant *variant = NULL; - variant = nm_dbus_utils_get_property (G_OBJECT (obj), - property_info->parent.signature, - property_info->property_name); + variant = _obj_get_property (reg_data, i, FALSE); g_variant_builder_add (builder, "{sv}", property_info->parent.name, @@ -1309,14 +1347,13 @@ _obj_collect_properties_all (NMDBusObject *obj, g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sa{sv}}")); c_list_for_each_entry (reg_data, &obj->internal.registration_lst_head, registration_lst) { - const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); GVariantBuilder properties_builder; g_variant_builder_add (builder, "{sa{sv}}", - interface_info->parent.name, + _reg_data_get_interface_info (reg_data)->parent.name, _obj_collect_properties_per_interface (obj, - interface_info, + reg_data, &properties_builder)); } diff --git a/src/nm-dbus-utils.c b/src/nm-dbus-utils.c index 514dc798c6..eab6cb6125 100644 --- a/src/nm-dbus-utils.c +++ b/src/nm-dbus-utils.c @@ -35,7 +35,8 @@ const GDBusSignalInfo nm_signal_info_property_changed_legacy = NM_DEFINE_GDBUS_S GDBusPropertyInfo * nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info, - const char *property_name) + const char *property_name, + guint *property_idx) { guint i; @@ -48,8 +49,10 @@ nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interfac for (i = 0; interface_info->properties[i]; i++) { GDBusPropertyInfo *info = interface_info->properties[i]; - if (nm_streq (info->name, property_name)) + if (nm_streq (info->name, property_name)) { + NM_SET_OUT (property_idx, i); return info; + } } } diff --git a/src/nm-dbus-utils.h b/src/nm-dbus-utils.h index 0a28bebcae..4b0ec5c477 100644 --- a/src/nm-dbus-utils.h +++ b/src/nm-dbus-utils.h @@ -152,7 +152,8 @@ extern const GDBusSignalInfo nm_signal_info_property_changed_legacy; /*****************************************************************************/ GDBusPropertyInfo *nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info, - const char *property_name); + const char *property_name, + guint *property_idx); GDBusMethodInfo *nm_dbus_utils_interface_info_lookup_method (const GDBusInterfaceInfo *interface_info, const char *method_name); From 987c584bb5dabd673c4d75e93ab487d0a379bfc4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Mar 2018 20:45:58 +0100 Subject: [PATCH 02/10] core/dbus: manually inline helper function to emit InterfacesAdded signal --- src/nm-dbus-manager.c | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/nm-dbus-manager.c b/src/nm-dbus-manager.c index 88fd9a40fd..70e17d1edc 100644 --- a/src/nm-dbus-manager.c +++ b/src/nm-dbus-manager.c @@ -110,9 +110,8 @@ NM_DEFINE_SINGLETON_GETTER (NMDBusManager, nm_dbus_manager_get, NM_TYPE_DBUS_MAN static const GDBusInterfaceInfo interface_info_objmgr; static const GDBusSignalInfo signal_info_objmgr_interfaces_added; static const GDBusSignalInfo signal_info_objmgr_interfaces_removed; - -static void _objmgr_emit_interfaces_added (NMDBusManager *self, - NMDBusObject *obj); +static GVariantBuilder *_obj_collect_properties_all (NMDBusObject *obj, + GVariantBuilder *builder); /*****************************************************************************/ @@ -927,6 +926,7 @@ _obj_register (NMDBusManager *self, GType gtype; NMDBusObjectClass *klasses[10]; const NMDBusInterfaceInfoExtended *const*prev_interface_infos = NULL; + GVariantBuilder builder; nm_assert (c_list_is_empty (&obj->internal.registration_lst_head)); nm_assert (priv->connection); @@ -1004,7 +1004,15 @@ _obj_register (NMDBusManager *self, * * In general, it's ok to export an object with frozen signals. But you better make sure * that all properties are in a self-consistent state when exporting the object. */ - _objmgr_emit_interfaces_added (self, obj); + g_dbus_connection_emit_signal (priv->connection, + NULL, + OBJECT_MANAGER_SERVER_BASE_PATH, + interface_info_objmgr.name, + signal_info_objmgr_interfaces_added.name, + g_variant_new ("(oa{sa{sv}})", + obj->internal.path, + _obj_collect_properties_all (obj, &builder)), + NULL); } static void @@ -1360,28 +1368,6 @@ _obj_collect_properties_all (NMDBusObject *obj, return builder; } -static void -_objmgr_emit_interfaces_added (NMDBusManager *self, - NMDBusObject *obj) -{ - NMDBusManagerPrivate *priv = NM_DBUS_MANAGER_GET_PRIVATE (self); - GVariantBuilder builder; - - nm_assert (NM_IS_DBUS_OBJECT (obj)); - nm_assert (priv->connection); - nm_assert (priv->objmgr_registration_id); - - g_dbus_connection_emit_signal (priv->connection, - NULL, - OBJECT_MANAGER_SERVER_BASE_PATH, - interface_info_objmgr.name, - signal_info_objmgr_interfaces_added.name, - g_variant_new ("(oa{sa{sv}})", - obj->internal.path, - _obj_collect_properties_all (obj, &builder)), - NULL); -} - static void dbus_vtable_objmgr_method_call (GDBusConnection *connection, const char *sender, From 4a705e1a0ce0eac24e55369f49fdb60db0b22b10 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Mar 2018 21:51:07 +0100 Subject: [PATCH 03/10] core: track devices in manager via embedded CList Instead of using a GSList for tracking the devices, use a CList. I think a CList is in most cases the more suitable data structure then GSList: - you can find out in O(1) whether the object is linked. That is nice, for example to assert in NMDevice's destructor that the object was unlinked, and we will use that later in nm_manager_get_device_by_path(). - you can unlink the element in O(1) and you can unlink the element without having access to the link's head - Contrary to GSList, this does not require an extra slice allocation for the link node. It quite possibliy consumes slightly less memory because the CList structure is embedded in a struct that we already allocate. Even if slice allocation would be perfect to only consume 2*sizeof(gpointer) for the link note, it would at most be as-good as CList. Quite possibly, there is an overhead though. - CList possibly has better memory locality, because the link structure and the data are close to each other. Something which could be seen as disavantage, is that with CList one device can only be tracked in one NMManager instance at a time. But that is fine. There exists only one NMManager instance for now, and even if we would ever introduce multiple managers, we probably would not associate one NMDevice instance with multiple managers. The advantages are arguably not huge, but CList is IMHO clearly the more suited data structure. No need to stick to a suboptimal data structure for the job. Refactor it. --- src/devices/nm-device.c | 4 + src/devices/nm-device.h | 1 + src/devices/wifi/nm-device-olpc-mesh.c | 8 +- src/devices/wifi/nm-iwd-manager.c | 9 +- src/nm-checkpoint-manager.c | 8 +- src/nm-checkpoint.c | 25 +- src/nm-manager.c | 309 ++++++++++++------------- src/nm-manager.h | 2 +- src/nm-policy.c | 40 ++-- 9 files changed, 204 insertions(+), 202 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index fc7c76b94d..d6ac3f9df5 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -14377,6 +14377,8 @@ nm_device_init (NMDevice *self) self->_priv = priv; + c_list_init (&self->devices_lst); + c_list_init (&priv->slaves); priv->netns = g_object_ref (NM_NETNS_GET); @@ -14495,6 +14497,8 @@ dispose (GObject *object) _LOGD (LOGD_DEVICE, "disposing"); + nm_assert (c_list_is_empty (&self->devices_lst)); + nm_clear_g_cancellable (&priv->deactivating_cancellable); nm_device_assume_state_reset (self); diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 4ed852bc65..f783cffbf3 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -174,6 +174,7 @@ struct _NMDevicePrivate; struct _NMDevice { NMDBusObject parent; struct _NMDevicePrivate *_priv; + CList devices_lst; }; /* The flags have an relaxing meaning, that means, specifying more flags, can make diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index c3d070d98d..d655406437 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -387,7 +387,8 @@ static void find_companion (NMDeviceOlpcMesh *self) { NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); - const GSList *list; + const CList *all_devices; + NMDevice *candidate; if (priv->companion) return; @@ -395,8 +396,9 @@ find_companion (NMDeviceOlpcMesh *self) nm_device_add_pending_action (NM_DEVICE (self), NM_PENDING_ACTION_WAITING_FOR_COMPANION, TRUE); /* Try to find the companion if it's already known to the NMManager */ - for (list = nm_manager_get_devices (priv->manager); list ; list = g_slist_next (list)) { - if (check_companion (self, NM_DEVICE (list->data))) { + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (candidate, all_devices, devices_lst) { + if (check_companion (self, candidate)) { nm_device_queue_recheck_available (NM_DEVICE (self), NM_DEVICE_STATE_REASON_NONE, NM_DEVICE_STATE_REASON_NONE); diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 0007b0fff5..ea903bc724 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -460,17 +460,16 @@ name_owner_changed (GObject *object, GParamSpec *pspec, gpointer user_data) g_clear_object (&priv->object_manager); prepare_object_manager (self); } else { - const GSList *devices, *iter; + const CList *all_devices; + NMDevice *device; if (!priv->running) return; priv->running = false; - devices = nm_manager_get_devices (priv->nm_manager); - for (iter = devices; iter; iter = iter->next) { - NMDevice *device = NM_DEVICE (iter->data); - + all_devices = nm_manager_get_devices (priv->nm_manager); + c_list_for_each_entry (device, all_devices, devices_lst) { if (!NM_IS_DEVICE_IWD (device)) continue; diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c index 44c1f49f48..7345f9a41d 100644 --- a/src/nm-checkpoint-manager.c +++ b/src/nm-checkpoint-manager.c @@ -174,14 +174,12 @@ nm_checkpoint_manager_create (NMCheckpointManager *self, if (!device_paths || !device_paths[0]) { const char *device_path; - const GSList *iter; + const CList *all_devices; GPtrArray *paths; paths = g_ptr_array_new (); - for (iter = nm_manager_get_devices (manager); - iter; - iter = g_slist_next (iter)) { - device = NM_DEVICE (iter->data); + all_devices = nm_manager_get_devices (manager); + c_list_for_each_entry (device, all_devices, devices_lst) { if (!nm_device_is_real (device)) continue; device_path = nm_dbus_object_get_path (NM_DBUS_OBJECT (device)); diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index 402d217f87..d85d2d5499 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -348,21 +348,20 @@ next_dev: } if (NM_FLAGS_HAS (priv->flags, NM_CHECKPOINT_CREATE_FLAG_DISCONNECT_NEW_DEVICES)) { - const GSList *list; + const CList *all_devices; NMDeviceState state; - NMDevice *dev; - for (list = nm_manager_get_devices (priv->manager); list ; list = g_slist_next (list)) { - dev = list->data; - if (!g_hash_table_contains (priv->devices, dev)) { - state = nm_device_get_state (dev); - if ( state > NM_DEVICE_STATE_DISCONNECTED - && state < NM_DEVICE_STATE_DEACTIVATING) { - _LOGD ("rollback: disconnecting new device %s", nm_device_get_iface (dev)); - nm_device_state_changed (dev, - NM_DEVICE_STATE_DEACTIVATING, - NM_DEVICE_STATE_REASON_USER_REQUESTED); - } + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (device, all_devices, devices_lst) { + if (g_hash_table_contains (priv->devices, device)) + continue; + state = nm_device_get_state (device); + if ( state > NM_DEVICE_STATE_DISCONNECTED + && state < NM_DEVICE_STATE_DEACTIVATING) { + _LOGD ("rollback: disconnecting new device %s", nm_device_get_iface (device)); + nm_device_state_changed (device, + NM_DEVICE_STATE_DEACTIVATING, + NM_DEVICE_STATE_REASON_USER_REQUESTED); } } diff --git a/src/nm-manager.c b/src/nm-manager.c index 1b18f44942..e23fdb1549 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -128,7 +128,8 @@ typedef struct { NMActiveConnection *activating_connection; NMMetered metered; - GSList *devices; + CList devices_lst_head; + NMState state; NMConfig *config; NMConnectivityState connectivity_state; @@ -925,27 +926,27 @@ impl_manager_reload (NMDBusObject *obj, /*****************************************************************************/ NMDevice * -nm_manager_get_device_by_path (NMManager *manager, const char *path) +nm_manager_get_device_by_path (NMManager *self, const char *path) { - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; - g_return_val_if_fail (path != NULL, NULL); + g_return_val_if_fail (path, NULL); - for (iter = NM_MANAGER_GET_PRIVATE (manager)->devices; iter; iter = iter->next) { - if (!strcmp (nm_dbus_object_get_path (NM_DBUS_OBJECT (iter->data)), path)) - return NM_DEVICE (iter->data); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if (!strcmp (nm_dbus_object_get_path (NM_DBUS_OBJECT (device)), path)) + return device; } return NULL; } NMDevice * -nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex) +nm_manager_get_device_by_ifindex (NMManager *self, int ifindex) { - GSList *iter; - - for (iter = NM_MANAGER_GET_PRIVATE (manager)->devices; iter; iter = iter->next) { - NMDevice *device = NM_DEVICE (iter->data); + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { if (nm_device_get_ifindex (device) == ifindex) return device; } @@ -954,19 +955,22 @@ nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex) } static NMDevice * -find_device_by_permanent_hw_addr (NMManager *manager, const char *hwaddr) +find_device_by_permanent_hw_addr (NMManager *self, const char *hwaddr) { - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; const char *device_addr; g_return_val_if_fail (hwaddr != NULL, NULL); - if (nm_utils_hwaddr_valid (hwaddr, -1)) { - for (iter = NM_MANAGER_GET_PRIVATE (manager)->devices; iter; iter = iter->next) { - device_addr = nm_device_get_permanent_hw_address (NM_DEVICE (iter->data)); - if (device_addr && nm_utils_hwaddr_matches (hwaddr, -1, device_addr, -1)) - return NM_DEVICE (iter->data); - } + if (!nm_utils_hwaddr_valid (hwaddr, -1)) + return NULL; + + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + device_addr = nm_device_get_permanent_hw_address (device); + if ( device_addr + && nm_utils_hwaddr_matches (hwaddr, -1, device_addr, -1)) + return device; } return NULL; } @@ -974,16 +978,15 @@ find_device_by_permanent_hw_addr (NMManager *manager, const char *hwaddr) static NMDevice * find_device_by_ip_iface (NMManager *self, const gchar *iface) { - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; - g_return_val_if_fail (iface != NULL, NULL); + g_return_val_if_fail (iface, NULL); - for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = g_slist_next (iter)) { - NMDevice *candidate = iter->data; - - if ( nm_device_is_real (candidate) - && g_strcmp0 (nm_device_get_ip_iface (candidate), iface) == 0) - return candidate; + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if ( nm_device_is_real (device) + && nm_streq0 (nm_device_get_ip_iface (device), iface)) + return device; } return NULL; } @@ -1011,12 +1014,11 @@ find_device_by_iface (NMManager *self, { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMDevice *fallback = NULL; - GSList *iter; + NMDevice *candidate; g_return_val_if_fail (iface != NULL, NULL); - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *candidate = iter->data; + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { if (strcmp (nm_device_get_iface (candidate), iface)) continue; @@ -1222,7 +1224,7 @@ static void check_if_startup_complete (NMManager *self) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GSList *iter; + NMDevice *device; if (!priv->startup) return; @@ -1235,12 +1237,10 @@ check_if_startup_complete (NMManager *self) return; } - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *dev = iter->data; - - if (nm_device_has_pending_action (dev)) { + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if (nm_device_has_pending_action (device)) { _LOGD (LOGD_CORE, "check_if_startup_complete returns FALSE because of %s", - nm_device_get_iface (dev)); + nm_device_get_iface (device)); return; } } @@ -1252,8 +1252,8 @@ check_if_startup_complete (NMManager *self) /* we no longer care about these signals. Startup-complete only * happens once. */ g_signal_handlers_disconnect_by_func (priv->settings, G_CALLBACK (settings_startup_complete_changed), self); - for (iter = priv->devices; iter; iter = iter->next) { - g_signal_handlers_disconnect_by_func (iter->data, + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_has_pending_action_changed), self); } @@ -1285,18 +1285,18 @@ _parent_notify_changed (NMManager *self, NMDevice *device, gboolean device_removed) { - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *candidate; nm_assert (NM_IS_DEVICE (device)); - nm_assert (NM_IS_MANAGER (self)); - for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; ) { - if (nm_device_parent_notify_changed (iter->data, device, device_removed)) { +again: + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { + if (nm_device_parent_notify_changed (candidate, device, device_removed)) { /* in the unlikely event that this changes anything, we start iterating * again, to be sure that the device list is up-to-date. */ - iter = NM_MANAGER_GET_PRIVATE (self)->devices; - } else - iter = iter->next; + goto again; + } } } @@ -1336,7 +1336,8 @@ remove_device (NMManager *self, g_signal_handlers_disconnect_matched (device, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self); nm_settings_device_removed (priv->settings, device, quitting); - priv->devices = g_slist_remove (priv->devices, device); + + c_list_unlink (&device->devices_lst); _parent_notify_changed (self, device, TRUE); @@ -1393,7 +1394,7 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection, NM const char *parent_name = NULL; NMSettingsConnection *parent_connection; NMDevice *parent, *first_compatible = NULL; - GSList *iter; + NMDevice *candidate; g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); @@ -1426,9 +1427,7 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection, NM /* Check if the parent connection is currently activated or is comaptible * with some known device. */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *candidate = iter->data; - + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { /* Unmanaged devices are not compatible with any connection */ if (!nm_device_get_managed (candidate, FALSE)) continue; @@ -1544,18 +1543,15 @@ NMDevice * nm_manager_get_device (NMManager *self, const char *ifname, NMDeviceType device_type) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GSList *iter; - NMDevice *d; + NMDevice *device; g_return_val_if_fail (ifname, NULL); g_return_val_if_fail (device_type != NM_DEVICE_TYPE_UNKNOWN, NULL); - for (iter = priv->devices; iter; iter = iter->next) { - d = iter->data; - - if ( nm_device_get_device_type (d) == device_type - && nm_streq0 (nm_device_get_iface (d), ifname)) - return d; + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if ( nm_device_get_device_type (device) == device_type + && nm_streq0 (nm_device_get_iface (device), ifname)) + return device; } return NULL; @@ -1591,9 +1587,9 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) NMDeviceFactory *factory; gs_free NMSettingsConnection **connections = NULL; guint i; - GSList *iter; gs_free char *iface = NULL; NMDevice *device = NULL, *parent = NULL; + NMDevice *dev_candidate; GError *error = NULL; NMLogLevel log_level; @@ -1609,17 +1605,15 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) } /* See if there's a device that is already compatible with this connection */ - for (iter = priv->devices; iter; iter = g_slist_next (iter)) { - NMDevice *candidate = iter->data; - - if (nm_device_check_connection_compatible (candidate, connection)) { - if (nm_device_is_real (candidate)) { + c_list_for_each_entry (dev_candidate, &priv->devices_lst_head, devices_lst) { + if (nm_device_check_connection_compatible (dev_candidate, connection)) { + if (nm_device_is_real (dev_candidate)) { _LOG3D (LOGD_DEVICE, connection, "already created virtual interface name %s", iface); return NULL; } - device = candidate; + device = dev_candidate; break; } } @@ -1840,10 +1834,10 @@ system_unmanaged_devices_changed_cb (NMSettings *settings, { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - const GSList *iter; + NMDevice *device; - for (iter = priv->devices; iter; iter = g_slist_next (iter)) - nm_device_set_unmanaged_by_user_settings (NM_DEVICE (iter->data)); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) + nm_device_set_unmanaged_by_user_settings (device); } static void @@ -1889,7 +1883,7 @@ manager_update_radio_enabled (NMManager *self, gboolean enabled) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GSList *iter; + NMDevice *device; /* Do nothing for radio types not yet implemented */ if (!rstate->prop) @@ -1902,9 +1896,7 @@ manager_update_radio_enabled (NMManager *self, return; /* enable/disable wireless devices as required */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *device = NM_DEVICE (iter->data); - + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { if (nm_device_get_rfkill_type (device) == rstate->rtype) { _LOG2D (LOGD_RFKILL, device, "rfkill: setting radio %s", enabled ? "enabled" : "disabled"); nm_device_set_enabled (device, enabled); @@ -2389,18 +2381,17 @@ device_ip_iface_changed (NMDevice *device, GParamSpec *pspec, NMManager *self) { + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); const char *ip_iface = nm_device_get_ip_iface (device); NMDeviceType device_type = nm_device_get_device_type (device); - GSList *iter; + NMDevice *candidate; /* Remove NMDevice objects that are actually child devices of others, * when the other device finally knows its IP interface name. For example, * remove the PPP interface that's a child of a WWAN device, since it's * not really a standalone NMDevice. */ - for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) { - NMDevice *candidate = NM_DEVICE (iter->data); - + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { if ( candidate != device && g_strcmp0 (nm_device_get_iface (candidate), ip_iface) == 0 && nm_device_get_device_type (candidate) == device_type @@ -2458,10 +2449,10 @@ device_connectivity_changed (NMDevice *device, NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMConnectivityState best_state = NM_CONNECTIVITY_UNKNOWN; NMConnectivityState state; - const GSList *devices; + NMDevice *dev; - for (devices = priv->devices; devices; devices = devices->next) { - state = nm_device_get_connectivity_state (NM_DEVICE (devices->data)); + c_list_for_each_entry (dev, &priv->devices_lst_head, devices_lst) { + state = nm_device_get_connectivity_state (dev); if (state > best_state) best_state = state; } @@ -2523,6 +2514,7 @@ add_device (NMManager *self, NMDevice *device, GError **error) GSList *iter, *remove = NULL; int ifindex; const char *dbus_path; + NMDevice *candidate; /* No duplicates */ ifindex = nm_device_get_ifindex (device); @@ -2539,18 +2531,20 @@ add_device (NMManager *self, NMDevice *device, GError **error) * FIXME: use parent/child device relationships instead of removing * the child NMDevice entirely */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *candidate = iter->data; - - iface = nm_device_get_ip_iface (candidate); - if (nm_device_is_real (candidate) && nm_device_owns_iface (device, iface)) + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { + if ( nm_device_is_real (candidate) + && (iface = nm_device_get_ip_iface (candidate)) + && nm_device_owns_iface (device, iface)) remove = g_slist_prepend (remove, candidate); } for (iter = remove; iter; iter = iter->next) remove_device (self, NM_DEVICE (iter->data), FALSE, FALSE); g_slist_free (remove); - priv->devices = g_slist_append (priv->devices, g_object_ref (device)); + g_object_ref (device); + + nm_assert (c_list_is_empty (&device->devices_lst)); + c_list_link_tail (&priv->devices_lst_head, &device->devices_lst); g_signal_connect (device, NM_DEVICE_STATE_CHANGED, G_CALLBACK (manager_device_state_changed), @@ -2664,12 +2658,11 @@ factory_component_added_cb (NMDeviceFactory *factory, gpointer user_data) { NMManager *self = user_data; - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; - g_return_val_if_fail (self, FALSE); - - for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) { - if (nm_device_notify_component_added ((NMDevice *) iter->data, component)) + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + if (nm_device_notify_component_added (device, component)) return TRUE; } return FALSE; @@ -2699,9 +2692,10 @@ platform_link_added (NMManager *self, gboolean guess_assume, const NMConfigDeviceStateData *dev_state) { + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMDeviceFactory *factory; NMDevice *device = NULL; - GSList *iter; + NMDevice *candidate; g_return_if_fail (ifindex > 0); @@ -2709,8 +2703,7 @@ platform_link_added (NMManager *self, return; /* Let unrealized devices try to realize themselves with the link */ - for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) { - NMDevice *candidate = iter->data; + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { gboolean compatible = TRUE; gs_free_error GError *error = NULL; @@ -2937,12 +2930,12 @@ rfkill_manager_rfkill_changed_cb (NMRfkillManager *rfkill_mgr, nm_manager_rfkill_update (NM_MANAGER (user_data), rtype); } -const GSList * +const CList * nm_manager_get_devices (NMManager *manager) { g_return_val_if_fail (NM_IS_MANAGER (manager), NULL); - return NM_MANAGER_GET_PRIVATE (manager)->devices; + return &NM_MANAGER_GET_PRIVATE (manager)->devices_lst_head; } static NMDevice * @@ -2951,9 +2944,10 @@ nm_manager_get_best_device_for_connection (NMManager *self, gboolean for_user_request, GHashTable *unavailable_devices) { - const GSList *devices, *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMActiveConnection *ac; NMDevice *act_device; + NMDevice *device; NMDeviceCheckConAvailableFlags flags; ac = active_connection_find_first_by_connection (self, connection); @@ -2966,9 +2960,7 @@ nm_manager_get_best_device_for_connection (NMManager *self, flags = for_user_request ? NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST : NM_DEVICE_CHECK_CON_AVAILABLE_NONE; /* Pick the first device that's compatible with the connection. */ - devices = nm_manager_get_devices (self); - for (iter = devices; iter; iter = g_slist_next (iter)) { - NMDevice *device = NM_DEVICE (iter->data); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { if (unavailable_devices && g_hash_table_contains (unavailable_devices, device)) continue; @@ -2981,30 +2973,34 @@ nm_manager_get_best_device_for_connection (NMManager *self, return NULL; } -static void -_get_devices (NMManager *self, - GDBusMethodInvocation *context, - gboolean all_devices) +static const char ** +_get_devices_paths (NMManager *self, + gboolean all_devices) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - gs_free const char **paths = NULL; + const char **paths = NULL; guint i; - GSList *iter; + NMDevice *device; - paths = g_new (const char *, g_slist_length (priv->devices) + 1); + paths = g_new (const char *, c_list_length (&priv->devices_lst_head) + 1); - for (i = 0, iter = priv->devices; iter; iter = iter->next) { + i = 0; + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { const char *path; - path = nm_dbus_object_get_path (NM_DBUS_OBJECT (iter->data)); - if ( path - && (all_devices || nm_device_is_real (iter->data))) - paths[i++] = path; + path = nm_dbus_object_get_path (NM_DBUS_OBJECT (device)); + if (!path) + continue; + + if ( !all_devices + && !nm_device_is_real (device)) + continue; + + paths[i++] = path; } paths[i++] = NULL; - g_dbus_method_invocation_return_value (context, - g_variant_new ("(^ao)", (char **) paths)); + return paths; } static void @@ -3017,8 +3013,11 @@ impl_manager_get_devices (NMDBusObject *obj, GVariant *parameters) { NMManager *self = NM_MANAGER (obj); + gs_free const char **paths = NULL; - _get_devices (self, invocation, FALSE); + paths = _get_devices_paths (self, FALSE); + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(^ao)", (char **) paths)); } static void @@ -3031,8 +3030,11 @@ impl_manager_get_all_devices (NMDBusObject *obj, GVariant *parameters) { NMManager *self = NM_MANAGER (obj); + gs_free const char **paths = NULL; - _get_devices (self, invocation, TRUE); + paths = _get_devices_paths (self, TRUE); + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(^ao)", (char **) paths)); } static void @@ -3137,7 +3139,6 @@ find_master (NMManager *self, const char *master; NMDevice *master_device = NULL; NMSettingsConnection *master_connection = NULL; - GSList *iter; s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); @@ -3166,10 +3167,10 @@ find_master (NMManager *self, /* Try master as a connection UUID */ master_connection = nm_settings_get_connection_by_uuid (priv->settings, master); if (master_connection) { - /* Check if the master connection is activated on some device already */ - for (iter = priv->devices; iter; iter = g_slist_next (iter)) { - NMDevice *candidate = NM_DEVICE (iter->data); + NMDevice *candidate; + /* Check if the master connection is activated on some device already */ + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { if (candidate == device) continue; @@ -3239,7 +3240,6 @@ ensure_master_active_connection (NMManager *self, NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMActiveConnection *master_ac = NULL; NMDeviceState master_state; - GSList *iter; g_assert (connection); g_assert (master_connection || master_device); @@ -3315,10 +3315,10 @@ ensure_master_active_connection (NMManager *self, NM_MANAGER_ERROR_DEPENDENCY_FAILED, "Device unmanaged or not available for activation"); } else if (master_connection) { - /* Find a compatible device and activate it using this connection */ - for (iter = priv->devices; iter; iter = g_slist_next (iter)) { - NMDevice *candidate = NM_DEVICE (iter->data); + NMDevice *candidate; + /* Find a compatible device and activate it using this connection */ + c_list_for_each_entry (candidate, &priv->devices_lst_head, devices_lst) { if (candidate == device) { /* A device obviously can't be its own master */ continue; @@ -4891,7 +4891,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); gboolean suspending, waking_from_suspend; - GSList *iter; + NMDevice *device; suspending = sleeping_changed && priv->sleeping; waking_from_suspend = sleeping_changed && !priv->sleeping; @@ -4902,9 +4902,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) /* FIXME: are there still hardware devices that need to be disabled around * suspend/resume? */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *device = iter->data; - + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { /* FIXME: shouldn't we be unmanaging software devices if !suspending? */ if (nm_device_is_software (device)) continue; @@ -4932,9 +4930,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) if (waking_from_suspend) { sleep_devices_clear (self); - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *device = iter->data; - + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { if (nm_device_is_software (device)) continue; @@ -4961,8 +4957,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) nm_manager_rfkill_update (self, RFKILL_TYPE_UNKNOWN); /* Re-manage managed devices */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *device = NM_DEVICE (iter->data); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { guint i; if (nm_device_is_software (device)) { @@ -5456,7 +5451,6 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, GError *error = NULL; NMAuthCallResult result; ConnectivityCheckData *data; - const GSList *devices; priv->auth_chains = g_slist_remove (priv->auth_chains, chain); @@ -5473,13 +5467,15 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to recheck connectivity"); } else { + NMDevice *device; + /* it's allowed */ data = g_slice_new0 (ConnectivityCheckData); data->context = context; - for (devices = priv->devices; devices; devices = devices->next) { + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { data->remaining++; - nm_device_check_connectivity (NM_DEVICE (devices->data), + nm_device_check_connectivity (device, device_connectivity_done, data); } @@ -5525,15 +5521,14 @@ start_factory (NMDeviceFactory *factory, gpointer user_data) void nm_manager_write_device_state (NMManager *self) { - const GSList *devices; NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; gs_unref_hashtable GHashTable *seen_ifindexes = NULL; gint nm_owned; seen_ifindexes = g_hash_table_new (nm_direct_hash, NULL); - for (devices = priv->devices; devices; devices = devices->next) { - NMDevice *device = NM_DEVICE (devices->data); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { int ifindex; gboolean managed; NMConfigDeviceStateManagedType managed_type; @@ -5674,10 +5669,10 @@ void nm_manager_stop (NMManager *self) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMDevice *device; - /* Remove all devices */ - while (priv->devices) - remove_device (self, NM_DEVICE (priv->devices->data), TRUE, TRUE); + while ((device = c_list_first_entry (&priv->devices_lst_head, NMDevice, devices_lst))) + remove_device (self, device, TRUE, TRUE); _active_connection_cleanup (self); @@ -5689,21 +5684,20 @@ handle_firmware_changed (gpointer user_data) { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GSList *iter; + NMDevice *device; priv->fw_changed_id = 0; /* Try to re-enable devices with missing firmware */ - for (iter = priv->devices; iter; iter = iter->next) { - NMDevice *candidate = NM_DEVICE (iter->data); - NMDeviceState state = nm_device_get_state (candidate); + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + NMDeviceState state = nm_device_get_state (device); - if ( nm_device_get_firmware_missing (candidate) + if ( nm_device_get_firmware_missing (device) && (state == NM_DEVICE_STATE_UNAVAILABLE)) { - _LOG2I (LOGD_CORE, candidate, "firmware may now be available"); + _LOG2I (LOGD_CORE, device, "firmware may now be available"); /* Re-set unavailable state to try bringing the device up again */ - nm_device_state_changed (candidate, + nm_device_state_changed (device, NM_DEVICE_STATE_UNAVAILABLE, NM_DEVICE_STATE_REASON_NONE); } @@ -6416,6 +6410,7 @@ nm_manager_init (NMManager *self) GFile *file; c_list_init (&priv->link_cb_lst); + c_list_init (&priv->devices_lst_head); c_list_init (&priv->active_connections_lst_head); c_list_init (&priv->delete_volatile_connection_lst_head); @@ -6484,12 +6479,6 @@ nm_manager_init (NMManager *self) priv->sleep_devices = g_hash_table_new (nm_direct_hash, NULL); } -static gboolean -device_is_real (GObject *device, gpointer user_data) -{ - return nm_device_is_real (NM_DEVICE (device)); -} - static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) @@ -6588,7 +6577,9 @@ get_property (GObject *object, guint prop_id, g_value_set_boolean (value, priv->sleeping); break; case PROP_DEVICES: - nm_dbus_utils_g_value_set_object_path_array (value, priv->devices, device_is_real, NULL); + g_value_take_boxed (value, + nm_utils_strv_make_deep_copied (_get_devices_paths (self, + FALSE))); break; case PROP_METERED: g_value_set_uint (value, priv->metered); @@ -6599,7 +6590,9 @@ get_property (GObject *object, guint prop_id, nm_global_dns_config_to_dbus (dns_config, value); break; case PROP_ALL_DEVICES: - nm_dbus_utils_g_value_set_object_path_array (value, priv->devices, NULL, NULL); + g_value_take_boxed (value, + nm_utils_strv_make_deep_copied (_get_devices_paths (self, + TRUE))); break; case PROP_CHECKPOINTS: strv = NULL; @@ -6700,7 +6693,7 @@ dispose (GObject *object) g_clear_object (&priv->auth_mgr); } - g_assert (priv->devices == NULL); + nm_assert (c_list_is_empty (&priv->devices_lst_head)); nm_clear_g_source (&priv->ac_cleanup_id); diff --git a/src/nm-manager.h b/src/nm-manager.h index 57936dfb8b..638ceed6ea 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -105,7 +105,7 @@ void nm_manager_write_device_state (NMManager *manager); /* Device handling */ -const GSList * nm_manager_get_devices (NMManager *manager); +const CList * nm_manager_get_devices (NMManager *manager); NMDevice * nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex); diff --git a/src/nm-policy.c b/src/nm-policy.c index 7cbf4864b0..72351efe06 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -385,7 +385,8 @@ get_best_ip_device (NMPolicy *self, gboolean fully_activated) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *iter; + const CList *all_devices; + NMDevice *device; NMDevice *best_device; NMDevice *prev_device; guint32 best_metric = G_MAXUINT32; @@ -400,8 +401,8 @@ get_best_ip_device (NMPolicy *self, ? (fully_activated ? priv->default_device4 : priv->activating_device4) : (fully_activated ? priv->default_device6 : priv->activating_device6); - for (iter = nm_manager_get_devices (priv->manager); iter; iter = iter->next) { - NMDevice *device = NM_DEVICE (iter->data); + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (device, all_devices, devices_lst) { NMDeviceState state; const NMPObject *r; NMConnection *connection; @@ -461,15 +462,16 @@ static gboolean all_devices_not_active (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *iter = nm_manager_get_devices (priv->manager); + const CList *all_devices; + NMDevice *device; - while (iter != NULL) { + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (device, all_devices, devices_lst) { NMDeviceState state; - state = nm_device_get_state (NM_DEVICE (iter->data)); + state = nm_device_get_state (device); if ( state <= NM_DEVICE_STATE_DISCONNECTED || state >= NM_DEVICE_STATE_DEACTIVATING) { - iter = g_slist_next (iter); continue; } return FALSE; @@ -2184,12 +2186,14 @@ schedule_activate_all_cb (gpointer user_data) { NMPolicy *self = user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *iter; + const CList *all_devices; + NMDevice *device; priv->schedule_activate_all_id = 0; - for (iter = nm_manager_get_devices (priv->manager); iter; iter = g_slist_next (iter)) - schedule_activate_check (self, iter->data); + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (device, all_devices, devices_lst) + schedule_activate_check (self, device); return G_SOURCE_REMOVE; } @@ -2223,7 +2227,8 @@ firewall_state_changed (NMFirewallManager *manager, { NMPolicy *self = (NMPolicy *) user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *iter; + const CList *all_devices; + NMDevice *device; if (initialized_now) { /* the firewall manager was initializing, but all requests @@ -2236,8 +2241,9 @@ firewall_state_changed (NMFirewallManager *manager, return; /* add interface of each device to correct zone */ - for (iter = nm_manager_get_devices (priv->manager); iter; iter = g_slist_next (iter)) - nm_device_update_firewall_zone (iter->data); + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (device, all_devices, devices_lst) + nm_device_update_firewall_zone (device); } static void @@ -2282,14 +2288,14 @@ connection_updated (NMSettings *settings, { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF (priv); - const GSList *iter; + const CList *all_devices; NMDevice *device = NULL; + NMDevice *dev; if (by_user) { /* find device with given connection */ - for (iter = nm_manager_get_devices (priv->manager); iter; iter = g_slist_next (iter)) { - NMDevice *dev = NM_DEVICE (iter->data); - + all_devices = nm_manager_get_devices (priv->manager); + c_list_for_each_entry (dev, all_devices, devices_lst) { if (nm_device_get_settings_connection (dev) == connection) { device = dev; break; From 199f2df50f6a268682954d05abf8af073016a9d0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 24 Mar 2018 11:31:29 +0100 Subject: [PATCH 04/10] manager: convert hwaddr to binary once in find_device_by_permanent_hw_addr() For comparing MAC addresses, they anyway have to be normalized to binary. Convert it once outside the loop and pass the binary form to nm_utils_hwaddr_matches(). Otherwise, we need to re-convert it every time. --- src/nm-manager.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index e23fdb1549..6cfa59e4a9 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -960,16 +960,18 @@ find_device_by_permanent_hw_addr (NMManager *self, const char *hwaddr) NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMDevice *device; const char *device_addr; + guint8 hwaddr_bin[NM_UTILS_HWADDR_LEN_MAX]; + gsize hwaddr_len; g_return_val_if_fail (hwaddr != NULL, NULL); - if (!nm_utils_hwaddr_valid (hwaddr, -1)) + if (!_nm_utils_hwaddr_aton (hwaddr, hwaddr_bin, sizeof (hwaddr_bin), &hwaddr_len)) return NULL; c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { device_addr = nm_device_get_permanent_hw_address (device); if ( device_addr - && nm_utils_hwaddr_matches (hwaddr, -1, device_addr, -1)) + && nm_utils_hwaddr_matches (hwaddr_bin, hwaddr_len, device_addr, -1)) return device; } return NULL; From 9fafd26f68c551499d6da8f1621a2ff5354cae55 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Mar 2018 21:49:41 +0100 Subject: [PATCH 05/10] core: rework lookup for exported objects by path to use index We already track an index of exported objects in NMDBusManager. Actually, that index was unused previously. We either could drop it, or use it. Let's use it. --- src/nm-manager.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 6cfa59e4a9..b941be66f8 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -799,18 +799,20 @@ nm_manager_get_activatable_connections (NMManager *manager, guint *out_len, gboo } static NMActiveConnection * -active_connection_get_by_path (NMManager *manager, const char *path) +active_connection_get_by_path (NMManager *self, const char *path) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMActiveConnection *ac; - nm_assert (path); + ac = (NMActiveConnection *) nm_dbus_manager_lookup_object (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)), + path); + if ( !ac + || !NM_IS_ACTIVE_CONNECTION (ac) + || c_list_is_empty (&ac->active_connections_lst)) + return NULL; - c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { - if (nm_streq0 (path, nm_dbus_object_get_path (NM_DBUS_OBJECT (ac)))) - return ac; - } - return NULL; + nm_assert (c_list_contains (&priv->active_connections_lst_head, &ac->active_connections_lst)); + return ac; } /*****************************************************************************/ @@ -933,11 +935,15 @@ nm_manager_get_device_by_path (NMManager *self, const char *path) g_return_val_if_fail (path, NULL); - c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { - if (!strcmp (nm_dbus_object_get_path (NM_DBUS_OBJECT (device)), path)) - return device; - } - return NULL; + device = (NMDevice *) nm_dbus_manager_lookup_object (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)), + path); + if ( !device + || !NM_IS_DEVICE (device) + || c_list_is_empty (&device->devices_lst)) + return NULL; + + nm_assert (c_list_contains (&priv->devices_lst_head, &device->devices_lst)); + return device; } NMDevice * @@ -5376,7 +5382,7 @@ impl_manager_set_logging (NMDBusObject *obj, * that the caller is still alive so that clients are forced to wait and * we'll be able to switch to polkit without breaking behavior. */ - if (!nm_dbus_manager_ensure_uid (nm_dbus_manager_get (), + if (!nm_dbus_manager_ensure_uid (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)), invocation, G_MAXULONG, NM_MANAGER_ERROR, From d7b1a911d913a2b6f5b26bcab01965e3a533081d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 24 Mar 2018 23:51:39 +0100 Subject: [PATCH 06/10] wifi: rework tracking of wifi-aps to use CList - no longer track APs in a hash table with their exported path as key. The exported path is already tracked by NMDBusManager's lookup index, so we can reuse that for fast lookup by path. Otherwise, track the APs in a CList per device. - as we now track APs in a CList, their order is well defined. We no longer need to sort APs and obsoletes nm_wifi_aps_get_sorted() and simplifies nm_wifi_aps_find_first_compatible(). --- src/devices/wifi/nm-device-iwd.c | 184 +++++++++++------------------- src/devices/wifi/nm-device-iwd.h | 2 +- src/devices/wifi/nm-device-wifi.c | 114 +++++++----------- src/devices/wifi/nm-device-wifi.h | 2 +- src/devices/wifi/nm-wifi-ap.c | 151 +++++++++++------------- src/devices/wifi/nm-wifi-ap.h | 25 ++-- src/devices/wifi/nm-wifi-common.c | 31 +++-- src/devices/wifi/nm-wifi-common.h | 5 + 8 files changed, 222 insertions(+), 292 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 40980b06f0..eeff5bd32d 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -40,6 +40,7 @@ #include "nm-core-internal.h" #include "nm-config.h" #include "nm-iwd-manager.h" +#include "nm-dbus-manager.h" #include "devices/nm-device-logging.h" _LOG_DECLARE_SELF(NMDeviceIwd); @@ -66,8 +67,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { GDBusObject * dbus_obj; GDBusProxy * dbus_proxy; - GHashTable * aps; - GHashTable * new_aps; + CList aps_lst_head; NMWifiAP * current_ap; GCancellable * cancellable; NMDeviceWifiCapabilities capabilities; @@ -104,6 +104,8 @@ G_DEFINE_TYPE (NMDeviceIwd, nm_device_iwd, NM_TYPE_DEVICE) static void schedule_periodic_scan (NMDeviceIwd *self, NMDeviceState current_state); +/*****************************************************************************/ + static void _ap_dump (NMDeviceIwd *self, NMLogLevel log_level, @@ -119,20 +121,6 @@ _ap_dump (NMDeviceIwd *self, nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); } -static void -_emit_access_point_added_removed (NMDeviceIwd *self, - NMWifiAP *ap, - gboolean is_added /* or else is removed */) -{ - nm_dbus_object_emit_signal (NM_DBUS_OBJECT (self), - &nm_interface_info_device_wireless, - is_added - ? &nm_signal_info_wireless_access_point_added - : &nm_signal_info_wireless_access_point_removed, - "(o)", - nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); -} - /* Callers ensure we're not removing current_ap */ static void ap_add_remove (NMDeviceIwd *self, @@ -143,23 +131,25 @@ ap_add_remove (NMDeviceIwd *self, NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); if (is_adding) { - g_hash_table_insert (priv->aps, - (gpointer) nm_dbus_object_export (NM_DBUS_OBJECT (ap)), - g_object_ref (ap)); + g_object_ref (ap); + ap->wifi_device = NM_DEVICE (self); + c_list_link_tail (&priv->aps_lst_head, &ap->aps_lst); + nm_dbus_object_export (NM_DBUS_OBJECT (ap)); _ap_dump (self, LOGL_DEBUG, ap, "added", 0); - } else + nm_device_wifi_emit_signal_access_point (NM_DEVICE (self), ap, TRUE); + } else { + ap->wifi_device = NULL; + c_list_unlink (&ap->aps_lst); _ap_dump (self, LOGL_DEBUG, ap, "removed", 0); - - _emit_access_point_added_removed (self, ap, is_adding); - - if (!is_adding) { - g_hash_table_remove (priv->aps, nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - nm_dbus_object_unexport (NM_DBUS_OBJECT (ap)); - g_object_unref (ap); } _notify (self, PROP_ACCESS_POINTS); + if (!is_adding) { + nm_device_wifi_emit_signal_access_point (NM_DEVICE (self), ap, FALSE); + nm_dbus_object_clear_and_unexport (&ap); + } + nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); if (recheck_available_connections) nm_device_recheck_available_connections (NM_DEVICE (self)); @@ -194,59 +184,20 @@ set_current_ap (NMDeviceIwd *self, NMWifiAP *new_ap, gboolean recheck_available_ _notify (self, PROP_MODE); } -static gboolean -update_ap_func (gpointer key, gpointer value, gpointer user_data) -{ - NMWifiAP *ap = value; - NMDeviceIwd *self = user_data; - NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - NMWifiAP *new_ap = NULL; - - if (priv->new_aps) - new_ap = g_hash_table_lookup (priv->new_aps, - nm_wifi_ap_get_supplicant_path (ap)); - - if (new_ap) { - g_hash_table_steal (priv->new_aps, - nm_wifi_ap_get_supplicant_path (ap)); - - if (nm_wifi_ap_set_strength (ap, nm_wifi_ap_get_strength (new_ap))) - _ap_dump (self, LOGL_TRACE, ap, "updated", 0); - - g_object_unref (new_ap); - return FALSE; - } - - if (ap == priv->current_ap) - /* Normally IWD will prevent the current AP from being - * removed from the list and set a low signal strength, - * but just making sure. - */ - return FALSE; - - _ap_dump (self, LOGL_DEBUG, ap, "removed", 0); - - _emit_access_point_added_removed (self, ap, FALSE); - - nm_dbus_object_unexport (NM_DBUS_OBJECT (ap)); - g_object_unref (ap); - - return TRUE; -} - static void remove_all_aps (NMDeviceIwd *self) { NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); + NMWifiAP *ap, *ap_safe; - if (!g_hash_table_size (priv->aps)) + if (c_list_is_empty (&priv->aps_lst_head)) return; set_current_ap (self, NULL, FALSE); - g_hash_table_foreach_remove (priv->aps, update_ap_func, self); + c_list_for_each_entry_safe (ap, ap_safe, &priv->aps_lst_head, aps_lst) + ap_add_remove (self, FALSE, ap, FALSE); - _notify (self, PROP_ACCESS_POINTS); nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); nm_device_recheck_available_connections (NM_DEVICE (self)); } @@ -285,9 +236,10 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) GVariantIter *networks; const gchar *path, *name, *type; int16_t signal; - NMWifiAP *ap; + NMWifiAP *ap, *ap_safe; gboolean changed = FALSE; GHashTableIter ap_iter; + gs_unref_hashtable GHashTable *new_aps = NULL; variant = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), res, G_VARIANT_TYPE ("(a(osns))"), @@ -298,7 +250,7 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) return; } - priv->new_aps = g_hash_table_new (nm_str_hash, g_str_equal); + new_aps = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_object_unref); g_variant_get (variant, "(a(osns))", &networks); @@ -348,27 +300,47 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) nm_wifi_ap_set_strength (ap, nm_wifi_utils_level_to_quality (signal / 100)); nm_wifi_ap_set_freq (ap, 2417); nm_wifi_ap_set_max_bitrate (ap, 65000); - g_hash_table_insert (priv->new_aps, + g_hash_table_insert (new_aps, (gpointer) nm_wifi_ap_get_supplicant_path (ap), ap); } g_variant_iter_free (networks); - if (g_hash_table_foreach_remove (priv->aps, update_ap_func, self)) - changed = TRUE; + c_list_for_each_entry_safe (ap, ap_safe, &priv->aps_lst_head, aps_lst) { - g_hash_table_iter_init (&ap_iter, priv->new_aps); - while (g_hash_table_iter_next (&ap_iter, NULL, (gpointer) &ap)) { - ap_add_remove (self, TRUE, ap, FALSE); + ap = g_hash_table_lookup (new_aps, + nm_wifi_ap_get_supplicant_path (ap)); + if (ap) { + if (nm_wifi_ap_set_strength (ap, nm_wifi_ap_get_strength (ap))) { + _ap_dump (self, LOGL_TRACE, ap, "updated", 0); + changed = TRUE; + } + g_hash_table_remove (new_aps, + nm_wifi_ap_get_supplicant_path (ap)); + continue; + } + + if (ap == priv->current_ap) { + /* Normally IWD will prevent the current AP from being + * removed from the list and set a low signal strength, + * but just making sure. + */ + continue; + } + + ap_add_remove (self, FALSE, ap, FALSE); changed = TRUE; } - g_hash_table_destroy (priv->new_aps); - priv->new_aps = NULL; + g_hash_table_iter_init (&ap_iter, new_aps); + while (g_hash_table_iter_next (&ap_iter, NULL, (gpointer) &ap)) { + ap_add_remove (self, TRUE, ap, FALSE); + g_hash_table_iter_remove (&ap_iter); + changed = TRUE; + } if (changed) { - _notify (self, PROP_ACCESS_POINTS); nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); nm_device_recheck_available_connections (NM_DEVICE (self)); } @@ -585,13 +557,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) return TRUE; } -static NMWifiAP * -get_ap_by_path (NMDeviceIwd *self, const char *path) -{ - g_return_val_if_fail (path != NULL, NULL); - return g_hash_table_lookup (NM_DEVICE_IWD_GET_PRIVATE (self)->aps, path); -} - static gboolean check_connection_available (NMDevice *device, NMConnection *connection, @@ -629,7 +594,7 @@ check_connection_available (NMDevice *device, if (specific_object) { NMWifiAP *ap; - ap = get_ap_by_path (self, specific_object); + ap = nm_wifi_ap_lookup_for_device (NM_DEVICE (self), specific_object); return ap ? nm_wifi_ap_check_compatible (ap, connection) : FALSE; } @@ -637,7 +602,7 @@ check_connection_available (NMDevice *device, return TRUE; /* Check at least one AP is compatible with this connection */ - return !!nm_wifi_aps_find_first_compatible (priv->aps, connection, TRUE); + return !!nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); } static gboolean @@ -691,7 +656,7 @@ complete_connection (NMDevice *device, } /* Find a compatible AP in the scan list */ - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); if (!ap) { g_set_error_literal (error, NM_DEVICE_ERROR, @@ -700,7 +665,7 @@ complete_connection (NMDevice *device, return FALSE; } } else { - ap = get_ap_by_path (self, specific_object); + ap = nm_wifi_ap_lookup_for_device (NM_DEVICE (self), specific_object); if (!ap) { g_set_error (error, NM_DEVICE_ERROR, @@ -859,7 +824,7 @@ can_auto_connect (NMDevice *device, return FALSE; } - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); if (ap) { /* All good; connection is usable */ NM_SET_OUT (specific_object, g_strdup (nm_dbus_object_get_path (NM_DBUS_OBJECT (ap)))); @@ -869,10 +834,10 @@ can_auto_connect (NMDevice *device, return FALSE; } -GHashTable * +const CList * _nm_device_iwd_get_aps (NMDeviceIwd *self) { - return NM_DEVICE_IWD_GET_PRIVATE (self)->aps; + return &NM_DEVICE_IWD_GET_PRIVATE (self)->aps_lst_head; } static gboolean @@ -1247,9 +1212,9 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) g_return_val_if_fail (s_wireless, NM_ACT_STAGE_RETURN_FAILURE); ap_path = nm_active_connection_get_specific_object (NM_ACTIVE_CONNECTION (req)); - ap = ap_path ? get_ap_by_path (self, ap_path) : NULL; + ap = ap_path ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) : NULL; if (!ap) { - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); /* TODO: assuming hidden networks aren't supported do we need * to consider the case of APs that are not in the scan list @@ -1540,8 +1505,7 @@ get_property (GObject *object, guint prop_id, { NMDeviceIwd *self = NM_DEVICE_IWD (object); NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - gsize i; - char **list; + const char **list; switch (prop_id) { case PROP_MODE: @@ -1557,10 +1521,8 @@ get_property (GObject *object, guint prop_id, g_value_set_uint (value, priv->capabilities); break; case PROP_ACCESS_POINTS: - list = (char **) nm_wifi_aps_get_sorted_paths (priv->aps, TRUE); - for (i = 0; list[i]; i++) - list[i] = g_strdup (list[i]); - g_value_take_boxed (value, list); + list = nm_wifi_aps_get_paths (&priv->aps_lst_head, TRUE); + g_value_take_boxed (value, nm_utils_strv_make_deep_copied (list)); break; case PROP_ACTIVE_ACCESS_POINT: nm_dbus_utils_g_value_set_object_path (value, priv->current_ap); @@ -1830,7 +1792,7 @@ nm_device_iwd_init (NMDeviceIwd *self) { NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - priv->aps = g_hash_table_new (nm_str_hash, g_str_equal); + c_list_init (&priv->aps_lst_head); /* Make sure the manager is running */ (void) nm_iwd_manager_get (); @@ -1867,19 +1829,8 @@ dispose (GObject *object) remove_all_aps (self); G_OBJECT_CLASS (nm_device_iwd_parent_class)->dispose (object); -} -static void -finalize (GObject *object) -{ - NMDeviceIwd *self = NM_DEVICE_IWD (object); - NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); - - nm_assert (g_hash_table_size (priv->aps) == 0); - - g_hash_table_unref (priv->aps); - - G_OBJECT_CLASS (nm_device_iwd_parent_class)->finalize (object); + nm_assert (c_list_is_empty (&priv->aps_lst_head)); } static void @@ -1894,7 +1845,6 @@ nm_device_iwd_class_init (NMDeviceIwdClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; object_class->dispose = dispose; - object_class->finalize = finalize; dbus_object_class->interface_infos = NM_DBUS_INTERFACE_INFOS (&nm_interface_info_device_wireless); diff --git a/src/devices/wifi/nm-device-iwd.h b/src/devices/wifi/nm-device-iwd.h index 6d4ff06bfc..699ba1bb2b 100644 --- a/src/devices/wifi/nm-device-iwd.h +++ b/src/devices/wifi/nm-device-iwd.h @@ -53,7 +53,7 @@ void nm_device_iwd_set_dbus_object (NMDeviceIwd *device, GDBusObject *object); gboolean nm_device_iwd_agent_psk_query (NMDeviceIwd *device, GDBusMethodInvocation *invocation); -GHashTable *_nm_device_iwd_get_aps (NMDeviceIwd *self); +const CList *_nm_device_iwd_get_aps (NMDeviceIwd *self); void _nm_device_iwd_request_scan (NMDeviceIwd *self, GVariant *options, diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 7c64d594b7..de4af42cfc 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -32,6 +32,7 @@ #include "nm-common-macros.h" #include "devices/nm-device.h" #include "devices/nm-device-private.h" +#include "nm-dbus-manager.h" #include "nm-utils.h" #include "NetworkManagerUtils.h" #include "nm-act-request.h" @@ -86,7 +87,8 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { gint8 invalid_strength_counter; - GHashTable * aps; + CList aps_lst_head; + NMWifiAP * current_ap; guint32 rate; bool enabled:1; /* rfkilled or not */ @@ -344,30 +346,6 @@ supplicant_interface_release (NMDeviceWifi *self) _notify_scanning (self); } -static NMWifiAP * -get_ap_by_path (NMDeviceWifi *self, const char *path) -{ - g_return_val_if_fail (path != NULL, NULL); - return g_hash_table_lookup (NM_DEVICE_WIFI_GET_PRIVATE (self)->aps, path); - -} - -static NMWifiAP * -get_ap_by_supplicant_path (NMDeviceWifi *self, const char *path) -{ - GHashTableIter iter; - NMWifiAP *ap; - - g_return_val_if_fail (path != NULL, NULL); - - g_hash_table_iter_init (&iter, NM_DEVICE_WIFI_GET_PRIVATE (self)->aps); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { - if (g_strcmp0 (path, nm_wifi_ap_get_supplicant_path (ap)) == 0) - return ap; - } - return NULL; -} - static void update_seen_bssids_cache (NMDeviceWifi *self, NMWifiAP *ap) { @@ -488,28 +466,25 @@ ap_add_remove (NMDeviceWifi *self, NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); if (is_adding) { - g_hash_table_insert (priv->aps, - (gpointer) nm_dbus_object_export (NM_DBUS_OBJECT (ap)), - g_object_ref (ap)); + g_object_ref (ap); + ap->wifi_device = NM_DEVICE (self); + c_list_link_tail (&priv->aps_lst_head, &ap->aps_lst); + nm_dbus_object_export (NM_DBUS_OBJECT (ap)); _ap_dump (self, LOGL_DEBUG, ap, "added", 0); - } else + nm_device_wifi_emit_signal_access_point (NM_DEVICE (self), ap, TRUE); + } else { + ap->wifi_device = NULL; + c_list_unlink (&ap->aps_lst); _ap_dump (self, LOGL_DEBUG, ap, "removed", 0); - - nm_dbus_object_emit_signal (NM_DBUS_OBJECT (self), - &nm_interface_info_device_wireless, - is_adding - ? &nm_signal_info_wireless_access_point_added - : &nm_signal_info_wireless_access_point_removed, - "(o)", - nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - - if (!is_adding) { - g_hash_table_remove (priv->aps, nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); - nm_dbus_object_clear_and_unexport (&ap); } _notify (self, PROP_ACCESS_POINTS); + if (!is_adding) { + nm_device_wifi_emit_signal_access_point (NM_DEVICE (self), ap, FALSE); + nm_dbus_object_clear_and_unexport (&ap); + } + nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); if (recheck_available_connections) nm_device_recheck_available_connections (NM_DEVICE (self)); @@ -519,20 +494,15 @@ static void remove_all_aps (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - GHashTableIter iter; NMWifiAP *ap; - if (!g_hash_table_size (priv->aps)) + if (c_list_is_empty (&priv->aps_lst_head)) return; set_current_ap (self, NULL, FALSE); -again: - g_hash_table_iter_init (&iter, priv->aps); - if (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { + while ((ap = c_list_first_entry (&priv->aps_lst_head, NMWifiAP, aps_lst))) ap_add_remove (self, FALSE, ap, FALSE); - goto again; - } nm_device_recheck_available_connections (NM_DEVICE (self)); } @@ -702,7 +672,7 @@ check_connection_available (NMDevice *device, if (specific_object) { NMWifiAP *ap; - ap = get_ap_by_path (self, specific_object); + ap = nm_wifi_ap_lookup_for_device (NM_DEVICE (self), specific_object); return ap ? nm_wifi_ap_check_compatible (ap, connection) : FALSE; } @@ -726,7 +696,7 @@ check_connection_available (NMDevice *device, return TRUE; /* check at least one AP is compatible with this connection */ - return !!nm_wifi_aps_find_first_compatible (priv->aps, connection, TRUE); + return !!nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); } static gboolean @@ -774,7 +744,7 @@ complete_connection (NMDevice *device, if (!nm_streq0 (mode, NM_SETTING_WIRELESS_MODE_AP)) { /* Find a compatible AP in the scan list */ - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); /* If we still don't have an AP, then the WiFI settings needs to be * fully specified by the client. Might not be able to find an AP @@ -796,7 +766,7 @@ complete_connection (NMDevice *device, return FALSE; ap = NULL; } else { - ap = get_ap_by_path (self, specific_object); + ap = nm_wifi_ap_lookup_for_device (NM_DEVICE (self), specific_object); if (!ap) { g_set_error (error, NM_DEVICE_ERROR, @@ -988,7 +958,7 @@ can_auto_connect (NMDevice *device, return FALSE; } - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); if (ap) { /* All good; connection is usable */ NM_SET_OUT (specific_object, g_strdup (nm_dbus_object_get_path (NM_DBUS_OBJECT (ap)))); @@ -998,10 +968,10 @@ can_auto_connect (NMDevice *device, return FALSE; } -GHashTable * +const CList * _nm_device_wifi_get_aps (NMDeviceWifi *self) { - return NM_DEVICE_WIFI_GET_PRIVATE (self)->aps; + return &NM_DEVICE_WIFI_GET_PRIVATE (self)->aps_lst_head; } static void @@ -1472,17 +1442,15 @@ ap_list_dump (gpointer user_data) priv->ap_dump_id = 0; if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) { - gs_free NMWifiAP **list = NULL; - gsize i; + NMWifiAP *ap; gint32 now_s = nm_utils_get_monotonic_timestamp_s (); _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%u next:%u]", now_s, priv->last_scan, priv->scheduled_scan_time); - list = nm_wifi_aps_get_sorted (priv->aps, TRUE); - for (i = 0; list[i]; i++) - _ap_dump (self, LOGL_DEBUG, list[i], "dump", now_s); + c_list_for_each_entry (ap, &priv->aps_lst_head, aps_lst) + _ap_dump (self, LOGL_DEBUG, ap, "dump", now_s); } return G_SOURCE_REMOVE; } @@ -1553,7 +1521,7 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, if (NM_DEVICE_WIFI_GET_PRIVATE (self)->mode == NM_802_11_MODE_AP) return; - found_ap = get_ap_by_supplicant_path (self, object_path); + found_ap = nm_wifi_aps_find_by_supplicant_path (&priv->aps_lst_head, object_path); if (found_ap) { if (!nm_wifi_ap_update_from_properties (found_ap, object_path, properties)) return; @@ -1609,7 +1577,7 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, g_return_if_fail (object_path != NULL); priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - ap = get_ap_by_supplicant_path (self, object_path); + ap = nm_wifi_aps_find_by_supplicant_path (&priv->aps_lst_head, object_path); if (!ap) return; @@ -2153,7 +2121,7 @@ supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, current_bss = nm_supplicant_interface_get_current_bss (iface); if (current_bss) - new_ap = get_ap_by_supplicant_path (self, current_bss); + new_ap = nm_wifi_aps_find_by_supplicant_path (&priv->aps_lst_head, current_bss); if (new_ap != priv->current_ap) { const char *new_bssid = NULL; @@ -2514,11 +2482,11 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) /* AP mode never uses a specific object or existing scanned AP */ if (priv->mode != NM_802_11_MODE_AP) { ap_path = nm_active_connection_get_specific_object (NM_ACTIVE_CONNECTION (req)); - ap = ap_path ? get_ap_by_path (self, ap_path) : NULL; + ap = ap_path ? nm_wifi_ap_lookup_for_device (NM_DEVICE (self), ap_path) : NULL; if (ap) goto done; - ap = nm_wifi_aps_find_first_compatible (priv->aps, connection, FALSE); + ap = nm_wifi_aps_find_first_compatible (&priv->aps_lst_head, connection); } if (ap) { @@ -3134,8 +3102,7 @@ get_property (GObject *object, guint prop_id, { NMDeviceWifi *self = NM_DEVICE_WIFI (object); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - gsize i; - char **list; + const char **list; switch (prop_id) { case PROP_MODE: @@ -3148,10 +3115,8 @@ get_property (GObject *object, guint prop_id, g_value_set_uint (value, priv->capabilities); break; case PROP_ACCESS_POINTS: - list = (char **) nm_wifi_aps_get_sorted_paths (priv->aps, TRUE); - for (i = 0; list[i]; i++) - list[i] = g_strdup (list[i]); - g_value_take_boxed (value, list); + list = nm_wifi_aps_get_paths (&priv->aps_lst_head, TRUE); + g_value_take_boxed (value, nm_utils_strv_make_deep_copied (list)); break; case PROP_ACTIVE_ACCESS_POINT: nm_dbus_utils_g_value_set_object_path (value, priv->current_ap); @@ -3190,8 +3155,9 @@ nm_device_wifi_init (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + c_list_init (&priv->aps_lst_head); + priv->mode = NM_802_11_MODE_INFRA; - priv->aps = g_hash_table_new (nm_str_hash, g_str_equal); } static void @@ -3249,9 +3215,7 @@ finalize (GObject *object) NMDeviceWifi *self = NM_DEVICE_WIFI (object); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - nm_assert (g_hash_table_size (priv->aps) == 0); - - g_hash_table_unref (priv->aps); + nm_assert (c_list_is_empty (&priv->aps_lst_head)); G_OBJECT_CLASS (nm_device_wifi_parent_class)->finalize (object); } diff --git a/src/devices/wifi/nm-device-wifi.h b/src/devices/wifi/nm-device-wifi.h index 632bbac1f4..c13b00defc 100644 --- a/src/devices/wifi/nm-device-wifi.h +++ b/src/devices/wifi/nm-device-wifi.h @@ -47,7 +47,7 @@ GType nm_device_wifi_get_type (void); NMDevice * nm_device_wifi_new (const char *iface, NMDeviceWifiCapabilities capabilities); -GHashTable *_nm_device_wifi_get_aps (NMDeviceWifi *self); +const CList *_nm_device_wifi_get_aps (NMDeviceWifi *self); void _nm_device_wifi_request_scan (NMDeviceWifi *self, GVariant *options, diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 60767032cd..dd6d1deb6e 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -33,6 +33,8 @@ #include "nm-utils.h" #include "nm-core-internal.h" #include "platform/nm-platform.h" +#include "devices/nm-device.h" +#include "nm-dbus-manager.h" #define PROTO_WPA "wpa" #define PROTO_RSN "rsn" @@ -52,7 +54,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMWifiAP, PROP_LAST_SEEN, ); -typedef struct { +struct _NMWifiAPPrivate { char *supplicant_path; /* D-Bus object path of this AP from wpa_supplicant */ /* Scanned or cached values */ @@ -71,20 +73,17 @@ typedef struct { bool fake:1; /* Whether or not the AP is from a scan */ bool hotspot:1; /* Whether the AP is a local device's hotspot network */ gint32 last_seen; /* Timestamp when the AP was seen lastly (obtained via nm_utils_get_monotonic_timestamp_s()) */ -} NMWifiAPPrivate; - -struct _NMWifiAP { - NMDBusObject parent; - NMWifiAPPrivate _priv; }; +typedef struct _NMWifiAPPrivate NMWifiAPPrivate; + struct _NMWifiAPClass { NMDBusObjectClass parent; }; G_DEFINE_TYPE (NMWifiAP, nm_wifi_ap, NM_TYPE_DBUS_OBJECT) -#define NM_WIFI_AP_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMWifiAP, NM_IS_WIFI_AP) +#define NM_WIFI_AP_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMWifiAP, NM_IS_WIFI_AP) /*****************************************************************************/ @@ -96,25 +95,6 @@ nm_wifi_ap_get_supplicant_path (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->supplicant_path; } -guint64 -nm_wifi_ap_get_id (NMWifiAP *ap) -{ - const char *path; - guint64 i; - - g_return_val_if_fail (NM_IS_WIFI_AP (ap), 0); - - path = nm_dbus_object_get_path (NM_DBUS_OBJECT (ap)); - g_return_val_if_fail (path, 0); - - nm_assert (g_str_has_prefix (path, NM_DBUS_PATH_ACCESS_POINT"/")); - - i = _nm_utils_ascii_str_to_int64 (&path[NM_STRLEN (NM_DBUS_PATH_ACCESS_POINT"/")], 10, 1, G_MAXINT64, 0); - - nm_assert (i); - return i; -} - const GByteArray * nm_wifi_ap_get_ssid (const NMWifiAP *ap) { @@ -1178,9 +1158,15 @@ get_property (GObject *object, guint prop_id, /*****************************************************************************/ static void -nm_wifi_ap_init (NMWifiAP *ap) +nm_wifi_ap_init (NMWifiAP *self) { - NMWifiAPPrivate *priv = NM_WIFI_AP_GET_PRIVATE (ap); + NMWifiAPPrivate *priv; + + priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_WIFI_AP, NMWifiAPPrivate); + + self->_priv = priv; + + c_list_init (&self->aps_lst); priv->mode = NM_802_11_MODE_INFRA; priv->flags = NM_802_11_AP_FLAGS_NONE; @@ -1340,7 +1326,11 @@ error: static void finalize (GObject *object) { - NMWifiAPPrivate *priv = NM_WIFI_AP_GET_PRIVATE ((NMWifiAP *) object); + NMWifiAP *self = NM_WIFI_AP (object); + NMWifiAPPrivate *priv = NM_WIFI_AP_GET_PRIVATE (self); + + nm_assert (!self->wifi_device); + nm_assert (c_list_is_empty (&self->aps_lst)); g_free (priv->supplicant_path); if (priv->ssid) @@ -1391,6 +1381,8 @@ nm_wifi_ap_class_init (NMWifiAPClass *ap_class) GObjectClass *object_class = G_OBJECT_CLASS (ap_class); NMDBusObjectClass *dbus_object_class = NM_DBUS_OBJECT_CLASS (ap_class); + g_type_class_add_private (object_class, sizeof (NMWifiAPPrivate)); + dbus_object_class->export_path = NM_DBUS_EXPORT_PATH_NUMBERED (NM_DBUS_PATH_ACCESS_POINT); dbus_object_class->interface_infos = NM_DBUS_INTERFACE_INFOS (&interface_info_access_point); @@ -1459,86 +1451,81 @@ nm_wifi_ap_class_init (NMWifiAPClass *ap_class) /*****************************************************************************/ -static int -ap_id_compare (gconstpointer p_a, gconstpointer p_b, gpointer user_data) +const char ** +nm_wifi_aps_get_paths (const CList *aps_lst_head, gboolean include_without_ssid) { - guint64 a_id = nm_wifi_ap_get_id (*((NMWifiAP **) p_a)); - guint64 b_id = nm_wifi_ap_get_id (*((NMWifiAP **) p_b)); - - return a_id < b_id ? -1 : (a_id == b_id ? 0 : 1); -} - -NMWifiAP ** -nm_wifi_aps_get_sorted (GHashTable *aps, gboolean include_without_ssid) -{ - NMWifiAP **list; - GHashTableIter iter; NMWifiAP *ap; gsize i, n; + const char **list; + const char *path; - n = g_hash_table_size (aps); - list = g_new (NMWifiAP *, n + 1); + n = c_list_length (aps_lst_head); + list = g_new (const char *, n + 1); i = 0; if (n > 0) { - g_hash_table_iter_init (&iter, aps); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { + c_list_for_each_entry (ap, aps_lst_head, aps_lst) { nm_assert (i < n); - if ( include_without_ssid - || nm_wifi_ap_get_ssid (ap)) - list[i++] = ap; + if ( !include_without_ssid + && !nm_wifi_ap_get_ssid (ap)) + continue; + + path = nm_dbus_object_get_path (NM_DBUS_OBJECT (ap)); + nm_assert (path); + + list[i++] = path; } nm_assert (i <= n); nm_assert (!include_without_ssid || i == n); - - g_qsort_with_data (list, - i, - sizeof (gpointer), - ap_id_compare, - NULL); } list[i] = NULL; return list; } -const char ** -nm_wifi_aps_get_sorted_paths (GHashTable *aps, gboolean include_without_ssid) +NMWifiAP * +nm_wifi_aps_find_first_compatible (const CList *aps_lst_head, + NMConnection *connection) { - gpointer *list; - gsize i, j; + NMWifiAP *ap; - list = (gpointer *) nm_wifi_aps_get_sorted (aps, include_without_ssid); - for (i = 0, j = 0; list[i]; i++) { - NMWifiAP *ap = list[i]; - const char *path; + g_return_val_if_fail (connection, NULL); - /* update @list inplace to hold instead the export-path. */ - path = nm_dbus_object_get_path (NM_DBUS_OBJECT (ap)); - nm_assert (path); - list[j++] = (gpointer) path; + c_list_for_each_entry (ap, aps_lst_head, aps_lst) { + if (nm_wifi_ap_check_compatible (ap, connection)) + return ap; } - return (const char **) list; + return NULL; } NMWifiAP * -nm_wifi_aps_find_first_compatible (GHashTable *aps, - NMConnection *connection, - gboolean allow_unstable_order) +nm_wifi_aps_find_by_supplicant_path (const CList *aps_lst_head, const char *path) { - GHashTableIter iter; NMWifiAP *ap; - NMWifiAP *cand_ap = NULL; - g_return_val_if_fail (connection != NULL, NULL); + g_return_val_if_fail (path != NULL, NULL); - g_hash_table_iter_init (&iter, aps); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &ap)) { - if (!nm_wifi_ap_check_compatible (ap, connection)) - continue; - if (allow_unstable_order) + c_list_for_each_entry (ap, aps_lst_head, aps_lst) { + if (nm_streq0 (path, nm_wifi_ap_get_supplicant_path (ap))) return ap; - if (!cand_ap || (nm_wifi_ap_get_id (cand_ap) < nm_wifi_ap_get_id (ap))) - cand_ap = ap; } - return cand_ap; + return NULL; +} + +/*****************************************************************************/ + +NMWifiAP * +nm_wifi_ap_lookup_for_device (NMDevice *device, const char *exported_path) +{ + NMWifiAP *ap; + + g_return_val_if_fail (NM_IS_DEVICE (device), NULL); + + ap = (NMWifiAP *) nm_dbus_manager_lookup_object (nm_dbus_object_get_manager (NM_DBUS_OBJECT (device)), + exported_path); + if ( !ap + || !NM_IS_WIFI_AP (ap) + || ap->wifi_device != device) + return NULL; + + return ap; } diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 314f2265f1..4fdeee935c 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -44,7 +44,13 @@ #define NM_WIFI_AP_STRENGTH "strength" #define NM_WIFI_AP_LAST_SEEN "last-seen" -typedef struct _NMWifiAP NMWifiAP; +typedef struct { + NMDBusObject parent; + NMDevice *wifi_device; + CList aps_lst; + struct _NMWifiAPPrivate *_priv; +} NMWifiAP; + typedef struct _NMWifiAPClass NMWifiAPClass; GType nm_wifi_ap_get_type (void); @@ -66,7 +72,6 @@ gboolean nm_wifi_ap_complete_connection (NMWifiAP *self, GError **error); const char * nm_wifi_ap_get_supplicant_path (NMWifiAP *ap); -guint64 nm_wifi_ap_get_id (NMWifiAP *ap); const GByteArray *nm_wifi_ap_get_ssid (const NMWifiAP *ap); gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, @@ -95,12 +100,14 @@ const char *nm_wifi_ap_to_string (const NMWifiAP *self, gulong buf_len, gint32 now_s); -NMWifiAP **nm_wifi_aps_get_sorted (GHashTable *aps, - gboolean include_without_ssid); -const char **nm_wifi_aps_get_sorted_paths (GHashTable *aps, - gboolean include_without_ssid); -NMWifiAP *nm_wifi_aps_find_first_compatible (GHashTable *aps, - NMConnection *connection, - gboolean allow_unstable_order); +const char **nm_wifi_aps_get_paths (const CList *aps_lst_head, + gboolean include_without_ssid); + +NMWifiAP *nm_wifi_aps_find_first_compatible (const CList *aps_lst_head, + NMConnection *connection); + +NMWifiAP *nm_wifi_aps_find_by_supplicant_path (const CList *aps_lst_head, const char *path); + +NMWifiAP *nm_wifi_ap_lookup_for_device (NMDevice *device, const char *exported_path); #endif /* __NM_WIFI_AP_H__ */ diff --git a/src/devices/wifi/nm-wifi-common.c b/src/devices/wifi/nm-wifi-common.c index 005aac9d60..47c0ce6784 100644 --- a/src/devices/wifi/nm-wifi-common.c +++ b/src/devices/wifi/nm-wifi-common.c @@ -25,6 +25,7 @@ #include "devices/nm-device.h" #include "nm-wifi-ap.h" #include "nm-device-wifi.h" +#include "nm-dbus-manager.h" #if WITH_IWD #include "nm-device-iwd.h" @@ -32,7 +33,23 @@ /*****************************************************************************/ -static GHashTable * +void +nm_device_wifi_emit_signal_access_point (NMDevice *device, + NMWifiAP *ap, + gboolean is_added /* or else is_removed */) +{ + nm_dbus_object_emit_signal (NM_DBUS_OBJECT (device), + &nm_interface_info_device_wireless, + is_added + ? &nm_signal_info_wireless_access_point_added + : &nm_signal_info_wireless_access_point_removed, + "(o)", + nm_dbus_object_get_path (NM_DBUS_OBJECT (ap))); +} + +/*****************************************************************************/ + +static const CList * _dispatch_get_aps (NMDevice *device) { #if WITH_IWD @@ -70,12 +87,12 @@ impl_device_wifi_get_access_points (NMDBusObject *obj, { gs_free const char **list = NULL; GVariant *v; - GHashTable *aps; + const CList *all_aps; /* NOTE: this handler is called both for NMDevicwWifi and NMDeviceIwd. */ - aps = _dispatch_get_aps (NM_DEVICE (obj)); - list = nm_wifi_aps_get_sorted_paths (aps, FALSE); + all_aps = _dispatch_get_aps (NM_DEVICE (obj)); + list = nm_wifi_aps_get_paths (all_aps, FALSE); v = g_variant_new_objv (list, -1); g_dbus_method_invocation_return_value (invocation, g_variant_new_tuple (&v, 1)); @@ -92,12 +109,12 @@ impl_device_wifi_get_all_access_points (NMDBusObject *obj, { gs_free const char **list = NULL; GVariant *v; - GHashTable *aps; + const CList *all_aps; /* NOTE: this handler is called both for NMDevicwWifi and NMDeviceIwd. */ - aps = _dispatch_get_aps (NM_DEVICE (obj)); - list = nm_wifi_aps_get_sorted_paths (aps, TRUE); + all_aps = _dispatch_get_aps (NM_DEVICE (obj)); + list = nm_wifi_aps_get_paths (all_aps, TRUE); v = g_variant_new_objv (list, -1); g_dbus_method_invocation_return_value (invocation, g_variant_new_tuple (&v, 1)); diff --git a/src/devices/wifi/nm-wifi-common.h b/src/devices/wifi/nm-wifi-common.h index 4df4a5903b..91cbeb558d 100644 --- a/src/devices/wifi/nm-wifi-common.h +++ b/src/devices/wifi/nm-wifi-common.h @@ -22,9 +22,14 @@ #define __NM_WIFI_COMMON_H__ #include "nm-dbus-utils.h" +#include "nm-wifi-ap.h" /*****************************************************************************/ +void nm_device_wifi_emit_signal_access_point (NMDevice *device, + NMWifiAP *ap, + gboolean is_added /* or else is_removed */); + extern const NMDBusInterfaceInfoExtended nm_interface_info_device_wireless; extern const GDBusSignalInfo nm_signal_info_wireless_access_point_added; extern const GDBusSignalInfo nm_signal_info_wireless_access_point_removed; From 938d9a82cf3cc1dce5a30914db925ae2d3dbe54c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 25 Mar 2018 14:16:07 +0200 Subject: [PATCH 07/10] shared: add nm_utils_hash_keys_to_array() helper --- shared/nm-utils/nm-shared-utils.c | 39 +++++++++++++++++-------------- shared/nm-utils/nm-shared-utils.h | 18 +++++++++++--- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 25b867538c..556594aca3 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -1149,31 +1149,34 @@ nm_utils_named_values_from_str_dict (GHashTable *hash, guint *out_len) return values; } -const char ** -nm_utils_strdict_get_keys (const GHashTable *hash, - gboolean sorted, - guint *out_length) +gpointer * +nm_utils_hash_keys_to_array (GHashTable *hash, + GCompareDataFunc compare_func, + gpointer user_data, + guint *out_len) { - const char **names; - guint length; + guint len; + gpointer *keys; + /* by convention, we never return an empty array. In that + * case, always %NULL. */ if ( !hash - || !g_hash_table_size ((GHashTable *) hash)) { - NM_SET_OUT (out_length, 0); + || g_hash_table_size (hash) == 0) { + NM_SET_OUT (out_len, 0); return NULL; } - names = (const char **) g_hash_table_get_keys_as_array ((GHashTable *) hash, &length); - if ( sorted - && length > 1) { - g_qsort_with_data (names, - length, - sizeof (char *), - nm_strcmp_p_with_data, - NULL); + keys = g_hash_table_get_keys_as_array (hash, &len); + if ( len > 1 + && compare_func) { + g_qsort_with_data (keys, + len, + sizeof (gpointer), + compare_func, + user_data); } - NM_SET_OUT (out_length, length); - return names; + NM_SET_OUT (out_len, len); + return keys; } char ** diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 122f1187bd..37f0621389 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -460,9 +460,21 @@ typedef struct { NMUtilsNamedValue *nm_utils_named_values_from_str_dict (GHashTable *hash, guint *out_len); -const char **nm_utils_strdict_get_keys (const GHashTable *hash, - gboolean sorted, - guint *out_length); +gpointer *nm_utils_hash_keys_to_array (GHashTable *hash, + GCompareDataFunc compare_func, + gpointer user_data, + guint *out_len); + +static inline const char ** +nm_utils_strdict_get_keys (const GHashTable *hash, + gboolean sorted, + guint *out_length) +{ + return (const char **) nm_utils_hash_keys_to_array ((GHashTable *) hash, + sorted ? nm_strcmp_p_with_data : NULL, + NULL, + out_length); +} char **nm_utils_strv_make_deep_copied (const char **strv); From 0f1dc3bca3ed3cc2a9e7dacc34cbb2654965770d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Mar 2018 08:17:11 +0200 Subject: [PATCH 08/10] config/trivial: rename dns_config local variable The variable serves a similar purpose, but is called "dns", "conf", and "dns_config". Choose one name: "dns_config". --- src/nm-config-data.c | 130 +++++++++++++++++++++---------------------- src/nm-config-data.h | 18 +++--- 2 files changed, 74 insertions(+), 74 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index e71ac95a63..13d6acc2f8 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -706,53 +706,53 @@ nm_config_data_log (const NMConfigData *self, /*****************************************************************************/ const char *const * -nm_global_dns_config_get_searches (const NMGlobalDnsConfig *dns) +nm_global_dns_config_get_searches (const NMGlobalDnsConfig *dns_config) { - g_return_val_if_fail (dns, NULL); + g_return_val_if_fail (dns_config, NULL); - return (const char *const *) dns->searches; + return (const char *const *) dns_config->searches; } const char *const * -nm_global_dns_config_get_options (const NMGlobalDnsConfig *dns) +nm_global_dns_config_get_options (const NMGlobalDnsConfig *dns_config) { - g_return_val_if_fail (dns, NULL); + g_return_val_if_fail (dns_config, NULL); - return (const char *const *) dns->options; + return (const char *const *) dns_config->options; } guint -nm_global_dns_config_get_num_domains (const NMGlobalDnsConfig *dns) +nm_global_dns_config_get_num_domains (const NMGlobalDnsConfig *dns_config) { - g_return_val_if_fail (dns, 0); - g_return_val_if_fail (dns->domains, 0); + g_return_val_if_fail (dns_config, 0); + g_return_val_if_fail (dns_config->domains, 0); - return g_hash_table_size (dns->domains); + return g_hash_table_size (dns_config->domains); } NMGlobalDnsDomain * -nm_global_dns_config_get_domain (const NMGlobalDnsConfig *dns, guint i) +nm_global_dns_config_get_domain (const NMGlobalDnsConfig *dns_config, guint i) { NMGlobalDnsDomain *domain; - g_return_val_if_fail (dns, NULL); - g_return_val_if_fail (dns->domains, NULL); - g_return_val_if_fail (dns->domain_list, NULL); - g_return_val_if_fail (i < g_strv_length (dns->domain_list), NULL); + g_return_val_if_fail (dns_config, NULL); + g_return_val_if_fail (dns_config->domains, NULL); + g_return_val_if_fail (dns_config->domain_list, NULL); + g_return_val_if_fail (i < g_strv_length (dns_config->domain_list), NULL); - domain = g_hash_table_lookup (dns->domains, dns->domain_list[i]); + domain = g_hash_table_lookup (dns_config->domains, dns_config->domain_list[i]); g_return_val_if_fail (domain, NULL); return domain; } -NMGlobalDnsDomain *nm_global_dns_config_lookup_domain (const NMGlobalDnsConfig *dns, const char *name) +NMGlobalDnsDomain *nm_global_dns_config_lookup_domain (const NMGlobalDnsConfig *dns_config, const char *name) { - g_return_val_if_fail (dns, NULL); - g_return_val_if_fail (dns->domains, NULL); + g_return_val_if_fail (dns_config, NULL); + g_return_val_if_fail (dns_config->domains, NULL); g_return_val_if_fail (name, NULL); - return g_hash_table_lookup (dns->domains, name); + return g_hash_table_lookup (dns_config->domains, name); } const char * @@ -779,42 +779,42 @@ nm_global_dns_domain_get_options (const NMGlobalDnsDomain *domain) } gboolean -nm_global_dns_config_is_internal (const NMGlobalDnsConfig *dns) +nm_global_dns_config_is_internal (const NMGlobalDnsConfig *dns_config) { - return dns->internal; + return dns_config->internal; } gboolean -nm_global_dns_config_is_empty (const NMGlobalDnsConfig *dns) +nm_global_dns_config_is_empty (const NMGlobalDnsConfig *dns_config) { - g_return_val_if_fail (dns, TRUE); - g_return_val_if_fail (dns->domains, TRUE); + g_return_val_if_fail (dns_config, TRUE); + g_return_val_if_fail (dns_config->domains, TRUE); - return (!dns->searches || g_strv_length (dns->searches) == 0) - && (!dns->options || g_strv_length (dns->options) == 0) - && g_hash_table_size (dns->domains) == 0; + return (!dns_config->searches || g_strv_length (dns_config->searches) == 0) + && (!dns_config->options || g_strv_length (dns_config->options) == 0) + && g_hash_table_size (dns_config->domains) == 0; } void -nm_global_dns_config_update_checksum (const NMGlobalDnsConfig *dns, GChecksum *sum) +nm_global_dns_config_update_checksum (const NMGlobalDnsConfig *dns_config, GChecksum *sum) { NMGlobalDnsDomain *domain; GList *keys, *key; guint i; - g_return_if_fail (dns); - g_return_if_fail (dns->domains); + g_return_if_fail (dns_config); + g_return_if_fail (dns_config->domains); g_return_if_fail (sum); - for (i = 0; dns->searches && dns->searches[i]; i++) - g_checksum_update (sum, (guchar *) dns->searches[i], strlen (dns->searches[i])); - for (i = 0; dns->options && dns->options[i]; i++) - g_checksum_update (sum, (guchar *) dns->options[i], strlen (dns->options[i])); + for (i = 0; dns_config->searches && dns_config->searches[i]; i++) + g_checksum_update (sum, (guchar *) dns_config->searches[i], strlen (dns_config->searches[i])); + for (i = 0; dns_config->options && dns_config->options[i]; i++) + g_checksum_update (sum, (guchar *) dns_config->options[i], strlen (dns_config->options[i])); - keys = g_list_sort (g_hash_table_get_keys (dns->domains), (GCompareFunc) strcmp); + keys = g_list_sort (g_hash_table_get_keys (dns_config->domains), (GCompareFunc) strcmp); for (key = keys; key; key = g_list_next (key)) { - domain = g_hash_table_lookup (dns->domains, key->data); + domain = g_hash_table_lookup (dns_config->domains, key->data); g_assert (domain != NULL); g_checksum_update (sum, (guchar *) domain->name, strlen (domain->name)); @@ -838,14 +838,14 @@ global_dns_domain_free (NMGlobalDnsDomain *domain) } void -nm_global_dns_config_free (NMGlobalDnsConfig *conf) +nm_global_dns_config_free (NMGlobalDnsConfig *dns_config) { - if (conf) { - g_strfreev (conf->searches); - g_strfreev (conf->options); - g_free (conf->domain_list); - g_hash_table_unref (conf->domains); - g_free (conf); + if (dns_config) { + g_strfreev (dns_config->searches); + g_strfreev (dns_config->options); + g_free (dns_config->domain_list); + g_hash_table_unref (dns_config->domains); + g_free (dns_config); } } @@ -858,18 +858,18 @@ nm_config_data_get_global_dns_config (const NMConfigData *self) } static void -global_dns_config_update_domain_list (NMGlobalDnsConfig *dns) +global_dns_config_update_domain_list (NMGlobalDnsConfig *dns_config) { guint length; - g_free (dns->domain_list); - dns->domain_list = (char **) g_hash_table_get_keys_as_array (dns->domains, &length); + g_free (dns_config->domain_list); + dns_config->domain_list = (char **) g_hash_table_get_keys_as_array (dns_config->domains, &length); } static NMGlobalDnsConfig * load_global_dns (GKeyFile *keyfile, gboolean internal) { - NMGlobalDnsConfig *conf; + NMGlobalDnsConfig *dns_config; char *group, *domain_prefix; gs_strfreev char **groups = NULL; int g, i, j, domain_prefix_len; @@ -887,13 +887,13 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) if (!nm_config_keyfile_has_global_dns_config (keyfile, internal)) return NULL; - conf = g_malloc0 (sizeof (NMGlobalDnsConfig)); - conf->domains = g_hash_table_new_full (nm_str_hash, g_str_equal, - g_free, (GDestroyNotify) global_dns_domain_free); + dns_config = g_malloc0 (sizeof (NMGlobalDnsConfig)); + dns_config->domains = g_hash_table_new_full (nm_str_hash, g_str_equal, + g_free, (GDestroyNotify) global_dns_domain_free); strv = g_key_file_get_string_list (keyfile, group, "searches", NULL, NULL); if (strv) - conf->searches = _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); + dns_config->searches = _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); strv = g_key_file_get_string_list (keyfile, group, "options", NULL, NULL); if (strv) { @@ -905,7 +905,7 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) g_free (strv[i]); } strv[j] = NULL; - conf->options = strv; + dns_config->options = strv; } groups = g_key_file_get_groups (keyfile, NULL); @@ -949,7 +949,7 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) domain->servers = servers; domain->options = options; - g_hash_table_insert (conf->domains, strdup (name), domain); + g_hash_table_insert (dns_config->domains, strdup (name), domain); if (!strcmp (name, "*")) default_found = TRUE; @@ -958,40 +958,40 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) if (!default_found) { nm_log_dbg (LOGD_CORE, "%s global DNS configuration is missing default domain, ignore it", internal ? "internal" : "user"); - nm_global_dns_config_free (conf); + nm_global_dns_config_free (dns_config); return NULL; } - conf->internal = internal; - global_dns_config_update_domain_list (conf); - return conf; + dns_config->internal = internal; + global_dns_config_update_domain_list (dns_config); + return dns_config; } void -nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns, GValue *value) +nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns_config, GValue *value) { GVariantBuilder conf_builder, domains_builder, domain_builder; NMGlobalDnsDomain *domain; GHashTableIter iter; g_variant_builder_init (&conf_builder, G_VARIANT_TYPE ("a{sv}")); - if (!dns) + if (!dns_config) goto out; - if (dns->searches) { + if (dns_config->searches) { g_variant_builder_add (&conf_builder, "{sv}", "searches", - g_variant_new_strv ((const char *const *) dns->searches, -1)); + g_variant_new_strv ((const char *const *) dns_config->searches, -1)); } - if (dns->options) { + if (dns_config->options) { g_variant_builder_add (&conf_builder, "{sv}", "options", - g_variant_new_strv ((const char *const *) dns->options, -1)); + g_variant_new_strv ((const char *const *) dns_config->options, -1)); } g_variant_builder_init (&domains_builder, G_VARIANT_TYPE ("a{sv}")); - g_hash_table_iter_init (&iter, dns->domains); + g_hash_table_iter_init (&iter, dns_config->domains); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &domain)) { g_variant_builder_init (&domain_builder, G_VARIANT_TYPE ("a{sv}")); diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 87cb83ba12..a11b38b206 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -206,18 +206,18 @@ gboolean nm_config_data_is_intern_atomic_group (const NMConfigData *self, const GKeyFile *nm_config_data_clone_keyfile_intern (const NMConfigData *self); -const char *const *nm_global_dns_config_get_searches (const NMGlobalDnsConfig *dns); -const char *const *nm_global_dns_config_get_options (const NMGlobalDnsConfig *dns); -guint nm_global_dns_config_get_num_domains (const NMGlobalDnsConfig *dns); -NMGlobalDnsDomain *nm_global_dns_config_get_domain (const NMGlobalDnsConfig *dns, guint i); -NMGlobalDnsDomain *nm_global_dns_config_lookup_domain (const NMGlobalDnsConfig *dns, const char *name); +const char *const *nm_global_dns_config_get_searches (const NMGlobalDnsConfig *dns_config); +const char *const *nm_global_dns_config_get_options (const NMGlobalDnsConfig *dns_config); +guint nm_global_dns_config_get_num_domains (const NMGlobalDnsConfig *dns_config); +NMGlobalDnsDomain *nm_global_dns_config_get_domain (const NMGlobalDnsConfig *dns_config, guint i); +NMGlobalDnsDomain *nm_global_dns_config_lookup_domain (const NMGlobalDnsConfig *dns_config, const char *name); const char *nm_global_dns_domain_get_name (const NMGlobalDnsDomain *domain); const char *const *nm_global_dns_domain_get_servers (const NMGlobalDnsDomain *domain); const char *const *nm_global_dns_domain_get_options (const NMGlobalDnsDomain *domain); -gboolean nm_global_dns_config_is_internal (const NMGlobalDnsConfig *dns); -gboolean nm_global_dns_config_is_empty (const NMGlobalDnsConfig *dns); -void nm_global_dns_config_update_checksum (const NMGlobalDnsConfig *dns, GChecksum *sum); -void nm_global_dns_config_free (NMGlobalDnsConfig *conf); +gboolean nm_global_dns_config_is_internal (const NMGlobalDnsConfig *dns_config); +gboolean nm_global_dns_config_is_empty (const NMGlobalDnsConfig *dns_config); +void nm_global_dns_config_update_checksum (const NMGlobalDnsConfig *dns_config, GChecksum *sum); +void nm_global_dns_config_free (NMGlobalDnsConfig *dns_config); NMGlobalDnsConfig *nm_global_dns_config_from_dbus (const GValue *value, GError **error); void nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns_config, GValue *value); From cd48bc74b69794a1f60c2f08cc503f8ca43322ae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Mar 2018 08:18:51 +0200 Subject: [PATCH 09/10] config: cleanup fields in NMGlobalDnsConfig - consistently set options, searches, domains fields to %NULL, if there are no values. - in nm_global_dns_config_update_checksum(), ensure that we uniquely hash values. E.g. a config with "searches[a], options=[b]" should hash differently from "searches=[ab], options=[]". - in nm_global_dns_config_to_dbus(), reuse the sorted domain list. We already have it, and it guarantees a consistent ordering of fields. - in global_dns_domain_from_dbus(), fix memleaks if D-Bus strdict contains duplicate entries. --- src/dns/nm-dns-manager.c | 34 +++--- src/nm-config-data.c | 186 ++++++++++++++++++++------------- src/tests/config/test-config.c | 2 +- 3 files changed, 135 insertions(+), 87 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 56be5aab35..ea8ce2d805 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -970,30 +970,36 @@ merge_global_dns_config (NMResolvConfData *rc, NMGlobalDnsConfig *global_conf) const char *const *searches; const char *const *options; const char *const *servers; - gint i; + guint i; if (!global_conf) return FALSE; searches = nm_global_dns_config_get_searches (global_conf); - options = nm_global_dns_config_get_options (global_conf); - - for (i = 0; searches && searches[i]; i++) { - if (domain_is_routing (searches[i])) - continue; - if (!domain_is_valid (searches[i], FALSE)) - continue; - add_string_item (rc->searches, searches[i], TRUE); + if (searches) { + for (i = 0; searches[i]; i++) { + if (domain_is_routing (searches[i])) + continue; + if (!domain_is_valid (searches[i], FALSE)) + continue; + add_string_item (rc->searches, searches[i], TRUE); + } } - for (i = 0; options && options[i]; i++) - add_string_item (rc->options, options[i], TRUE); + options = nm_global_dns_config_get_options (global_conf); + if (options) { + for (i = 0; options[i]; i++) + add_string_item (rc->options, options[i], TRUE); + } default_domain = nm_global_dns_config_lookup_domain (global_conf, "*"); - g_assert (default_domain); + nm_assert (default_domain); + servers = nm_global_dns_domain_get_servers (default_domain); - for (i = 0; servers && servers[i]; i++) - add_string_item (rc->nameservers, servers[i], TRUE); + if (servers) { + for (i = 0; servers[i]; i++) + add_string_item (rc->nameservers, servers[i], TRUE); + } return TRUE; } diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 13d6acc2f8..638d5f3cbd 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -54,7 +54,7 @@ struct _NMGlobalDnsConfig { char **searches; char **options; GHashTable *domains; - char **domain_list; + const char **domain_list; gboolean internal; }; @@ -705,12 +705,12 @@ nm_config_data_log (const NMConfigData *self, /*****************************************************************************/ -const char *const * +const char *const* nm_global_dns_config_get_searches (const NMGlobalDnsConfig *dns_config) { g_return_val_if_fail (dns_config, NULL); - return (const char *const *) dns_config->searches; + return (const char *const*) dns_config->searches; } const char *const * @@ -718,16 +718,15 @@ nm_global_dns_config_get_options (const NMGlobalDnsConfig *dns_config) { g_return_val_if_fail (dns_config, NULL); - return (const char *const *) dns_config->options; + return (const char *const*) dns_config->options; } guint nm_global_dns_config_get_num_domains (const NMGlobalDnsConfig *dns_config) { g_return_val_if_fail (dns_config, 0); - g_return_val_if_fail (dns_config->domains, 0); - return g_hash_table_size (dns_config->domains); + return dns_config->domains ? g_hash_table_size (dns_config->domains) : 0; } NMGlobalDnsDomain * @@ -737,22 +736,22 @@ nm_global_dns_config_get_domain (const NMGlobalDnsConfig *dns_config, guint i) g_return_val_if_fail (dns_config, NULL); g_return_val_if_fail (dns_config->domains, NULL); - g_return_val_if_fail (dns_config->domain_list, NULL); - g_return_val_if_fail (i < g_strv_length (dns_config->domain_list), NULL); + g_return_val_if_fail (i < g_hash_table_size (dns_config->domains), NULL); + + nm_assert (NM_PTRARRAY_LEN (dns_config->domain_list) == g_hash_table_size (dns_config->domains)); domain = g_hash_table_lookup (dns_config->domains, dns_config->domain_list[i]); - g_return_val_if_fail (domain, NULL); + nm_assert (domain); return domain; } NMGlobalDnsDomain *nm_global_dns_config_lookup_domain (const NMGlobalDnsConfig *dns_config, const char *name) { g_return_val_if_fail (dns_config, NULL); - g_return_val_if_fail (dns_config->domains, NULL); g_return_val_if_fail (name, NULL); - return g_hash_table_lookup (dns_config->domains, name); + return dns_config->domains ? g_hash_table_lookup (dns_config->domains, name) : NULL; } const char * @@ -775,6 +774,7 @@ const char *const * nm_global_dns_domain_get_options (const NMGlobalDnsDomain *domain) { g_return_val_if_fail (domain, NULL); + return (const char *const *) domain->options; } @@ -788,42 +788,59 @@ gboolean nm_global_dns_config_is_empty (const NMGlobalDnsConfig *dns_config) { g_return_val_if_fail (dns_config, TRUE); - g_return_val_if_fail (dns_config->domains, TRUE); - return (!dns_config->searches || g_strv_length (dns_config->searches) == 0) - && (!dns_config->options || g_strv_length (dns_config->options) == 0) - && g_hash_table_size (dns_config->domains) == 0; + return !dns_config->searches + && !dns_config->options + && !dns_config->domain_list; } void nm_global_dns_config_update_checksum (const NMGlobalDnsConfig *dns_config, GChecksum *sum) { NMGlobalDnsDomain *domain; - GList *keys, *key; guint i; + guint8 v8; g_return_if_fail (dns_config); - g_return_if_fail (dns_config->domains); g_return_if_fail (sum); - for (i = 0; dns_config->searches && dns_config->searches[i]; i++) - g_checksum_update (sum, (guchar *) dns_config->searches[i], strlen (dns_config->searches[i])); - for (i = 0; dns_config->options && dns_config->options[i]; i++) - g_checksum_update (sum, (guchar *) dns_config->options[i], strlen (dns_config->options[i])); + v8 = NM_HASH_COMBINE_BOOLS (guint8, + !dns_config->searches, + !dns_config->options, + !dns_config->domain_list); + g_checksum_update (sum, (guchar *) &v8, 1); - keys = g_list_sort (g_hash_table_get_keys (dns_config->domains), (GCompareFunc) strcmp); - for (key = keys; key; key = g_list_next (key)) { - - domain = g_hash_table_lookup (dns_config->domains, key->data); - g_assert (domain != NULL); - g_checksum_update (sum, (guchar *) domain->name, strlen (domain->name)); - - for (i = 0; domain->servers && domain->servers[i]; i++) - g_checksum_update (sum, (guchar *) domain->servers[i], strlen (domain->servers[i])); - for (i = 0; domain->options && domain->options[i]; i++) - g_checksum_update (sum, (guchar *) domain->options[i], strlen (domain->options[i])); + if (dns_config->searches) { + for (i = 0; dns_config->searches[i]; i++) + g_checksum_update (sum, (guchar *) dns_config->searches[i], strlen (dns_config->searches[i]) + 1); + } + if (dns_config->options) { + for (i = 0; dns_config->options[i]; i++) + g_checksum_update (sum, (guchar *) dns_config->options[i], strlen (dns_config->options[i]) + 1); + } + + if (dns_config->domain_list) { + for (i = 0; dns_config->domain_list[i]; i++) { + domain = g_hash_table_lookup (dns_config->domains, dns_config->domain_list[i]); + nm_assert (domain); + + v8 = NM_HASH_COMBINE_BOOLS (guint8, + !domain->servers, + !domain->options); + g_checksum_update (sum, (guchar *) &v8, 1); + + g_checksum_update (sum, (guchar *) domain->name, strlen (domain->name) + 1); + + if (domain->servers) { + for (i = 0; domain->servers && domain->servers[i]; i++) + g_checksum_update (sum, (guchar *) domain->servers[i], strlen (domain->servers[i]) + 1); + } + if (domain->options) { + for (i = 0; domain->options[i]; i++) + g_checksum_update (sum, (guchar *) domain->options[i], strlen (domain->options[i]) + 1); + } + } } - g_list_free (keys); } static void @@ -844,7 +861,8 @@ nm_global_dns_config_free (NMGlobalDnsConfig *dns_config) g_strfreev (dns_config->searches); g_strfreev (dns_config->options); g_free (dns_config->domain_list); - g_hash_table_unref (dns_config->domains); + if (dns_config->domains) + g_hash_table_unref (dns_config->domains); g_free (dns_config); } } @@ -858,12 +876,16 @@ nm_config_data_get_global_dns_config (const NMConfigData *self) } static void -global_dns_config_update_domain_list (NMGlobalDnsConfig *dns_config) +global_dns_config_seal_domains (NMGlobalDnsConfig *dns_config) { - guint length; + nm_assert (dns_config); + nm_assert (dns_config->domains); + nm_assert (!dns_config->domain_list); - g_free (dns_config->domain_list); - dns_config->domain_list = (char **) g_hash_table_get_keys_as_array (dns_config->domains, &length); + if (g_hash_table_size (dns_config->domains) == 0) + nm_clear_pointer (&dns_config->domains, g_hash_table_unref); + else + dns_config->domain_list = nm_utils_strdict_get_keys (dns_config->domains, TRUE, NULL); } static NMGlobalDnsConfig * @@ -892,8 +914,13 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) g_free, (GDestroyNotify) global_dns_domain_free); strv = g_key_file_get_string_list (keyfile, group, "searches", NULL, NULL); - if (strv) - dns_config->searches = _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); + if (strv) { + _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); + if (!strv[0]) + g_free (strv); + else + dns_config->searches = strv; + } strv = g_key_file_get_string_list (keyfile, group, "options", NULL, NULL); if (strv) { @@ -904,8 +931,12 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) else g_free (strv[i]); } - strv[j] = NULL; - dns_config->options = strv; + if (j == 0) + g_free (strv); + else { + strv[j] = NULL; + dns_config->options = strv; + } } groups = g_key_file_get_groups (keyfile, NULL); @@ -928,20 +959,23 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) else g_free (strv[i]); } - if (j) { + if (j == 0) + g_free (strv); + else { strv[j] = NULL; servers = strv; } - else - g_free (strv); } if (!servers) continue; strv = g_key_file_get_string_list (keyfile, groups[g], "options", NULL, NULL); - if (strv) + if (strv) { options = _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); + if (!options[0]) + nm_clear_g_free (&options); + } name = strdup (&groups[g][domain_prefix_len]); domain = g_malloc0 (sizeof (NMGlobalDnsDomain)); @@ -963,7 +997,7 @@ load_global_dns (GKeyFile *keyfile, gboolean internal) } dns_config->internal = internal; - global_dns_config_update_domain_list (dns_config); + global_dns_config_seal_domains (dns_config); return dns_config; } @@ -972,8 +1006,7 @@ void nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns_config, GValue *value) { GVariantBuilder conf_builder, domains_builder, domain_builder; - NMGlobalDnsDomain *domain; - GHashTableIter iter; + guint i; g_variant_builder_init (&conf_builder, G_VARIANT_TYPE ("a{sv}")); if (!dns_config) @@ -990,27 +1023,30 @@ nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns_config, GValue *value } g_variant_builder_init (&domains_builder, G_VARIANT_TYPE ("a{sv}")); + if (dns_config->domain_list) { + for (i = 0; dns_config->domain_list[i]; i++) { + NMGlobalDnsDomain *domain; - g_hash_table_iter_init (&iter, dns_config->domains); - while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &domain)) { + domain = g_hash_table_lookup (dns_config->domains, dns_config->domain_list[i]); - g_variant_builder_init (&domain_builder, G_VARIANT_TYPE ("a{sv}")); + g_variant_builder_init (&domain_builder, G_VARIANT_TYPE ("a{sv}")); - if (domain->servers) { - g_variant_builder_add (&domain_builder, "{sv}", "servers", - g_variant_new_strv ((const char *const *) domain->servers, -1)); + if (domain->servers) { + g_variant_builder_add (&domain_builder, "{sv}", "servers", + g_variant_new_strv ((const char *const *) domain->servers, -1)); + } + if (domain->options) { + g_variant_builder_add (&domain_builder, "{sv}", "options", + g_variant_new_strv ((const char *const *) domain->options, -1)); + } + + g_variant_builder_add (&domains_builder, "{sv}", domain->name, + g_variant_builder_end (&domain_builder)); } - if (domain->options) { - g_variant_builder_add (&domain_builder, "{sv}", "options", - g_variant_new_strv ((const char *const *) domain->options, -1)); - } - - g_variant_builder_add (&domains_builder, "{sv}", domain->name, - g_variant_builder_end (&domain_builder)); } - g_variant_builder_add (&conf_builder, "{sv}", "domains", g_variant_builder_end (&domains_builder)); + out: g_value_take_variant (value, g_variant_builder_end (&conf_builder)); } @@ -1044,15 +1080,20 @@ global_dns_domain_from_dbus (char *name, GVariant *variant) else g_free (strv[i]); } - if (j) { - strv[j] = NULL; - domain->servers = strv; - } else + if (j == 0) g_free (strv); + else { + strv[j] = NULL; + g_strfreev (domain->servers); + domain->servers = strv; + } } else if ( !g_strcmp0 (key, "options") && g_variant_is_of_type (val, G_VARIANT_TYPE ("as"))) { strv = g_variant_dup_strv (val, NULL); + g_strfreev (domain->options); domain->options = _nm_utils_strv_cleanup (strv, TRUE, TRUE, TRUE); + if (!domain->options[0]) + nm_clear_g_free (&domain->options); } g_variant_unref (val); @@ -1111,11 +1152,12 @@ nm_global_dns_config_from_dbus (const GValue *value, GError **error) else g_free (strv[i]); } - - if (strv) + if (j == 0) + g_free (strv); + else { strv[j] = NULL; - - dns_config->options = strv; + dns_config->options = strv; + } } else if ( !g_strcmp0 (key, "domains") && g_variant_is_of_type (val, G_VARIANT_TYPE ("a{sv}"))) { NMGlobalDnsDomain *domain; @@ -1145,7 +1187,7 @@ nm_global_dns_config_from_dbus (const GValue *value, GError **error) return NULL; } - global_dns_config_update_domain_list (dns_config); + global_dns_config_seal_domains (dns_config); return dns_config; } diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 8398841d39..a6b886ce43 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -254,7 +254,7 @@ test_config_global_dns (void) NMConfig *config; const NMGlobalDnsConfig *dns; NMGlobalDnsDomain *domain; - const char *const *strv; + const char *const*strv; config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", NULL); From e49a32936c6b7544750e60c6ec0816049f8fdfb9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 25 Mar 2018 14:16:17 +0200 Subject: [PATCH 10/10] all: use nm_utils_hash_keys_to_array() --- libnm-core/nm-keyfile-writer.c | 8 +--- libnm-core/nm-setting-ip-config.c | 2 +- libnm-core/nm-setting-user.c | 19 +++----- src/devices/nm-lldp-listener.c | 43 +++++++++++-------- src/dns/nm-dns-systemd-resolved.c | 12 ++---- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 11 +---- 6 files changed, 38 insertions(+), 57 deletions(-) diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c index 8af7c086e7..f92faef5bb 100644 --- a/libnm-core/nm-keyfile-writer.c +++ b/libnm-core/nm-keyfile-writer.c @@ -338,13 +338,9 @@ write_hash_of_string (GKeyFile *file, } hash = g_value_get_boxed (value); - keys = (const char **) g_hash_table_get_keys_as_array (hash, &l); - if (!keys) - return; - g_qsort_with_data (keys, l, sizeof (const char *), nm_strcmp_p_with_data, NULL); - - for (i = 0; keys[i]; i++) { + keys = nm_utils_strdict_get_keys (hash, TRUE, &l); + for (i = 0; i < l; i++) { const char *property, *data; gboolean write_item = TRUE; diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index e62e8c63ea..e581aee557 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -1129,7 +1129,7 @@ _nm_ip_route_get_attributes_direct (NMIPRoute *route) * * Returns: (array length=out_length) (transfer container): a %NULL-terminated array * of attribute names or %NULL if there are no attributes. The order of the returned - * names is undefined. + * names depends on @sorted. **/ const char ** _nm_ip_route_get_attribute_names (const NMIPRoute *route, gboolean sorted, guint *out_length) diff --git a/libnm-core/nm-setting-user.c b/libnm-core/nm-setting-user.c index 8fc96b278f..ccc030aacd 100644 --- a/libnm-core/nm-setting-user.c +++ b/libnm-core/nm-setting-user.c @@ -226,7 +226,6 @@ nm_setting_user_get_keys (NMSettingUser *setting, guint *out_len) { NMSettingUser *self = setting; NMSettingUserPrivate *priv; - guint len; g_return_val_if_fail (NM_IS_SETTING_USER (self), NULL); @@ -237,19 +236,13 @@ nm_setting_user_get_keys (NMSettingUser *setting, guint *out_len) return priv->keys; } - if (!priv->data || !g_hash_table_size (priv->data)) { - NM_SET_OUT (out_len, 0); - return (const char **) &priv->keys; - } + priv->keys = nm_utils_strdict_get_keys (priv->data, + TRUE, + out_len); - priv->keys = (const char **) g_hash_table_get_keys_as_array (priv->data, &len); - g_qsort_with_data (priv->keys, - len, - sizeof (const char *), - nm_strcmp_p_with_data, - NULL); - NM_SET_OUT (out_len, len); - return priv->keys; + /* don't return %NULL, but hijack the @keys fields as a pseudo + * empty strv array. */ + return priv->keys ?: ((const char **) &priv->keys); } /*****************************************************************************/ diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index 2ed2a7d9fa..f637825b5a 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -286,19 +286,21 @@ lldp_neighbor_id_hash (gconstpointer ptr) } static int -lldp_neighbor_id_cmp (gconstpointer a, gconstpointer b) +lldp_neighbor_id_cmp (const LldpNeighbor *x, const LldpNeighbor *y) { - const LldpNeighbor *x = a, *y = b; - int c; + NM_CMP_SELF (x, y); + NM_CMP_FIELD (x, y, chassis_id_type); + NM_CMP_FIELD (x, y, port_id_type); + NM_CMP_FIELD_STR0 (x, y, chassis_id); + NM_CMP_FIELD_STR0 (x, y, port_id); + return 0; +} - if (x->chassis_id_type != y->chassis_id_type) - return x->chassis_id_type < y->chassis_id_type ? -1 : 1; - if (x->port_id_type != y->port_id_type) - return x->port_id_type < y->port_id_type ? -1 : 1; - c = g_strcmp0 (x->chassis_id, y->chassis_id); - if (c == 0) - c = g_strcmp0 (x->port_id, y->port_id); - return c < 0 ? -1 : (c > 0 ? 1 : 0); +static int +lldp_neighbor_id_cmp_p (gconstpointer a, gconstpointer b, gpointer user_data) +{ + return lldp_neighbor_id_cmp (*((const LldpNeighbor *const*) a), + *((const LldpNeighbor *const*) b)); } static gboolean @@ -838,20 +840,23 @@ GVariant * nm_lldp_listener_get_neighbors (NMLldpListener *self) { NMLldpListenerPrivate *priv; - GVariantBuilder array_builder; - GList *neighbors, *iter; g_return_val_if_fail (NM_IS_LLDP_LISTENER (self), FALSE); priv = NM_LLDP_LISTENER_GET_PRIVATE (self); - if (!priv->variant) { + if (G_UNLIKELY (!priv->variant)) { + GVariantBuilder array_builder; + gs_free LldpNeighbor **neighbors = NULL; + guint i, n; + g_variant_builder_init (&array_builder, G_VARIANT_TYPE ("aa{sv}")); - neighbors = g_hash_table_get_keys (priv->lldp_neighbors); - neighbors = g_list_sort (neighbors, lldp_neighbor_id_cmp); - for (iter = neighbors; iter; iter = iter->next) - g_variant_builder_add_value (&array_builder, lldp_neighbor_to_variant (iter->data)); - g_list_free (neighbors); + neighbors = (LldpNeighbor **) nm_utils_hash_keys_to_array (priv->lldp_neighbors, + lldp_neighbor_id_cmp_p, + NULL, + &n); + for (i = 0; i < n; i++) + g_variant_builder_add_value (&array_builder, lldp_neighbor_to_variant (neighbors[i])); priv->variant = g_variant_ref_sink (g_variant_builder_end (&array_builder)); } return priv->variant; diff --git a/src/dns/nm-dns-systemd-resolved.c b/src/dns/nm-dns-systemd-resolved.c index 2e790d0b14..7da27e5f7f 100644 --- a/src/dns/nm-dns-systemd-resolved.c +++ b/src/dns/nm-dns-systemd-resolved.c @@ -330,14 +330,10 @@ update (NMDnsPlugin *plugin, free_pending_updates (self); - interfaces_keys = g_hash_table_get_keys_as_array (interfaces, &interfaces_len); - if (interfaces_len > 1) { - g_qsort_with_data (interfaces_keys, - interfaces_len, - sizeof (gpointer), - nm_cmp_int2ptr_p_with_data, - NULL); - } + interfaces_keys = nm_utils_hash_keys_to_array (interfaces, + nm_cmp_int2ptr_p_with_data, + NULL, + &interfaces_len); for (i = 0; i < interfaces_len; i++) { InterfaceConfig *ic = g_hash_table_lookup (interfaces, GINT_TO_POINTER (interfaces_keys[i])); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 1484dd43c8..3c97a43e6f 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -148,16 +148,7 @@ write_secrets (shvarFile *ifcfg, /* we purge all existing secrets. */ svUnsetAll (keyfile, SV_KEY_TYPE_ANY); - /* sort the keys. */ - secrets_keys = (const char **) g_hash_table_get_keys_as_array (secrets, &secrets_keys_n); - if (secrets_keys_n > 1) { - g_qsort_with_data (secrets_keys, - secrets_keys_n, - sizeof (const char *), - nm_strcmp_p_with_data, - NULL); - } - + secrets_keys = nm_utils_strdict_get_keys (secrets, TRUE, &secrets_keys_n); for (i = 0; i < secrets_keys_n; i++) { const char *k = secrets_keys[i]; const char *v = g_hash_table_lookup (secrets, k);