settings/ifupdown: use c-list for data structure of ifupdown parser

We already have a linked-list implementation. Use it.

src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c

@@ -32,12 +32,6 @@
 
 #include "nm-utils.h"
 
-struct _if_parser {
-	if_block* first;
-	if_block* last;
-	if_data* last_data;
-};
-
 /*****************************************************************************/
 
 static void _ifparser_source (if_parser *parser, const char *path, const char *en_dir, int quiet, int dir);
@@ -47,45 +41,39 @@ static void _ifparser_source (if_parser *parser, const char *path, const char *e
 static void
 add_block (if_parser *parser, const char *type, const char* name)
 {
-	if_block *ret = g_slice_new0 (struct _if_block);
-	ret->name = g_strdup (name);
-	ret->type = g_strdup (type);
-	if (parser->first == NULL)
-		parser->first = parser->last = ret;
-	else {
-		parser->last->next = ret;
-		parser->last = ret;
-	}
-	parser->last_data = NULL;
+	if_block *ifb;
+
+	ifb = g_slice_new (if_block);
+	ifb->name = g_strdup (name);
+	ifb->type = g_strdup (type);
+	c_list_init (&ifb->data_lst_head);
+	c_list_link_tail (&parser->block_lst_head, &ifb->block_lst);
 }
 
 static void
 add_data (if_parser *parser, const char *key, const char *data)
 {
-	if_data *ret;
+	if_block *last_block;
+	if_data *ifd;
 	char *idx;
 
+	last_block = c_list_last_entry (&parser->block_lst_head, if_block, block_lst);
+
 	/* Check if there is a block where we can attach our data */
-	if (parser->first == NULL)
+	if (!last_block)
 		return;
 
-	ret = g_slice_new0 (struct _if_data);
-	ret->key = g_strdup (key);
+	ifd = g_slice_new (if_data);
+	ifd->key = g_strdup (key);
+	ifd->data = g_strdup (data);
 
 	/* Normalize keys. Convert '_' to '-', as ifupdown accepts both variants.
 	 * When querying keys via ifparser_getkey(), use '-'. */
-	while ((idx = strrchr (ret->key, '_'))) {
-		*idx = '-';
-	}
-	ret->data = g_strdup (data);
+	idx = ifd->key;
+	while ((idx = strchr (idx, '_')))
+		*(idx++) = '-';
 
-	if (parser->last->info == NULL) {
-		parser->last->info = ret;
-		parser->last_data = ret;
-	} else {
-		parser->last_data->next = ret;
-		parser->last_data = parser->last_data->next;
-	}
+	c_list_link_tail (&last_block->data_lst_head, &ifd->data_lst);
 }
 
 /* join values in src with spaces into dst;  dst needs to be large enough */
@@ -309,7 +297,8 @@ ifparser_parse (const char *eni_file, int quiet)
 {
 	if_parser *parser;
 
-	parser = g_slice_new0 (if_parser);
+	parser = g_slice_new (if_parser);
+	c_list_init (&parser->block_lst_head);
 	_recursive_ifparser (parser, eni_file, quiet);
 	return parser;
 }
@@ -317,61 +306,68 @@ ifparser_parse (const char *eni_file, int quiet)
 static void
 _destroy_data (if_data *ifd)
 {
-	if (ifd == NULL)
-		return;
-	_destroy_data (ifd->next);
+	c_list_unlink_stale (&ifd->data_lst);
 	g_free (ifd->key);
 	g_free (ifd->data);
-	g_slice_free (struct _if_data, ifd);
-	return;
+	g_slice_free (if_data, ifd);
 }
 
 static void
 _destroy_block (if_block* ifb)
 {
-	if (ifb == NULL)
-		return;
-	_destroy_block (ifb->next);
-	_destroy_data (ifb->info);
+	if_data *ifd;
+
+	while ((ifd = c_list_first_entry (&ifb->data_lst_head, if_data, data_lst)))
+		_destroy_data (ifd);
+	c_list_unlink_stale (&ifb->block_lst);
 	g_free (ifb->name);
 	g_free (ifb->type);
-	g_slice_free (struct _if_block, ifb);
-	return;
+	g_slice_free (if_block, ifb);
 }
 
 void
 ifparser_destroy (if_parser *parser)
 {
-	_destroy_block (parser->first);
+	if_block *ifb;
+
+	while ((ifb = c_list_first_entry (&parser->block_lst_head, if_block, block_lst)))
+		_destroy_block (ifb);
 	g_slice_free (if_parser, parser);
 }
 
-if_block *ifparser_getfirst (if_parser *parser)
+if_block *
+ifparser_getfirst (if_parser *parser)
 {
-	return parser->first;
+	return c_list_first_entry (&parser->block_lst_head, if_block, block_lst);
 }
 
-int ifparser_get_num_blocks (if_parser *parser)
+guint
+ifparser_get_num_blocks (if_parser *parser)
 {
-	int i = 0;
-	if_block *iter = parser->first;
-
-	while (iter) {
-		i++;
-		iter = iter->next;
-	}
-	return i;
+	return c_list_length (&parser->block_lst_head);
 }
 
 if_block *
 ifparser_getif (if_parser *parser, const char* iface)
 {
-	if_block *curr = parser->first;
-	while (curr != NULL) {
-		if (   nm_streq (curr->type, "iface")
-		    && nm_streq (curr->name, iface))
-			return curr;
-		curr = curr->next;
+	if_block *ifb;
+
+	c_list_for_each_entry (ifb, &parser->block_lst_head, block_lst) {
+		if (   nm_streq (ifb->type, "iface")
+		    && nm_streq (ifb->name, iface))
+			return ifb;
+	}
+	return NULL;
+}
+
+static if_data *
+ifparser_findkey (if_block* iface, const char *key)
+{
+	if_data *ifd;
+
+	c_list_for_each_entry (ifd, &iface->data_lst_head, data_lst) {
+		if (nm_streq (ifd->key, key))
+			return ifd;
 	}
 	return NULL;
 }
@@ -379,37 +375,20 @@ ifparser_getif (if_parser *parser, const char* iface)
 const char *
 ifparser_getkey (if_block* iface, const char *key)
 {
-	if_data *curr = iface->info;
-	while (curr != NULL) {
-		if (nm_streq (curr->key, key))
-			return curr->data;
-		curr = curr->next;
-	}
-	return NULL;
+	if_data *ifd;
+
+	ifd = ifparser_findkey (iface, key);
+	return ifd ? ifd->data : NULL;
 }
 
 gboolean
 ifparser_haskey (if_block* iface, const char *key)
 {
-	if_data *curr = iface->info;
-
-	while (curr != NULL) {
-		if (nm_streq (curr->key, key))
-			return TRUE;
-		curr = curr->next;
-	}
-	return FALSE;
+	return !!ifparser_findkey (iface, key);
 }
 
-int
+guint
 ifparser_get_num_info (if_block* iface)
 {
-	int i = 0;
-	if_data *iter = iface->info;
-
-	while (iter) {
-		i++;
-		iter = iter->next;
-	}
-	return i;
+	return c_list_length (&iface->data_lst_head);
 }

src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h

@@ -23,20 +23,24 @@
 #ifndef _INTERFACE_PARSER_H
 #define _INTERFACE_PARSER_H
 
-typedef struct _if_data {
+#include "c-list/src/c-list.h"
+
+typedef struct {
+	CList data_lst;
 	char *key;
 	char *data;
-	struct _if_data *next;
 } if_data;
 
-typedef struct _if_block {
+typedef struct {
+	CList block_lst;
+	CList data_lst_head;
 	char *type;
 	char *name;
-	if_data *info;
-	struct _if_block *next;
 } if_block;
 
-typedef struct _if_parser if_parser;
+typedef struct {
+	CList block_lst_head;
+} if_parser;
 
 if_parser *ifparser_parse (const char *eni_file, int quiet);
 
@@ -48,7 +52,8 @@ if_block *ifparser_getif (if_parser *parser, const char* iface);
 if_block *ifparser_getfirst (if_parser *parser);
 const char *ifparser_getkey (if_block* iface, const char *key);
 gboolean ifparser_haskey (if_block* iface, const char *key);
-int ifparser_get_num_blocks (if_parser *parser);
-int ifparser_get_num_info (if_block* iface);
+
+guint ifparser_get_num_blocks (if_parser *parser);
+guint ifparser_get_num_info (if_block* iface);
 
 #endif

src/settings/plugins/ifupdown/nms-ifupdown-parser.c

@@ -49,24 +49,24 @@
 static const char*
 _ifupdownplugin_guess_connection_type (if_block *block)
 {
-	if_data *curr = block->info;
-	const char* ret_type = NULL;
-	const char* value = ifparser_getkey (block, "inet");
+	const char *ret_type = NULL;
 
-	if (nm_streq0 (value, "ppp"))
+	if(nm_streq0 (ifparser_getkey (block, "inet"), "ppp"))
 		ret_type = NM_SETTING_PPP_SETTING_NAME;
+	else {
+		if_data *ifb;
 
-	while (!ret_type && curr) {
-		if (   _str_has_prefix (curr->key, "wireless-", FALSE)
-		    || _str_has_prefix (curr->key, "wpa-", FALSE)) {
-			ret_type = NM_SETTING_WIRELESS_SETTING_NAME;
+		c_list_for_each_entry (ifb, &block->data_lst_head, data_lst) {
+			if (   _str_has_prefix (ifb->key, "wireless-", FALSE)
+			    || _str_has_prefix (ifb->key, "wpa-", FALSE)) {
+				ret_type = NM_SETTING_WIRELESS_SETTING_NAME;
+				break;
+			}
 		}
-		curr = curr->next;
+		if(!ret_type)
+			ret_type = NM_SETTING_WIRED_SETTING_NAME;
 	}
 
-	if (!ret_type)
-		ret_type = NM_SETTING_WIRED_SETTING_NAME;
-
 	nm_log_info (LOGD_SETTINGS, "guessed connection type (%s) = %s", block->name, ret_type);
 	return ret_type;
 }
@@ -93,8 +93,8 @@ static void
 update_wireless_setting_from_if_block (NMConnection *connection,
                                        if_block *block)
 {
-	if_data *curr = block->info;
-	const char* value = ifparser_getkey (block, "inet");
+	if_data *curr;
+	const char *value = ifparser_getkey (block, "inet");
 	struct _Mapping mapping[] = {
 		{"ssid", "ssid"},
 		{"essid", "ssid"},
@@ -110,7 +110,7 @@ update_wireless_setting_from_if_block (NMConnection *connection,
 	nm_log_info (LOGD_SETTINGS, "update wireless settings (%s).", block->name);
 	wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ());
 
-	while (curr) {
+	c_list_for_each_entry (curr, &block->data_lst_head, data_lst) {
 		if (_str_has_prefix (curr->key, "wireless-", TRUE)) {
 			const char* newkey = map_by_mapping (mapping, curr->key + NM_STRLEN ("wireless-"));
 
@@ -156,7 +156,6 @@ update_wireless_setting_from_if_block (NMConnection *connection,
 				nm_log_info (LOGD_SETTINGS, "setting wpa newkey(%s)=data(%s)", newkey, curr->data);
 			}
 		}
-		curr = curr->next;
 	}
 	nm_connection_add_setting (connection, (NMSetting*) wireless_setting);
 }
@@ -248,7 +247,7 @@ static void
 update_wireless_security_setting_from_if_block (NMConnection *connection,
                                                 if_block *block)
 {
-	if_data *curr = block->info;
+	if_data *curr;
 	const char* value = ifparser_getkey (block, "inet");
 	struct _Mapping mapping[] = {
 		{"psk", "psk"},
@@ -314,7 +313,7 @@ update_wireless_security_setting_from_if_block (NMConnection *connection,
 	nm_log_info (LOGD_SETTINGS, "update wireless security settings (%s).", block->name);
 	wireless_security_setting = NM_SETTING_WIRELESS_SECURITY (nm_setting_wireless_security_new ());
 
-	while (curr) {
+	c_list_for_each_entry (curr, &block->data_lst_head, data_lst) {
 		if (_str_has_prefix (curr->key, "wireless-", TRUE)) {
 			const char *key = curr->key + NM_STRLEN ("wireless-");
 			char *property_value = NULL;
@@ -391,7 +390,7 @@ wireless_next:
 				(*free_func) (typed_property_value);
 		}
 	next:
-		curr = curr->next;
+		;
 	}
 
 	if (security)

src/settings/plugins/ifupdown/nms-ifupdown-plugin.c

@@ -158,9 +158,9 @@ initialize (NMSettingsPlugin *plugin)
 	const char *block_name;
 	NMIfupdownConnection *conn;
 
-	/* Read in all the interfaces */
 	parser = ifparser_parse (ENI_INTERFACES_FILE, 0);
-	for (block = ifparser_getfirst (parser); block; block = block->next) {
+
+	c_list_for_each_entry (block, &parser->block_lst_head, block_lst) {
 
 		if (NM_IN_STRSET (block->type, "auto", "allow-hotplug")) {
 			if (!auto_ifaces)

src/settings/plugins/ifupdown/tests/test-ifupdown.c

@@ -137,9 +137,8 @@ compare_expected_to_ifparser (if_parser *parser, Expected *e)
 
 	g_assert_cmpint (g_slist_length (e->blocks), ==, ifparser_get_num_blocks (parser));
 
-	for (n = ifparser_getfirst (parser), biter = e->blocks;
-	     n && biter;
-	     n = n->next, biter = g_slist_next (biter)) {
+	biter = e->blocks;
+	c_list_for_each_entry (n, &parser->block_lst_head, block_lst) {
 		if_data *m;
 		ExpectedBlock *b = biter->data;
 
@@ -150,17 +149,22 @@ compare_expected_to_ifparser (if_parser *parser, Expected *e)
 
 		g_assert_cmpint (g_slist_length (b->keys), ==, ifparser_get_num_info (n));
 
-		for (m = n->info, kiter = b->keys;
-		     m && kiter;
-		     m = m->next, kiter = g_slist_next (kiter)) {
+		kiter = b->keys;
+		c_list_for_each_entry (m, &n->data_lst_head, data_lst) {
 			ExpectedKey *k = kiter->data;
 
 			g_assert (k->key && m->key);
 			g_assert_cmpstr (k->key, ==, m->key);
 			g_assert (k->data && m->data);
 			g_assert_cmpstr (k->data, ==, m->data);
+
+			kiter = g_slist_next (kiter);
 		}
+		g_assert (!kiter);
+
+		biter = g_slist_next (biter);
 	}
+	g_assert (!biter);
 }
 
 static void
@@ -169,7 +173,7 @@ dump_blocks (if_parser *parser)
 	if_block *n;
 
 	g_message ("\n***************************************************");
-	for (n = ifparser_getfirst (parser); n != NULL; n = n->next) {
+	c_list_for_each_entry (n, &parser->block_lst_head, block_lst) {
 		if_data *m;
 
 		// each block start with its type & name
@@ -178,8 +182,8 @@ dump_blocks (if_parser *parser)
 
 		// each key-value pair within a block is indented & separated by a tab
 		// (single quotes used to show typ & name baoundaries)
-		for (m = n->info; m != NULL; m = m->next)
-			   g_print("\t'%s'\t'%s'\n", m->key, m->data);
+		c_list_for_each_entry (m, &n->data_lst_head, data_lst)
+			g_print("\t'%s'\t'%s'\n", m->key, m->data);
 
 		// blocks are separated by an empty line
 		g_print("\n");
@@ -465,7 +469,7 @@ test17_read_static_ipv4 (void)
 	connection = nm_simple_connection_new();
 	g_assert (connection);
 
-	ifupdown_update_connection_from_if_block(connection, block, &error);
+	ifupdown_update_connection_from_if_block (connection, block, &error);
 	g_assert_no_error (error);
 
 	success = nm_connection_verify (connection, &error);
@@ -519,7 +523,7 @@ test18_read_static_ipv6 (void)
 	block = ifparser_getfirst (parser);
 	connection = nm_simple_connection_new();
 	g_assert (connection);
-	ifupdown_update_connection_from_if_block(connection, block, &error);
+	ifupdown_update_connection_from_if_block (connection, block, &error);
 	g_assert_no_error (error);
 
 	success = nm_connection_verify (connection, &error);
@@ -571,7 +575,7 @@ test19_read_static_ipv4_plen (void)
 	block = ifparser_getfirst (parser);
 	connection = nm_simple_connection_new();
 	g_assert (connection);
-	ifupdown_update_connection_from_if_block(connection, block, &error);
+	ifupdown_update_connection_from_if_block (connection, block, &error);
 	g_assert_no_error (error);
 
 	success = nm_connection_verify (connection, &error);