From 700a32e5ddd95cd2e11ee1faaaa97177d39fd27a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 13:09:09 +0200 Subject: [PATCH 01/18] cli: fix memleak in nm_vpn_openconnect_authenticate_helper() --- clients/common/nm-vpn-helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index ea905972f5..fbac26d82e 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -203,7 +203,7 @@ nm_vpn_openconnect_authenticate_helper (const char *host, int *status, GError **error) { - char *output = NULL; + gs_free char *output = NULL; gboolean ret; char **strv = NULL, **iter; char *argv[4]; From 84f2037648a68693faa7fa1768743921439113c9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Apr 2019 15:44:19 +0200 Subject: [PATCH 02/18] shared: add flags argument to nm_utils_strsplit_set() It will be useful to extend nm_utils_strsplit_set() with various flavors and subtly different behaviors. Add a flags argument to support these. --- clients/cli/connections.c | 4 +- clients/cli/settings.c | 2 +- clients/cli/utils.c | 3 +- clients/common/nm-meta-setting-desc.c | 24 ++++----- libnm-core/nm-setting-bridge.c | 2 +- libnm-core/nm-utils.c | 2 +- libnm-core/tests/test-general.c | 10 +++- shared/nm-utils/nm-shared-utils.c | 23 ++++---- shared/nm-utils/nm-shared-utils.h | 16 +++++- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 54 +++++++++---------- 10 files changed, 83 insertions(+), 57 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 9a71d3944d..97c46923ae 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1876,7 +1876,7 @@ parse_preferred_connection_order (const char *order, GError **error) gboolean inverse, unique; int i; - strv = nm_utils_strsplit_set (order, ":", FALSE); + strv = nm_utils_strsplit_set (order, ":"); if (!strv) { g_set_error (error, NMCLI_ERROR, 0, _("incorrect string '%s' of '--order' option"), order); @@ -2680,7 +2680,7 @@ parse_passwords (const char *passwd_file, GError **error) return NULL; } - strv = nm_utils_strsplit_set (contents, "\r\n", FALSE); + strv = nm_utils_strsplit_set (contents, "\r\n"); for (iter = strv; *iter; iter++) { gs_free char *iter_s = g_strdup (*iter); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index d050e24e28..70348dbfec 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -338,7 +338,7 @@ _set_fcn_precheck_connection_secondaries (NMClient *client, char **iter; gboolean modified = FALSE; - strv0 = nm_utils_strsplit_set (value, " \t,", FALSE); + strv0 = nm_utils_strsplit_set (value, " \t,"); if (!strv0) return TRUE; diff --git a/clients/cli/utils.c b/clients/cli/utils.c index 0e74ba8867..a8b81279ed 100644 --- a/clients/cli/utils.c +++ b/clients/cli/utils.c @@ -508,7 +508,8 @@ nmc_string_to_arg_array (const char *line, const char *delim, gboolean unquote, gs_free const char **arr0 = NULL; char **arr; - arr0 = nm_utils_strsplit_set (line ?: "", delim ?: " \t", FALSE); + arr0 = nm_utils_strsplit_set (line ?: "", + delim ?: " \t"); if (!arr0) arr = g_new0 (char *, 1); else diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index d47e1bd67e..25337cc3a3 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -197,19 +197,19 @@ _value_strsplit (const char *value, /* note that all modes remove empty tokens (",", "a,,b", ",,"). */ switch (split_mode) { case VALUE_STRSPLIT_MODE_STRIPPED: - strv = nm_utils_strsplit_set (value, NM_ASCII_SPACES",", FALSE); + strv = nm_utils_strsplit_set (value, NM_ASCII_SPACES","); break; case VALUE_STRSPLIT_MODE_OBJLIST: - strv = nm_utils_strsplit_set (value, ",", FALSE); + strv = nm_utils_strsplit_set (value, ","); break; case VALUE_STRSPLIT_MODE_OBJLIST_WITH_ESCAPE: - strv = nm_utils_strsplit_set (value, ",", TRUE); + strv = nm_utils_strsplit_set_full (value, ",", NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); break; case VALUE_STRSPLIT_MODE_MULTILIST: - strv = nm_utils_strsplit_set (value, " \t,", FALSE); + strv = nm_utils_strsplit_set (value, " \t,"); break; case VALUE_STRSPLIT_MODE_MULTILIST_WITH_ESCAPE: - strv = nm_utils_strsplit_set (value, MULTILIST_WITH_ESCAPE_CHARS, TRUE); + strv = nm_utils_strsplit_set_full (value, MULTILIST_WITH_ESCAPE_CHARS, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); break; default: nm_assert_not_reached (); @@ -306,7 +306,7 @@ _parse_ip_route (int family, nm_assert (!error || !*error); str_clean = nm_strstrip_avoid_copy_a (300, str, &str_clean_free); - routev = nm_utils_strsplit_set (str_clean, " \t", FALSE); + routev = nm_utils_strsplit_set (str_clean, " \t"); if (!routev) { g_set_error (error, 1, 0, "'%s' is not valid. %s", @@ -480,7 +480,7 @@ _parse_team_link_watcher (const char *str, nm_assert (!error || !*error); str_clean = nm_strstrip_avoid_copy_a (300, str, &str_clean_free); - watcherv = nm_utils_strsplit_set (str_clean, " \t", FALSE); + watcherv = nm_utils_strsplit_set (str_clean, " \t"); if (!watcherv) { g_set_error (error, 1, 0, "'%s' is not valid", str); return NULL; @@ -489,7 +489,7 @@ _parse_team_link_watcher (const char *str, for (i = 0; watcherv[i]; i++) { gs_free const char **pair = NULL; - pair = nm_utils_strsplit_set (watcherv[i], "=", FALSE); + 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'"); @@ -1935,7 +1935,7 @@ _set_fcn_optionlist (ARGS_SET_FCN) return _gobject_property_reset_default (setting, property_info->property_name); nstrv = 0; - strv = nm_utils_strsplit_set (value, ",", FALSE); + strv = nm_utils_strsplit_set (value, ","); if (strv) { strv_val = g_new (const char *, NM_PTRARRAY_LEN (strv)); for (i = 0; strv[i]; i++) { @@ -2187,7 +2187,7 @@ _set_fcn_gobject_bytes (ARGS_SET_FCN) } /* Otherwise, consider the following format: AA b 0xCc D */ - strv = nm_utils_strsplit_set (value, " \t", FALSE); + strv = nm_utils_strsplit_set (value, " \t"); array = g_byte_array_sized_new (NM_PTRARRAY_LEN (strv)); for (iter = strv; iter && *iter; iter++) { int v; @@ -2797,7 +2797,7 @@ _set_fcn_dcb_flags (ARGS_SET_FCN) const char *const*iter; /* Check for individual flag numbers */ - strv = nm_utils_strsplit_set (value, " \t,", FALSE); + strv = nm_utils_strsplit_set (value, " \t,"); for (iter = strv; iter && *iter; iter++) { t = _nm_utils_ascii_str_to_int64 (*iter, 0, 0, DCB_ALL_FLAGS, -1); @@ -3948,7 +3948,7 @@ _set_fcn_wired_s390_subchannels (ARGS_SET_FCN) if (_SET_FCN_DO_RESET_DEFAULT (property_info, modifier, value)) return _gobject_property_reset_default (setting, property_info->property_name); - strv = nm_utils_strsplit_set (value, " ,\t", FALSE); + strv = nm_utils_strsplit_set (value, " ,\t"); len = NM_PTRARRAY_LEN (strv); if (len != 2 && len != 3) { g_set_error (error, 1, 0, _("'%s' is not valid; 2 or 3 strings should be provided"), diff --git a/libnm-core/nm-setting-bridge.c b/libnm-core/nm-setting-bridge.c index f87abd0654..590f6ea2bd 100644 --- a/libnm-core/nm-setting-bridge.c +++ b/libnm-core/nm-setting-bridge.c @@ -432,7 +432,7 @@ nm_bridge_vlan_from_str (const char *str, GError **error) g_return_val_if_fail (str, NULL); g_return_val_if_fail (!error || !*error, NULL); - tokens = nm_utils_strsplit_set (str, " ", FALSE); + tokens = nm_utils_strsplit_set (str, " "); if (!tokens || !tokens[0]) { g_set_error_literal (error, NM_CONNECTION_ERROR, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index cf7ddb08f9..0d9118adee 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -2849,7 +2849,7 @@ _nm_sriov_vf_parse_vlans (NMSriovVF *vf, const char *str, GError **error) gs_free const char **vlans = NULL; guint i; - vlans = nm_utils_strsplit_set (str, ";", FALSE); + vlans = nm_utils_strsplit_set (str, ";"); if (!vlans) { g_set_error_literal (error, NM_CONNECTION_ERROR, diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 765957135a..4063dfcf20 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -276,7 +276,15 @@ _do_test_nm_utils_strsplit_set (gboolean escape, const char *str, ...) args = (const char *const*) args_array->pdata; - words = nm_utils_strsplit_set (str, " \t\n", escape); + if (!escape && nmtst_get_rand_bool ()) + words = nm_utils_strsplit_set (str, " \t\n"); + else { + words = nm_utils_strsplit_set_full (str, + " \t\n", + escape + ? NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING + : NM_UTILS_STRSPLIT_SET_FLAGS_NONE); + } if (!args[0]) { g_assert (!words); diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 5f317e4144..13a1a35f6e 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -972,11 +972,11 @@ _char_lookup_table_init (guint8 lookup[static 256], } /** - * nm_utils_strsplit_set: + * nm_utils_strsplit_set_full: * @str: the string to split. * @delimiters: the set of delimiters. If %NULL, defaults to " \t\n", * like bash's $IFS. - * @allow_escaping: whether delimiters can be escaped by a backslash + * @flags: additional flags for controlling the operation. * * This is a replacement for g_strsplit_set() which avoids copying * each word once (the entire strv array), but instead copies it once @@ -985,8 +985,8 @@ _char_lookup_table_init (guint8 lookup[static 256], * Another difference from g_strsplit_set() is that this never returns * empty words. Multiple delimiters are combined and treated as one. * - * If @allow_escaping is %TRUE, delimiters prefixed by a backslash are - * not treated as a separator. Such delimiters and their escape + * If @flags has %NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING, delimiters prefixed + * by a backslash are not treated as a separator. Such delimiters and their escape * character are copied to the current word without unescaping them. * * Returns: %NULL if @str is %NULL or contains only delimiters. @@ -1000,7 +1000,9 @@ _char_lookup_table_init (guint8 lookup[static 256], * like "g_strstrip((char *) iter[0])". */ const char ** -nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_escaping) +nm_utils_strsplit_set_full (const char *str, + const char *delimiters, + NMUtilsStrsplitSetFlags flags) { const char **ptr, **ptr0; gsize alloc_size, plen, i; @@ -1009,6 +1011,7 @@ nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_e char *s; guint8 delimiters_table[256]; gboolean escaped = FALSE; + const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); if (!str) return NULL; @@ -1033,7 +1036,7 @@ nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_e /* skip initial delimiters, and return of the remaining string is * empty. */ - while (_is_delimiter (str[0], delimiters_table, allow_escaping, escaped)) + while (_is_delimiter (str[0], delimiters_table, f_allow_escaping, escaped)) next_char (str, escaped); if (!str[0]) @@ -1067,11 +1070,11 @@ nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_e ptr[plen++] = s; - nm_assert (s[0] && !_is_delimiter (s[0], delimiters_table, allow_escaping, escaped)); + nm_assert (s[0] && !_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)); while (TRUE) { next_char (s, escaped); - if (_is_delimiter (s[0], delimiters_table, allow_escaping, escaped)) + if (_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)) break; if (s[0] == '\0') goto done; @@ -1079,7 +1082,7 @@ nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_e s[0] = '\0'; next_char (s, escaped); - while (_is_delimiter (s[0], delimiters_table, allow_escaping, escaped)) + while (_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)) next_char (s, escaped); if (s[0] == '\0') break; @@ -2272,7 +2275,7 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid) state = p[0]; - tokens = nm_utils_strsplit_set (p, " ", FALSE); + tokens = nm_utils_strsplit_set (p, " "); if (NM_PTRARRAY_LEN (tokens) < 20) goto fail; diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 95ed5c946f..75ef429dd7 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -332,7 +332,21 @@ int nm_utils_dbus_path_cmp (const char *dbus_path_a, const char *dbus_path_b); /*****************************************************************************/ -const char **nm_utils_strsplit_set (const char *str, const char *delimiters, gboolean allow_escaping); +typedef enum { + NM_UTILS_STRSPLIT_SET_FLAGS_NONE = 0, + NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING = (1u << 0), +} NMUtilsStrsplitSetFlags; + +const char **nm_utils_strsplit_set_full (const char *str, + const char *delimiter, + NMUtilsStrsplitSetFlags flags); + +static inline const char ** +nm_utils_strsplit_set (const char *str, + const char *delimiters) +{ + return nm_utils_strsplit_set_full (str, delimiters, NM_UTILS_STRSPLIT_SET_FLAGS_NONE); +} char *nm_utils_str_simpletokens_extract_next (char **p_line_start); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index f9722b8f37..8d39f827ca 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -457,7 +457,7 @@ make_connection_setting (const char *file, if (v) { gs_free const char **items = NULL; - items = nm_utils_strsplit_set (v, " ", FALSE); + items = nm_utils_strsplit_set (v, " "); for (iter = items; iter && *iter; iter++) { if (!nm_setting_connection_add_permission (s_con, "user", *iter, NULL)) PARSE_WARNING ("invalid USERS item '%s'", *iter); @@ -473,7 +473,7 @@ make_connection_setting (const char *file, if (v) { gs_free const char **items = NULL; - items = nm_utils_strsplit_set (v, " \t", FALSE); + items = nm_utils_strsplit_set (v, " \t"); for (iter = items; iter && *iter; iter++) { if (!nm_setting_connection_add_secondary (s_con, *iter)) PARSE_WARNING ("secondary connection UUID '%s' already added", *iter); @@ -889,7 +889,7 @@ parse_route_line (const char *line, * Maybe later we want to support some form of quotation here. * Which of course, would be incompatible with initscripts. */ - words_free = nm_utils_strsplit_set (line, " \t\n", FALSE); + words_free = nm_utils_strsplit_set (line, " \t\n"); words = words_free ?: NM_PTRARRAY_EMPTY (const char *); @@ -1327,7 +1327,7 @@ parse_dns_options (NMSettingIPConfig *ip_config, const char *value) if (!nm_setting_ip_config_has_dns_options (ip_config)) nm_setting_ip_config_clear_dns_options (ip_config, TRUE); - options = nm_utils_strsplit_set (value, " ", FALSE); + options = nm_utils_strsplit_set (value, " "); if (options) { for (item = options; *item; item++) { if (!nm_setting_ip_config_add_dns_option (ip_config, *item)) @@ -1443,7 +1443,7 @@ make_match_setting (shvarFile *ifcfg) if (!v) return NULL; - strv = nm_utils_strsplit_set (v, " \t", TRUE); + strv = nm_utils_strsplit_set_full (v, " \t", NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); if (strv) { for (i = 0; strv[i]; i++) { if (!s_match) @@ -1722,7 +1722,7 @@ make_ip4_setting (shvarFile *ifcfg, if (v) { gs_free const char **searches = NULL; - searches = nm_utils_strsplit_set (v, " ", FALSE); + searches = nm_utils_strsplit_set (v, " "); if (searches) { for (item = searches; *item; item++) { if (!nm_setting_ip_config_add_dns_search (s_ip4, *item)) @@ -1783,7 +1783,7 @@ make_ip4_setting (shvarFile *ifcfg, if (v) { gs_free const char **searches = NULL; - searches = nm_utils_strsplit_set (v, " ", FALSE); + searches = nm_utils_strsplit_set (v, " "); if (searches) { for (item = searches; *item; item++) { if (!nm_setting_ip_config_add_dns_search (s_ip4, *item)) @@ -2099,7 +2099,7 @@ make_ip6_setting (shvarFile *ifcfg, ipv6addr_secondaries ?: "", NULL); - list = nm_utils_strsplit_set (value, " ", FALSE); + list = nm_utils_strsplit_set (value, " "); for (iter = list, i = 0; iter && *iter; iter++, i++) { NMIPAddress *addr = NULL; @@ -2192,7 +2192,7 @@ make_ip6_setting (shvarFile *ifcfg, if (v) { gs_free const char **searches = NULL; - searches = nm_utils_strsplit_set (v, " ", FALSE); + searches = nm_utils_strsplit_set (v, " "); if (searches) { for (iter = searches; *iter; iter++) { if (!nm_setting_ip_config_add_dns_search (s_ip6, *iter)) @@ -2551,7 +2551,7 @@ read_dcb_percent_array (shvarFile *ifcfg, return TRUE; } - split = nm_utils_strsplit_set (val, ",", FALSE); + split = nm_utils_strsplit_set (val, ","); if (NM_PTRARRAY_LEN (split) != 8) { PARSE_WARNING ("invalid %s percentage list value '%s'", prop, val); g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, @@ -2963,7 +2963,7 @@ fill_wpa_ciphers (shvarFile *ifcfg, if (!p) return TRUE; - list = nm_utils_strsplit_set (p, " ", FALSE); + list = nm_utils_strsplit_set (p, " "); for (iter = list; iter && *iter; iter++, i++) { /* Ad-Hoc configurations cannot have pairwise ciphers, and can only * have one group cipher. Ignore any additional group ciphers and @@ -3233,7 +3233,7 @@ eap_peap_reader (const char *eap_method, } /* Handle options for the inner auth method */ - list = nm_utils_strsplit_set (v, " ", FALSE); + list = nm_utils_strsplit_set (v, " "); iter = list; if (iter) { if (NM_IN_STRSET (*iter, "MSCHAPV2", @@ -3311,7 +3311,7 @@ eap_ttls_reader (const char *eap_method, inner_auth = g_ascii_strdown (v, -1); /* Handle options for the inner auth method */ - list = nm_utils_strsplit_set (inner_auth, " ", FALSE); + list = nm_utils_strsplit_set (inner_auth, " "); iter = list; if (iter) { if (NM_IN_STRSET (*iter, "mschapv2", @@ -3372,7 +3372,7 @@ eap_fast_reader (const char *eap_method, if (fast_provisioning) { gs_free const char **list1 = NULL; - list1 = nm_utils_strsplit_set (fast_provisioning, " \t", FALSE); + list1 = nm_utils_strsplit_set (fast_provisioning, " \t"); for (iter = list1; iter && *iter; iter++) { if (strcmp (*iter, "allow-unauth") == 0) allow_unauth = TRUE; @@ -3406,7 +3406,7 @@ eap_fast_reader (const char *eap_method, } /* Handle options for the inner auth method */ - list = nm_utils_strsplit_set (inner_auth, " ", FALSE); + list = nm_utils_strsplit_set (inner_auth, " "); iter = list; if (iter) { if ( !strcmp (*iter, "MSCHAPV2") @@ -3486,7 +3486,7 @@ read_8021x_list_value (shvarFile *ifcfg, if (!v) return; - strv = nm_utils_strsplit_set (v, " \t", FALSE); + strv = nm_utils_strsplit_set (v, " \t"); if (strv) g_object_set (setting, prop_name, strv, NULL); } @@ -3515,7 +3515,7 @@ fill_8021x (shvarFile *ifcfg, return NULL; } - list = nm_utils_strsplit_set (v, " ", FALSE); + list = nm_utils_strsplit_set (v, " "); s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); @@ -3829,7 +3829,7 @@ transform_hwaddr_blacklist (const char *blacklist) const char **strv; gsize i, j; - strv = nm_utils_strsplit_set (blacklist, " \t", FALSE); + strv = nm_utils_strsplit_set (blacklist, " \t"); if (!strv) return NULL; for (i = 0, j = 0; strv[j]; j++) { @@ -4147,7 +4147,7 @@ parse_ethtool_option (const char *value, gs_free const char **words = NULL; guint i; - words = nm_utils_strsplit_set (value, NULL, FALSE); + words = nm_utils_strsplit_set (value, NULL); if (!words) return; @@ -4418,7 +4418,7 @@ parse_ethtool_options (shvarFile *ifcfg, NMConnection *connection) gs_free const char **opts = NULL; const char *const *iter; - opts = nm_utils_strsplit_set (ethtool_opts, ";", FALSE); + opts = nm_utils_strsplit_set (ethtool_opts, ";"); for (iter = opts; iter && iter[0]; iter++) { /* in case of repeated wol_passwords, parse_ethtool_option() * will do the right thing and clear wol_password before resetting. */ @@ -4514,7 +4514,7 @@ make_wired_setting (shvarFile *ifcfg, gs_free const char **chans = NULL; guint32 num_chans; - chans = nm_utils_strsplit_set (value, ",", FALSE); + chans = nm_utils_strsplit_set (value, ","); num_chans = NM_PTRARRAY_LEN (chans); if (num_chans < 2 || num_chans > 3) { PARSE_WARNING ("invalid SUBCHANNELS '%s' (%u channels, 2 or 3 expected)", @@ -4837,7 +4837,7 @@ make_bond_setting (shvarFile *ifcfg, gs_free const char **items = NULL; const char *const *iter; - items = nm_utils_strsplit_set (v, " ", FALSE); + items = nm_utils_strsplit_set (v, " "); for (iter = items; iter && *iter; iter++) { gs_strfreev char **keys = NULL; const char *key, *val; @@ -5117,7 +5117,7 @@ handle_bridging_opts (NMSetting *setting, gs_free const char **items = NULL; const char *const *iter; - items = nm_utils_strsplit_set (value, " ", FALSE); + items = nm_utils_strsplit_set (value, " "); for (iter = items; iter && *iter; iter++) { gs_strfreev char **keys = NULL; const char *key, *val; @@ -5151,7 +5151,7 @@ read_bridge_vlans (shvarFile *ifcfg, array = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_bridge_vlan_unref); - strv = nm_utils_strsplit_set (value, ",", FALSE); + strv = nm_utils_strsplit_set (value, ","); if (strv) { for (iter = strv; *iter; iter++) { vlan = nm_bridge_vlan_from_str (*iter, &local); @@ -5382,7 +5382,7 @@ parse_prio_map_list (NMSettingVlan *s_vlan, v = svGetValueStr (ifcfg, key, &value); if (!v) return; - list = nm_utils_strsplit_set (v, ",", FALSE); + list = nm_utils_strsplit_set (v, ","); for (iter = list; iter && *iter; iter++) { if (!strchr (*iter, ':')) @@ -5487,7 +5487,7 @@ make_vlan_setting (shvarFile *ifcfg, gs_free const char **strv = NULL; const char *const *ptr; - strv = nm_utils_strsplit_set (v, ", ", FALSE); + strv = nm_utils_strsplit_set (v, ", "); for (ptr = strv; ptr && *ptr; ptr++) { if (nm_streq (*ptr, "GVRP") && gvrp == -1) vlan_flags |= NM_VLAN_FLAG_GVRP; @@ -5629,7 +5629,7 @@ check_dns_search_domains (shvarFile *ifcfg, NMSetting *s_ip4, NMSetting *s_ip6) gs_free const char **searches = NULL; const char *const *item; - searches = nm_utils_strsplit_set (v, " ", FALSE); + searches = nm_utils_strsplit_set (v, " "); if (searches) { for (item = searches; *item; item++) { if (!nm_setting_ip_config_add_dns_search (NM_SETTING_IP_CONFIG (s_ip6), *item)) From ce456f5b77ff3ef71247f140b4af2eed98a39617 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Apr 2019 16:02:11 +0200 Subject: [PATCH 03/18] all: don't accept %NULL as delimiters for nm_utils_strsplit_set() The caller should make a conscious decision which delimiters to use. Unfortunately, there is a variety of different demiters in use. This should be unitfied and the callers should use one of a few specific set of delimiters. This could be unified by (re)using a define as delimiters, like strv = nm_utils_strsplit_set_full (value, MULTILIST_WITH_ESCAPE_CHARS, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); where MULTILIST_WITH_ESCAPE_CHARS has a particular meaning that should be reused for similar uses. However, leaving the delimiter at NULL is not good because it's unclear who wants that default behavior (and what the default should be). Don't allow that. There are almost no callers that relied on this default anyway. --- shared/nm-utils/nm-shared-utils.c | 7 ++++--- src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 13a1a35f6e..1554d4cd07 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -974,8 +974,7 @@ _char_lookup_table_init (guint8 lookup[static 256], /** * nm_utils_strsplit_set_full: * @str: the string to split. - * @delimiters: the set of delimiters. If %NULL, defaults to " \t\n", - * like bash's $IFS. + * @delimiters: the set of delimiters. * @flags: additional flags for controlling the operation. * * This is a replacement for g_strsplit_set() which avoids copying @@ -1017,8 +1016,10 @@ nm_utils_strsplit_set_full (const char *str, return NULL; /* initialize lookup table for delimiter */ - if (!delimiters) + if (!delimiters) { + nm_assert_not_reached (); delimiters = " \t\n"; + } _char_lookup_table_init (delimiters_table, delimiters); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 8d39f827ca..93d48dfdd4 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -4147,7 +4147,7 @@ parse_ethtool_option (const char *value, gs_free const char **words = NULL; guint i; - words = nm_utils_strsplit_set (value, NULL); + words = nm_utils_strsplit_set (value, " \t\n"); if (!words) return; From e2217a26e750f0e6918e344218fdc04372f265b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Apr 2019 16:20:12 +0200 Subject: [PATCH 04/18] shared: refactor lookup of delimiter tables in nm_utils_strsplit_set_full() --- shared/nm-utils/nm-shared-utils.c | 36 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 1554d4cd07..f723ef0fc2 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -971,6 +971,14 @@ _char_lookup_table_init (guint8 lookup[static 256], lookup[(guint8) ((candidates++)[0])] = 1; } +static gboolean +_char_lookup_has (const guint8 lookup[static 256], + char ch) +{ + nm_assert (lookup[(guint8) '\0'] == 0); + return lookup[(guint8) ch] != 0; +} + /** * nm_utils_strsplit_set_full: * @str: the string to split. @@ -1008,7 +1016,7 @@ nm_utils_strsplit_set_full (const char *str, gsize str_len; char *s0; char *s; - guint8 delimiters_table[256]; + guint8 ch_lookup[256]; gboolean escaped = FALSE; const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); @@ -1021,10 +1029,10 @@ nm_utils_strsplit_set_full (const char *str, delimiters = " \t\n"; } - _char_lookup_table_init (delimiters_table, delimiters); + _char_lookup_table_init (ch_lookup, delimiters); -#define _is_delimiter(ch, delimiters_table, allow_esc, esc) \ - ((delimiters_table)[(guint8) (ch)] != 0 && (!allow_esc || !esc)) +#define _is_delimiter(ch_lookup, ch, allow_esc, esc) \ + (_char_lookup_has ((ch_lookup), (ch)) && (!allow_esc || !esc)) #define next_char(p, esc) \ G_STMT_START { \ @@ -1037,7 +1045,7 @@ nm_utils_strsplit_set_full (const char *str, /* skip initial delimiters, and return of the remaining string is * empty. */ - while (_is_delimiter (str[0], delimiters_table, f_allow_escaping, escaped)) + while (_is_delimiter (ch_lookup, str[0], f_allow_escaping, escaped)) next_char (str, escaped); if (!str[0]) @@ -1071,11 +1079,11 @@ nm_utils_strsplit_set_full (const char *str, ptr[plen++] = s; - nm_assert (s[0] && !_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)); + nm_assert (s[0] && !_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)); while (TRUE) { next_char (s, escaped); - if (_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)) + if (_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)) break; if (s[0] == '\0') goto done; @@ -1083,7 +1091,7 @@ nm_utils_strsplit_set_full (const char *str, s[0] = '\0'; next_char (s, escaped); - while (_is_delimiter (s[0], delimiters_table, f_allow_escaping, escaped)) + while (_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)) next_char (s, escaped); if (s[0] == '\0') break; @@ -2430,8 +2438,6 @@ _nm_utils_user_data_unpack (gpointer user_data, int nargs, ...) /*****************************************************************************/ -#define _CH_LOOKUP(ch_lookup, ch) (!!((ch_lookup)[(guint8) (ch)])) - const char * _nm_utils_escape_plain (const char *str, const char *candidates, char **to_free) { @@ -2449,7 +2455,7 @@ _nm_utils_escape_plain (const char *str, const char *candidates, char **to_free) while (TRUE) { if (!*ptr) return str; - if (_CH_LOOKUP (ch_lookup, *ptr)) + if (_char_lookup_has (ch_lookup, *ptr)) break; ptr++; } @@ -2459,7 +2465,7 @@ _nm_utils_escape_plain (const char *str, const char *candidates, char **to_free) r = ret; *to_free = ret; while (*ptr) { - if (_CH_LOOKUP (ch_lookup, *ptr)) + if (_char_lookup_has (ch_lookup, *ptr)) *r++ = '\\'; *r++ = *ptr++; } @@ -2482,13 +2488,13 @@ _nm_utils_unescape_plain (char *str, const char *candidates, gboolean do_strip) _char_lookup_table_init (ch_lookup, candidates ?: NM_ASCII_SPACES); if (do_strip) { - while (str[i] && _CH_LOOKUP (ch_lookup, str[i])) + while (str[i] && _char_lookup_has (ch_lookup, str[i])) i++; } for (; str[i]; i++) { if ( str[i] == '\\' - && _CH_LOOKUP (ch_lookup, str[i+1])) { + && _char_lookup_has (ch_lookup, str[i+1])) { preserve_space_at = j; i++; } @@ -2498,7 +2504,7 @@ _nm_utils_unescape_plain (char *str, const char *candidates, gboolean do_strip) if (do_strip && j > 0) { while ( --j > preserve_space_at - && _CH_LOOKUP (ch_lookup, str[j])) + && _char_lookup_has (ch_lookup, str[j])) str[j] = '\0'; } From 453b3ea3626071efccdfa2872579acf37f1e5e33 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Apr 2019 16:41:21 +0200 Subject: [PATCH 05/18] shared: refactor allowed-escaped handling in nm_utils_strsplit_set_full() Drop the next_char() and is_delimiter() macros. They are difficult to understand, because they both have a state-variable (escaped). Instead, the state of whether we handle an escape or not, shall only depend on the current line of code. --- shared/nm-utils/nm-shared-utils.c | 49 +++++++++++++------------------ 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index f723ef0fc2..21658ca876 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -1017,36 +1017,22 @@ nm_utils_strsplit_set_full (const char *str, char *s0; char *s; guint8 ch_lookup[256]; - gboolean escaped = FALSE; const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); if (!str) return NULL; - /* initialize lookup table for delimiter */ if (!delimiters) { nm_assert_not_reached (); delimiters = " \t\n"; } - _char_lookup_table_init (ch_lookup, delimiters); -#define _is_delimiter(ch_lookup, ch, allow_esc, esc) \ - (_char_lookup_has ((ch_lookup), (ch)) && (!allow_esc || !esc)) + nm_assert ( !f_allow_escaping + || !_char_lookup_has (ch_lookup, '\\')); -#define next_char(p, esc) \ - G_STMT_START { \ - if (esc) \ - esc = FALSE; \ - else \ - esc = p[0] == '\\'; \ - p++; \ - } G_STMT_END - - /* skip initial delimiters, and return of the remaining string is - * empty. */ - while (_is_delimiter (ch_lookup, str[0], f_allow_escaping, escaped)) - next_char (str, escaped); + while (_char_lookup_has (ch_lookup, str[0])) + str++; if (!str[0]) return NULL; @@ -1079,23 +1065,30 @@ nm_utils_strsplit_set_full (const char *str, ptr[plen++] = s; - nm_assert (s[0] && !_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)); + nm_assert (s[0] && !_char_lookup_has (ch_lookup, s[0])); - while (TRUE) { - next_char (s, escaped); - if (_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)) - break; - if (s[0] == '\0') + while (!_char_lookup_has (ch_lookup, s[0])) { + if (G_UNLIKELY ( s[0] == '\\' + && f_allow_escaping)) { + s++; + if (s[0] == '\0') + goto done; + s++; + } else if (s[0] == '\0') goto done; + else + s++; } + nm_assert (_char_lookup_has (ch_lookup, s[0])); s[0] = '\0'; - next_char (s, escaped); - while (_is_delimiter (ch_lookup, s[0], f_allow_escaping, escaped)) - next_char (s, escaped); + s++; + while (_char_lookup_has (ch_lookup, s[0])) + s++; if (s[0] == '\0') - break; + goto done; } + done: ptr[plen] = NULL; From 5c1f93943e39d30cbf6ebb53ccdb74d5e04d50bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Apr 2019 17:32:59 +0200 Subject: [PATCH 06/18] shared: add NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY flag for nm_utils_strsplit_set_full() Previously, nm_utils_strsplit_set_full() would always remove empty tokens. Add a flag NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY to avoid that. This makes nm_utils_strsplit_set_full() return the same result as g_strsplit_set() and a direct replacement for it -- except for "", where we return %NULL. --- libnm-core/tests/test-general.c | 339 +++++++++++++++++++++++++++--- shared/nm-utils/nm-shared-utils.c | 58 +++-- shared/nm-utils/nm-shared-utils.h | 14 +- 3 files changed, 364 insertions(+), 47 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 4063dfcf20..175b48b505 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -259,55 +259,279 @@ test_nm_g_slice_free_fcn (void) /*****************************************************************************/ static void -_do_test_nm_utils_strsplit_set (gboolean escape, const char *str, ...) +_do_test_nm_utils_strsplit_set_f_one (NMUtilsStrsplitSetFlags flags, + const char *str, + gsize words_len, + const char *const*exp_words) { - gs_unref_ptrarray GPtrArray *args_array = g_ptr_array_new (); - const char *const*args; + const char *DELIMITERS = " \t\n"; +#define DELIMITERS_C ' ', '\t', '\n' + gs_free const char **words = NULL; - const char *arg; - gsize i; - va_list ap; + gsize i, j, k; + const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); + const gboolean f_preserve_empty = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY); + const char *s1; + gsize initial_offset; + gs_strfreev char **words_g = NULL; - va_start (ap, str); - while ((arg = va_arg (ap, const char *))) - g_ptr_array_add (args_array, (gpointer) arg); - va_end (ap); - g_ptr_array_add (args_array, NULL); + g_assert (!NM_FLAGS_ANY (flags, ~( NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING + | NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY))); - args = (const char *const*) args_array->pdata; + /* assert that the epected words are valid (and don't contain unescaped delimiters). */ + for (i = 0; i < words_len; i++) { + const char *w = exp_words[i]; - if (!escape && nmtst_get_rand_bool ()) - words = nm_utils_strsplit_set (str, " \t\n"); - else { - words = nm_utils_strsplit_set_full (str, - " \t\n", - escape - ? NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING - : NM_UTILS_STRSPLIT_SET_FLAGS_NONE); + g_assert (w); + if (!f_preserve_empty) + g_assert (w[0]); + for (k = 0; w[k]; ) { + if ( f_allow_escaping + && w[k] == '\\') { + k++; + if (w[k] == '\0') + break; + k++; + continue; + } + g_assert (!NM_IN_SET (w[k], DELIMITERS_C)); + k++; + } + if (!f_allow_escaping) + g_assert (!NM_STRCHAR_ANY (w, ch, NM_IN_SET (ch, DELIMITERS_C))); } - if (!args[0]) { + initial_offset = (f_preserve_empty || !str) + ? 0u + : strspn (str, DELIMITERS); + + /* first compare our expected values with what g_strsplit_set() would + * do. */ + words_g = str ? g_strsplit_set (str, DELIMITERS, -1) : NULL; + if (str == NULL) { + g_assert_cmpint (words_len, ==, 0); + g_assert (!words_g); + } else if (nm_streq0 (str, "")) { + g_assert_cmpint (words_len, ==, 0); + g_assert (words_g); + g_assert (!words_g[0]); + } else { + g_assert (words_g); + g_assert (words_g[0]); + if (!f_allow_escaping) { + if (!f_preserve_empty) { + for (i = 0, j = 0; words_g[i]; i++) { + if (words_g[i][0] == '\0') + g_free (words_g[i]); + else + words_g[j++] = words_g[i]; + } + words_g[j] = NULL; + } + if (f_preserve_empty) + g_assert_cmpint (words_len, >, 0); + for (i = 0; i < words_len; i++) { + g_assert (exp_words[i]); + g_assert_cmpstr (exp_words[i], ==, words_g[i]); + } + g_assert (words_g[words_len] == NULL); + g_assert_cmpint (NM_PTRARRAY_LEN (words_g), ==, words_len); + g_assert (_nm_utils_strv_cmp_n (exp_words, words_len, NM_CAST_STRV_CC (words_g), -1) == 0); + } + } + + if ( flags == NM_UTILS_STRSPLIT_SET_FLAGS_NONE + && nmtst_get_rand_bool ()) + words = nm_utils_strsplit_set (str, DELIMITERS); + else if ( flags == NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY + && nmtst_get_rand_bool ()) + words = nm_utils_strsplit_set_with_empty (str, DELIMITERS); + else + words = nm_utils_strsplit_set_full (str, DELIMITERS, flags); + + g_assert_cmpint (NM_PTRARRAY_LEN (words), ==, words_len); + + if (words_len == 0) { g_assert (!words); g_assert ( !str - || NM_STRCHAR_ALL (str, ch, NM_IN_SET (ch, ' ', '\t', '\n'))); + || NM_STRCHAR_ALL (str, ch, NM_IN_SET (ch, DELIMITERS_C))); return; } + g_assert (words); - for (i = 0; args[i] || words[i]; i++) { - g_assert (args[i]); - g_assert (words[i]); - g_assert (args[i][0]); - g_assert (escape || NM_STRCHAR_ALL (args[i], ch, !NM_IN_SET (ch, ' ', '\t', '\n'))); - g_assert_cmpstr (args[i], ==, words[i]); + for (i = 0; i < words_len; i++) + g_assert_cmpstr (exp_words[i], ==, words[i]); + g_assert (words[words_len] == NULL); + + g_assert (_nm_utils_strv_cmp_n (exp_words, words_len, words, -1) == 0); + + s1 = words[0]; + g_assert (s1 >= (char *) &words[words_len + 1]); + s1 = &s1[strlen (str)]; + for (i = 1; i < words_len; i++) { + g_assert (&(words[i - 1])[strlen (words[i - 1])] < words[i]); + g_assert (words[i] <= s1); + } + + /* while strsplit removes all delimiters, we can relatively easily find them + * in the original string. Assert that the original string and the pointer offsets + * of words correspond. In particular, find idx_delim_after and idx_delim_before + * to determine which delimiter was after/before a word. */ + { + gsize idx_word_start; + gsize idx_delim_after_old = G_MAXSIZE; + + idx_word_start = initial_offset; + for (i = 0; i < words_len; i++) { + const gsize l_i = strlen (words[i]); + gsize idx_delim_after; + gsize idx_delim_before; + + /* find the delimiter *after* words[i]. We can do that by looking at the next + * word and calculating the pointer difference. + * + * The delimiter after the very last word is '\0' and requires strlen() to find. */ + idx_delim_after = initial_offset + ((words[i] - words[0]) + l_i); + if (idx_delim_after != idx_word_start + l_i) { + g_assert (!f_preserve_empty); + g_assert_cmpint (idx_word_start + l_i, <, idx_delim_after); + idx_word_start = idx_delim_after - l_i; + } + if (i + 1 < words_len) { + gsize x = initial_offset + ((words[i + 1] - words[0]) - 1); + + if (idx_delim_after != x) { + g_assert (!f_preserve_empty); + g_assert_cmpint (idx_delim_after, <, x); + for (k = idx_delim_after; k <= x; k++) + g_assert (NM_IN_SET (str[k], DELIMITERS_C)); + } + g_assert (NM_IN_SET (str[idx_delim_after], DELIMITERS_C)); + } else { + if (f_preserve_empty) + g_assert (NM_IN_SET (str[idx_delim_after], '\0')); + else + g_assert (NM_IN_SET (str[idx_delim_after], '\0', DELIMITERS_C)); + } + + /* find the delimiter *before* words[i]. */ + if (i == 0) { + /* there is only a delimiter *before*, with !f_preserve_empty and leading + * delimiters. */ + idx_delim_before = G_MAXSIZE; + if (initial_offset > 0) { + g_assert (!f_preserve_empty); + idx_delim_before = initial_offset - 1; + } + } else + idx_delim_before = initial_offset + (words[i] - words[0]) - 1; + if (idx_delim_before != G_MAXSIZE) + g_assert (NM_IN_SET (str[idx_delim_before], DELIMITERS_C)); + if (idx_delim_after_old != idx_delim_before) { + g_assert (!f_preserve_empty); + if (i == 0) { + g_assert_cmpint (initial_offset, >, 0); + g_assert_cmpint (idx_delim_before, !=, G_MAXSIZE); + g_assert_cmpint (idx_delim_before, ==, initial_offset - 1); + } else { + g_assert_cmpint (idx_delim_after_old, !=, G_MAXSIZE); + g_assert_cmpint (idx_delim_before, !=, G_MAXSIZE); + g_assert_cmpint (idx_delim_after_old, <, idx_delim_before); + for (k = idx_delim_after_old; k <= idx_delim_before; k++) + g_assert (NM_IN_SET (str[k], DELIMITERS_C)); + } + } + + for (k = 0; k < l_i; ) { + if ( f_allow_escaping + && str[idx_word_start + k] == '\\') { + k++; + if (k >= l_i) + break; + k++; + continue; + } + g_assert (!NM_IN_SET (str[idx_word_start + k], DELIMITERS_C)); + k++; + } + g_assert (strncmp (words[i], &str[idx_word_start], l_i) == 0); + + if (i > 0) { + const char *s = &(words[i - 1])[strlen (words[i - 1]) + 1]; + + if (s != words[i]) { + g_assert (!f_preserve_empty); + g_assert (s < words[i]); + } + } + + idx_word_start += l_i + 1; + idx_delim_after_old = idx_delim_after; + } } } -#define do_test_nm_utils_strsplit_set(str, ...) \ - _do_test_nm_utils_strsplit_set (str, ##__VA_ARGS__, NULL) +static void +_do_test_nm_utils_strsplit_set_f (NMUtilsStrsplitSetFlags flags, + const char *str, + gsize words_len, + const char *const*exp_words) +{ + _do_test_nm_utils_strsplit_set_f_one (flags, str, words_len, exp_words); + + if (NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY)) { + gs_unref_ptrarray GPtrArray *exp_words2 = NULL; + gsize k; + + exp_words2 = g_ptr_array_new (); + for (k = 0; k < words_len; k++) { + if (exp_words[k][0] != '\0') + g_ptr_array_add (exp_words2, (gpointer) exp_words[k]); + } + + _do_test_nm_utils_strsplit_set_f_one (flags & (~NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY), + str, + exp_words2->len, + (const char *const*) exp_words2->pdata); + } +} + +#define do_test_nm_utils_strsplit_set_f(flags, str, ...) \ + _do_test_nm_utils_strsplit_set_f (flags, \ + str, \ + NM_NARG (__VA_ARGS__), \ + NM_MAKE_STRV (__VA_ARGS__)) + +#define do_test_nm_utils_strsplit_set(allow_escaping, str, ...) \ + do_test_nm_utils_strsplit_set_f ( (allow_escaping) \ + ? NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING \ + : NM_UTILS_STRSPLIT_SET_FLAGS_NONE, \ + str, \ + ##__VA_ARGS__) static void test_nm_utils_strsplit_set (void) { + gs_unref_ptrarray GPtrArray *words_exp = NULL; + guint test_run; + + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_NONE, NULL); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_NONE, ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_NONE, " "); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_NONE, "a b", "a", "b"); + + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, NULL); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, " ", "", ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, " ", "", "", ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "a ", "a", "", ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "a b", "a", "", "b"); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, " ab b", "", "ab", "", "b"); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "ab b", "ab", "", "b"); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "abb", "abb"); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "abb bb ", "abb", "", "bb", ""); + do_test_nm_utils_strsplit_set_f (NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, "abb bcb ", "abb", "bcb", ""); + do_test_nm_utils_strsplit_set (FALSE, NULL); do_test_nm_utils_strsplit_set (FALSE, ""); do_test_nm_utils_strsplit_set (FALSE, "\t"); @@ -331,6 +555,61 @@ test_nm_utils_strsplit_set (void) do_test_nm_utils_strsplit_set (TRUE, "foo\\", "foo\\"); do_test_nm_utils_strsplit_set (TRUE, "bar foo\\", "bar", "foo\\"); do_test_nm_utils_strsplit_set (TRUE, "\\ a b\\ \\ c", "\\ a", "b\\ \\ ", "c"); + + words_exp = g_ptr_array_new_with_free_func (g_free); + for (test_run = 0; test_run < 100; test_run++) { + gboolean f_allow_escaping = nmtst_get_rand_bool (); + guint words_len = nmtst_get_rand_int () % 100; + gs_free char *str = NULL; + guint i; + + g_ptr_array_set_size (words_exp, 0); + for (i = 0; i < words_len; i++) { + guint word_len; + char *word; + guint j; + + word_len = nmtst_get_rand_int (); + if ((word_len % 100) < 30) + word_len = 0; + else + word_len = (word_len >> 10) % 100; + word = g_new (char, word_len + 3); + for (j = 0; j < word_len; ) { + guint32 p = nmtst_get_rand_int (); + static const char delimiters_arr[] = { DELIMITERS_C }; + static const char regular_chars[] = "abcdefghijklmnopqrstuvwxyz"; + + if ( !f_allow_escaping + || (p % 1000) < 700) { + if (((p >> 20) % 100) < 20) + word[j++] = '\\'; + word[j++] = regular_chars[(p >> 11) % (G_N_ELEMENTS (regular_chars) - 1)]; + continue; + } + word[j++] = '\\'; + word[j++] = delimiters_arr[(p >> 11) % G_N_ELEMENTS (delimiters_arr)]; + } + word[j] = '\0'; + g_ptr_array_add (words_exp, word); + } + g_ptr_array_add (words_exp, NULL); + + str = g_strjoinv (" ", (char **) words_exp->pdata); + + if ( str[0] == '\0' + && words_len > 0) { + g_assert (words_len == 1); + g_assert_cmpstr (words_exp->pdata[0], ==, ""); + words_len = 0; + } + + _do_test_nm_utils_strsplit_set_f ( (f_allow_escaping ? NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING : NM_UTILS_STRSPLIT_SET_FLAGS_NONE) + | NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, + str, + words_len, + (const char *const*) words_exp->pdata); + } } /*****************************************************************************/ diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 21658ca876..e8f3563c09 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -989,17 +989,26 @@ _char_lookup_has (const guint8 lookup[static 256], * each word once (the entire strv array), but instead copies it once * and all words point into that internal copy. * - * Another difference from g_strsplit_set() is that this never returns - * empty words. Multiple delimiters are combined and treated as one. + * Note that for @str %NULL and "", this always returns %NULL too. That differs + * from g_strsplit_set(), which would return an empty strv array for "". + * + * Note that g_strsplit_set() returns empty words as well. By default, + * nm_utils_strsplit_set_full() strips all empty tokens (that is, repeated + * delimiters. With %NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY, empty tokens + * are not removed. * * If @flags has %NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING, delimiters prefixed * by a backslash are not treated as a separator. Such delimiters and their escape - * character are copied to the current word without unescaping them. + * character are copied to the current word without unescaping them. In general, + * nm_utils_strsplit_set_full() does not remove any backslash escape characters + * and does not unescaping. It only considers them for skipping to split at + * an escaped delimiter. * - * Returns: %NULL if @str is %NULL or contains only delimiters. - * Otherwise, a %NULL terminated strv array containing non-empty - * words, split at the delimiter characters (delimiter characters - * are removed). + * Returns: %NULL if @str is %NULL or "". + * If @str only contains delimiters and %NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY + * is not set, it also returns %NULL. + * Otherwise, a %NULL terminated strv array containing the split words. + * (delimiter characters are removed). * The strings to which the result strv array points to are allocated * after the returned result itself. Don't free the strings themself, * but free everything with g_free(). @@ -1012,12 +1021,15 @@ nm_utils_strsplit_set_full (const char *str, NMUtilsStrsplitSetFlags flags) { const char **ptr, **ptr0; - gsize alloc_size, plen, i; + gsize alloc_size; + gsize plen; + gsize i; gsize str_len; char *s0; char *s; guint8 ch_lookup[256]; const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); + const gboolean f_preseve_empty = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY); if (!str) return NULL; @@ -1031,11 +1043,18 @@ nm_utils_strsplit_set_full (const char *str, nm_assert ( !f_allow_escaping || !_char_lookup_has (ch_lookup, '\\')); - while (_char_lookup_has (ch_lookup, str[0])) - str++; + if (!f_preseve_empty) { + while (_char_lookup_has (ch_lookup, str[0])) + str++; + } - if (!str[0]) + if (!str[0]) { + /* We return %NULL here, also with NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY. + * That makes nm_utils_strsplit_set_full() with NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY + * different from g_strsplit_set(), which would in this case return an empty array. + * If you need to handle %NULL, and "" specially, then check the input string first. */ return NULL; + } str_len = strlen (str) + 1; alloc_size = 8; @@ -1065,7 +1084,12 @@ nm_utils_strsplit_set_full (const char *str, ptr[plen++] = s; - nm_assert (s[0] && !_char_lookup_has (ch_lookup, s[0])); + if (s[0] == '\0') { + nm_assert (f_preseve_empty); + goto done; + } + nm_assert ( f_preseve_empty + || !_char_lookup_has (ch_lookup, s[0])); while (!_char_lookup_has (ch_lookup, s[0])) { if (G_UNLIKELY ( s[0] == '\\' @@ -1083,10 +1107,12 @@ nm_utils_strsplit_set_full (const char *str, nm_assert (_char_lookup_has (ch_lookup, s[0])); s[0] = '\0'; s++; - while (_char_lookup_has (ch_lookup, s[0])) - s++; - if (s[0] == '\0') - goto done; + if (!f_preseve_empty) { + while (_char_lookup_has (ch_lookup, s[0])) + s++; + if (s[0] == '\0') + goto done; + } } done: diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 75ef429dd7..8ec6fa2f5a 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -334,13 +334,25 @@ int nm_utils_dbus_path_cmp (const char *dbus_path_a, const char *dbus_path_b); typedef enum { NM_UTILS_STRSPLIT_SET_FLAGS_NONE = 0, - NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING = (1u << 0), + NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY = (1u << 0), + NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING = (1u << 1), } NMUtilsStrsplitSetFlags; const char **nm_utils_strsplit_set_full (const char *str, const char *delimiter, NMUtilsStrsplitSetFlags flags); +static inline const char ** +nm_utils_strsplit_set_with_empty (const char *str, + const char *delimiters) +{ + /* this returns the same result as g_strsplit_set(str, delimiters, -1), except + * it does not deep-clone the strv array. + * Also, for @str == "", this returns %NULL while g_strsplit_set() would return + * an empty strv array. */ + return nm_utils_strsplit_set_full (str, delimiters, NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY); +} + static inline const char ** nm_utils_strsplit_set (const char *str, const char *delimiters) From a15e70889c603bc0fd981a187c2b0ff3bbca2ea5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 12:59:21 +0200 Subject: [PATCH 07/18] dhcp: cleanup ip4_process_dhclient_rfc3442_routes() - use nm_utils_strsplit_set_full() instead of g_strsplit_set() to avoid allocating a full strv array. - refactor the code to return early and use cleanup attribute for freeing memory. - return TRUE/FALSE from process_dhclient_rfc3442_route(). It's simpler to understand than returning the moved pointer and a success output variable. --- src/dhcp/nm-dhcp-utils.c | 99 +++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 56 deletions(-) diff --git a/src/dhcp/nm-dhcp-utils.c b/src/dhcp/nm-dhcp-utils.c index 5227eea777..a8e0413da3 100644 --- a/src/dhcp/nm-dhcp-utils.c +++ b/src/dhcp/nm-dhcp-utils.c @@ -100,67 +100,56 @@ out: return have_routes; } -static const char ** -process_dhclient_rfc3442_route (const char **octets, - NMPlatformIP4Route *route, - gboolean *success) +static gboolean +process_dhclient_rfc3442_route (const char *const**p_octets, + NMPlatformIP4Route *route) { - const char **o = octets; - int addr_len = 0, i = 0; - long int tmp; - char *next_hop; - guint32 tmp_addr; + const char *const*o = *p_octets; + gs_free char *next_hop = NULL; + int addr_len; + int v_plen; + in_addr_t tmp_addr; + in_addr_t v_network = 0; - *success = FALSE; - - if (!*o) - return o; /* no prefix */ - - tmp = strtol (*o, NULL, 10); - if (tmp < 0 || tmp > 32) /* 32 == max IP4 prefix length */ - return o; - - memset (route, 0, sizeof (*route)); - route->plen = tmp; + v_plen = _nm_utils_ascii_str_to_int64 (*o, 10, 0, 32, -1); + if (v_plen == -1) + return FALSE; o++; - if (tmp > 0) - addr_len = ((tmp - 1) / 8) + 1; + addr_len = v_plen > 0 + ? ((v_plen - 1) / 8) + 1 + : 0; /* ensure there's at least the address + next hop left */ - if (g_strv_length ((char **) o) < addr_len + 4) - goto error; + if (NM_PTRARRAY_LEN (o) < addr_len + 4) + return FALSE; - if (tmp) { + if (v_plen > 0) { const char *addr[4] = { "0", "0", "0", "0" }; - char *str_addr; + gs_free char *str_addr = NULL; + int i; for (i = 0; i < addr_len; i++) addr[i] = *o++; str_addr = g_strjoin (".", addr[0], addr[1], addr[2], addr[3], NULL); - if (inet_pton (AF_INET, str_addr, &tmp_addr) <= 0) { - g_free (str_addr); - goto error; - } - g_free (str_addr); - route->network = nm_utils_ip4_address_clear_host_address (tmp_addr, tmp); + if (inet_pton (AF_INET, str_addr, &tmp_addr) <= 0) + return FALSE; + v_network = nm_utils_ip4_address_clear_host_address (tmp_addr, v_plen); } - /* Handle next hop */ next_hop = g_strjoin (".", o[0], o[1], o[2], o[3], NULL); - if (inet_pton (AF_INET, next_hop, &tmp_addr) <= 0) { - g_free (next_hop); - goto error; - } - route->gateway = tmp_addr; - g_free (next_hop); + o += 4; + if (inet_pton (AF_INET, next_hop, &tmp_addr) <= 0) + return FALSE; - *success = TRUE; - return o + 4; /* advance to past the next hop */ - -error: - return o; + *route = (NMPlatformIP4Route) { + .network = v_network, + .plen = v_plen, + .gateway = tmp_addr, + }; + *p_octets = o; + return TRUE; } static gboolean @@ -171,23 +160,23 @@ ip4_process_dhclient_rfc3442_routes (const char *iface, NMIP4Config *ip4_config, guint32 *gwaddr) { - char **octets, **o; + gs_free const char **octets = NULL; + const char *const*o; gboolean have_routes = FALSE; - NMPlatformIP4Route route; - gboolean success; - o = octets = g_strsplit_set (str, " .", 0); - if (g_strv_length (octets) < 5) { + octets = nm_utils_strsplit_set_with_empty (str, " ."); + if (NM_PTRARRAY_LEN (octets) < 5) { _LOG2W (LOGD_DHCP4, iface, "ignoring invalid classless static routes '%s'", str); - goto out; + return FALSE; } + o = octets; while (*o) { - memset (&route, 0, sizeof (route)); - o = (char **) process_dhclient_rfc3442_route ((const char **) o, &route, &success); - if (!success) { + NMPlatformIP4Route route; + + if (!process_dhclient_rfc3442_route (&o, &route)) { _LOG2W (LOGD_DHCP4, iface, "ignoring invalid classless static routes"); - break; + return have_routes; } have_routes = TRUE; @@ -211,8 +200,6 @@ ip4_process_dhclient_rfc3442_routes (const char *iface, } } -out: - g_strfreev (octets); return have_routes; } From f00d71cec1cae1b953ca7f3ec2236b77a8da25f5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 13:39:32 +0200 Subject: [PATCH 08/18] dhcp: cleanup nm_dhcp_dhclient_save_duid() --- src/dhcp/nm-dhcp-dhclient-utils.c | 43 ++++++++++++++----------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index cbd706fa69..ec2d168abc 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -590,14 +590,12 @@ nm_dhcp_dhclient_save_duid (const char *leasefile, GError **error) { gs_free char *escaped_duid = NULL; - gs_strfreev char **lines = NULL; - char **iter, *l; - GString *s; - gboolean success; + gs_free const char **lines = NULL; + nm_auto_free_gstring GString *s = NULL; + const char *const*iter; gsize len = 0; g_return_val_if_fail (leasefile != NULL, FALSE); - if (!duid) { nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "missing duid"); @@ -605,19 +603,17 @@ nm_dhcp_dhclient_save_duid (const char *leasefile, } escaped_duid = nm_dhcp_dhclient_escape_duid (duid); - g_return_val_if_fail (escaped_duid != NULL, FALSE); + nm_assert (escaped_duid); if (g_file_test (leasefile, G_FILE_TEST_EXISTS)) { - char *contents = NULL; + gs_free char *contents = NULL; if (!g_file_get_contents (leasefile, &contents, &len, error)) { g_prefix_error (error, "failed to read lease file %s: ", leasefile); return FALSE; } - g_assert (contents); - lines = g_strsplit_set (contents, "\n\r", -1); - g_free (contents); + lines = nm_utils_strsplit_set_with_empty (contents, "\n\r"); } s = g_string_sized_new (len + 50); @@ -625,37 +621,38 @@ nm_dhcp_dhclient_save_duid (const char *leasefile, /* Preserve existing leasefile contents */ if (lines) { - for (iter = lines; iter && *iter; iter++) { - l = *iter; - while (g_ascii_isspace (*l)) - l++; + for (iter = lines; *iter; iter++) { + const char *str = *iter; + const char *l; + /* If we find an uncommented DUID in the file, check if * equal to the one we are going to write: if so, no need * to update the lease file, otherwise skip the old DUID. */ + l = nm_str_skip_leading_spaces (str); if (g_str_has_prefix (l, DUID_PREFIX)) { gs_strfreev char **split = NULL; split = g_strsplit (l, "\"", -1); - if (nm_streq0 (split[1], escaped_duid)) { - g_string_free (s, TRUE); + if ( split[0] + && nm_streq0 (split[1], escaped_duid)) return TRUE; - } + continue; } - if (*iter[0]) - g_string_append (s, *iter); + if (str) + g_string_append (s, str); /* avoid to add an extra '\n' at the end of file */ if ((iter[1]) != NULL) g_string_append_c (s, '\n'); } } - success = g_file_set_contents (leasefile, s->str, -1, error); - if (!success) + if (!g_file_set_contents (leasefile, s->str, -1, error)) { g_prefix_error (error, "failed to set DUID in lease file %s: ", leasefile); + return FALSE; + } - g_string_free (s, TRUE); - return success; + return TRUE; } From be4fd39ab964899de70fda3b0d671f80e56fc5eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 15:21:37 +0200 Subject: [PATCH 09/18] dhcp: cleanup grab_request_options() --- src/dhcp/nm-dhcp-dhclient-utils.c | 45 +++++++++++++++++++------------ 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index ec2d168abc..2a37ce9253 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -62,37 +62,48 @@ add_request (GPtrArray *array, const char *item) static gboolean grab_request_options (GPtrArray *store, const char* line) { - char **areq, **aiter; - gboolean end = FALSE; + gs_free const char **line_v = NULL; + gsize i; /* Grab each 'request' or 'also request' option and save for later */ - areq = g_strsplit_set (line, "\t ,", -1); - for (aiter = areq; aiter && *aiter; aiter++) { - if (!strlen (g_strstrip (*aiter))) - continue; + line_v = nm_utils_strsplit_set (line, "\t ,"); + for (i = 0; line_v && line_v[i]; i++) { + const char *ss = nm_str_skip_leading_spaces (line_v[i]); + gsize l; + gboolean end = FALSE; - if (*aiter[0] == ';') { + if (!ss[0]) + continue; + if (ss[0] == ';') { /* all done */ - end = TRUE; - break; + return TRUE; } - if (!g_ascii_isalnum ((*aiter)[0])) + if (!g_ascii_isalnum (ss[0])) continue; - if ((*aiter)[strlen (*aiter) - 1] == ';') { + l = strlen (ss); + + while ( l > 0 + && g_ascii_isspace (ss[l - 1])) { + ((char *) ss)[l - 1] = '\0'; + l--; + } + if ( l > 0 + && ss[l - 1] == ';') { /* Remove the EOL marker */ - (*aiter)[strlen (*aiter) - 1] = '\0'; + ((char *) ss)[l - 1] = '\0'; end = TRUE; } - add_request (store, *aiter); + if (ss[0]) + add_request (store, ss); + + if (end) + return TRUE; } - if (areq) - g_strfreev (areq); - - return end; + return FALSE; } static void From e072489cc6a939ba20d5c5734c248969ef02d126 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 15:30:08 +0200 Subject: [PATCH 10/18] dhcp: cleanup nm_dhcp_dhclient_read_duid() --- src/dhcp/nm-dhcp-dhclient-utils.c | 40 +++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index 2a37ce9253..b959b3b59d 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -564,9 +564,9 @@ error: GBytes * nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error) { - GBytes *duid = NULL; - char *contents; - char **line, **split, *p, *e; + gs_free char *contents = NULL; + gs_free const char **contents_v = NULL; + gsize i; if (!g_file_test (leasefile, G_FILE_TEST_EXISTS)) return NULL; @@ -574,25 +574,29 @@ nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error) if (!g_file_get_contents (leasefile, &contents, NULL, error)) return NULL; - split = g_strsplit_set (contents, "\n\r", -1); - for (line = split; line && *line && (duid == NULL); line++) { - p = g_strstrip (*line); - if (g_str_has_prefix (p, DUID_PREFIX)) { - p += strlen (DUID_PREFIX); + contents_v = nm_utils_strsplit_set (contents, "\n\r"); + for (i = 0; contents_v && contents_v[i]; i++) { + const char *p = nm_str_skip_leading_spaces (contents_v[i]); + GBytes *duid; - /* look for trailing "; */ - e = p + strlen (p) - 2; - if (strcmp (e, "\";") != 0) - continue; - *e = '\0'; + if (!NM_STR_HAS_PREFIX (p, DUID_PREFIX)) + continue; - duid = nm_dhcp_dhclient_unescape_duid (p); - } + p += NM_STRLEN (DUID_PREFIX); + + g_strchomp ((char *) p); + + if (!NM_STR_HAS_SUFFIX (p, "\";")) + continue; + + ((char *) p)[strlen (p) - 2] = '\0'; + + duid = nm_dhcp_dhclient_unescape_duid (p); + if (duid) + return duid; } - g_free (contents); - g_strfreev (split); - return duid; + return NULL; } gboolean From 994df9244fe3346877984cb17d52affafb145367 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 15:36:49 +0200 Subject: [PATCH 11/18] dhcp: cleanup nm_dhcp_dhclient_create_config() --- src/dhcp/nm-dhcp-dhclient-utils.c | 51 ++++++++++++++++--------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index b959b3b59d..9d1bdceb17 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -50,10 +50,10 @@ static void add_request (GPtrArray *array, const char *item) { - int i; + guint i; for (i = 0; i < array->len; i++) { - if (!strcmp (g_ptr_array_index (array, i), item)) + if (nm_streq (array->pdata[i], item)) return; } g_ptr_array_add (array, g_strdup (item)); @@ -289,8 +289,9 @@ nm_dhcp_dhclient_create_config (const char *interface, const char *orig_contents, GBytes **out_new_client_id) { - GString *new_contents; - GPtrArray *fqdn_opts, *reqs; + nm_auto_free_gstring GString *new_contents = NULL; + gs_unref_ptrarray GPtrArray *fqdn_opts = NULL; + gs_unref_ptrarray GPtrArray *reqs = NULL; gboolean reset_reqlist = FALSE; int i; @@ -299,11 +300,11 @@ nm_dhcp_dhclient_create_config (const char *interface, nm_assert (!out_new_client_id || !*out_new_client_id); new_contents = g_string_new (_("# Created by NetworkManager\n")); - fqdn_opts = g_ptr_array_sized_new (5); reqs = g_ptr_array_new_full (5, g_free); if (orig_contents) { - char **lines, **line; + gs_free const char **lines = NULL; + gsize line_i; int nest = 0; gboolean in_alsoreq = FALSE; gboolean in_req = FALSE; @@ -312,19 +313,23 @@ nm_dhcp_dhclient_create_config (const char *interface, g_string_append_printf (new_contents, _("# Merged from %s\n\n"), orig_path); intf[0] = '\0'; - lines = g_strsplit_set (orig_contents, "\n\r", 0); - for (line = lines; lines && *line; line++) { - char *p = *line; + lines = nm_utils_strsplit_set (orig_contents, "\n\r"); + for (line_i = 0; lines && lines[line_i]; line_i++) { + const char *line = nm_str_skip_leading_spaces (lines[line_i]); + const char *p; - if (!strlen (g_strstrip (p))) + if (line[0] == '\0') continue; + g_strchomp ((char *) line); + + p = line; if (in_req) { /* pass */ } else if (strchr (p, '{')) { nest++; if ( !intf[0] - && g_str_has_prefix (p, "interface")) + && NM_STR_HAS_PREFIX (p, "interface")) if (read_interface (p, intf, sizeof (intf))) continue; } else if (strchr (p, '}')) { @@ -374,6 +379,8 @@ nm_dhcp_dhclient_create_config (const char *interface, * default ones set by NM, add them later */ if (!strncmp (p, FQDN_TAG_PREFIX, NM_STRLEN (FQDN_TAG_PREFIX))) { + if (!fqdn_opts) + fqdn_opts = g_ptr_array_new_full (5, g_free); g_ptr_array_add (fqdn_opts, g_strdup (p + NM_STRLEN (FQDN_TAG_PREFIX))); continue; } @@ -408,12 +415,9 @@ nm_dhcp_dhclient_create_config (const char *interface, } /* Existing configuration line is OK, add it to new configuration */ - g_string_append (new_contents, *line); + g_string_append (new_contents, line); g_string_append_c (new_contents, '\n'); } - - if (lines) - g_strfreev (lines); } else g_string_append_c (new_contents, '\n'); @@ -447,17 +451,16 @@ nm_dhcp_dhclient_create_config (const char *interface, /* And add it to the dhclient configuration */ for (i = 0; i < reqs->len; i++) g_string_append_printf (new_contents, "also request %s;\n", (char *) reqs->pdata[i]); - g_ptr_array_free (reqs, TRUE); - for (i = 0; i < fqdn_opts->len; i++) { - char *t = g_ptr_array_index (fqdn_opts, i); + if (fqdn_opts) { + for (i = 0; i < fqdn_opts->len; i++) { + const char *t = fqdn_opts->pdata[i]; - if (i == 0) - g_string_append_printf (new_contents, "\n# FQDN options from %s\n", orig_path); - g_string_append_printf (new_contents, FQDN_TAG_PREFIX "%s\n", t); - g_free (t); + if (i == 0) + g_string_append_printf (new_contents, "\n# FQDN options from %s\n", orig_path); + g_string_append_printf (new_contents, FQDN_TAG_PREFIX "%s\n", t); + } } - g_ptr_array_free (fqdn_opts, TRUE); g_string_append_c (new_contents, '\n'); @@ -469,7 +472,7 @@ nm_dhcp_dhclient_create_config (const char *interface, interface, anycast_addr); } - return g_string_free (new_contents, FALSE); + return g_string_free (g_steal_pointer (&new_contents), FALSE); } /* Roughly follow what dhclient's quotify_buf() and pretty_escape() functions do */ From a55c10754a69e0e5b4046a1d6972fa2a635928e5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 13:46:46 +0200 Subject: [PATCH 12/18] dcb: cleanup do_helper() in "nm-dcb.c" --- src/nm-dcb.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/nm-dcb.c b/src/nm-dcb.c index a63fdf3d96..b2e59154f0 100644 --- a/src/nm-dcb.c +++ b/src/nm-dcb.c @@ -37,9 +37,11 @@ do_helper (const char *iface, const char *fmt, ...) { - char **argv = NULL, **split = NULL, *cmdline, *errmsg = NULL; - gboolean success = FALSE; - guint i, u; + gs_free const char **split = NULL; + gs_free char *cmdline = NULL; + gs_free const char **argv = NULL; + gsize i; + gsize u; va_list args; g_return_val_if_fail (fmt != NULL, FALSE); @@ -48,35 +50,31 @@ do_helper (const char *iface, cmdline = g_strdup_vprintf (fmt, args); va_end (args); - split = g_strsplit_set (cmdline, " ", 0); + split = nm_utils_strsplit_set_with_empty (cmdline, " "); if (!split) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, "failure parsing %s command line", helper_names[which]); - goto out; + return FALSE; } /* Allocate space for path, custom arg, interface name, arguments, and NULL */ - i = u = 0; - argv = g_new0 (char *, g_strv_length (split) + 4); + i = 0; + argv = g_new (const char *, NM_PTRARRAY_LEN (split) + 4); argv[i++] = NULL; /* Placeholder for dcbtool path */ if (which == DCBTOOL) { argv[i++] = "sc"; argv[i++] = (char *) iface; } - while (u < g_strv_length (split)) - argv[i++] = split[u++]; + for (u = 0; split[u]; u++) + argv[i++] = split[u]; argv[i++] = NULL; - success = run_func (argv, which, user_data, error); - if (!success && error) - g_assert (*error); -out: - if (split) - g_strfreev (split); - g_free (argv); - g_free (cmdline); - g_free (errmsg); - return success; + if (!run_func ((char **) argv, which, user_data, error)) { + g_assert (!error || !*error); + return FALSE; + } + + return TRUE; } gboolean From b33e2b72da36717dd88beed2a0e0d42a6826f53a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 15:02:07 +0200 Subject: [PATCH 13/18] ibft: cleanup read_connections() --- src/settings/plugins/ibft/nms-ibft-plugin.c | 23 ++++---- src/settings/plugins/ibft/nms-ibft-reader.c | 64 +++++++++------------ src/settings/plugins/ibft/nms-ibft-reader.h | 8 +++ src/settings/plugins/ibft/tests/test-ibft.c | 17 +++--- 4 files changed, 53 insertions(+), 59 deletions(-) diff --git a/src/settings/plugins/ibft/nms-ibft-plugin.c b/src/settings/plugins/ibft/nms-ibft-plugin.c index 00b25068fc..47051995c1 100644 --- a/src/settings/plugins/ibft/nms-ibft-plugin.c +++ b/src/settings/plugins/ibft/nms-ibft-plugin.c @@ -64,31 +64,30 @@ static void read_connections (NMSIbftPlugin *self) { NMSIbftPluginPrivate *priv = NMS_IBFT_PLUGIN_GET_PRIVATE (self); - GSList *blocks = NULL, *iter; - GError *error = NULL; + nm_auto_free_ibft_blocks GSList *blocks = NULL; + GSList *iter; + gs_free_error GError *error = NULL; NMSIbftConnection *connection; if (!nms_ibft_reader_load_blocks ("/sbin/iscsiadm", &blocks, &error)) { nm_log_dbg (LOGD_SETTINGS, "ibft: failed to read iscsiadm records: %s", error->message); - g_error_free (error); return; } for (iter = blocks; iter; iter = iter->next) { connection = nms_ibft_connection_new (iter->data, &error); - if (connection) { - nm_log_info (LOGD_SETTINGS, "ibft: read connection '%s'", - nm_settings_connection_get_id (NM_SETTINGS_CONNECTION (connection))); - g_hash_table_insert (priv->connections, - g_strdup (nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (connection))), - connection); - } else { + if (!connection) { nm_log_warn (LOGD_SETTINGS, "ibft: failed to read iscsiadm record: %s", error->message); g_clear_error (&error); + continue; } - } - g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref); + nm_log_info (LOGD_SETTINGS, "ibft: read connection '%s'", + nm_settings_connection_get_id (NM_SETTINGS_CONNECTION (connection))); + g_hash_table_insert (priv->connections, + g_strdup (nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (connection))), + connection); + } } static GSList * diff --git a/src/settings/plugins/ibft/nms-ibft-reader.c b/src/settings/plugins/ibft/nms-ibft-reader.c index c6c1437651..fde3038321 100644 --- a/src/settings/plugins/ibft/nms-ibft-reader.c +++ b/src/settings/plugins/ibft/nms-ibft-reader.c @@ -46,8 +46,7 @@ remove_most_whitespace (const char *src) char *s_new, *s2; const char *svalue; - while (*src && g_ascii_isspace (*src)) - src++; + src = nm_str_skip_leading_spaces (src); svalue = strchr (src, '='); if (!svalue || svalue == src) @@ -94,24 +93,25 @@ nms_ibft_reader_load_blocks (const char *iscsiadm_path, { const char *argv[4] = { iscsiadm_path, "-m", "fw", NULL }; const char *envp[1] = { NULL }; - GSList *blocks = NULL; - char *out = NULL, *err = NULL; - int status = 0; - char **lines = NULL, **iter; + nm_auto_free_ibft_blocks GSList *blocks = NULL; + gs_free char *out = NULL; + gs_free char *err = NULL; + gs_free const char **lines = NULL; GPtrArray *block_lines = NULL; - gboolean success = FALSE; + gsize i; + int status = 0; g_return_val_if_fail (iscsiadm_path != NULL, FALSE); g_return_val_if_fail (out_blocks != NULL && *out_blocks == NULL, FALSE); if (!g_spawn_sync ("/", (char **) argv, (char **) envp, 0, NULL, NULL, &out, &err, &status, error)) - goto done; + return FALSE; if (!WIFEXITED (status)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "iBFT: %s exited abnormally.", iscsiadm_path); - goto done; + return FALSE; } if (WEXITSTATUS (status) != 0) { @@ -127,59 +127,47 @@ nms_ibft_reader_load_blocks (const char *iscsiadm_path, g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "iBFT: %s exited with error %d. Message: '%s'", iscsiadm_path, WEXITSTATUS (status), err ?: "(none)"); - goto done; + return FALSE; } nm_log_dbg (LOGD_SETTINGS, "iBFT records:\n%s", out); - lines = g_strsplit_set (out, "\n\r", -1); - for (iter = lines; iter && *iter; iter++) { - if (!*iter[0]) - continue; + lines = nm_utils_strsplit_set (out, "\n\r"); + for (i = 0; lines && lines[i]; i++) { + const char *ss = lines[i]; - if (!g_ascii_strncasecmp (*iter, TAG_BEGIN, NM_STRLEN (TAG_BEGIN))) { + if (!g_ascii_strncasecmp (ss, TAG_BEGIN, NM_STRLEN (TAG_BEGIN))) { if (block_lines) { PARSE_WARNING ("malformed iscsiadm record: missing END RECORD."); - g_ptr_array_unref (block_lines); + nm_clear_pointer (&block_lines, g_ptr_array_unref); } /* Start new record */ block_lines = g_ptr_array_new_full (15, g_free); - } else if (!g_ascii_strncasecmp (*iter, TAG_END, NM_STRLEN (TAG_END))) { + } else if (!g_ascii_strncasecmp (ss, TAG_END, NM_STRLEN (TAG_END))) { if (block_lines) { if (block_lines->len) - blocks = g_slist_prepend (blocks, block_lines); + blocks = g_slist_prepend (blocks, g_steal_pointer (&block_lines)); else - g_ptr_array_unref (block_lines); - block_lines = NULL; + g_ptr_array_unref (g_steal_pointer (&block_lines)); } } else if (block_lines) { - char *s = remove_most_whitespace (*iter); + char *s = remove_most_whitespace (ss); - if (s) + if (!s) { + PARSE_WARNING ("malformed iscsiadm record: no = in '%s'.", ss); + nm_clear_pointer (&block_lines, g_ptr_array_unref); + } else g_ptr_array_add (block_lines, s); - else { - PARSE_WARNING ("malformed iscsiadm record: no = in '%s'.", *iter); - g_clear_pointer (&block_lines, g_ptr_array_unref); - } } } if (block_lines) { PARSE_WARNING ("malformed iscsiadm record: missing # END RECORD."); - g_clear_pointer (&block_lines, g_ptr_array_unref); + nm_clear_pointer (&block_lines, g_ptr_array_unref); } - success = TRUE; -done: - if (lines) - g_strfreev (lines); - g_free (out); - g_free (err); - if (success) - *out_blocks = blocks; - else - g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref); - return success; + *out_blocks = g_steal_pointer (&blocks); + return TRUE; } #define ISCSI_HWADDR_TAG "iface.hwaddress" diff --git a/src/settings/plugins/ibft/nms-ibft-reader.h b/src/settings/plugins/ibft/nms-ibft-reader.h index 27500cc50b..baa81e99c4 100644 --- a/src/settings/plugins/ibft/nms-ibft-reader.h +++ b/src/settings/plugins/ibft/nms-ibft-reader.h @@ -23,6 +23,14 @@ #include "nm-connection.h" +static inline void +_nm_auto_free_ibft_blocks (GSList **p_blocks) +{ + if (*p_blocks) + g_slist_free_full (*p_blocks, (GDestroyNotify) g_ptr_array_unref); +} +#define nm_auto_free_ibft_blocks nm_auto (_nm_auto_free_ibft_blocks) + gboolean nms_ibft_reader_load_blocks (const char *iscsiadm_path, GSList **out_blocks, GError **error); diff --git a/src/settings/plugins/ibft/tests/test-ibft.c b/src/settings/plugins/ibft/tests/test-ibft.c index 4c45f574d2..f5b584a17b 100644 --- a/src/settings/plugins/ibft/tests/test-ibft.c +++ b/src/settings/plugins/ibft/tests/test-ibft.c @@ -40,16 +40,16 @@ static GPtrArray * read_block (const char *iscsiadm_path, const char *expected_mac) { - GSList *blocks = NULL, *iter; + nm_auto_free_ibft_blocks GSList *blocks = NULL; + GSList *iter; GPtrArray *block = NULL; GError *error = NULL; gboolean success; success = nms_ibft_reader_load_blocks (iscsiadm_path, &blocks, &error); - g_assert_no_error (error); - g_assert (success); - g_assert (blocks); + nmtst_assert_success (success, error); + g_assert (blocks); for (iter = blocks; iter; iter = iter->next) { const char *s_hwaddr = NULL; @@ -63,7 +63,6 @@ read_block (const char *iscsiadm_path, const char *expected_mac) } g_assert (block); - g_slist_free_full (blocks, (GDestroyNotify) g_ptr_array_unref); return block; } @@ -176,7 +175,7 @@ static void test_read_ibft_malformed (gconstpointer user_data) { const char *iscsiadm_path = user_data; - GSList *blocks = NULL; + nm_auto_free_ibft_blocks GSList *blocks = NULL; GError *error = NULL; gboolean success; @@ -185,9 +184,9 @@ test_read_ibft_malformed (gconstpointer user_data) NMTST_EXPECT_NM_WARN ("*malformed iscsiadm record*"); success = nms_ibft_reader_load_blocks (iscsiadm_path, &blocks, &error); - g_assert_no_error (error); - g_assert (success); - g_assert (blocks == NULL); + nmtst_assert_success (success, error); + + g_assert (!blocks); g_test_assert_expected_messages (); } From c9ca7d06374e109e40e80c5e67dc98e21d4ff89f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Apr 2019 15:14:49 +0200 Subject: [PATCH 14/18] cli: cleanup nm_vpn_openconnect_authenticate_helper() --- clients/common/nm-vpn-helpers.c | 61 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index fbac26d82e..47a827409f 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -181,18 +181,17 @@ _extract_variable_value (char *line, const char *tag, char **value) { char *p1, *p2; - if (g_str_has_prefix (line, tag)) { - p1 = line + strlen (tag); - p2 = line + strlen (line) - 1; - if ((*p1 == '\'' || *p1 == '"') && (*p1 == *p2)) { - p1++; - *p2 = '\0'; - } - if (value) - *value = g_strdup (p1); - return TRUE; + if (!g_str_has_prefix (line, tag)) + return FALSE; + + p1 = line + strlen (tag); + p2 = line + strlen (line) - 1; + if ((*p1 == '\'' || *p1 == '"') && (*p1 == *p2)) { + p1++; + *p2 = '\0'; } - return FALSE; + NM_SET_OUT (value, g_strdup (p1)); + return TRUE; } gboolean @@ -204,9 +203,8 @@ nm_vpn_openconnect_authenticate_helper (const char *host, GError **error) { gs_free char *output = NULL; - gboolean ret; - char **strv = NULL, **iter; - char *argv[4]; + gs_free const char **output_v = NULL; + const char *const*iter; const char *path; const char *const DEFAULT_PATHS[] = { "/sbin/", @@ -223,17 +221,17 @@ nm_vpn_openconnect_authenticate_helper (const char *host, if (!path) return FALSE; - argv[0] = (char *) path; - argv[1] = "--authenticate"; - argv[2] = (char *) host; - argv[3] = NULL; - - ret = g_spawn_sync (NULL, argv, NULL, - G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN, - NULL, NULL, &output, NULL, - status, error); - - if (!ret) + if (!g_spawn_sync (NULL, + (char **) NM_MAKE_STRV (path, "--authenticate", host), + NULL, + G_SPAWN_SEARCH_PATH + | G_SPAWN_CHILD_INHERITS_STDIN, + NULL, + NULL, + &output, + NULL, + status, + error)) return FALSE; /* Parse output and set cookie, gateway and gwcert @@ -242,13 +240,14 @@ nm_vpn_openconnect_authenticate_helper (const char *host, * HOST='1.2.3.4' * FINGERPRINT='sha1:32bac90cf09a722e10ecc1942c67fe2ac8c21e2e' */ - strv = g_strsplit_set (output ?: "", "\r\n", 0); - for (iter = strv; iter && *iter; iter++) { - _extract_variable_value (*iter, "COOKIE=", cookie); - _extract_variable_value (*iter, "HOST=", gateway); - _extract_variable_value (*iter, "FINGERPRINT=", gwcert); + output_v = nm_utils_strsplit_set_with_empty (output, "\r\n"); + for (iter = output_v; iter && *iter; iter++) { + char *s_mutable = (char *) *iter; + + _extract_variable_value (s_mutable, "COOKIE=", cookie); + _extract_variable_value (s_mutable, "HOST=", gateway); + _extract_variable_value (s_mutable, "FINGERPRINT=", gwcert); } - g_strfreev (strv); return TRUE; } From 34e60bf2282e700347aef5b2b00c5fa3121d0d40 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 13:26:18 +0200 Subject: [PATCH 15/18] cli: cleanup split_required_fields_for_con_show() - return early and use cleanup attribute for freeing memory - use nm_utils_strsplit_set_with_empty() instead of g_strsplit_set(). --- clients/cli/connections.c | 92 ++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 97c46923ae..d3193ccb3c 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1528,15 +1528,12 @@ split_required_fields_for_con_show (const char *input, char **active_flds, GError **error) { - char **fields, **iter; - char *dot; - GString *str1, *str2; - gboolean found; + gs_free const char **fields = NULL; + const char *const*iter; + nm_auto_free_gstring GString *str1 = NULL; + nm_auto_free_gstring GString *str2 = NULL; gboolean group_profile = FALSE; gboolean group_active = FALSE; - gboolean success = TRUE; - gboolean is_all, is_common; - int i; if (!input) { *profile_flds = NULL; @@ -1547,25 +1544,30 @@ split_required_fields_for_con_show (const char *input, str1 = g_string_new (NULL); str2 = g_string_new (NULL); - /* Split supplied fields string */ - fields = g_strsplit_set (input, ",", -1); + fields = nm_utils_strsplit_set_with_empty (input, ","); for (iter = fields; iter && *iter; iter++) { - g_strstrip (*iter); - dot = strchr (*iter, '.'); + char *s_mutable = (char *) (*iter); + char *dot; + gboolean is_all; + gboolean is_common; + gboolean found; + int i; + + g_strstrip (s_mutable); + dot = strchr (s_mutable, '.'); if (dot) *dot = '\0'; - is_all = !dot && strcasecmp (*iter, "all") == 0; - is_common = !dot && strcasecmp (*iter, "common") == 0; + is_all = !dot && strcasecmp (s_mutable, "all") == 0; + is_common = !dot && strcasecmp (s_mutable, "common") == 0; found = FALSE; - for (i = 0; i < _NM_META_SETTING_TYPE_NUM; i++) { if ( is_all || is_common - || !strcasecmp (*iter, nm_meta_setting_infos[i].setting_name)) { + || !strcasecmp (s_mutable, nm_meta_setting_infos[i].setting_name)) { if (dot) *dot = '.'; - g_string_append (str1, *iter); + g_string_append (str1, s_mutable); g_string_append_c (str1, ','); found = TRUE; break; @@ -1573,12 +1575,13 @@ split_required_fields_for_con_show (const char *input, } if (found) continue; + for (i = 0; nmc_fields_con_active_details_groups[i]; i++) { if ( is_all || is_common - || !strcasecmp (*iter, nmc_fields_con_active_details_groups[i]->name)) { + || !strcasecmp (s_mutable, nmc_fields_con_active_details_groups[i]->name)) { if (dot) *dot = '.'; - g_string_append (str2, *iter); + g_string_append (str2, s_mutable); g_string_append_c (str2, ','); found = TRUE; break; @@ -1587,55 +1590,46 @@ split_required_fields_for_con_show (const char *input, if (!found) { if (dot) *dot = '.'; - if (!strcasecmp (*iter, CON_SHOW_DETAIL_GROUP_PROFILE)) + if (!strcasecmp (s_mutable, CON_SHOW_DETAIL_GROUP_PROFILE)) group_profile = TRUE; - else if (!strcasecmp (*iter, CON_SHOW_DETAIL_GROUP_ACTIVE)) + else if (!strcasecmp (s_mutable, CON_SHOW_DETAIL_GROUP_ACTIVE)) group_active = TRUE; else { - char *allowed1 = nm_meta_abstract_infos_get_names_str ((const NMMetaAbstractInfo *const*) nm_meta_setting_infos_editor_p (), NULL); - char *allowed2 = nm_meta_abstract_infos_get_names_str ((const NMMetaAbstractInfo *const*) nmc_fields_con_active_details_groups, NULL); + gs_free char *allowed1 = nm_meta_abstract_infos_get_names_str ((const NMMetaAbstractInfo *const*) nm_meta_setting_infos_editor_p (), NULL); + gs_free char *allowed2 = nm_meta_abstract_infos_get_names_str ((const NMMetaAbstractInfo *const*) nmc_fields_con_active_details_groups, NULL); + g_set_error (error, NMCLI_ERROR, 0, _("invalid field '%s'; allowed fields: %s and %s, or %s,%s"), - *iter, allowed1, allowed2, CON_SHOW_DETAIL_GROUP_PROFILE, CON_SHOW_DETAIL_GROUP_ACTIVE); - g_free (allowed1); - g_free (allowed2); - success = FALSE; - break; + s_mutable, allowed1, allowed2, CON_SHOW_DETAIL_GROUP_PROFILE, CON_SHOW_DETAIL_GROUP_ACTIVE); + return FALSE; } } } - if (fields) - g_strfreev (fields); /* Handle pseudo groups: profile, active */ - if (success && group_profile) { + if (group_profile) { if (str1->len > 0) { g_set_error (error, NMCLI_ERROR, 0, _("'%s' has to be alone"), CON_SHOW_DETAIL_GROUP_PROFILE); - success = FALSE; - } else - g_string_assign (str1, "all,"); + return FALSE; + } + g_string_assign (str1, "all,"); } - if (success && group_active) { + if (group_active) { if (str2->len > 0) { g_set_error (error, NMCLI_ERROR, 0, _("'%s' has to be alone"), CON_SHOW_DETAIL_GROUP_ACTIVE); - success = FALSE; - } else - g_string_assign (str2, "all,"); + return FALSE; + } + g_string_assign (str2, "all,"); } - if (success) { - if (str1->len > 0) - g_string_truncate (str1, str1->len - 1); - if (str2->len > 0) - g_string_truncate (str2, str2->len - 1); - *profile_flds = g_string_free (str1, str1->len == 0); - *active_flds = g_string_free (str2, str2->len == 0); - } else { - g_string_free (str1, TRUE); - g_string_free (str2, TRUE); - } - return success; + if (str1->len > 0) + g_string_truncate (str1, str1->len - 1); + if (str2->len > 0) + g_string_truncate (str2, str2->len - 1); + *profile_flds = g_string_free (g_steal_pointer (&str1), str1->len == 0); + *active_flds = g_string_free (g_steal_pointer (&str2), str2->len == 0); + return TRUE; } typedef enum { From 3e0366a3ff6ba3aad8928c885e2073a1264e48e0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 13:06:00 +0200 Subject: [PATCH 16/18] all: replace g_strsplit_set() by nm_utils_strsplit_set*() --- clients/common/nm-meta-setting-desc.c | 10 ++--- src/devices/nm-device-bond.c | 18 ++++----- src/nm-logging.c | 40 +++++++++---------- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 19 ++++----- 4 files changed, 40 insertions(+), 47 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 25337cc3a3..2b0386265c 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -2837,12 +2837,12 @@ dcb_parse_uint_array (const char *val, guint out_array[static 8], GError **error) { - gs_strfreev char **items = NULL; - char **iter; + gs_free const char **items = NULL; + const char *const*iter; gsize i; - items = g_strsplit_set (val, ",", -1); - if (g_strv_length (items) != 8) { + items = nm_utils_strsplit_set_with_empty (val, ","); + if (NM_PTRARRAY_LEN (items) != 8) { g_set_error_literal (error, 1, 0, _("must contain 8 comma-separated numbers")); return FALSE; } @@ -2851,8 +2851,6 @@ dcb_parse_uint_array (const char *val, for (iter = items; *iter; iter++) { gint64 num; - *iter = g_strstrip (*iter); - num = _nm_utils_ascii_str_to_int64 (*iter, 10, 0, other ?: max, -1); /* If number is greater than 'max' it must equal 'other' */ diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 6dabdfe855..37159fca8c 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -185,20 +185,18 @@ set_arp_targets (NMDevice *device, const char *delim, const char *prefix) { - char **items, **iter, *tmp; + gs_free const char **value_v = NULL; + gsize i; - if (!value || !*value) + value_v = nm_utils_strsplit_set (value, delim); + if (!value_v) return; + for (i = 0; value_v[i]; i++) { + gs_free char *tmp = NULL; - items = g_strsplit_set (value, delim, 0); - for (iter = items; iter && *iter; iter++) { - if (*iter[0]) { - tmp = g_strdup_printf ("%s%s", prefix, *iter); - set_bond_attr (device, mode, NM_SETTING_BOND_OPTION_ARP_IP_TARGET, tmp); - g_free (tmp); - } + tmp = g_strdup_printf ("%s%s", prefix, value_v[i]); + set_bond_attr (device, mode, NM_SETTING_BOND_OPTION_ARP_IP_TARGET, tmp); } - g_strfreev (items); } static void diff --git a/src/nm-logging.c b/src/nm-logging.c index 1ee9464541..511a4fdd66 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -302,7 +302,8 @@ nm_logging_setup (const char *level, NMLogDomain new_log_state[_LOGL_N_REAL]; NMLogLevel cur_log_level; NMLogLevel new_log_level; - char **tmp, **iter; + gs_free const char **domains_v = NULL; + gsize i_d; int i; gboolean had_platform_debug; gs_free char *domains_free = NULL; @@ -337,28 +338,24 @@ nm_logging_setup (const char *level, } } - tmp = g_strsplit_set (domains, ", ", 0); - for (iter = tmp; iter && *iter; iter++) { + domains_v = nm_utils_strsplit_set (domains, ", "); + for (i_d = 0; domains_v && domains_v[i_d]; i_d++) { + const char *s = domains_v[i_d]; + const char *p; const LogDesc *diter; NMLogLevel domain_log_level; NMLogDomain bits; - char *p; /* LOGD_VPN_PLUGIN is protected, that is, when setting ALL or DEFAULT, * it does not enable the verbose levels DEBUG and TRACE, because that * may expose sensitive data. */ NMLogDomain protect = LOGD_NONE; - if (!strlen (*iter)) - continue; - - p = strchr (*iter, ':'); + p = strchr (s, ':'); if (p) { - *p = '\0'; - if (!match_log_level (p + 1, &domain_log_level, error)) { - g_strfreev (tmp); + *((char *) p) = '\0'; + if (!match_log_level (p + 1, &domain_log_level, error)) return FALSE; - } } else domain_log_level = new_log_level; @@ -372,26 +369,26 @@ nm_logging_setup (const char *level, } /* Check for combined domains */ - if (!g_ascii_strcasecmp (*iter, LOGD_ALL_STRING)) { + if (!g_ascii_strcasecmp (s, LOGD_ALL_STRING)) { bits = LOGD_ALL; protect = LOGD_VPN_PLUGIN; - } else if (!g_ascii_strcasecmp (*iter, LOGD_DEFAULT_STRING)) { + } else if (!g_ascii_strcasecmp (s, LOGD_DEFAULT_STRING)) { bits = LOGD_DEFAULT; protect = LOGD_VPN_PLUGIN; - } else if (!g_ascii_strcasecmp (*iter, LOGD_DHCP_STRING)) + } else if (!g_ascii_strcasecmp (s, LOGD_DHCP_STRING)) bits = LOGD_DHCP; - else if (!g_ascii_strcasecmp (*iter, LOGD_IP_STRING)) + else if (!g_ascii_strcasecmp (s, LOGD_IP_STRING)) bits = LOGD_IP; /* Check for compatibility domains */ - else if (!g_ascii_strcasecmp (*iter, "HW")) + else if (!g_ascii_strcasecmp (s, "HW")) bits = LOGD_PLATFORM; - else if (!g_ascii_strcasecmp (*iter, "WIMAX")) + else if (!g_ascii_strcasecmp (s, "WIMAX")) continue; else { for (diter = &domain_desc[0]; diter->name; diter++) { - if (!g_ascii_strcasecmp (diter->name, *iter)) { + if (!g_ascii_strcasecmp (diter->name, s)) { bits = diter->num; break; } @@ -400,7 +397,7 @@ nm_logging_setup (const char *level, if (!bits) { if (!bad_domains) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_LOG_DOMAIN, - _("Unknown log domain '%s'"), *iter); + _("Unknown log domain '%s'"), s); return FALSE; } @@ -408,7 +405,7 @@ nm_logging_setup (const char *level, g_string_append (unrecognized, ", "); else unrecognized = g_string_new (NULL); - g_string_append (unrecognized, *iter); + g_string_append (unrecognized, s); continue; } } @@ -429,7 +426,6 @@ nm_logging_setup (const char *level, } } } - g_strfreev (tmp); g_clear_pointer (&gl_main.logging_domains_to_string, g_free); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 93d48dfdd4..acf0c55ac6 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -4548,22 +4548,23 @@ make_wired_setting (shvarFile *ifcfg, value = svGetValueStr_cp (ifcfg, "OPTIONS"); if (value) { - char **options, **iter; + gs_free const char **options = NULL; + gsize i; - iter = options = g_strsplit_set (value, " ", 0); - while (iter && *iter) { - char *equals = strchr (*iter, '='); + options = nm_utils_strsplit_set_with_empty (value, " "); + for (i = 0; options && options[i]; i++) { + const char *line = options[i]; + const char *equals; gboolean valid = FALSE; + equals = strchr (line, '='); if (equals) { - *equals = '\0'; - valid = nm_setting_wired_add_s390_option (s_wired, *iter, equals + 1); + ((char *) equals)[0] = '\0'; + valid = nm_setting_wired_add_s390_option (s_wired, line, equals + 1); } if (!valid) - PARSE_WARNING ("invalid s390 OPTION '%s'", *iter); - iter++; + PARSE_WARNING ("invalid s390 OPTION '%s'", line); } - g_strfreev (options); nm_clear_g_free (&value); } From c1f340401ffeb6d22d90d3dff841bfbc50466c61 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 Apr 2019 15:58:40 +0200 Subject: [PATCH 17/18] ifcfg-rh: various cleanups using the cleanup attribute --- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 82 +++++++------------ 1 file changed, 29 insertions(+), 53 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index acf0c55ac6..3393fca2e2 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -82,7 +82,8 @@ static char * get_full_file_path (const char *ifcfg_path, const char *file_path) { const char *base = file_path; - char *p, *ret, *dirname; + gs_free char *dirname = NULL; + char *p; g_return_val_if_fail (ifcfg_path != NULL, NULL); g_return_val_if_fail (file_path != NULL, NULL); @@ -95,9 +96,7 @@ get_full_file_path (const char *ifcfg_path, const char *file_path) base = p + 1; dirname = g_path_get_dirname (ifcfg_path); - ret = g_build_path ("/", dirname, base, NULL); - g_free (dirname); - return ret; + return g_build_path ("/", dirname, base, NULL); } /*****************************************************************************/ @@ -379,7 +378,7 @@ make_connection_setting (const char *file, NMSettingConnection *s_con; NMSettingConnectionLldp lldp; const char *ifcfg_name = NULL; - char *new_id; + gs_free char *new_id = NULL; const char *uuid; gs_free char *uuid_free = NULL; gs_free char *value = NULL; @@ -396,7 +395,6 @@ make_connection_setting (const char *file, new_id = make_connection_name (ifcfg, ifcfg_name, suggested, prefix); g_object_set (s_con, NM_SETTING_CONNECTION_ID, new_id, NULL); - g_free (new_id); /* Try for a UUID key before falling back to hashing the file name */ uuid = svGetValueStr (ifcfg, "UUID", &uuid_free); @@ -1809,7 +1807,8 @@ static void read_aliases (NMSettingIPConfig *s_ip4, gboolean read_defroute, const char *filename) { GDir *dir; - char *dirname, *base; + gs_free char *dirname = NULL; + gs_free char *base = NULL; NMIPAddress *base_addr = NULL; GError *err = NULL; @@ -1820,9 +1819,9 @@ read_aliases (NMSettingIPConfig *s_ip4, gboolean read_defroute, const char *file base_addr = nm_setting_ip_config_get_address (s_ip4, 0); dirname = g_path_get_dirname (filename); - g_return_if_fail (dirname != NULL); + nm_assert (dirname != NULL); base = g_path_get_basename (filename); - g_return_if_fail (base != NULL); + nm_assert (base != NULL); dir = g_dir_open (dirname, 0, &err); if (dir) { @@ -1911,9 +1910,6 @@ read_aliases (NMSettingIPConfig *s_ip4, gboolean read_defroute, const char *file PARSE_WARNING ("can not read directory '%s': %s", dirname, err->message); g_error_free (err); } - - g_free (base); - g_free (dirname); } static NMSetting * @@ -1922,10 +1918,9 @@ make_ip6_setting (shvarFile *ifcfg, gboolean routes_read, GError **error) { - NMSettingIPConfig *s_ip6 = NULL; + gs_unref_object NMSettingIPConfig *s_ip6 = NULL; const char *v; gs_free char *value = NULL; - char *route6_path = NULL; gboolean ipv6init, ipv6forwarding, dhcp6 = FALSE; char *method = NM_SETTING_IP6_CONFIG_METHOD_MANUAL; const char *ipv6addr, *ipv6addr_secondaries; @@ -2060,7 +2055,7 @@ make_ip6_setting (shvarFile *ifcfg, /* Don't bother to read IP, DNS and routes when IPv6 is disabled */ if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) == 0) - return NM_SETTING (s_ip6); + return NM_SETTING (g_steal_pointer (&s_ip6)); nm_clear_g_free (&value); v = svGetValueStr (ifcfg, "DHCPV6_DUID", &value); @@ -2104,7 +2099,7 @@ make_ip6_setting (shvarFile *ifcfg, NMIPAddress *addr = NULL; if (!parse_full_ip6_address (ifcfg, *iter, i, &addr, error)) - goto error; + return NULL; if (!nm_setting_ip_config_add_address (s_ip6, addr)) PARSE_WARNING ("duplicate IP6 address"); @@ -2129,7 +2124,7 @@ make_ip6_setting (shvarFile *ifcfg, if (!nm_utils_ipaddr_valid (AF_INET6, v)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid IP6 address '%s'", v); - goto error; + return NULL; } g_object_set (s_ip6, NM_SETTING_IP_CONFIG_GATEWAY, v, NULL); @@ -2172,18 +2167,19 @@ make_ip6_setting (shvarFile *ifcfg, /* Ignore IPv4 addresses */ } else { PARSE_WARNING ("invalid DNS server address %s", v); - goto error; + return NULL; } } if (!routes_read) { /* NOP */ } else { + gs_free char *route6_path = NULL; + /* Read static routes from route6- file */ route6_path = utils_get_route6_path (svFileGetName (ifcfg)); if (!read_route_file (AF_INET6, route6_path, s_ip6, error)) - goto error; - g_free (route6_path); + return NULL; } /* DNS searches */ @@ -2212,12 +2208,7 @@ make_ip6_setting (shvarFile *ifcfg, priority, NULL); - return NM_SETTING (s_ip6); - -error: - g_free (route6_path); - g_object_unref (s_ip6); - return NULL; + return NM_SETTING (g_steal_pointer (&s_ip6)); } static NMSetting * @@ -2588,10 +2579,9 @@ make_dcb_setting (shvarFile *ifcfg, NMSetting **out_setting, GError **error) { - NMSettingDcb *s_dcb = NULL; + gs_unref_object NMSettingDcb *s_dcb = NULL; gboolean dcb_on; NMSettingDcbFlags flags = NM_SETTING_DCB_FLAG_NONE; - char *val; g_return_val_if_fail (out_setting != NULL, FALSE); @@ -2600,31 +2590,28 @@ make_dcb_setting (shvarFile *ifcfg, return TRUE; s_dcb = (NMSettingDcb *) nm_setting_dcb_new (); - g_assert (s_dcb); /* FCOE */ if (!read_dcb_app (ifcfg, s_dcb, "FCOE", &dcb_flags_props[DCB_APP_FCOE_FLAGS], NM_SETTING_DCB_APP_FCOE_PRIORITY, error)) { - g_object_unref (s_dcb); return FALSE; } if (nm_setting_dcb_get_app_fcoe_flags (s_dcb) & NM_SETTING_DCB_FLAG_ENABLE) { + gs_free char *val = NULL; + val = svGetValueStr_cp (ifcfg, KEY_DCB_APP_FCOE_MODE); if (val) { - if (strcmp (val, NM_SETTING_DCB_FCOE_MODE_FABRIC) == 0 || - strcmp (val, NM_SETTING_DCB_FCOE_MODE_VN2VN) == 0) + if (NM_IN_STRSET (val, NM_SETTING_DCB_FCOE_MODE_FABRIC, + NM_SETTING_DCB_FCOE_MODE_VN2VN)) g_object_set (G_OBJECT (s_dcb), NM_SETTING_DCB_APP_FCOE_MODE, val, NULL); else { PARSE_WARNING ("invalid FCoE mode '%s'", val); g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "invalid FCoE mode"); - g_free (val); - g_object_unref (s_dcb); return FALSE; } - g_free (val); } } @@ -2633,7 +2620,6 @@ make_dcb_setting (shvarFile *ifcfg, &dcb_flags_props[DCB_APP_ISCSI_FLAGS], NM_SETTING_DCB_APP_ISCSI_PRIORITY, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2642,7 +2628,6 @@ make_dcb_setting (shvarFile *ifcfg, &dcb_flags_props[DCB_APP_FIP_FLAGS], NM_SETTING_DCB_APP_FIP_PRIORITY, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2657,7 +2642,6 @@ make_dcb_setting (shvarFile *ifcfg, "PFC", nm_setting_dcb_set_priority_flow_control, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2673,7 +2657,6 @@ make_dcb_setting (shvarFile *ifcfg, TRUE, nm_setting_dcb_set_priority_group_id, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2686,7 +2669,6 @@ make_dcb_setting (shvarFile *ifcfg, TRUE, nm_setting_dcb_set_priority_group_bandwidth, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2699,7 +2681,6 @@ make_dcb_setting (shvarFile *ifcfg, FALSE, nm_setting_dcb_set_priority_bandwidth, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2711,7 +2692,6 @@ make_dcb_setting (shvarFile *ifcfg, "STRICT", nm_setting_dcb_set_priority_strict_bandwidth, error)) { - g_object_unref (s_dcb); return FALSE; } @@ -2723,11 +2703,10 @@ make_dcb_setting (shvarFile *ifcfg, FALSE, nm_setting_dcb_set_priority_traffic_class, error)) { - g_object_unref (s_dcb); return FALSE; } - *out_setting = NM_SETTING (s_dcb); + *out_setting = NM_SETTING (g_steal_pointer (&s_dcb)); return TRUE; } @@ -2834,7 +2813,7 @@ make_wep_setting (shvarFile *ifcfg, GError **error) { gs_unref_object NMSettingWirelessSecurity *s_wsec = NULL; - char *value; + gs_free char *value = NULL; shvarFile *keys_ifcfg = NULL; int default_key_idx = 0; gboolean has_default_key = FALSE; @@ -2849,13 +2828,12 @@ make_wep_setting (shvarFile *ifcfg, if (default_key_idx == 0) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid default WEP key '%s'", value); - g_free (value); return NULL; } has_default_key = TRUE; default_key_idx--; /* convert to [0...3] */ g_object_set (s_wsec, NM_SETTING_WIRELESS_SECURITY_WEP_TX_KEYIDX, (guint) default_key_idx, NULL); - g_free (value); + nm_clear_g_free (&value); } /* Read WEP key flags */ @@ -2902,23 +2880,21 @@ make_wep_setting (shvarFile *ifcfg, value = svGetValueStr_cp (ifcfg, "SECURITYMODE"); if (value) { - char *lcase; + gs_free char *lcase = NULL; lcase = g_ascii_strdown (value, -1); - g_free (value); + nm_clear_g_free (&value); - if (!strcmp (lcase, "open")) { + if (nm_streq (lcase, "open")) { g_object_set (s_wsec, NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "open", NULL); - } else if (!strcmp (lcase, "restricted")) { + } else if (nm_streq (lcase, "restricted")) { g_object_set (s_wsec, NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "shared", NULL); } else { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid WEP authentication algorithm '%s'", lcase); - g_free (lcase); return NULL; } - g_free (lcase); } /* If no WEP keys were given, and the keys are not agent-owned, and no From a1425a4c91d2b653dfdeea5972b12262abf251c3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 5 Apr 2019 22:44:49 +0200 Subject: [PATCH 18/18] shared: pre-calculate number of tokens in nm_utils_strsplit_set_full() Instead of growing the buffer for the tokens (and reallocating), do one pre-run over the string and count the delimiters. This way we know how much space we need and we don't need to reallocate. Interestingly, this is notably slower than the previous implementation, because previously if would not bother determining the right number of tokens but just over-allocate with a reasonable guess of 8 and grow the buffer exponentially. Still, I like this better because while it may be slower in common scenarios, it allocates the exact number of buffer space. --- shared/nm-utils/nm-shared-utils.c | 110 ++++++++++++++++++------------ 1 file changed, 66 insertions(+), 44 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index e8f3563c09..879d1e7b9e 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -1020,12 +1020,11 @@ nm_utils_strsplit_set_full (const char *str, const char *delimiters, NMUtilsStrsplitSetFlags flags) { - const char **ptr, **ptr0; - gsize alloc_size; - gsize plen; - gsize i; - gsize str_len; - char *s0; + const char **ptr; + gsize num_tokens; + gsize i_token; + gsize str_len_p1; + const char *c_str; char *s; guint8 ch_lookup[256]; const gboolean f_allow_escaping = NM_FLAGS_HAS (flags, NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING); @@ -1056,37 +1055,68 @@ nm_utils_strsplit_set_full (const char *str, return NULL; } - str_len = strlen (str) + 1; - alloc_size = 8; - - /* we allocate the buffer larger, so to copy @str at the - * end of it as @s0. */ - ptr0 = g_malloc ((sizeof (const char *) * (alloc_size + 1)) + str_len); - s0 = (char *) &ptr0[alloc_size + 1]; - memcpy (s0, str, str_len); - - plen = 0; - s = s0; - ptr = ptr0; + num_tokens = 1; + c_str = str; while (TRUE) { - if (plen >= alloc_size) { - const char **ptr_old = ptr; - /* reallocate the buffer. Note that for now the string - * continues to be in ptr0/s0. We fix that at the end. */ - alloc_size *= 2; - ptr = g_malloc ((sizeof (const char *) * (alloc_size + 1)) + str_len); - memcpy (ptr, ptr_old, sizeof (const char *) * plen); - if (ptr_old != ptr0) - g_free (ptr_old); + while (G_LIKELY (!_char_lookup_has (ch_lookup, c_str[0]))) { + if (c_str[0] == '\0') + goto done1; + c_str++; } - ptr[plen++] = s; + /* we assume escapings are not frequent. After we found + * this delimiter, check whether it was escaped by counting + * the backslashed before. */ + if (f_allow_escaping) { + const char *c2 = c_str; + + while ( c2 > str + && c2[-1] == '\\') + c2--; + if (((c_str - c2) % 2) != 0) { + /* the delimiter is escaped. This was not an accepted delimiter. */ + c_str++; + continue; + } + } + + c_str++; + + /* if we drop empty tokens, then we now skip over all consecutive delimiters. */ + if (!f_preseve_empty) { + while (_char_lookup_has (ch_lookup, c_str[0])) + c_str++; + if (c_str[0] == '\0') + break; + } + + num_tokens++; + } + +done1: + + nm_assert (c_str[0] == '\0'); + + str_len_p1 = (c_str - str) + 1; + + nm_assert (str[str_len_p1 - 1] == '\0'); + + ptr = g_malloc ((sizeof (const char *) * (num_tokens + 1)) + str_len_p1); + s = (char *) &ptr[num_tokens + 1]; + memcpy (s, str, str_len_p1); + + i_token = 0; + + while (TRUE) { + + nm_assert (i_token < num_tokens); + ptr[i_token++] = s; if (s[0] == '\0') { nm_assert (f_preseve_empty); - goto done; + goto done2; } nm_assert ( f_preseve_empty || !_char_lookup_has (ch_lookup, s[0])); @@ -1096,10 +1126,10 @@ nm_utils_strsplit_set_full (const char *str, && f_allow_escaping)) { s++; if (s[0] == '\0') - goto done; + goto done2; s++; } else if (s[0] == '\0') - goto done; + goto done2; else s++; } @@ -1107,26 +1137,18 @@ nm_utils_strsplit_set_full (const char *str, nm_assert (_char_lookup_has (ch_lookup, s[0])); s[0] = '\0'; s++; + if (!f_preseve_empty) { while (_char_lookup_has (ch_lookup, s[0])) s++; if (s[0] == '\0') - goto done; + goto done2; } } -done: - ptr[plen] = NULL; - - if (ptr != ptr0) { - /* we reallocated the buffer. We must copy over the - * string @s0 and adjust the pointers. */ - s = (char *) &ptr[alloc_size + 1]; - memcpy (s, s0, str_len); - for (i = 0; i < plen; i++) - ptr[i] = &s[ptr[i] - s0]; - g_free (ptr0); - } +done2: + nm_assert (i_token == num_tokens); + ptr[i_token] = NULL; return ptr; }