From 51a3d8a861ae585ec12c775ef77117ff73fb9728 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 8 Dec 2017 10:40:04 +0100 Subject: [PATCH 1/4] libnm: make nm_setting_802_1x_set_private_key() self-assignment safe nmcli calls nm_setting_802_1x_set_private_key() with a password pointer that it just got from the setting connection itself. Make this less fragile, by not freeing the current password before assigning it. --- libnm-core/nm-setting-8021x.c | 85 +++++++++++++++-------------------- 1 file changed, 37 insertions(+), 48 deletions(-) diff --git a/libnm-core/nm-setting-8021x.c b/libnm-core/nm-setting-8021x.c index af195211dc..98e12ca81d 100644 --- a/libnm-core/nm-setting-8021x.c +++ b/libnm-core/nm-setting-8021x.c @@ -2247,7 +2247,7 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *setting, { NMSetting8021xPrivate *priv; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - gboolean key_cleared = FALSE, password_cleared = FALSE; + gboolean password_changed = FALSE; GError *local_err = NULL; g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE); @@ -2281,39 +2281,35 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *setting, priv = NM_SETTING_802_1X_GET_PRIVATE (setting); - /* Clear out any previous private key data */ - if (priv->private_key) { - g_bytes_unref (priv->private_key); - priv->private_key = NULL; - key_cleared = TRUE; - } - - if (priv->private_key_password) { - g_free (priv->private_key_password); - priv->private_key_password = NULL; - password_cleared = TRUE; - } - if (value == NULL) { - if (key_cleared) + if (priv->private_key) { + g_clear_pointer (&priv->private_key, g_bytes_unref); g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PRIVATE_KEY); - if (password_cleared) + } + if (nm_clear_g_free (&priv->private_key_password)) g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD); return TRUE; } - priv->private_key_password = g_strdup (password); + /* this makes password self-assignment safe. */ + if (!nm_streq0 (priv->private_key_password, password)) { + g_free (priv->private_key_password); + priv->private_key_password = g_strdup (password); + password_changed = TRUE; + } + + g_bytes_unref (priv->private_key); if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { /* FIXME: potential race after verifying the private key above */ /* FIXME: ensure blob doesn't start with file:// */ priv->private_key = file_to_secure_bytes (value); - g_assert (priv->private_key); + nm_assert (priv->private_key); } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) priv->private_key = path_to_scheme_value (value); - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11) + else { + nm_assert (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11); priv->private_key = g_bytes_new (value, strlen (value) + 1); - else - g_assert_not_reached (); + } /* As required by NM and wpa_supplicant, set the client-cert * property to the same PKCS#12 data. @@ -2326,11 +2322,10 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *setting, } g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PRIVATE_KEY); - if (password_cleared || password) + if (password_changed) g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD); - if (out_format) - *out_format = (NMSetting8021xCKFormat) format; + NM_SET_OUT (out_format, (NMSetting8021xCKFormat) format); return priv->private_key != NULL; } @@ -2594,7 +2589,7 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *setting, { NMSetting8021xPrivate *priv; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - gboolean key_cleared = FALSE, password_cleared = FALSE; + gboolean password_changed = FALSE; GError *local_err = NULL; g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE); @@ -2628,39 +2623,34 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *setting, priv = NM_SETTING_802_1X_GET_PRIVATE (setting); - /* Clear out any previous private key data */ - if (priv->phase2_private_key) { - g_bytes_unref (priv->phase2_private_key); - priv->phase2_private_key = NULL; - key_cleared = TRUE; - } - - if (priv->phase2_private_key_password) { - g_free (priv->phase2_private_key_password); - priv->phase2_private_key_password = NULL; - password_cleared = TRUE; - } - if (value == NULL) { - if (key_cleared) + if (priv->phase2_private_key) { + g_clear_pointer (&priv->phase2_private_key, g_bytes_unref); g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); - if (password_cleared) + } + if (nm_clear_g_free (&priv->phase2_private_key_password)) g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD); return TRUE; } - priv->phase2_private_key_password = g_strdup (password); + /* this makes password self-assignment safe. */ + if (!nm_streq0 (priv->phase2_private_key_password, password)) { + g_free (priv->phase2_private_key_password); + priv->phase2_private_key_password = g_strdup (password); + password_changed = TRUE; + } + if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { /* FIXME: potential race after verifying the private key above */ /* FIXME: ensure blob doesn't start with file:// */ priv->phase2_private_key = file_to_secure_bytes (value); - g_assert (priv->phase2_private_key); + nm_assert (priv->phase2_private_key); } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) priv->phase2_private_key = path_to_scheme_value (value); - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11) + else { + nm_assert (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11); priv->phase2_private_key = g_bytes_new (value, strlen (value) + 1); - else - g_assert_not_reached (); + } /* As required by NM and wpa_supplicant, set the client-cert * property to the same PKCS#12 data. @@ -2674,11 +2664,10 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *setting, } g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); - if (password_cleared || password) + if (password_changed) g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD); - if (out_format) - *out_format = (NMSetting8021xCKFormat) format; + NM_SET_OUT (out_format, (NMSetting8021xCKFormat) format); return priv->phase2_private_key != NULL; } From 97acd737bb0a4ea0d79a074a60ddad2c33606faa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 8 Dec 2017 10:44:23 +0100 Subject: [PATCH 2/4] cli: rework DEFINE_SETTER_PRIV_KEY() nmc_strsplit_set()'s max_token argument is broken, because it *first* calls g_strsplit_set() and then removes empty tokens. It wasn't an issue, because DEFINE_SETTER_PRIV_KEY() would first already remove leading spaces, and who uses multiple spaces anyway... Anyway, refactor DEFINE_SETTER_PRIV_KEY() to not use it. --- clients/common/nm-meta-setting-desc.c | 32 +++++++++++++-------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 57b4ce0922..726b336870 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -2148,29 +2148,27 @@ _get_fcn_802_1x_phase2_private_key (ARGS_GET_FCN) static gboolean \ def_func (ARGS_SET_FCN) \ { \ - char **strv = NULL; \ - char *val_strip = g_strstrip (g_strdup (value)); \ - char *p = val_strip; \ - const char *path, *password; \ + gs_free char *path = NULL; \ gs_free char *password_free = NULL; \ + char *password; \ NMSetting8021xCKScheme scheme = NM_SETTING_802_1X_CK_SCHEME_PATH; \ - gboolean success; \ \ - if (strncmp (val_strip, NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11, NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11)) == 0) \ + value = nm_str_skip_leading_spaces (value); \ + \ + if (strncmp (value, NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11, NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11)) == 0) \ scheme = NM_SETTING_802_1X_CK_SCHEME_PKCS11; \ - else if (strncmp (val_strip, NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH, NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH)) == 0) \ - p += NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH); \ + else if (strncmp (value, NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH, NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH)) == 0) \ + value += NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PATH); \ \ - strv = nmc_strsplit_set (p, " \t,", 2); \ - path = strv[0]; \ - if (g_strv_length (strv) == 2) \ - password = strv[1]; \ - else \ + path = g_strdup (value); \ + password = path + strcspn (path, " \t"); \ + if (password[0] != '\0') { \ + password[0] = '\0'; \ + while (NM_IN_SET (password[0], ' ', '\t')) \ + password++; \ + } else \ password = password_free = g_strdup (pwd_func (NM_SETTING_802_1X (setting))); \ - success = set_func (NM_SETTING_802_1X (setting), path, password, scheme, NULL, error); \ - g_free (val_strip); \ - g_strfreev (strv); \ - return success; \ + return set_func (NM_SETTING_802_1X (setting), path, password, scheme, NULL, error); \ } DEFINE_SETTER_STR_LIST_MULTI (check_and_add_eap_method, From 886994359411e08d66a6b4bc8bacb68360d5176a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 8 Dec 2017 12:51:45 +0100 Subject: [PATCH 3/4] cli: refactor splitting of first command line argument nmc_strsplit_set() handles max_token wrong. It cannot call g_strsplit_set() with max_token first, and then split empty words. You cannot use g_strsplit_set() to achieve what nmc_strsplit_set() wants to do, unless you first split all tokens, then them construct them together again -- thereby loosing the delimiters. Anyway, there are just a few caller that do essentially the same. Refactor the code to not use nmc_strsplit_set(). --- clients/cli/connections.c | 154 ++++++++++++++++++++++---------------- 1 file changed, 90 insertions(+), 64 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index b2268e59ef..2da31b0789 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -5855,50 +5855,80 @@ typedef enum { NMC_EDITOR_MAIN_CMD_QUIT, } NmcEditorMainCmd; +static void +_split_cmd (const char *cmd, char **out_arg0, const char **out_argr) +{ + gs_free char *arg0 = NULL; + const char *argr = NULL; + gsize l; + + NM_SET_OUT (out_arg0, NULL); + NM_SET_OUT (out_argr, NULL); + + if (!cmd) + return; + while (NM_IN_SET (cmd[0], ' ', '\t')) + cmd++; + if (!cmd[0]) + return; + + l = strcspn (cmd, " \t"); + arg0 = g_strndup (cmd, l); + cmd += l; + if (cmd[0]) { + while (NM_IN_SET (cmd[0], ' ', '\t')) + cmd++; + if (cmd[0]) + argr = cmd; + } + + NM_SET_OUT (out_arg0, g_steal_pointer (&arg0)); + NM_SET_OUT (out_argr, argr); +} + static NmcEditorMainCmd parse_editor_main_cmd (const char *cmd, char **cmd_arg) { NmcEditorMainCmd editor_cmd = NMC_EDITOR_MAIN_CMD_UNKNOWN; - char **vec; + gs_free char *cmd_arg0 = NULL; + const char *cmd_argr; - vec = nmc_strsplit_set (cmd, " \t", 2); - if (g_strv_length (vec) < 1) { - if (cmd_arg) - *cmd_arg = NULL; - return NMC_EDITOR_MAIN_CMD_UNKNOWN; - } + _split_cmd (cmd, &cmd_arg0, &cmd_argr); + if (!cmd_arg0) + goto fail; - if (matches (vec[0], "goto")) + if (matches (cmd_arg0, "goto")) editor_cmd = NMC_EDITOR_MAIN_CMD_GOTO; - else if (matches (vec[0], "remove")) + else if (matches (cmd_arg0, "remove")) editor_cmd = NMC_EDITOR_MAIN_CMD_REMOVE; - else if (matches (vec[0], "set")) + else if (matches (cmd_arg0, "set")) editor_cmd = NMC_EDITOR_MAIN_CMD_SET; - else if (matches (vec[0], "describe")) + else if (matches (cmd_arg0, "describe")) editor_cmd = NMC_EDITOR_MAIN_CMD_DESCRIBE; - else if (matches (vec[0], "print")) + else if (matches (cmd_arg0, "print")) editor_cmd = NMC_EDITOR_MAIN_CMD_PRINT; - else if (matches (vec[0], "verify")) + else if (matches (cmd_arg0, "verify")) editor_cmd = NMC_EDITOR_MAIN_CMD_VERIFY; - else if (matches (vec[0], "save")) + else if (matches (cmd_arg0, "save")) editor_cmd = NMC_EDITOR_MAIN_CMD_SAVE; - else if (matches (vec[0], "activate")) + else if (matches (cmd_arg0, "activate")) editor_cmd = NMC_EDITOR_MAIN_CMD_ACTIVATE; - else if (matches (vec[0], "back")) + else if (matches (cmd_arg0, "back")) editor_cmd = NMC_EDITOR_MAIN_CMD_BACK; - else if (matches (vec[0], "help") || strcmp (vec[0], "?") == 0) + else if (matches (cmd_arg0, "help") || strcmp (cmd_arg0, "?") == 0) editor_cmd = NMC_EDITOR_MAIN_CMD_HELP; - else if (matches (vec[0], "quit")) + else if (matches (cmd_arg0, "quit")) editor_cmd = NMC_EDITOR_MAIN_CMD_QUIT; - else if (matches (vec[0], "nmcli")) + else if (matches (cmd_arg0, "nmcli")) editor_cmd = NMC_EDITOR_MAIN_CMD_NMCLI; + else + goto fail; - /* set pointer to command argument */ - if (cmd_arg) - *cmd_arg = vec[1] ? g_strstrip (g_strdup (vec[1])) : NULL; - - g_strfreev (vec); + NM_SET_OUT (cmd_arg, g_strdup (cmd_argr)); return editor_cmd; +fail: + NM_SET_OUT (cmd_arg, NULL); + return NMC_EDITOR_MAIN_CMD_UNKNOWN; } static void @@ -6047,40 +6077,39 @@ static NmcEditorSubCmd parse_editor_sub_cmd (const char *cmd, char **cmd_arg) { NmcEditorSubCmd editor_cmd = NMC_EDITOR_SUB_CMD_UNKNOWN; - char **vec; + gs_free char *cmd_arg0 = NULL; + const char *cmd_argr; - vec = nmc_strsplit_set (cmd, " \t", 2); - if (g_strv_length (vec) < 1) { - if (cmd_arg) - *cmd_arg = NULL; - return NMC_EDITOR_SUB_CMD_UNKNOWN; - } + _split_cmd (cmd, &cmd_arg0, &cmd_argr); + if (!cmd_arg0) + goto fail; - if (matches (vec[0], "set")) + if (matches (cmd_arg0, "set")) editor_cmd = NMC_EDITOR_SUB_CMD_SET; - else if (matches (vec[0], "add")) + else if (matches (cmd_arg0, "add")) editor_cmd = NMC_EDITOR_SUB_CMD_ADD; - else if (matches (vec[0], "change")) + else if (matches (cmd_arg0, "change")) editor_cmd = NMC_EDITOR_SUB_CMD_CHANGE; - else if (matches (vec[0], "remove")) + else if (matches (cmd_arg0, "remove")) editor_cmd = NMC_EDITOR_SUB_CMD_REMOVE; - else if (matches (vec[0], "describe")) + else if (matches (cmd_arg0, "describe")) editor_cmd = NMC_EDITOR_SUB_CMD_DESCRIBE; - else if (matches (vec[0], "print")) + else if (matches (cmd_arg0, "print")) editor_cmd = NMC_EDITOR_SUB_CMD_PRINT; - else if (matches (vec[0], "back")) + else if (matches (cmd_arg0, "back")) editor_cmd = NMC_EDITOR_SUB_CMD_BACK; - else if (matches (vec[0], "help") || strcmp (vec[0], "?") == 0) + else if (matches (cmd_arg0, "help") || strcmp (cmd_arg0, "?") == 0) editor_cmd = NMC_EDITOR_SUB_CMD_HELP; - else if (matches (vec[0], "quit")) + else if (matches (cmd_arg0, "quit")) editor_cmd = NMC_EDITOR_SUB_CMD_QUIT; + else + goto fail; - /* set pointer to command argument */ - if (cmd_arg) - *cmd_arg = g_strdup (vec[1]); - - g_strfreev (vec); + NM_SET_OUT (cmd_arg, g_strdup (cmd_argr)); return editor_cmd; +fail: + NM_SET_OUT (cmd_arg, NULL); + return NMC_EDITOR_SUB_CMD_UNKNOWN; } static void @@ -6616,29 +6645,26 @@ property_edit_submenu (NmCli *nmc, static void split_editor_main_cmd_args (const char *str, char **setting, char **property, char **value) { - char **args, **items; + gs_free char *cmd_arg0 = NULL; + const char *cmd_argr; + const char *s; - if (!str) + NM_SET_OUT (setting, NULL); + NM_SET_OUT (property, NULL); + NM_SET_OUT (value, NULL); + + _split_cmd (str, &cmd_arg0, &cmd_argr); + if (!cmd_arg0) return; - args = nmc_strsplit_set (str, " \t", 2); - if (args[0]) { - items = nmc_strsplit_set (args[0], ".", 2); - if (g_strv_length (items) == 2) { - if (setting) - *setting = g_strdup (items[0]); - if (property) - *property = g_strdup (items[1]); - } else { - if (property) - *property = g_strdup (items[0]); - } - g_strfreev (items); - - if (value && args[1]) - *value = g_strstrip (g_strdup (args[1])); + NM_SET_OUT (value, g_strdup (cmd_argr)); + s = strchr (cmd_arg0, '.'); + if (s && s > cmd_arg0) { + NM_SET_OUT (setting, g_strndup (cmd_arg0, s - cmd_arg0)); + NM_SET_OUT (property, g_strdup (&s[1])); + } else { + NM_SET_OUT (property, cmd_arg0); } - g_strfreev (args); } static NMSetting * From 5d3736ac65f05aadd5b794f3f2595451eab3d2f7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 8 Dec 2017 10:44:36 +0100 Subject: [PATCH 4/4] 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). --- clients/cli/connections.c | 49 ++-- clients/cli/settings.c | 8 +- clients/cli/utils.c | 10 +- clients/cli/utils.h | 1 - clients/common/nm-client-utils.c | 12 - clients/common/nm-client-utils.h | 2 - clients/common/nm-meta-setting-desc.c | 390 +++++++++++++------------- 7 files changed, 223 insertions(+), 249 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 2da31b0789..a501f34320 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1555,18 +1555,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; } @@ -1608,7 +1608,6 @@ parse_preferred_connection_order (const char *order, GError **error) g_array_append_val (order_arr, val); } - g_strfreev (strv); return order_arr; } @@ -2261,48 +2260,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' */ @@ -2311,21 +2312,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 index 69275aad48..86035ba4a0 100644 --- 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 index f20e24ce09..3814e6830b 100644 --- a/clients/cli/utils.c +++ b/clients/cli/utils.c @@ -603,9 +603,14 @@ 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; @@ -613,7 +618,7 @@ nmc_string_to_arg_array (const char *line, const char *delim, gboolean unquote, 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 index ffc5b6f312..48e96e0126 100644 --- 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 index 8a28db2dd5..398c94cf16 100644 --- a/clients/common/nm-client-utils.c +++ b/clients/common/nm-client-utils.c @@ -173,18 +173,6 @@ finish: 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 index ac18fe9475..5b2ba67837 100644 --- 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, diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 726b336870..b2e8b9b4a4 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -117,7 +117,6 @@ _parse_ip_route (int family, GError **error) { const int MAX_PREFIX = (family == AF_INET) ? 32 : 128; - char *plen = NULL; const char *next_hop = NULL; const char *canon_dest; int prefix; @@ -125,9 +124,11 @@ _parse_ip_route (int family, GError *local = NULL; gint64 metric = -1; guint i; - gs_strfreev char **routev = NULL; + gs_free const char **routev = NULL; gs_free char *str_clean = NULL; - char *dest; + gs_free char *dest_clone = NULL; + const char *dest; + const char *plen; gs_unref_hashtable GHashTable *attrs = NULL; GHashTable *tmp_attrs; #define ROUTE_SYNTAX _("The valid syntax is: 'ip[/prefix] [next-hop] [metric] [attribute=val]... [,ip[/prefix] ...]'") @@ -137,8 +138,8 @@ _parse_ip_route (int family, nm_assert (!error || !*error); str_clean = g_strstrip (g_strdup (str)); - routev = nmc_strsplit_set (str_clean, " \t", 0); - if (!routev || !routev[0]) { + routev = nm_utils_strsplit_set (str_clean, " \t"); + if (!routev) { g_set_error (error, 1, 0, "'%s' is not valid. %s", str, ROUTE_SYNTAX); @@ -147,8 +148,13 @@ _parse_ip_route (int family, dest = routev[0]; plen = strchr (dest, '/'); /* prefix delimiter */ - if (plen) - *plen++ = '\0'; + if (plen) { + dest_clone = g_strdup (dest); + plen = &dest_clone[plen - dest]; + dest = dest_clone; + *((char *) plen) = '\0'; + plen++; + } prefix = MAX_PREFIX; if (plen) { if ((prefix = _nm_utils_ascii_str_to_int64 (plen, 10, 1, MAX_PREFIX, -1)) == -1) { @@ -295,7 +301,7 @@ static NMTeamLinkWatcher * _parse_team_link_watcher (const char *str, GError **error) { - gs_strfreev char **watcherv = NULL; + gs_free const char **watcherv = NULL; gs_free char *str_clean = NULL; guint i; gs_free const char *name = NULL; @@ -308,17 +314,17 @@ _parse_team_link_watcher (const char *str, nm_assert (!error || !*error); str_clean = g_strstrip (g_strdup (str)); - watcherv = nmc_strsplit_set (str_clean, " \t", 0); - if (!watcherv || !watcherv[0]) { + watcherv = nm_utils_strsplit_set (str_clean, " \t"); + if (!watcherv) { g_set_error (error, 1, 0, "'%s' is not valid", str); return NULL; } for (i = 0; watcherv[i]; i++) { - gs_strfreev char **pair = NULL; + gs_free const char **pair = NULL; - pair = nmc_strsplit_set (watcherv[i], "=", 0); - if (!pair[0]) { + pair = nm_utils_strsplit_set (watcherv[i], "="); + if (!pair) { g_set_error (error, 1, 0, "'%s' is not valid: %s", watcherv[i], "properties should be specified as 'key=value'"); return NULL; @@ -1170,22 +1176,21 @@ _set_fcn_gobject_mac (ARGS_SET_FCN) static gboolean _set_fcn_gobject_secret_flags (ARGS_SET_FCN) { - char **strv = NULL, **iter; + gs_free const char **strv = NULL; + const char **iter; unsigned long flags = 0, val_int; - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + nm_assert (!error || !*error); - strv = nmc_strsplit_set (value, " \t,", 0); + strv = nm_utils_strsplit_set (value, " \t,"); for (iter = strv; iter && *iter; iter++) { if (!nmc_string_to_uint (*iter, TRUE, 0, ALL_SECRET_FLAGS, &val_int)) { g_set_error (error, 1, 0, _("'%s' is not a valid flag number; use <0-%d>"), *iter, ALL_SECRET_FLAGS); - g_strfreev (strv); return FALSE; } flags += val_int; } - g_strfreev (strv); /* Validate the flags number */ if (flags > ALL_SECRET_FLAGS) { @@ -1572,18 +1577,19 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) const char **valid_strv, \ GError **error) \ { \ - char **strv = NULL, **iter; \ + gs_free const char **strv = NULL; \ + gsize i; \ const char *item; \ - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); \ - strv = nmc_strsplit_set (value, " \t,", 0); \ - for (iter = strv; iter && *iter; iter++) { \ - if (!(item = nmc_string_is_valid (g_strstrip (*iter), valid_strv, error))) { \ - g_strfreev (strv); \ - return FALSE; \ + nm_assert (!error || !*error); \ + strv = nm_utils_strsplit_set (value, " \t,"); \ + if (strv) { \ + for (i = 0; strv[i]; i++) { \ + if (!(item = nmc_string_is_valid (strv[i], valid_strv, error))) { \ + return FALSE; \ + } \ + set_func (s_macro (setting), item); \ } \ - set_func (s_macro (setting), item); \ } \ - g_strfreev (strv); \ return TRUE; \ } @@ -1591,43 +1597,42 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) static gboolean \ def_func (ARGS_SET_FCN) \ { \ - char **strv = NULL, **iter; \ + gs_free const char **strv = NULL; \ + const char **iter; \ const char **(*valid_func1_p) (s_type *) = valid_func1; \ const char * (*valid_func2_p) (const char *, const char *, GError **) = valid_func2; \ const char *opt_name, *opt_val; \ \ - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); \ + nm_assert (!error || !*error); \ \ - strv = nmc_strsplit_set (value, ",", 0); \ + strv = nm_utils_strsplit_set (value, ","); \ for (iter = strv; iter && *iter; iter++) { \ - char *left = g_strstrip (*iter); \ + gs_free char *left_clone = g_strstrip (g_strdup (*iter)); \ + char *left = left_clone; \ char *right = strchr (left, '='); \ if (!right) { \ g_set_error (error, 1, 0, _("'%s' is not valid; use