Commit 4a705e1a authored by Thomas Haller's avatar Thomas Haller

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.
parent 987c584b
......@@ -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);
......
......@@ -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
......
......@@ -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);
......
......@@ -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;
......
......@@ -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));
......
......@@ -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);
}
}
......
This diff is collapsed.
......@@ -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);
......
......@@ -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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment