From cf546ee7898e955affa03992bea9c39326d363fb Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 6 May 2020 15:51:52 +0200
Subject: [PATCH] ifcfg-rh: refactor write_match_setting()

- write_match_setting() never fails. Don't let it return a boolean
  error result.

- drop "if (!name || !name[0])" checks. It's not possibly to configure
  a name %NULL in NMSettingMatch (without triggering assertions). Also,
  an empty name "" is not valid, so we wouldn't expect it. There is one
  problem with the way how we concatenate the string list: it uses
  spaces as separator, while stripping spaces. That means, in the
  currenty format, an empty token "" cannot be expressed. On the other
  hand, serializing it would lead to duplicate spaces, that get dropped
  during re-read. So the empty name wasn't valid from the start, but it
  also cannot be encoded.

- use nm_gstring_add_space_delimiter() and nm_gstring_prepare().
---
 .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c    | 68 +++++--------------
 1 file changed, 18 insertions(+), 50 deletions(-)

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 cc5cc75ab2..f3f5647dbc 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -2374,81 +2374,50 @@ write_tc_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
 	return TRUE;
 }
 
-static gboolean
-write_match_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
+static void
+write_match_setting (NMConnection *connection, shvarFile *ifcfg)
 {
+	nm_auto_free_gstring GString *str = NULL;
 	NMSettingMatch *s_match;
 	guint i, num;
+	const char *name;
 
 	s_match = (NMSettingMatch *) nm_connection_get_setting (connection, NM_TYPE_SETTING_MATCH);
 	if (!s_match)
-		return TRUE;
+		return;
 
 	num = nm_setting_match_get_num_drivers (s_match);
-	{
-		nm_auto_free_gstring GString *str = NULL;
+	if (num > 0) {
+		nm_gstring_prepare (&str);
 		for (i = 0; i < num; i++) {
-			const char *name;
-
 			name = nm_setting_match_get_driver (s_match, i);
-			if (!name || !name[0])
-				continue;
-
-			if (!str)
-				str = g_string_new ("");
-			else
-				g_string_append_c (str, ' ');
+			nm_gstring_add_space_delimiter (str);
 			nm_utils_escaped_tokens_escape_gstr (name, NM_ASCII_SPACES, str);
 		}
-
-		if (str)
-			svSetValueStr (ifcfg, "MATCH_DRIVER", str->str);
+		svSetValueStr (ifcfg, "MATCH_DRIVER", str->str);
 	}
 
 	num = nm_setting_match_get_num_interface_names (s_match);
-	{
-		nm_auto_free_gstring GString *str = NULL;
+	if (num > 0) {
+		nm_gstring_prepare (&str);
 		for (i = 0; i < num; i++) {
-			const char *name;
-
 			name = nm_setting_match_get_interface_name (s_match, i);
-			if (!name || !name[0])
-				continue;
-
-			if (!str)
-				str = g_string_new ("");
-			else
-				g_string_append_c (str, ' ');
+			nm_gstring_add_space_delimiter (str);
 			nm_utils_escaped_tokens_escape_gstr (name, NM_ASCII_SPACES, str);
 		}
-
-		if (str)
-			svSetValueStr (ifcfg, "MATCH_INTERFACE_NAME", str->str);
+		svSetValueStr (ifcfg, "MATCH_INTERFACE_NAME", str->str);
 	}
 
-
 	num = nm_setting_match_get_num_kernel_command_lines (s_match);
-	{
-		nm_auto_free_gstring GString *str = NULL;
+	if (num > 0) {
+		nm_gstring_prepare (&str);
 		for (i = 0; i < num; i++) {
-			const char *name;
-
 			name = nm_setting_match_get_kernel_command_line (s_match, i);
-			if (!name || !name[0])
-				continue;
-
-			if (!str)
-				str = g_string_new ("");
-			else
-				g_string_append_c (str, ' ');
+			nm_gstring_add_space_delimiter (str);
 			nm_utils_escaped_tokens_escape_gstr (name, NM_ASCII_SPACES, str);
 		}
-
-		if (str)
-			svSetValueStr (ifcfg, "MATCH_KERNEL_COMMAND_LINE", str->str);
+		svSetValueStr (ifcfg, "MATCH_KERNEL_COMMAND_LINE", str->str);
 	}
-
-	return TRUE;
 }
 
 static void
@@ -3208,8 +3177,7 @@ do_write_construct (NMConnection *connection,
 	if (!write_user_setting (connection, ifcfg, error))
 		return FALSE;
 
-	if (!write_match_setting (connection, ifcfg, error))
-		return FALSE;
+	write_match_setting (connection, ifcfg);
 
 	write_sriov_setting (connection, ifcfg);