cli: drop nmc_strsplit_set()

In most cases, it copies the entire strv needlessly. We can do better. Also, the max_tokens argument is handled wrongly (albeit not used anywhere anymore).
NetworkManager · Dec 8, 2017 · ccef603 · ccef603
1 parent 5e66596
commit ccef603
Show file tree

Hide file tree

Showing 7 changed files with 223 additions and 249 deletions.
diff --git a/clients/cli/connections.c b/clients/cli/connections.c
@@ -1554,18 +1554,18 @@ get_invisible_active_connections (NmCli *nmc)
 static GArray *
 parse_preferred_connection_order (const char *order, GError **error)
 {
-	char **strv, **iter;
+	gs_free const char **strv = NULL;
+	const char *const*iter;
 	const char *str;
 	GArray *order_arr;
 	NmcSortOrder val;
 	gboolean inverse, unique;
 	int i;
 
-	strv = nmc_strsplit_set (order, ":", -1);
-	if (!strv || !*strv) {
+	strv = nm_utils_strsplit_set (order, ":");
+	if (!strv) {
 		g_set_error (error, NMCLI_ERROR, 0,
 		             _("incorrect string '%s' of '--order' option"), order);
-		g_strfreev (strv);
 		return NULL;
 	}
 
@@ -1607,7 +1607,6 @@ parse_preferred_connection_order (const char *order, GError **error)
 			g_array_append_val (order_arr, val);
 	}
 
-	g_strfreev (strv);
 	return order_arr;
 }
 
@@ -2260,48 +2259,50 @@ activate_connection_cb (GObject *client, GAsyncResult *result, gpointer user_dat
 static GHashTable *
 parse_passwords (const char *passwd_file, GError **error)
 {
-	GHashTable *pwds_hash;
-	char *contents = NULL;
+	gs_unref_hashtable GHashTable *pwds_hash = NULL;
+	gs_free char *contents = NULL;
 	gsize len = 0;
 	GError *local_err = NULL;
-	char **lines, **iter;
+	gs_free const char **strv = NULL;
+	const char *const*iter;
 	char *pwd_spec, *pwd, *prop;
 	const char *setting;
 
 	pwds_hash = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_free);
 
 	if (!passwd_file)
-		return pwds_hash;
+		return g_steal_pointer (&pwds_hash);
 
-        /* Read the passwords file */
+	/* Read the passwords file */
 	if (!g_file_get_contents (passwd_file, &contents, &len, &local_err)) {
 		g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
 		             _("failed to read passwd-file '%s': %s"),
 		             passwd_file, local_err->message);
 		g_error_free (local_err);
-		g_hash_table_destroy (pwds_hash);
 		return NULL;
 	}
 
-	lines = nmc_strsplit_set (contents, "\r\n", -1);
-	for (iter = lines; *iter; iter++) {
-		pwd = strchr (*iter, ':');
+	strv = nm_utils_strsplit_set (contents, "\r\n");
+	for (iter = strv; *iter; iter++) {
+		gs_free char *iter_s = g_strdup (*iter);
+
+		pwd = strchr (iter_s, ':');
 		if (!pwd) {
 			g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
 			             _("missing colon in 'password' entry '%s'"), *iter);
-			goto failure;
+			return NULL;
 		}
 		*(pwd++) = '\0';
 
-		prop = strchr (*iter, '.');
+		prop = strchr (iter_s, '.');
 		if (!prop) {
 			g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
 			             _("missing dot in 'password' entry '%s'"), *iter);
-			goto failure;
+			return NULL;
 		}
 		*(prop++) = '\0';
 
-		setting = *iter;
+		setting = iter_s;
 		while (g_ascii_isspace (*setting))
 			setting++;
 		/* Accept wifi-sec or wifi instead of cumbersome '802-11-wireless-security' */
@@ -2310,21 +2311,13 @@ parse_passwords (const char *passwd_file, GError **error)
 		if (nm_setting_lookup_type (setting) == G_TYPE_INVALID) {
 			g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
 			             _("invalid setting name in 'password' entry '%s'"), setting);
-			goto failure;
+			return NULL;
 		}
 
 		pwd_spec = g_strdup_printf ("%s.%s", setting, prop);
 		g_hash_table_insert (pwds_hash, pwd_spec, g_strdup (pwd));
 	}
-	g_strfreev (lines);
-	g_free (contents);
-	return pwds_hash;
-
-failure:
-	g_strfreev (lines);
-	g_free (contents);
-	g_hash_table_destroy (pwds_hash);
-	return NULL;
+	return g_steal_pointer (&pwds_hash);
 }
 
 

diff --git a/clients/cli/settings.c b/clients/cli/settings.c
@@ -313,16 +313,18 @@ _set_fcn_precheck_connection_secondaries (const char *value,
 {
 	const GPtrArray *connections;
 	NMConnection *con;
+	gs_free const char **strv0 = NULL;
 	gs_strfreev char **strv = NULL;
 	char **iter;
-	gboolean modified;
+	gboolean modified = FALSE;
 
-	strv = nmc_strsplit_set (value, " \t,", 0);
-	if (!strv)
+	strv0 = nm_utils_strsplit_set (value, " \t,");
+	if (!strv0)
 		return TRUE;
 
 	connections = nm_client_get_connections (nm_cli.client);
 
+	strv = g_strdupv ((char **) strv0);
 	for (iter = strv; *iter; iter++) {
 		if (nm_utils_is_uuid (*iter)) {
 			con = nmc_find_connection (connections, "uuid", *iter, NULL, FALSE);

diff --git a/clients/cli/utils.c b/clients/cli/utils.c
@@ -603,17 +603,22 @@ int
 nmc_string_to_arg_array (const char *line, const char *delim, gboolean unquote,
                          char ***argv, int *argc)
 {
+	gs_free const char **arr0 = NULL;
 	char **arr;
 
-	arr = nmc_strsplit_set (line ? line : "", delim ? delim : " \t", 0);
+	arr0 = nm_utils_strsplit_set (line ?: "", delim ?: " \t");
+	if (!arr0)
+		arr = g_new0 (char *, 1);
+	else
+		arr = g_strdupv ((char **) arr0);
 
 	if (unquote) {
 		int i = 0;
 		char *s;
 		size_t l;
 		const char *quotes = "\"'";
 
-		while (arr && arr[i]) {
+		while (arr[i]) {
 			s = arr[i];
 			l = strlen (s);
 			if (l >= 2) {
@@ -628,7 +633,6 @@ nmc_string_to_arg_array (const char *line, const char *delim, gboolean unquote,
 
 	*argv = arr;
 	*argc = g_strv_length (arr);
-
 	return 0;
 }
 

diff --git a/clients/cli/utils.h b/clients/cli/utils.h
@@ -53,7 +53,6 @@ int nmc_string_to_arg_array (const char *line, const char *delim, gboolean unquo
                              char ***argv, int *argc);
 const char *nmc_string_is_valid (const char *input, const char **allowed, GError **error);
 char * nmc_util_strv_for_display (const char *const*strv, gboolean brackets);
-char **nmc_strsplit_set (const char *str, const char *delimiter, int max_tokens);
 int nmc_string_screen_width (const char *start, const char *end);
 void set_val_str  (NmcOutputField fields_array[], guint32 index, char *value);
 void set_val_strc (NmcOutputField fields_array[], guint32 index, const char *value);

diff --git a/clients/common/nm-client-utils.c b/clients/common/nm-client-utils.c
@@ -173,18 +173,6 @@ nmc_string_is_valid (const char *input, const char **allowed, GError **error)
 	return ret;
 }
 
-/*
- * Wrapper function for g_strsplit_set() that removes empty strings
- * from the vector as they are not useful in most cases.
- */
-char **
-nmc_strsplit_set (const char *str, const char *delimiter, int max_tokens)
-{
-	/* remove empty strings */
-	return _nm_utils_strv_cleanup (g_strsplit_set (str, delimiter, max_tokens),
-	                               FALSE, TRUE, FALSE);
-}
-
 gboolean
 matches (const char *cmd, const char *pattern)
 {

diff --git a/clients/common/nm-client-utils.h b/clients/common/nm-client-utils.h
@@ -32,8 +32,6 @@ typedef enum {
 
 const char *nmc_string_is_valid (const char *input, const char **allowed, GError **error);
 
-char **nmc_strsplit_set (const char *str, const char *delimiter, int max_tokens);
-
 gboolean nmc_string_to_uint (const char *str,
                              gboolean range_check,
                              unsigned long int min,