ifcfg-rh: ensure keys/route files are monitored for changes too

keys- and route- files weren't passing the should_ignore_file()
check in dir_changed() because should_ignore_file() was only
taking ifcfg- files into account.  Generalize most of the ifcfg name
handling functions so that should_ignore_file() will now handle
all three variants.

Add testcases to ensure that the name handling does what we want
it to, and optimize memory usage of utils_get_ifcfg_name() a bit.

system-settings/plugins/ifcfg-rh/common.h

@@ -26,6 +26,7 @@
 #define IFCFG_TAG "ifcfg-"
 #define KEYS_TAG "keys-"
 #define ROUTE_TAG "route-"
+
 #define BAK_TAG ".bak"
 #define TILDE_TAG "~"
 #define ORIG_TAG ".orig"

system-settings/plugins/ifcfg-rh/plugin.c

@@ -45,6 +45,7 @@
 #include "nm-inotify-helper.h"
 #include "shvar.h"
 #include "writer.h"
+#include "utils.h"
 
 static void system_config_interface_init (NMSystemConfigInterface *system_config_interface_class);
 
@@ -153,44 +154,6 @@ read_one_connection (SCPluginIfcfg *plugin, const char *filename)
 	return connection;
 }
 
-static gboolean
-check_suffix (const char *base, const char *tag)
-{
-	int len, tag_len;
-
-	g_return_val_if_fail (base != NULL, TRUE);
-	g_return_val_if_fail (tag != NULL, TRUE);
-
-	len = strlen (base);
-	tag_len = strlen (tag);
-	if ((len > tag_len) && !strcasecmp (base + len - tag_len, tag))
-		return TRUE;
-	return FALSE;
-}
-
-static gboolean
-should_ignore_file (const char *filename)
-{
-	char *base;
-	gboolean ignore = TRUE;
-
-	g_return_val_if_fail (filename != NULL, TRUE);
-
-	base = g_path_get_basename (filename);
-	g_return_val_if_fail (base != NULL, TRUE);
-
-	if (   !strncmp (base, IFCFG_TAG, strlen (IFCFG_TAG))
-		&& !check_suffix (base, BAK_TAG)
-		&& !check_suffix (base, TILDE_TAG)
-		&& !check_suffix (base, ORIG_TAG)
-		&& !check_suffix (base, REJ_TAG)
-		&& !check_suffix (base, RPMNEW_TAG))
-		ignore = FALSE;
-
-	g_free (base);
-	return ignore;
-}
-
 static void
 read_connections (SCPluginIfcfg *plugin)
 {
@@ -204,7 +167,7 @@ read_connections (SCPluginIfcfg *plugin)
 		while ((item = g_dir_read_name (dir))) {
 			char *full_path;
 
-			if (should_ignore_file (item))
+			if (utils_should_ignore_file (item, TRUE))
 				continue;
 
 			full_path = g_build_filename (IFCFG_DIR, item, NULL);
@@ -331,7 +294,6 @@ handle_connection_remove_or_new (SCPluginIfcfg *plugin,
 		}
 	}
 }
-
 static void
 dir_changed (GFileMonitor *monitor,
 		   GFile *file,
@@ -341,16 +303,20 @@ dir_changed (GFileMonitor *monitor,
 {
 	SCPluginIfcfg *plugin = SC_PLUGIN_IFCFG (user_data);
 	SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (plugin);
-	char *name;
+	char *path, *name;
 	NMIfcfgConnection *connection;
 	gboolean do_remove = FALSE, do_new = FALSE;
 
-	name = g_file_get_path (file);
+	path = g_file_get_path (file);
-	if (should_ignore_file (name)) {
+	if (utils_should_ignore_file (path, FALSE)) {
-		g_free (name);
+		g_free (path);
 		return;
 	}
 
+	/* Given any ifcfg, keys, or routes file, get the ifcfg file path */
+	name = utils_get_ifcfg_path (path);
+	g_free (path);
+
 	connection = g_hash_table_lookup (priv->connections, name);
 	if (!connection) {
 		do_new = TRUE;

system-settings/plugins/ifcfg-rh/reader.c

@@ -111,11 +111,11 @@ make_connection_setting (const char *file,
                          const char *suggested)
 {
 	NMSettingConnection *s_con;
-	char *ifcfg_name = NULL;
+	const char *ifcfg_name = NULL;
 	char *new_id = NULL, *uuid = NULL, *value;
 	char *ifcfg_id;
 
-	ifcfg_name = utils_get_ifcfg_name (file);
+	ifcfg_name = utils_get_ifcfg_name (file, TRUE);
 	if (!ifcfg_name)
 		return NULL;
 
@@ -180,7 +180,6 @@ make_connection_setting (const char *file,
 		g_free (value);
 	}
 
-	g_free (ifcfg_name);
 	return NM_SETTING (s_con);
 }
 
@@ -2583,7 +2582,7 @@ connection_from_file (const char *filename,
 	shvarFile *parsed;
 	char *type, *nmc = NULL, *bootproto;
 	NMSetting *s_ip4;
-	char *ifcfg_name = NULL;
+	const char *ifcfg_name = NULL;
 	gboolean nm_controlled = TRUE;
 
 	g_return_val_if_fail (filename != NULL, NULL);
@@ -2601,13 +2600,12 @@ connection_from_file (const char *filename,
 	if (!iscsiadm_path)
 		iscsiadm_path = SBINDIR "/iscsiadm";
 
-	ifcfg_name = utils_get_ifcfg_name (filename);
+	ifcfg_name = utils_get_ifcfg_name (filename, TRUE);
 	if (!ifcfg_name) {
 		g_set_error (error, ifcfg_plugin_error_quark (), 0,
 		             "Ignoring connection '%s' because it's not an ifcfg file.", filename);
 		return NULL;
 	}
-	g_free (ifcfg_name);
 
 	parsed = svNewFile (filename);
 	if (!parsed) {

system-settings/plugins/ifcfg-rh/tests/Makefile.am

@@ -6,7 +6,7 @@ INCLUDES = \
 	-I$(top_srcdir)/libnm-glib \
 	-I$(top_srcdir)/system-settings/plugins/ifcfg-rh
 
-noinst_PROGRAMS = test-ifcfg-rh
+noinst_PROGRAMS = test-ifcfg-rh test-ifcfg-rh-utils
 
 test_ifcfg_rh_SOURCES = \
 	test-ifcfg-rh.c
@@ -23,9 +23,19 @@ test_ifcfg_rh_LDADD = \
 	$(top_builddir)/system-settings/plugins/ifcfg-rh/libifcfg-rh-io.la \
 	$(DBUS_LIBS)
 
+test_ifcfg_rh_utils_SOURCES = \
+	test-ifcfg-rh-utils.c
+
+test_ifcfg_rh_utils_CPPFLAGS = \
+	$(GLIB_CFLAGS)
+
+test_ifcfg_rh_utils_LDADD = \
+	$(top_builddir)/system-settings/plugins/ifcfg-rh/libifcfg-rh-io.la
+
 if WITH_TESTS
 
 check-local: test-ifcfg-rh
+	$(abs_builddir)/test-ifcfg-rh-utils
 	$(abs_builddir)/test-ifcfg-rh
 
 endif

system-settings/plugins/ifcfg-rh/utils.c

@@ -115,59 +115,136 @@ utils_hexstr2bin (const char *hex, size_t len)
 
 /* End from hostap */
 
-char *
+static gboolean
-utils_cert_path (const char *parent, const char *suffix)
+check_suffix (const char *base, const char *tag)
 {
-	char *name, *dir, *path;
+	int len, tag_len;
 
-	name = utils_get_ifcfg_name (parent);
+	g_return_val_if_fail (base != NULL, TRUE);
-	dir = g_path_get_dirname (parent);
+	g_return_val_if_fail (tag != NULL, TRUE);
-	path = g_strdup_printf ("%s/%s-%s", dir, name, suffix);
+
-	g_free (dir);
+	len = strlen (base);
-	g_free (name);
+	tag_len = strlen (tag);
-	return path;
+	if ((len > tag_len) && !strcasecmp (base + len - tag_len, tag))
+		return TRUE;
+	return FALSE;
+}
+
+gboolean
+utils_should_ignore_file (const char *filename, gboolean only_ifcfg)
+{
+	char *base;
+	gboolean ignore = TRUE;
+	gboolean is_ifcfg = FALSE;
+	gboolean is_other = FALSE;
+
+	g_return_val_if_fail (filename != NULL, TRUE);
+
+	base = g_path_get_basename (filename);
+	g_return_val_if_fail (base != NULL, TRUE);
+
+	/* Only handle ifcfg, keys, and routes files */
+	if (!strncmp (base, IFCFG_TAG, strlen (IFCFG_TAG)))
+		is_ifcfg = TRUE;
+
+	if (only_ifcfg == FALSE) {
+		if (   !strncmp (base, KEYS_TAG, strlen (KEYS_TAG))
+		    || !strncmp (base, ROUTE_TAG, strlen (ROUTE_TAG)))
+				is_other = TRUE;
+	}
+
+	/* But not those that have certain suffixes */
+	if (   (is_ifcfg || is_other)
+	    && !check_suffix (base, BAK_TAG)
+	    && !check_suffix (base, TILDE_TAG)
+	    && !check_suffix (base, ORIG_TAG)
+	    && !check_suffix (base, REJ_TAG)
+	    && !check_suffix (base, RPMNEW_TAG))
+		ignore = FALSE;
+
+	g_free (base);
+	return ignore;
 }
 
 char *
-utils_get_ifcfg_name (const char *file)
+utils_cert_path (const char *parent, const char *suffix)
 {
-	char *ifcfg_name;
+	const char *name;
+	char *dir, *path;
+
+	g_return_val_if_fail (parent != NULL, NULL);
+	g_return_val_if_fail (suffix != NULL, NULL);
+
+	name = utils_get_ifcfg_name (parent, FALSE);
+	dir = g_path_get_dirname (parent);
+	path = g_strdup_printf ("%s/%s-%s", dir, name, suffix);
+	g_free (dir);
+	return path;
+}
+
+const char *
+utils_get_ifcfg_name (const char *file, gboolean only_ifcfg)
+{
+	const char *name = NULL, *start = NULL;
 	char *base;
 
+	g_return_val_if_fail (file != NULL, NULL);
+
 	base = g_path_get_basename (file);
 	if (!base)
 		return NULL;
 
-	ifcfg_name = g_strdup (base + strlen (IFCFG_TAG));
+	/* Find the point in 'file' where 'base' starts.  We use 'file' since it's
+	 * const and thus will survive after we free 'base'.
+	 */
+	start = file + strlen (file) - strlen (base);
+	g_assert (strcmp (start, base) == 0);
 	g_free (base);
-	return ifcfg_name;
+
+	if (!strncmp (start, IFCFG_TAG, strlen (IFCFG_TAG)))
+		name = start + strlen (IFCFG_TAG);
+	else if (only_ifcfg == FALSE)  {
+		if (!strncmp (start, KEYS_TAG, strlen (KEYS_TAG)))
+			name = start + strlen (KEYS_TAG);
+		else if (!strncmp (start, ROUTE_TAG, strlen (ROUTE_TAG)))
+			name = start + strlen (ROUTE_TAG);
+	}
+
+	return name;
 }
 
-/* Used to get an extra file path for ifcfg-<name> in the form <tag><name>.
+/* Used to get any ifcfg/extra file path from any other ifcfg/extra path
- * Currently used for: keys-<name>
+ * in the form <tag><name>.
- *                     route-<name>
  */
-char *
+static char *
 utils_get_extra_path (const char *parent, const char *tag)
 {
-	char *ifcfg_name;
+	char *item_path = NULL, *dirname;
-	char *extra_file = NULL;
+	const char *name;
-	char *tmp = NULL;
 
-	ifcfg_name = utils_get_ifcfg_name (parent);
+	g_return_val_if_fail (parent != NULL, NULL);
-	if (!ifcfg_name)
+	g_return_val_if_fail (tag != NULL, NULL);
+
+	dirname = g_path_get_dirname (parent);
+	if (!dirname)
 		return NULL;
 
-	tmp = g_path_get_dirname (parent);
+	name = utils_get_ifcfg_name (parent, FALSE);
-	if (!tmp)
+	if (name) {
-		goto out;
+		if (!strcmp (dirname, "."))
+			item_path = g_strdup_printf ("%s%s", tag, name);
+		else
+			item_path = g_strdup_printf ("%s/%s%s", dirname, tag, name);
+	}
+	g_free (dirname);
 
-	extra_file = g_strdup_printf ("%s/%s%s", tmp, tag, ifcfg_name);
+	return item_path;
+}
 
-out:
+char *
-	g_free (tmp);
+utils_get_ifcfg_path (const char *parent)
-	g_free (ifcfg_name);
+{
-	return extra_file;
+	return utils_get_extra_path (parent, IFCFG_TAG);
 }
 
 char *

system-settings/plugins/ifcfg-rh/utils.h

@@ -31,9 +31,11 @@ char *utils_hexstr2bin (const char *hex, size_t len);
 
 char *utils_cert_path (const char *parent, const char *suffix);
 
-char *utils_get_ifcfg_name (const char *file);
+const char *utils_get_ifcfg_name (const char *file, gboolean only_ifcfg);
 
-char *utils_get_extra_path (const char *parent, const char *tag);
+gboolean utils_should_ignore_file (const char *filename, gboolean only_ifcfg);
+
+char *utils_get_ifcfg_path (const char *parent);
 char *utils_get_keys_path (const char *parent);
 char *utils_get_route_path (const char *parent);