ibft: cleanup read_connections()

4 files changed, 54 insertions, 60 deletions

diff --git a/src/settings/plugins/ibft/nms-ibft-plugin.c b/src/settings/plugins/ibft/nms-ibft-plugin.c
index 00b25068fc..47051995c1 100644
--- a/src/settings/plugins/ibft/nms-ibft-plugin.c
+++ b/src/settings/plugins/ibft/nms-ibft-plugin.c
@@ -64,31 +64,30 @@ static void
 read_connections (NMSIbftPlugin *self)
 {
 	NMSIbftPluginPrivate *priv = NMS_IBFT_PLUGIN_GET_PRIVATE (self);
-	GSList *blocks = NULL, *iter;
-	GError *error = NULL;
+	nm_auto_free_ibft_blocks GSList *blocks = NULL;
+	GSList *iter;
+	gs_free_error GError *error = NULL;
 	NMSIbftConnection *connection;
 
 	if (!nms_ibft_reader_load_blocks ("/sbin/iscsiadm", &blocks, &error)) {
 		nm_log_dbg (LOGD_SETTINGS, "ibft: failed to read iscsiadm records: %s", error->message);
-		g_error_free (error);
 		return;
 	}
 
 	for (iter = blocks; iter; iter = iter->next) {
 		connection = nms_ibft_connection_new (iter->data, &error);
-		if (connection) {
-			nm_log_info (LOGD_SETTINGS, "ibft: read connection '%s'",
-			             nm_settings_connection_get_id (NM_SETTINGS_CONNECTION (connection)));
-			g_hash_table_insert (priv->connections,
-			                     g_strdup (nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (connection))),
-			                     connection);
-		} else {
+		if (!connection) {
 			nm_log_warn (LOGD_SETTINGS, "ibft: failed to read iscsiadm record: %s", error->message);
 			g_clear_error (&error);
+			continue;
 		}
-	}
 
-	g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref);
+		nm_log_info (LOGD_SETTINGS, "ibft: read connection '%s'",
+		             nm_settings_connection_get_id (NM_SETTINGS_CONNECTION (connection)));
+		g_hash_table_insert (priv->connections,
+		                     g_strdup (nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (connection))),
+		                     connection);
+	}
 }
 
 static GSList *
diff --git a/src/settings/plugins/ibft/nms-ibft-reader.c b/src/settings/plugins/ibft/nms-ibft-reader.c
index c6c1437651..fde3038321 100644
--- a/src/settings/plugins/ibft/nms-ibft-reader.c
+++ b/src/settings/plugins/ibft/nms-ibft-reader.c
@@ -46,8 +46,7 @@ remove_most_whitespace (const char *src)
 	char *s_new, *s2;
 	const char *svalue;
 
-	while (*src && g_ascii_isspace (*src))
-		src++;
+	src = nm_str_skip_leading_spaces (src);
 
 	svalue = strchr (src, '=');
 	if (!svalue || svalue == src)
@@ -94,24 +93,25 @@ nms_ibft_reader_load_blocks (const char *iscsiadm_path,
 {
 	const char *argv[4] = { iscsiadm_path, "-m", "fw", NULL };
 	const char *envp[1] = { NULL };
-	GSList *blocks = NULL;
-	char *out = NULL, *err = NULL;
-	int status = 0;
-	char **lines = NULL, **iter;
+	nm_auto_free_ibft_blocks GSList *blocks = NULL;
+	gs_free char *out = NULL;
+	gs_free char *err = NULL;
+	gs_free const char **lines = NULL;
 	GPtrArray *block_lines = NULL;
-	gboolean success = FALSE;
+	gsize i;
+	int status = 0;
 
 	g_return_val_if_fail (iscsiadm_path != NULL, FALSE);
 	g_return_val_if_fail (out_blocks != NULL && *out_blocks == NULL, FALSE);
 
 	if (!g_spawn_sync ("/", (char **) argv, (char **) envp, 0,
 	                   NULL, NULL, &out, &err, &status, error))
-		goto done;
+		return FALSE;
 
 	if (!WIFEXITED (status)) {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 		             "iBFT: %s exited abnormally.", iscsiadm_path);
-		goto done;
+		return FALSE;
 	}
 
 	if (WEXITSTATUS (status) != 0) {
@@ -127,59 +127,47 @@ nms_ibft_reader_load_blocks (const char *iscsiadm_path,
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 		             "iBFT: %s exited with error %d.  Message: '%s'",
 		             iscsiadm_path, WEXITSTATUS (status), err ?: "(none)");
-		goto done;
+		return FALSE;
 	}
 
 	nm_log_dbg (LOGD_SETTINGS, "iBFT records:\n%s", out);
 
-	lines = g_strsplit_set (out, "\n\r", -1);
-	for (iter = lines; iter && *iter; iter++) {
-		if (!*iter[0])
-			continue;
+	lines = nm_utils_strsplit_set (out, "\n\r");
+	for (i = 0; lines && lines[i]; i++) {
+		const char *ss = lines[i];
 
-		if (!g_ascii_strncasecmp (*iter, TAG_BEGIN, NM_STRLEN (TAG_BEGIN))) {
+		if (!g_ascii_strncasecmp (ss, TAG_BEGIN, NM_STRLEN (TAG_BEGIN))) {
 			if (block_lines) {
 				PARSE_WARNING ("malformed iscsiadm record: missing END RECORD.");
-				g_ptr_array_unref (block_lines);
+				nm_clear_pointer (&block_lines, g_ptr_array_unref);
 			}
 			/* Start new record */
 			block_lines = g_ptr_array_new_full (15, g_free);
-		} else if (!g_ascii_strncasecmp (*iter, TAG_END, NM_STRLEN (TAG_END))) {
+		} else if (!g_ascii_strncasecmp (ss, TAG_END, NM_STRLEN (TAG_END))) {
 			if (block_lines) {
 				if (block_lines->len)
-					blocks = g_slist_prepend (blocks, block_lines);
+					blocks = g_slist_prepend (blocks, g_steal_pointer (&block_lines));
 				else
-					g_ptr_array_unref (block_lines);
-				block_lines = NULL;
+					g_ptr_array_unref (g_steal_pointer (&block_lines));
 			}
 		} else if (block_lines) {
-			char *s = remove_most_whitespace (*iter);
+			char *s = remove_most_whitespace (ss);
 
-			if (s)
+			if (!s) {
+				PARSE_WARNING ("malformed iscsiadm record: no = in '%s'.", ss);
+				nm_clear_pointer (&block_lines, g_ptr_array_unref);
+			} else
 				g_ptr_array_add (block_lines, s);
-			else {
-				PARSE_WARNING ("malformed iscsiadm record: no = in '%s'.", *iter);
-				g_clear_pointer (&block_lines, g_ptr_array_unref);
-			}
 		}
 	}
 
 	if (block_lines) {
 		PARSE_WARNING ("malformed iscsiadm record: missing # END RECORD.");
-		g_clear_pointer (&block_lines, g_ptr_array_unref);
+		nm_clear_pointer (&block_lines, g_ptr_array_unref);
 	}
-	success = TRUE;
-
-done:
-	if (lines)
-		g_strfreev (lines);
-	g_free (out);
-	g_free (err);
-	if (success)
-		*out_blocks = blocks;
-	else
-		g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref);
-	return success;
+
+	*out_blocks = g_steal_pointer (&blocks);
+	return TRUE;
 }
 
 #define ISCSI_HWADDR_TAG     "iface.hwaddress"
diff --git a/src/settings/plugins/ibft/nms-ibft-reader.h b/src/settings/plugins/ibft/nms-ibft-reader.h
index 27500cc50b..baa81e99c4 100644
--- a/src/settings/plugins/ibft/nms-ibft-reader.h
+++ b/src/settings/plugins/ibft/nms-ibft-reader.h
@@ -23,6 +23,14 @@
 
 #include "nm-connection.h"
 
+static inline void
+_nm_auto_free_ibft_blocks (GSList **p_blocks)
+{
+	if (*p_blocks)
+		g_slist_free_full (*p_blocks, (GDestroyNotify) g_ptr_array_unref);
+}
+#define nm_auto_free_ibft_blocks nm_auto (_nm_auto_free_ibft_blocks)
+
 gboolean nms_ibft_reader_load_blocks (const char *iscsiadm_path,
                                       GSList **out_blocks,
                                       GError **error);
diff --git a/src/settings/plugins/ibft/tests/test-ibft.c b/src/settings/plugins/ibft/tests/test-ibft.c
index 4c45f574d2..f5b584a17b 100644
--- a/src/settings/plugins/ibft/tests/test-ibft.c
+++ b/src/settings/plugins/ibft/tests/test-ibft.c
@@ -40,16 +40,16 @@
 static GPtrArray *
 read_block (const char *iscsiadm_path, const char *expected_mac)
 {
-	GSList *blocks = NULL, *iter;
+	nm_auto_free_ibft_blocks GSList *blocks = NULL;
+	GSList *iter;
 	GPtrArray *block = NULL;
 	GError *error = NULL;
 	gboolean success;
 
 	success = nms_ibft_reader_load_blocks (iscsiadm_path, &blocks, &error);
-	g_assert_no_error (error);
-	g_assert (success);
-	g_assert (blocks);
+	nmtst_assert_success (success, error);
 
+	g_assert (blocks);
 	for (iter = blocks; iter; iter = iter->next) {
 		const char *s_hwaddr = NULL;
 
@@ -63,7 +63,6 @@ read_block (const char *iscsiadm_path, const char *expected_mac)
 	}
 	g_assert (block);
 
-	g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref);
 	return block;
 }
 
@@ -176,7 +175,7 @@ static void
 test_read_ibft_malformed (gconstpointer user_data)
 {
 	const char *iscsiadm_path = user_data;
-	GSList *blocks = NULL;
+	nm_auto_free_ibft_blocks GSList *blocks = NULL;
 	GError *error = NULL;
 	gboolean success;
 
@@ -185,9 +184,9 @@ test_read_ibft_malformed (gconstpointer user_data)
 	NMTST_EXPECT_NM_WARN ("*malformed iscsiadm record*");
 
 	success = nms_ibft_reader_load_blocks (iscsiadm_path, &blocks, &error);
-	g_assert_no_error (error);
-	g_assert (success);
-	g_assert (blocks == NULL);
+	nmtst_assert_success (success, error);
+
+	g_assert (!blocks);
 
 	g_test_assert_expected_messages ();
 }