fixup! wake-on-lan: add option to keep existing settings

Initscripts does:
    oldifs=$IFS;
    IFS=';';
    [ -n "${ETHTOOL_DELAY}" ] && /bin/usleep ${ETHTOOL_DELAY}
    for opts in $ETHTOOL_OPTS ; do
        IFS=$oldifs;
        if [[ "${opts}" =~ [[:space:]]*- ]]; then
            /sbin/ethtool $opts
        else
            /sbin/ethtool -s ${REALDEVICE} $opts
        fi
        IFS=';';
    done
    IFS=$oldifs;

thus, we want to split on ';', otherwise we parse
  "wol d;something else"
wrong.

Also, g_strsplit_set() returns multiple empty tokens. So
we must skip over empty tokens in case of "wol    d".

The @use_password was wrong, because we would warn if sopass is specified
before wol:
  "sopass AA:BB:CC:DD:EE:FF wol g"

More resilently handle wrong configurations:
  "wol pu wol m" => gives m.
  "wol pu wol" => should give NONE and warn (instead of "pu").

Also accept tab as separator.

2 files changed, 81 insertions, 36 deletions

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index 82bcde0400..dc4e61acde 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -3550,26 +3550,49 @@ wireless_connection_from_ifcfg (const char *file,
 }
 
 static void
-parse_ethtool_options (shvarFile *ifcfg, NMSettingWired *s_wired, char *value)
+parse_ethtool_option (const char *value, NMSettingWiredWakeOnLan *out_flags, char **out_password)
 {
-	NMSettingWiredWakeOnLan wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_IGNORE;
-	gboolean use_password = FALSE;
-	char **words = NULL, **iter = NULL, *flag;
+	gs_strfreev char **words = NULL;
+	const char **iter = NULL, *flag;
+	gboolean has_flags = FALSE;
+	gboolean has_password = FALSE;
 
-	if (!value)
+	if (!value || !value[0])
 		return;
 
-	if (value[0]) {
-		words = g_strsplit_set (value, " ", 0);
-		iter = words;
-	}
+	words = g_strsplit_set (value, "\t ", 0);
+	iter = (const char **) words;
+
+	while (iter[0]) {
+		gboolean is_wol;
+
+		if (g_str_equal (iter[0], "wol"))
+			is_wol = TRUE;
+		else if (g_str_equal (iter[0], "sopass"))
+			is_wol = FALSE;
+		else {
+			/* Silently skip unknown options */
+			iter++;
+			continue;
+		}
+
+		iter++;
+
+		/* g_strsplit_set() returns empty tokens, meaning that we must skip over repeated
+		 * space characters like to parse "wol     d". */
+		while (iter[0] && !*iter[0])
+			iter++;
 
-	while (iter && iter[0]) {
-		if (g_str_equal (iter[0], "wol") && iter[1] && *iter[1]) {
+		if (is_wol) {
+			NMSettingWiredWakeOnLan wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_NONE;
+			has_flags = TRUE;
 
-			wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_NONE;
+			if (!iter[0]) {
+				PARSE_WARNING ("Wake-on-LAN options missing");
+				break;
+			}
 
-			for (flag = iter[1]; *flag; flag++) {
+			for (flag = iter[0]; *flag; flag++) {
 				switch (*flag) {
 				case 'p':
 					wol_flags |= NM_SETTING_WIRED_WAKE_ON_LAN_PHY;
@@ -3590,42 +3613,65 @@ parse_ethtool_options (shvarFile *ifcfg, NMSettingWired *s_wired, char *value)
 					wol_flags |= NM_SETTING_WIRED_WAKE_ON_LAN_MAGIC;
 					break;
 				case 's':
-					use_password = TRUE;
 					break;
 				case 'd':
 					wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_NONE;
-					use_password = FALSE;
 					break;
 				default:
 					PARSE_WARNING ("unrecognized Wake-on-LAN option '%c'", *flag);
 				}
 			}
 
-			if (!NM_FLAGS_HAS (wol_flags, NM_SETTING_WIRED_WAKE_ON_LAN_MAGIC))
-				use_password = FALSE;
+			*out_flags = wol_flags;
+		} else {
+			has_password = TRUE;
 
-			iter += 2;
-			continue;
-		}
+			if (!iter[0]) {
+				PARSE_WARNING ("Wake-on-LAN password missing");
+				break;
+			}
 
-		if (g_str_equal (iter[0], "sopass") && iter[1] && *iter[1]) {
-			if (use_password) {
-				if (nm_utils_hwaddr_valid (iter[1], ETH_ALEN))
-					g_object_set (s_wired, NM_SETTING_WIRED_WAKE_ON_LAN_PASSWORD, iter[1], NULL);
-				else
-					PARSE_WARNING ("Wake-on-LAN password '%s' is invalid", iter[1]);
-			} else
-				PARSE_WARNING ("Wake-on-LAN password not expected");
-			iter += 2;
-			continue;
-		}
 
-		/* Silently skip unknown options */
+			g_clear_pointer (out_password, g_free);
+			if (nm_utils_hwaddr_valid (iter[0], ETH_ALEN))
+				*out_password = g_strdup (iter[0]);
+			else
+				PARSE_WARNING ("Wake-on-LAN password '%s' is invalid", iter[0]);
+		}
 		iter++;
 	}
+}
+
+static void
+parse_ethtool_options (shvarFile *ifcfg, NMSettingWired *s_wired, const char *value)
+{
+	NMSettingWiredWakeOnLan wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_DEFAULT;
+	gs_free char *wol_password = NULL;
+	gboolean ignore_wol_password = FALSE;
+
+	if (value) {
+		gs_strfreev char **opts = NULL;
+		const char **iter;
+
+		wol_flags = NM_SETTING_WIRED_WAKE_ON_LAN_IGNORE;
+
+		opts = g_strsplit_set (value, ";", 0);
+		for (iter = (const char **) opts; iter[0]; iter++) {
+			/* in case of repeated wol_passwords, parse_ethtool_option()
+			 * will do the right thing and clear wol_password before resetting. */
+			parse_ethtool_option (iter[0], &wol_flags, &wol_password);
+		}
+	}
 
-	g_object_set (s_wired, NM_SETTING_WIRED_WAKE_ON_LAN, wol_flags, NULL);
-	g_strfreev (words);
+	if (   wol_password
+	    && !NM_FLAGS_HAS (wol_flags, NM_SETTING_WIRED_WAKE_ON_LAN_MAGIC)) {
+		PARSE_WARNING ("Wake-on-LAN password not expected");
+		ignore_wol_password = TRUE;
+	}
+	g_object_set (s_wired,
+	              NM_SETTING_WIRED_WAKE_ON_LAN, wol_flags,
+	              NM_SETTING_WIRED_WAKE_ON_LAN_PASSWORD, ignore_wol_password ? NULL : wol_password,
+	              NULL);
 }
 
 static NMSetting *
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 2e8b68f3ea..f75247b190 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -6291,8 +6291,7 @@ test_write_wired_static (void)
 	g_assert_cmpint (nm_setting_ip_config_get_route_metric (reread_s_ip4), ==, 204);
 	g_assert_cmpint (nm_setting_ip_config_get_route_metric (reread_s_ip6), ==, 206);
 
-	ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE,
-	        "wired-static-write", "written and re-read connection weren't the same.");
+	nmtst_assert_connection_equals (connection, FALSE, reread, FALSE);
 
 	route6file = utils_get_route6_path (testfile);
 	unlink (route6file);