From 4e7eadd6d3425f301277448329d53e93b09236b9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Mar 2019 20:28:44 +0100 Subject: [PATCH 01/98] cli: fix appending team link-watchers The set_fcn() function is supposed to only append. For the set-all mode, the caller ensures to clear the array first. --- clients/common/nm-meta-setting-desc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 6e35228ad6..838661c2e4 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -4144,7 +4144,6 @@ _set_fcn_team_link_watchers (ARGS_SET_FCN) const char *const*iter; NMTeamLinkWatcher *watcher; - nm_setting_team_clear_link_watchers (NM_SETTING_TEAM (setting)); strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { watcher = _parse_team_link_watcher (*iter, error); @@ -4218,7 +4217,6 @@ _set_fcn_team_port_link_watchers (ARGS_SET_FCN) const char *const*iter; NMTeamLinkWatcher *watcher; - nm_setting_team_port_clear_link_watchers (NM_SETTING_TEAM_PORT (setting)); strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { watcher = _parse_team_link_watcher (*iter, error); From e187bd08feb8c2ea8ed6716e76c23476353ca7d0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Mar 2019 20:30:21 +0100 Subject: [PATCH 02/98] cli: fix leaking error variables setting vfs, qdiscs and tfilters --- clients/common/nm-meta-setting-desc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 838661c2e4..5dc5433f26 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -3871,7 +3871,7 @@ _set_fcn_sriov_vfs (ARGS_SET_FCN) gs_free const char **strv = NULL; const char *const*iter; NMSriovVF *vf; - GError *local = NULL; + gs_free_error GError *local = NULL; strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { @@ -3893,7 +3893,7 @@ _set_fcn_tc_config_qdiscs (ARGS_SET_FCN) gs_free const char **strv = NULL; const char *const*iter; NMTCQdisc *tc_qdisc; - GError *local = NULL; + gs_free_error GError *local = NULL; strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { @@ -3998,7 +3998,7 @@ _set_fcn_tc_config_tfilters (ARGS_SET_FCN) gs_free const char **strv = NULL; const char *const*iter; NMTCTfilter *tc_tfilter; - GError *local = NULL; + gs_free_error GError *local = NULL; strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { From 731d251cc08f5f5d01526cf220f61963f1c43fa7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 17 Mar 2019 13:38:07 +0100 Subject: [PATCH 03/98] shared: don't implement nm_utils_parse_inaddr() based on nm_utils_parse_inaddr_bin() nm_utils_parse_inaddr() is trivial enough to reimplement it, instead of calling nm_utils_parse_inaddr_bin(). Calling nm_utils_parse_inaddr_bin() involves several things that don't matter for nm_utils_parse_inaddr() -- like assigning out_addr_family or returning the binary address. --- shared/nm-utils/nm-shared-utils.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 93e0b97e20..5e4ba0d2ab 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -635,8 +635,16 @@ nm_utils_parse_inaddr (int addr_family, NMIPAddr addrbin; char addrstr_buf[MAX (INET_ADDRSTRLEN, INET6_ADDRSTRLEN)]; - if (!nm_utils_parse_inaddr_bin (addr_family, text, &addr_family, &addrbin)) + g_return_val_if_fail (text, FALSE); + + if (addr_family == AF_UNSPEC) + addr_family = strchr (text, ':') ? AF_INET6 : AF_INET; + else + g_return_val_if_fail (NM_IN_SET (addr_family, AF_INET, AF_INET6), FALSE); + + if (inet_pton (addr_family, text, &addrbin) != 1) return FALSE; + NM_SET_OUT (out_addr, g_strdup (inet_ntop (addr_family, &addrbin, addrstr_buf, sizeof (addrstr_buf)))); return TRUE; } From 395738900fd4de294696dd0a4a882f42a5e9943e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 21:21:43 +0100 Subject: [PATCH 04/98] libnm: don't use strlen() for checking for non-empty string It's well understood that these are NUL terminated strings. We don't need strlen() to check that the strings aren't empty. --- libnm-core/nm-setting-connection.c | 6 ++---- libnm-core/nm-setting-vpn.c | 14 ++++++-------- libnm-core/nm-setting-wired.c | 9 +++------ libnm-core/tests/test-general.c | 26 +++++++++++++------------- libnm/nm-secret-agent-old.c | 3 +-- 5 files changed, 25 insertions(+), 33 deletions(-) diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index e8d008e33d..79ffa477e4 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -397,8 +397,7 @@ nm_setting_connection_add_permission (NMSettingConnection *setting, GSList *iter; g_return_val_if_fail (NM_IS_SETTING_CONNECTION (setting), FALSE); - g_return_val_if_fail (ptype, FALSE); - g_return_val_if_fail (strlen (ptype) > 0, FALSE); + g_return_val_if_fail (ptype && ptype[0], FALSE); g_return_val_if_fail (detail == NULL, FALSE); /* Only "user" for now... */ @@ -470,8 +469,7 @@ nm_setting_connection_remove_permission_by_value (NMSettingConnection *setting, GSList *iter; g_return_val_if_fail (NM_IS_SETTING_CONNECTION (setting), FALSE); - g_return_val_if_fail (ptype, FALSE); - g_return_val_if_fail (strlen (ptype) > 0, FALSE); + g_return_val_if_fail (ptype && ptype[0], FALSE); g_return_val_if_fail (detail == NULL, FALSE); /* Only "user" for now... */ diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index 20a0ae14a0..fc4b538ad2 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -173,10 +173,8 @@ nm_setting_vpn_add_data_item (NMSettingVpn *setting, const char *item) { g_return_if_fail (NM_IS_SETTING_VPN (setting)); - g_return_if_fail (key != NULL); - g_return_if_fail (strlen (key) > 0); - g_return_if_fail (item != NULL); - g_return_if_fail (strlen (item) > 0); + g_return_if_fail (key && key[0]); + g_return_if_fail (item && item[0]); g_hash_table_insert (NM_SETTING_VPN_GET_PRIVATE (setting)->data, g_strdup (key), g_strdup (item)); @@ -242,6 +240,7 @@ nm_setting_vpn_remove_data_item (NMSettingVpn *setting, const char *key) gboolean found; g_return_val_if_fail (NM_IS_SETTING_VPN (setting), FALSE); + g_return_val_if_fail (key, FALSE); found = g_hash_table_remove (NM_SETTING_VPN_GET_PRIVATE (setting)->data, key); if (found) @@ -350,10 +349,8 @@ nm_setting_vpn_add_secret (NMSettingVpn *setting, const char *secret) { g_return_if_fail (NM_IS_SETTING_VPN (setting)); - g_return_if_fail (key != NULL); - g_return_if_fail (strlen (key) > 0); - g_return_if_fail (secret != NULL); - g_return_if_fail (strlen (secret) > 0); + g_return_if_fail (key && key[0]); + g_return_if_fail (secret && secret[0]); g_hash_table_insert (NM_SETTING_VPN_GET_PRIVATE (setting)->secrets, g_strdup (key), g_strdup (secret)); @@ -419,6 +416,7 @@ nm_setting_vpn_remove_secret (NMSettingVpn *setting, const char *key) gboolean found; g_return_val_if_fail (NM_IS_SETTING_VPN (setting), FALSE); + g_return_val_if_fail (key, FALSE); found = g_hash_table_remove (NM_SETTING_VPN_GET_PRIVATE (setting)->secrets, key); if (found) diff --git a/libnm-core/nm-setting-wired.c b/libnm-core/nm-setting-wired.c index ef23294fda..da786d21f0 100644 --- a/libnm-core/nm-setting-wired.c +++ b/libnm-core/nm-setting-wired.c @@ -483,8 +483,7 @@ nm_setting_wired_get_s390_option_by_key (NMSettingWired *setting, const char *key) { g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), NULL); - g_return_val_if_fail (key != NULL, NULL); - g_return_val_if_fail (strlen (key), NULL); + g_return_val_if_fail (key && key[0], NULL); return g_hash_table_lookup (NM_SETTING_WIRED_GET_PRIVATE (setting)->s390_options, key); } @@ -511,8 +510,7 @@ nm_setting_wired_add_s390_option (NMSettingWired *setting, size_t value_len; g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE); - g_return_val_if_fail (key != NULL, FALSE); - g_return_val_if_fail (strlen (key), FALSE); + g_return_val_if_fail (key && key[0], FALSE); g_return_val_if_fail (g_strv_contains (valid_s390_opts, key), FALSE); g_return_val_if_fail (value != NULL, FALSE); @@ -544,8 +542,7 @@ nm_setting_wired_remove_s390_option (NMSettingWired *setting, gboolean found; g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE); - g_return_val_if_fail (key != NULL, FALSE); - g_return_val_if_fail (strlen (key), FALSE); + g_return_val_if_fail (key && key[0], FALSE); found = g_hash_table_remove (NM_SETTING_WIRED_GET_PRIVATE (setting)->s390_options, key); if (found) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 7d97296ab4..9fcf7d006c 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -809,54 +809,54 @@ test_setting_vpn_items (void) nm_setting_vpn_remove_data_item (s_vpn, "foobar4-flags"); /* Try to add some blank values and make sure they are rejected */ - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_data_item (s_vpn, NULL, NULL); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (key) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_data_item (s_vpn, "", ""); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (item != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (item && item[0])); nm_setting_vpn_add_data_item (s_vpn, "foobar1", NULL); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (item) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (item && item[0])); nm_setting_vpn_add_data_item (s_vpn, "foobar1", ""); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_data_item (s_vpn, NULL, "blahblah1"); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (key) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_data_item (s_vpn, "", "blahblah1"); g_test_assert_expected_messages (); nm_setting_vpn_foreach_data_item (s_vpn, vpn_check_empty_func, NULL); /* Try to add some blank secrets and make sure they are rejected */ - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_secret (s_vpn, NULL, NULL); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (key) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_secret (s_vpn, "", ""); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (secret != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (secret && secret[0])); nm_setting_vpn_add_secret (s_vpn, "foobar1", NULL); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (secret) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (secret && secret[0])); nm_setting_vpn_add_secret (s_vpn, "foobar1", ""); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key != NULL)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_secret (s_vpn, NULL, "blahblah1"); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (strlen (key) > 0)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_secret (s_vpn, "", "blahblah1"); g_test_assert_expected_messages (); @@ -2347,7 +2347,7 @@ test_setting_connection_permissions_helpers (void) g_assert (!success); /* Ensure a bad [type] is rejected */ - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (ptype)); + NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (ptype && ptype[0])); success = nm_setting_connection_add_permission (s_con, NULL, "blah", NULL); g_test_assert_expected_messages (); g_assert (!success); diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index 56a88b004f..40485ab6c0 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -892,8 +892,7 @@ nm_secret_agent_old_get_secrets (NMSecretAgentOld *self, g_return_if_fail (NM_IS_SECRET_AGENT_OLD (self)); g_return_if_fail (NM_IS_CONNECTION (connection)); g_return_if_fail (nm_connection_get_path (connection)); - g_return_if_fail (setting_name != NULL); - g_return_if_fail (strlen (setting_name) > 0); + g_return_if_fail (setting_name && setting_name[0]); g_return_if_fail (!(flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)); g_return_if_fail (!(flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS)); g_return_if_fail (callback != NULL); From aaaccfd26461b42a61cbc4852af5df2f770ecdaa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 13:43:17 +0100 Subject: [PATCH 05/98] libnm: refactor parsing of NMVlanQosMapping and support wildcard for "to" - avoid the memory allocations by not using g_strsplit(). - add a helper function priority_map_parse_str(). This will be used later, to avoid allocating a NMVlanQosMapping result, when we don't need it on the heap. - for the priority mappings, the "from" part is the key and must be unique. As such, it would make sense to say $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '1:*' or $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '1:' to delete any mapping for that priority, regardless of the "to" part. Add an option to leave the "to" part unspecified. This will be used later. --- libnm-core/nm-setting-vlan.c | 95 +++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 5b8a49d48b..88843e7706 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -116,38 +116,87 @@ get_max_prio (NMVlanPriorityMap map, gboolean from) g_assert_not_reached (); } +static gboolean +priority_map_parse_str (NMVlanPriorityMap map_type, + const char *str, + gboolean allow_wildcard_to, + guint32 *out_from, + guint32 *out_to, + gboolean *out_has_wildcard_to) +{ + const char *s2; + gint64 v1, v2; + + nm_assert (str); + + s2 = strchr (str, ':'); + + if (!s2) { + if (!allow_wildcard_to) + return FALSE; + v1 = _nm_utils_ascii_str_to_int64 (str, 10, 0, G_MAXUINT32, -1); + v2 = -1; + } else { + gs_free char *s1_free = NULL; + gsize s1_len = (s2 - str); + + s2 = nm_str_skip_leading_spaces (&s2[1]); + if ( s2[0] == '\0' + || ( s2[0] == '*' + && NM_STRCHAR_ALL (&s2[1], ch, g_ascii_isspace (ch)))) { + if (!allow_wildcard_to) + return FALSE; + v2 = -1; + } else { + v2 = _nm_utils_ascii_str_to_int64 (s2, 10, 0, G_MAXUINT32, -1); + if ( v2 < 0 + || v2 > get_max_prio (map_type, FALSE)) + return FALSE; + } + + v1 = _nm_utils_ascii_str_to_int64 (nm_strndup_a (100, str, s1_len, &s1_free), + 10, 0, G_MAXUINT32, -1); + } + + if ( v1 < 0 + || v1 > get_max_prio (map_type, TRUE)) + return FALSE; + + NM_SET_OUT (out_from, v1); + NM_SET_OUT (out_to, v2 < 0 + ? 0u + : (guint) v2); + NM_SET_OUT (out_has_wildcard_to, v2 < 0); + return TRUE; +} + +static NMVlanQosMapping * +priority_map_new (guint32 from, guint32 to) +{ + NMVlanQosMapping *mapping; + + mapping = g_new (NMVlanQosMapping, 1); + *mapping = (NMVlanQosMapping) { + .from = from, + .to = to, + }; + return mapping; +} + static NMVlanQosMapping * priority_map_new_from_str (NMVlanPriorityMap map, const char *str) { - NMVlanQosMapping *p = NULL; - char **t = NULL; - guint32 len; - guint64 from, to; + guint32 from, to; - g_return_val_if_fail (str && str[0], NULL); - - t = g_strsplit (str, ":", 0); - len = g_strv_length (t); - if (len == 2) { - from = g_ascii_strtoull (t[0], NULL, 10); - to = g_ascii_strtoull (t[1], NULL, 10); - - if ((from <= get_max_prio (map, TRUE)) && (to <= get_max_prio (map, FALSE))) { - G_STATIC_ASSERT (sizeof (*p) == sizeof (p->from) + sizeof (p->to)); - p = g_malloc (sizeof (NMVlanQosMapping)); - p->from = from; - p->to = to; - } - } - - g_strfreev (t); - return p; + if (!priority_map_parse_str (map, str, FALSE, &from, &to, NULL)) + return NULL; + return priority_map_new (from, to); } static void priority_map_free (NMVlanQosMapping *map) { - g_return_if_fail (map != NULL); + nm_assert (map); g_free (map); } From d0f1e68b3ef837140e8eafdb6c62069daf84fe57 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 17 Mar 2019 12:12:30 +0100 Subject: [PATCH 06/98] shared: add "shared/nm-libnm-core-utils.h" utils We have code in "shared/nm-utils" which are general purpose helpers, independent of "libnm", "libnm-core", "clients" and "src". We have shared code like "shared/nm-ethtool-utils.h" and "shared/nm-meta-setting.h", which is statically linked, shared code that contains libnm related helpers. But these helpers already have a specific use (e.g. they are related to ethtool or NMSetting metadata). Add a general purpose helper that: - depends (and extends) libnm-core - contains unrelated helpers - can be shared (meaning it will be statically linked). - this code can be used by any library user of "libnm.so" (nmcli, nm-applet) and by "libnm-core" itself. Thus, "src/" and "libnm/" may also use this code indirectly, via "libnm-core/". --- Makefile.am | 4 ++++ clients/common/meson.build | 2 +- libnm-core/meson.build | 1 + shared/meson.build | 2 ++ shared/nm-libnm-core-utils.c | 22 ++++++++++++++++++++++ shared/nm-libnm-core-utils.h | 23 +++++++++++++++++++++++ 6 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 shared/nm-libnm-core-utils.c create mode 100644 shared/nm-libnm-core-utils.h diff --git a/Makefile.am b/Makefile.am index 21c43884b7..a8353656da 100644 --- a/Makefile.am +++ b/Makefile.am @@ -669,6 +669,7 @@ libnm_core_lib_h_pub_mkenums = \ libnm-core/nm-core-enum-types.h libnm_core_lib_h_priv = \ shared/nm-ethtool-utils.h \ + shared/nm-libnm-core-utils.h \ shared/nm-meta-setting.h \ libnm-core/nm-crypto.h \ libnm-core/nm-crypto-impl.h \ @@ -731,6 +732,7 @@ libnm_core_lib_c_settings_real = \ libnm_core_lib_c_real = \ $(libnm_core_lib_c_settings_real) \ shared/nm-ethtool-utils.c \ + shared/nm-libnm-core-utils.c \ shared/nm-meta-setting.c \ libnm-core/nm-crypto.c \ libnm-core/nm-connection.c \ @@ -3914,6 +3916,8 @@ clients_common_libnmc_la_SOURCES = \ \ shared/nm-ethtool-utils.c \ shared/nm-ethtool-utils.h \ + shared/nm-libnm-core-utils.c \ + shared/nm-libnm-core-utils.h \ \ clients/common/nm-meta-setting-desc.c \ clients/common/nm-meta-setting-desc.h \ diff --git a/clients/common/meson.build b/clients/common/meson.build index b4b6bcacd3..c137d4463e 100644 --- a/clients/common/meson.build +++ b/clients/common/meson.build @@ -55,7 +55,7 @@ libnmc = static_library( sources: files( 'nm-meta-setting-access.c', 'nm-meta-setting-desc.c', - ) + shared_nm_meta_setting_c + shared_nm_ethtool_utils_c + [settings_docs_source], + ) + shared_nm_meta_setting_c + shared_nm_ethtool_utils_c + shared_nm_libnm_core_utils_c + [settings_docs_source], dependencies: deps, c_args: cflags, link_with: libnmc_base, diff --git a/libnm-core/meson.build b/libnm-core/meson.build index d10dd1c551..beee290e78 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -189,6 +189,7 @@ libnm_core_sources_all = libnm_core_sources libnm_core_sources_all += libnm_core_enum libnm_core_sources_all += shared_nm_meta_setting_c libnm_core_sources_all += shared_nm_ethtool_utils_c +libnm_core_sources_all += shared_nm_libnm_core_utils_c libnm_core_sources_all += [version_header] libnm_core = static_library( diff --git a/shared/meson.build b/shared/meson.build index a6e94d6b88..f9d9e62bff 100644 --- a/shared/meson.build +++ b/shared/meson.build @@ -75,6 +75,8 @@ version_header = configure_file( shared_nm_ethtool_utils_c = files('nm-ethtool-utils.c') +shared_nm_libnm_core_utils_c = files('nm-libnm-core-utils.c') + shared_nm_meta_setting_c = files('nm-meta-setting.c') shared_nm_test_utils_impl_c = files('nm-test-utils-impl.c') diff --git a/shared/nm-libnm-core-utils.c b/shared/nm-libnm-core-utils.c new file mode 100644 index 0000000000..244b318d72 --- /dev/null +++ b/shared/nm-libnm-core-utils.c @@ -0,0 +1,22 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + */ + +#include "nm-default.h" + +#include "nm-libnm-core-utils.h" + +/*****************************************************************************/ diff --git a/shared/nm-libnm-core-utils.h b/shared/nm-libnm-core-utils.h new file mode 100644 index 0000000000..fcdd425a54 --- /dev/null +++ b/shared/nm-libnm-core-utils.h @@ -0,0 +1,23 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + */ + +#ifndef __NM_LIBNM_SHARED_UTILS_H__ +#define __NM_LIBNM_SHARED_UTILS_H__ + +/****************************************************************************/ + +#endif /* __NM_LIBNM_SHARED_UTILS_H__ */ From 5079cd99423a0be5e5f0cfe4a7b59437f7ebf368 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 17 Mar 2019 12:21:00 +0100 Subject: [PATCH 07/98] libnm: move parsing VLAN priority mapping to "shared/nm-libnm-core-utils.h" The same code is used by nmcli. Obviously, clients also need to parse string representations. That begs the question whether this should be public API of libnm. Maybe, but don't decide that now, just reuse the code internally via "shared/nm-libnm-core-utils.h". --- libnm-core/nm-setting-vlan.c | 67 ++---------------------------------- shared/nm-libnm-core-utils.c | 54 +++++++++++++++++++++++++++++ shared/nm-libnm-core-utils.h | 23 +++++++++++++ 3 files changed, 79 insertions(+), 65 deletions(-) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 88843e7706..5e34834952 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -25,6 +25,7 @@ #include +#include "nm-libnm-core-utils.h" #include "nm-utils.h" #include "nm-core-types-internal.h" #include "nm-setting-connection.h" @@ -106,70 +107,6 @@ nm_setting_vlan_get_flags (NMSettingVlan *setting) return NM_SETTING_VLAN_GET_PRIVATE (setting)->flags; } -static guint32 -get_max_prio (NMVlanPriorityMap map, gboolean from) -{ - if (map == NM_VLAN_INGRESS_MAP) - return from ? MAX_8021P_PRIO : MAX_SKB_PRIO; - else if (map == NM_VLAN_EGRESS_MAP) - return from ? MAX_SKB_PRIO : MAX_8021P_PRIO; - g_assert_not_reached (); -} - -static gboolean -priority_map_parse_str (NMVlanPriorityMap map_type, - const char *str, - gboolean allow_wildcard_to, - guint32 *out_from, - guint32 *out_to, - gboolean *out_has_wildcard_to) -{ - const char *s2; - gint64 v1, v2; - - nm_assert (str); - - s2 = strchr (str, ':'); - - if (!s2) { - if (!allow_wildcard_to) - return FALSE; - v1 = _nm_utils_ascii_str_to_int64 (str, 10, 0, G_MAXUINT32, -1); - v2 = -1; - } else { - gs_free char *s1_free = NULL; - gsize s1_len = (s2 - str); - - s2 = nm_str_skip_leading_spaces (&s2[1]); - if ( s2[0] == '\0' - || ( s2[0] == '*' - && NM_STRCHAR_ALL (&s2[1], ch, g_ascii_isspace (ch)))) { - if (!allow_wildcard_to) - return FALSE; - v2 = -1; - } else { - v2 = _nm_utils_ascii_str_to_int64 (s2, 10, 0, G_MAXUINT32, -1); - if ( v2 < 0 - || v2 > get_max_prio (map_type, FALSE)) - return FALSE; - } - - v1 = _nm_utils_ascii_str_to_int64 (nm_strndup_a (100, str, s1_len, &s1_free), - 10, 0, G_MAXUINT32, -1); - } - - if ( v1 < 0 - || v1 > get_max_prio (map_type, TRUE)) - return FALSE; - - NM_SET_OUT (out_from, v1); - NM_SET_OUT (out_to, v2 < 0 - ? 0u - : (guint) v2); - NM_SET_OUT (out_has_wildcard_to, v2 < 0); - return TRUE; -} - static NMVlanQosMapping * priority_map_new (guint32 from, guint32 to) { @@ -188,7 +125,7 @@ priority_map_new_from_str (NMVlanPriorityMap map, const char *str) { guint32 from, to; - if (!priority_map_parse_str (map, str, FALSE, &from, &to, NULL)) + if (!nm_utils_vlan_priority_map_parse_str (map, str, FALSE, &from, &to, NULL)) return NULL; return priority_map_new (from, to); } diff --git a/shared/nm-libnm-core-utils.c b/shared/nm-libnm-core-utils.c index 244b318d72..d1e5f75460 100644 --- a/shared/nm-libnm-core-utils.c +++ b/shared/nm-libnm-core-utils.c @@ -20,3 +20,57 @@ #include "nm-libnm-core-utils.h" /*****************************************************************************/ + +gboolean +nm_utils_vlan_priority_map_parse_str (NMVlanPriorityMap map_type, + const char *str, + gboolean allow_wildcard_to, + guint32 *out_from, + guint32 *out_to, + gboolean *out_has_wildcard_to) +{ + const char *s2; + gint64 v1, v2; + + nm_assert (str); + + s2 = strchr (str, ':'); + + if (!s2) { + if (!allow_wildcard_to) + return FALSE; + v1 = _nm_utils_ascii_str_to_int64 (str, 10, 0, G_MAXUINT32, -1); + v2 = -1; + } else { + gs_free char *s1_free = NULL; + gsize s1_len = (s2 - str); + + s2 = nm_str_skip_leading_spaces (&s2[1]); + if ( s2[0] == '\0' + || ( s2[0] == '*' + && NM_STRCHAR_ALL (&s2[1], ch, g_ascii_isspace (ch)))) { + if (!allow_wildcard_to) + return FALSE; + v2 = -1; + } else { + v2 = _nm_utils_ascii_str_to_int64 (s2, 10, 0, G_MAXUINT32, -1); + if ( v2 < 0 + || (guint32) v2 > nm_utils_vlan_priority_map_get_max_prio (map_type, FALSE)) + return FALSE; + } + + v1 = _nm_utils_ascii_str_to_int64 (nm_strndup_a (100, str, s1_len, &s1_free), + 10, 0, G_MAXUINT32, -1); + } + + if ( v1 < 0 + || (guint32) v1 > nm_utils_vlan_priority_map_get_max_prio (map_type, TRUE)) + return FALSE; + + NM_SET_OUT (out_from, v1); + NM_SET_OUT (out_to, v2 < 0 + ? 0u + : (guint) v2); + NM_SET_OUT (out_has_wildcard_to, v2 < 0); + return TRUE; +} diff --git a/shared/nm-libnm-core-utils.h b/shared/nm-libnm-core-utils.h index fcdd425a54..5a846659e6 100644 --- a/shared/nm-libnm-core-utils.h +++ b/shared/nm-libnm-core-utils.h @@ -20,4 +20,27 @@ /****************************************************************************/ +#include "nm-setting-vlan.h" + +static inline guint32 +nm_utils_vlan_priority_map_get_max_prio (NMVlanPriorityMap map, gboolean from) +{ + if (map == NM_VLAN_INGRESS_MAP) { + return from + ? 7u /* MAX_8021P_PRIO */ + : (guint32) G_MAXUINT32 /* MAX_SKB_PRIO */; + } + nm_assert (map == NM_VLAN_EGRESS_MAP); + return from + ? (guint32) G_MAXUINT32 /* MAX_SKB_PRIO */ + : 7u /* MAX_8021P_PRIO */; +} + +gboolean nm_utils_vlan_priority_map_parse_str (NMVlanPriorityMap map_type, + const char *str, + gboolean allow_wildcard_to, + guint32 *out_from, + guint32 *out_to, + gboolean *out_has_wildcard_to); + #endif /* __NM_LIBNM_SHARED_UTILS_H__ */ From b56e430da9c6025fba4a2d6c7c216976a1b40185 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 17 Mar 2019 11:46:24 +0100 Subject: [PATCH 08/98] cli: reuse nm_utils_vlan_priority_map_parse_str() in _parse_vlan_priority_maps() --- clients/common/nm-meta-setting-desc.c | 48 ++++++++------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 5dc5433f26..31356f2e2e 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -26,6 +26,7 @@ #include "nm-common-macros.h" #include "nm-utils/nm-enum-utils.h" +#include "nm-libnm-core-utils.h" #include "nm-vpn-helpers.h" #include "nm-client-utils.h" @@ -409,48 +410,25 @@ _parse_vlan_priority_maps (const char *priority_map, NMVlanPriorityMap map_type, GError **error) { - char **mapping = NULL, **iter; - unsigned long from, to, from_max, to_max; + gs_strfreev char **mapping = NULL; + char **iter; - g_return_val_if_fail (priority_map != NULL, NULL); - g_return_val_if_fail (error == NULL || *error == NULL, NULL); - - if (map_type == NM_VLAN_INGRESS_MAP) { - from_max = MAX_8021P_PRIO; - to_max = MAX_SKB_PRIO; - } else { - from_max = MAX_SKB_PRIO; - to_max = MAX_8021P_PRIO; - } + nm_assert (priority_map); + nm_assert (!error || !*error); mapping = g_strsplit (priority_map, ",", 0); - for (iter = mapping; iter && *iter; iter++) { - char *left, *right; - - left = g_strstrip (*iter); - right = strchr (left, ':'); - if (!right) { + for (iter = mapping; *iter; iter++) { + if (!nm_utils_vlan_priority_map_parse_str (map_type, + *iter, + FALSE, + NULL, + NULL, + NULL)) { g_set_error (error, 1, 0, _("invalid priority map '%s'"), *iter); - g_strfreev (mapping); return NULL; } - *right++ = '\0'; - - if (!nmc_string_to_uint (left, TRUE, 0, from_max, &from)) { - g_set_error (error, 1, 0, _("priority '%s' is not valid (<0-%ld>)"), - left, from_max); - g_strfreev (mapping); - return NULL; - } - if (!nmc_string_to_uint (right, TRUE, 0, to_max, &to)) { - g_set_error (error, 1, 0, _("priority '%s' is not valid (<0-%ld>)"), - right, to_max); - g_strfreev (mapping); - return NULL; - } - *(right-1) = ':'; /* Put back ':' */ } - return mapping; + return g_steal_pointer (&mapping); } /* From bb4e5a7a62ce7322beaac55d3f14528bde43eec0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 17 Mar 2019 12:59:40 +0100 Subject: [PATCH 09/98] cli: don't treat empty string as valid number in nmc_string_to_uint_base() How hard can it be to use strtol()? Apparently very. You need to check errno, the endpointer, and also not pass in an empty string. Previously, nmc_string_to_uint_base() would silently accept "" as valid number. That's a bug. Btw, let's not use strtol() (or nmc_string_to_uint*()). We have _nm_utils_ascii_str_to_int64(), which gets all of this right. --- clients/common/nm-client-utils.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clients/common/nm-client-utils.c b/clients/common/nm-client-utils.c index 1241131a11..0a155dae40 100644 --- a/clients/common/nm-client-utils.c +++ b/clients/common/nm-client-utils.c @@ -82,6 +82,10 @@ nmc_string_to_uint_base (const char *str, char *end; unsigned long int tmp; + if (!str || !str[0]) + return FALSE; + + /* FIXME: don't use this function, replace by _nm_utils_ascii_str_to_int64() */ errno = 0; tmp = strtoul (str, &end, base); if (errno || *end != '\0' || (range_check && (tmp < min || tmp > max))) { From 25ef45ff3debe7d3df526f7727401116b4f3252e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 14:03:26 +0100 Subject: [PATCH 10/98] libnm/cli: support deleting VLAN egress/ingress priority map by "from" The "from" part is like a key for the egress/ingress priority map. Extend nm_setting_vlan_remove_priority_str_by_value() to accept only the "from" part when finding and deleting value. This allows for: $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:' $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:*' to fuzzy match the value to remove. --- clients/common/nm-meta-setting-desc.c | 7 +-- libnm-core/nm-setting-vlan.c | 76 ++++++++++++++++----------- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 31356f2e2e..71f6f1d5b0 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -408,6 +408,7 @@ _parse_team_link_watcher (const char *str, static char ** _parse_vlan_priority_maps (const char *priority_map, NMVlanPriorityMap map_type, + gboolean allow_wildcard_to, GError **error) { gs_strfreev char **mapping = NULL; @@ -420,7 +421,7 @@ _parse_vlan_priority_maps (const char *priority_map, for (iter = mapping; *iter; iter++) { if (!nm_utils_vlan_priority_map_parse_str (map_type, *iter, - FALSE, + allow_wildcard_to, NULL, NULL, NULL)) { @@ -4279,7 +4280,7 @@ _set_vlan_xgress_priority_map (NMSetting *setting, { char **prio_map, **p; - prio_map = _parse_vlan_priority_maps (value, map_type, error); + prio_map = _parse_vlan_priority_maps (value, map_type, FALSE, error); if (!prio_map) return FALSE; @@ -4320,7 +4321,7 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment, char **prio_map; gs_free char *v = g_strdup (value); - prio_map = _parse_vlan_priority_maps (v, map_type, error); + prio_map = _parse_vlan_priority_maps (v, map_type, TRUE, error); if (!prio_map) return FALSE; if (prio_map[1]) { diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 5e34834952..f07504aa3e 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -144,7 +144,7 @@ get_map (NMSettingVlan *self, NMVlanPriorityMap map) return NM_SETTING_VLAN_GET_PRIVATE (self)->ingress_priority_map; else if (map == NM_VLAN_EGRESS_MAP) return NM_SETTING_VLAN_GET_PRIVATE (self)->egress_priority_map; - g_assert_not_reached (); + nm_assert_not_reached (); return NULL; } @@ -193,7 +193,7 @@ set_map (NMSettingVlan *self, NMVlanPriorityMap map, GSList *list) NM_SETTING_VLAN_GET_PRIVATE (self)->egress_priority_map = list; _notify (self, PROP_EGRESS_PRIORITY_MAP); } else - g_assert_not_reached (); + nm_assert_not_reached (); } static gboolean @@ -475,6 +475,36 @@ nm_setting_vlan_remove_priority (NMSettingVlan *setting, set_map (setting, map, g_slist_delete_link (list, item)); } +static gboolean +priority_map_remove_by_value (NMSettingVlan *setting, + NMVlanPriorityMap map, + guint32 from, + guint32 to, + gboolean wildcard_to) +{ + GSList *list = NULL, *iter = NULL; + NMVlanQosMapping *item; + + nm_assert (NM_IS_SETTING_VLAN (setting)); + nm_assert (NM_IN_SET (map, NM_VLAN_INGRESS_MAP, NM_VLAN_EGRESS_MAP)); + + list = get_map (setting, map); + for (iter = list; iter; iter = g_slist_next (iter)) { + item = iter->data; + + if (item->from != from) + continue; + if ( !wildcard_to + && item->to != to) + continue; + + priority_map_free ((NMVlanQosMapping *) (iter->data)); + set_map (setting, map, g_slist_delete_link (list, iter)); + return TRUE; + } + return FALSE; +} + /** * nm_setting_vlan_remove_priority_by_value: * @setting: the #NMSettingVlan @@ -494,22 +524,10 @@ nm_setting_vlan_remove_priority_by_value (NMSettingVlan *setting, guint32 from, guint32 to) { - GSList *list = NULL, *iter = NULL; - NMVlanQosMapping *item; - g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE); g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE); - list = get_map (setting, map); - for (iter = list; iter; iter = g_slist_next (iter)) { - item = iter->data; - if (item->from == from && item->to == to) { - priority_map_free ((NMVlanQosMapping *) (iter->data)); - set_map (setting, map, g_slist_delete_link (list, iter)); - return TRUE; - } - } - return FALSE; + return priority_map_remove_by_value (setting, map, from, to, FALSE); } /** @@ -529,19 +547,15 @@ nm_setting_vlan_remove_priority_str_by_value (NMSettingVlan *setting, NMVlanPriorityMap map, const char *str) { - NMVlanQosMapping *item; - gboolean found; + gboolean is_wildcard_to; + guint32 from, to; g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE); g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE); - item = priority_map_new_from_str (map, str); - if (!item) + if (!nm_utils_vlan_priority_map_parse_str (map, str, TRUE, &from, &to, &is_wildcard_to)) return FALSE; - - found = nm_setting_vlan_remove_priority_by_value (setting, map, item->from, item->to); - g_free (item); - return found; + return priority_map_remove_by_value (setting, map, from, to, is_wildcard_to); } /** @@ -689,18 +703,16 @@ static GSList * priority_strv_to_maplist (NMVlanPriorityMap map, char **strv) { GSList *list = NULL; - int i; + gsize i; for (i = 0; strv && strv[i]; i++) { - NMVlanQosMapping *item; + guint32 from, to; - item = priority_map_new_from_str (map, strv[i]); - if (item) { - if (!check_replace_duplicate_priority (list, item->from, item->to)) - list = g_slist_prepend (list, item); - else - g_free (item); - } + if (!nm_utils_vlan_priority_map_parse_str (map, strv[i], FALSE, &from, &to, NULL)) + continue; + if (check_replace_duplicate_priority (list, from, to)) + continue; + list = g_slist_prepend (list, priority_map_new (from, to)); } return g_slist_sort (list, prio_map_compare); } From d178c2572844ef689909da07dc193a7dfab406d6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 08:08:04 +0100 Subject: [PATCH 11/98] libnm,cli: move cleanup macros to "shared/nm-libnm-core-utils.h" --- clients/common/nm-client-utils.h | 8 +---- libnm-core/nm-core-internal.h | 12 +------- shared/nm-libnm-core-utils.h | 30 +++++++++++++++++++ .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 4 +-- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/clients/common/nm-client-utils.h b/clients/common/nm-client-utils.h index a5bc05fab0..fd726ee93c 100644 --- a/clients/common/nm-client-utils.h +++ b/clients/common/nm-client-utils.h @@ -23,13 +23,7 @@ #include "nm-meta-setting.h" #include "nm-active-connection.h" #include "nm-device.h" - - -#define nm_auto_unref_ip_address nm_auto (_nm_ip_address_unref) -NM_AUTO_DEFINE_FCN0 (NMIPAddress *, _nm_ip_address_unref, nm_ip_address_unref) - -#define nm_auto_unref_wgpeer nm_auto (_nm_auto_unref_wgpeer) -NM_AUTO_DEFINE_FCN0 (NMWireGuardPeer *, _nm_auto_unref_wgpeer, nm_wireguard_peer_unref) +#include "nm-libnm-core-utils.h" const NMObject **nmc_objects_sort_by_path (const NMObject *const*objs, gssize len); diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index cf2be4a24d..9b123131ef 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -88,6 +88,7 @@ #include "nm-utils.h" #include "nm-vpn-dbus-interface.h" #include "nm-vpn-editor-plugin.h" +#include "nm-libnm-core-utils.h" /* IEEE 802.1D-1998 timer values */ #define NM_BR_MIN_HELLO_TIME 1 @@ -264,14 +265,6 @@ GHashTable *_nm_ip_route_get_attributes_direct (NMIPRoute *route); NMSriovVF *_nm_utils_sriov_vf_from_strparts (const char *index, const char *detail, gboolean ignore_unknown, GError **error); gboolean _nm_sriov_vf_attribute_validate_all (const NMSriovVF *vf, GError **error); -static inline void -_nm_auto_ip_route_unref (NMIPRoute **v) -{ - if (*v) - nm_ip_route_unref (*v); -} -#define nm_auto_ip_route_unref nm_auto (_nm_auto_ip_route_unref) - GPtrArray *_nm_utils_copy_array (const GPtrArray *array, NMUtilsCopyFunc copy_func, GDestroyNotify free_func); @@ -763,9 +756,6 @@ gboolean _nm_connection_find_secret (NMConnection *self, /*****************************************************************************/ -#define nm_auto_unref_wgpeer nm_auto(_nm_auto_unref_wgpeer) -NM_AUTO_DEFINE_FCN0 (NMWireGuardPeer *, _nm_auto_unref_wgpeer, nm_wireguard_peer_unref) - gboolean nm_utils_base64secret_normalize (const char *base64_key, gsize required_key_len, char **out_base64_key_norm); diff --git a/shared/nm-libnm-core-utils.h b/shared/nm-libnm-core-utils.h index 5a846659e6..6f0980b01f 100644 --- a/shared/nm-libnm-core-utils.h +++ b/shared/nm-libnm-core-utils.h @@ -20,7 +20,37 @@ /****************************************************************************/ +#include "nm-setting-connection.h" +#include "nm-setting-ip-config.h" +#include "nm-setting-sriov.h" +#include "nm-setting-team.h" #include "nm-setting-vlan.h" +#include "nm-setting-wireguard.h" + +/****************************************************************************/ + +#define nm_auto_unref_ip_address nm_auto (_nm_ip_address_unref) +NM_AUTO_DEFINE_FCN0 (NMIPAddress *, _nm_ip_address_unref, nm_ip_address_unref) + +#define nm_auto_unref_ip_route nm_auto (_nm_auto_unref_ip_route) +NM_AUTO_DEFINE_FCN0 (NMIPRoute *, _nm_auto_unref_ip_route, nm_ip_route_unref) + +#define nm_auto_unref_sriov_vf nm_auto (_nm_auto_unref_sriov_vf) +NM_AUTO_DEFINE_FCN0 (NMSriovVF *, _nm_auto_unref_sriov_vf, nm_sriov_vf_unref) + +#define nm_auto_unref_tc_qdisc nm_auto (_nm_auto_unref_tc_qdisc) +NM_AUTO_DEFINE_FCN0 (NMTCQdisc *, _nm_auto_unref_tc_qdisc, nm_tc_qdisc_unref) + +#define nm_auto_unref_tc_tfilter nm_auto (_nm_auto_unref_tc_tfilter) +NM_AUTO_DEFINE_FCN0 (NMTCTfilter *, _nm_auto_unref_tc_tfilter, nm_tc_tfilter_unref) + +#define nm_auto_unref_team_link_watcher nm_auto (_nm_auto_unref_team_link_watcher) +NM_AUTO_DEFINE_FCN0 (NMTeamLinkWatcher *, _nm_auto_unref_team_link_watcher, nm_team_link_watcher_unref) + +#define nm_auto_unref_wgpeer nm_auto (_nm_auto_unref_wgpeer) +NM_AUTO_DEFINE_FCN0 (NMWireGuardPeer *, _nm_auto_unref_wgpeer, nm_wireguard_peer_unref) + +/****************************************************************************/ static inline guint32 nm_utils_vlan_priority_map_get_max_prio (NMVlanPriorityMap map, gboolean from) 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 7c1db225c0..03d2cb6330 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -820,7 +820,7 @@ parse_route_line (const char *line, NMIPRoute **out_route, GError **error) { - nm_auto_ip_route_unref NMIPRoute *route = NULL; + nm_auto_unref_ip_route NMIPRoute *route = NULL; gs_free const char **words_free = NULL; const char *const*words; const char *s; @@ -1284,7 +1284,7 @@ read_route_file (int addr_family, for (line = strtok_r (contents, "\n", &contents_rest); line; line = strtok_r (NULL, "\n", &contents_rest)) { - nm_auto_ip_route_unref NMIPRoute *route = NULL; + nm_auto_unref_ip_route NMIPRoute *route = NULL; gs_free_error GError *local = NULL; int e; From 0055d8ed58b8e4b3ce73cb51a3a282ec7518e927 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 13:05:17 +0100 Subject: [PATCH 12/98] cli: don't fail removing non-existing option (pt1) Part 1, which addresses the issue for simple properties that have a plain remove-by-value function. Rationale: Removing a value/index that does not exist should not be a failure. Woule you like: $ nmcli connection modify "$PROFILE" autoconnect no $ nmcli connection modify "$PROFILE" autoconnect no Error: autoconnect is already disabled So, why would it be a good idea to fail during $ nmcli connection modify "$PROFILE" -vpn.data ca $ nmcli connection modify "$PROFILE" -vpn.data ca Error: failed to remove a value from vpn.data: invalid option 'ca'. Generally, it should not be an error to remove an option, as long as the option itself is valid. For example, $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map bogus should fail, but $ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map 1:5 should succeed even if there was nothing to remove. --- clients/common/nm-meta-setting-desc.c | 212 ++++++++------------------ shared/nm-libnm-core-utils.h | 7 + 2 files changed, 71 insertions(+), 148 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 71f6f1d5b0..e7d836e593 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -1690,6 +1690,30 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) return TRUE; \ } +#define _DEFINE_REMOVER_INDEX_OR_VALUE(def_func, s_macro, num_func, rem_func_idx, rem_func_cmd) \ + static gboolean \ + def_func (ARGS_REMOVE_FCN) \ + { \ + guint32 num; \ + \ + if (!value) { \ + num = num_func (s_macro (setting)); \ + if (idx < num) \ + rem_func_idx (s_macro (setting), idx); \ + return TRUE; \ + } \ + rem_func_cmd \ + } + +#define DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT(def_func, s_macro, num_func, rem_func_idx, rem_func_val) \ + _DEFINE_REMOVER_INDEX_OR_VALUE (def_func, s_macro, num_func, rem_func_idx, { \ + gs_free char *value_to_free = NULL; \ + \ + rem_func_val (s_macro (setting), \ + nm_strstrip_avoid_copy (value, &value_to_free)); \ + return TRUE; \ + }) + #define DEFINE_REMOVER_OPTION(def_func, s_macro, rem_func) \ static gboolean \ def_func (ARGS_REMOVE_FCN) \ @@ -2233,47 +2257,21 @@ _set_fcn_802_1x_eap (ARGS_SET_FCN) error); } -static gboolean -_validate_and_remove_eap_method (NMSetting8021x *setting, - const char *eap, - GError **error) -{ - gboolean ret; - - ret = nm_setting_802_1x_remove_eap_method_by_value (setting, eap); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain EAP method '%s'"), eap); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_802_1x_eap, - NM_SETTING_802_1X, - nm_setting_802_1x_get_num_eap_methods, - nm_setting_802_1x_remove_eap_method, - _validate_and_remove_eap_method) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_802_1x_eap, + NM_SETTING_802_1X, + nm_setting_802_1x_get_num_eap_methods, + nm_setting_802_1x_remove_eap_method, + nm_setting_802_1x_remove_eap_method_by_value) DEFINE_SETTER_CERT (_set_fcn_802_1x_ca_cert, nm_setting_802_1x_set_ca_cert) DEFINE_SETTER_STR_LIST (_set_fcn_802_1x_altsubject_matches, nm_setting_802_1x_add_altsubject_match) -static gboolean -_validate_and_remove_altsubject_match (NMSetting8021x *setting, - const char *altsubject_match, - GError **error) -{ - gboolean ret; - - ret = nm_setting_802_1x_remove_altsubject_match_by_value (setting, altsubject_match); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain alternative subject match '%s'"), - altsubject_match); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_802_1x_altsubject_matches, - NM_SETTING_802_1X, - nm_setting_802_1x_get_num_altsubject_matches, - nm_setting_802_1x_remove_altsubject_match, - _validate_and_remove_altsubject_match) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_802_1x_altsubject_matches, + NM_SETTING_802_1X, + nm_setting_802_1x_get_num_altsubject_matches, + nm_setting_802_1x_remove_altsubject_match, + nm_setting_802_1x_remove_altsubject_match_by_value) DEFINE_SETTER_CERT (_set_fcn_802_1x_client_cert, nm_setting_802_1x_set_client_cert) @@ -2281,25 +2279,11 @@ DEFINE_SETTER_CERT (_set_fcn_802_1x_phase2_ca_cert, nm_setting_802_1x_set_phase2 DEFINE_SETTER_STR_LIST (_set_fcn_802_1x_phase2_altsubject_matches, nm_setting_802_1x_add_phase2_altsubject_match) -static gboolean -_validate_and_remove_phase2_altsubject_match (NMSetting8021x *setting, - const char *phase2_altsubject_match, - GError **error) -{ - gboolean ret; - - ret = nm_setting_802_1x_remove_phase2_altsubject_match_by_value (setting, phase2_altsubject_match); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain \"phase2\" alternative subject match '%s'"), - phase2_altsubject_match); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_802_1x_phase2_altsubject_matches, - NM_SETTING_802_1X, - nm_setting_802_1x_get_num_phase2_altsubject_matches, - nm_setting_802_1x_remove_phase2_altsubject_match, - _validate_and_remove_phase2_altsubject_match) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_802_1x_phase2_altsubject_matches, + NM_SETTING_802_1X, + nm_setting_802_1x_get_num_phase2_altsubject_matches, + nm_setting_802_1x_remove_phase2_altsubject_match, + nm_setting_802_1x_remove_phase2_altsubject_match_by_value) DEFINE_SETTER_CERT (_set_fcn_802_1x_phase2_client_cert, nm_setting_802_1x_set_phase2_client_cert) @@ -2564,23 +2548,11 @@ _set_fcn_connection_permissions (ARGS_SET_FCN) return TRUE; } -static gboolean -_validate_and_remove_connection_permission (NMSettingConnection *setting, - const char *perm, - GError **error) -{ - gboolean ret; - - ret = nm_setting_connection_remove_permission_by_value (setting, "user", perm, NULL); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain permission '%s'"), perm); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_connection_permissions, - NM_SETTING_CONNECTION, - nm_setting_connection_get_num_permissions, - nm_setting_connection_remove_permission, - _validate_and_remove_connection_permission) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_connection_permissions, + NM_SETTING_CONNECTION, + nm_setting_connection_get_num_permissions, + nm_setting_connection_remove_permission, + nm_setting_connection_remove_permission_user) static gboolean _set_fcn_connection_master (ARGS_SET_FCN) @@ -3339,25 +3311,11 @@ _set_fcn_ip_config_dns_search (ARGS_SET_FCN) return TRUE; } -static gboolean -_validate_and_remove_ip_dns_search (NMSettingIPConfig *setting, - const char *dns_search, - GError **error) -{ - gboolean ret; - - ret = nm_setting_ip_config_remove_dns_search_by_value (setting, dns_search); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain DNS search domain '%s'"), - dns_search); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ip_config_dns_search, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_dns_searches, - nm_setting_ip_config_remove_dns_search, - _validate_and_remove_ip_dns_search) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_ip_config_dns_search, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_dns_searches, + nm_setting_ip_config_remove_dns_search, + nm_setting_ip_config_remove_dns_search_by_value) static gboolean _set_fcn_ip_config_dns_options (ARGS_SET_FCN) @@ -3379,25 +3337,11 @@ _set_fcn_ip_config_dns_options (ARGS_SET_FCN) return TRUE; } -static gboolean -_validate_and_remove_ip_dns_option (NMSettingIPConfig *setting, - const char *dns_option, - GError **error) -{ - gboolean ret; - - ret = nm_setting_ip_config_remove_dns_option_by_value (setting, dns_option); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain DNS option '%s'"), - dns_option); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ip_config_dns_options, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_dns_options, - nm_setting_ip_config_remove_dns_option, - _validate_and_remove_ip_dns_option) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_ip_config_dns_options, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_dns_options, + nm_setting_ip_config_remove_dns_option, + nm_setting_ip_config_remove_dns_option_by_value) static gboolean _set_fcn_ip4_config_addresses (ARGS_SET_FCN) @@ -3718,25 +3662,11 @@ _set_fcn_match_interface_name (ARGS_SET_FCN) return TRUE; } -static gboolean -_validate_and_remove_match_interface_name (NMSettingMatch *setting, - const char *interface_name, - GError **error) -{ - gboolean ret; - - ret = nm_setting_match_remove_interface_name_by_value (setting, interface_name); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain interface name '%s'"), - interface_name); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_match_interface_name, - NM_SETTING_MATCH, - nm_setting_match_get_num_interface_names, - nm_setting_match_remove_interface_name, - _validate_and_remove_match_interface_name) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_match_interface_name, + NM_SETTING_MATCH, + nm_setting_match_get_num_interface_names, + nm_setting_match_remove_interface_name, + nm_setting_match_remove_interface_name_by_value) static gconstpointer _get_fcn_olpc_mesh_ssid (ARGS_GET_FCN) @@ -4067,25 +3997,11 @@ _set_fcn_team_runner_tx_hash (ARGS_SET_FCN) return TRUE; } -static gboolean -_validate_and_remove_team_runner_tx_hash (NMSettingTeam *setting, - const char *tx_hash, - GError **error) -{ - if (!nm_setting_team_remove_runner_tx_hash_by_value (setting, tx_hash)) { - g_set_error (error, 1, 0, - _("the property doesn't contain string '%s'"), - tx_hash); - return FALSE; - } - - return TRUE; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_team_runner_tx_hash, - NM_SETTING_TEAM, - nm_setting_team_get_num_runner_tx_hash, - nm_setting_team_remove_runner_tx_hash, - _validate_and_remove_team_runner_tx_hash) +DEFINE_REMOVER_INDEX_OR_VALUE_DIRECT (_remove_fcn_team_runner_tx_hash, + NM_SETTING_TEAM, + nm_setting_team_get_num_runner_tx_hash, + nm_setting_team_remove_runner_tx_hash, + nm_setting_team_remove_runner_tx_hash_by_value) static gconstpointer _get_fcn_team_link_watchers (ARGS_GET_FCN) diff --git a/shared/nm-libnm-core-utils.h b/shared/nm-libnm-core-utils.h index 6f0980b01f..0fb5778599 100644 --- a/shared/nm-libnm-core-utils.h +++ b/shared/nm-libnm-core-utils.h @@ -73,4 +73,11 @@ gboolean nm_utils_vlan_priority_map_parse_str (NMVlanPriorityMap map_type, guint32 *out_to, gboolean *out_has_wildcard_to); +static inline void +nm_setting_connection_remove_permission_user (NMSettingConnection *setting, + const char *user) +{ + nm_setting_connection_remove_permission_by_value (setting, "user", user, NULL); +} + #endif /* __NM_LIBNM_SHARED_UTILS_H__ */ From 83fb6bcb42a076099412e6a41e479160fbb508a5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 13:05:17 +0100 Subject: [PATCH 13/98] cli: don't fail removing non-existing option (pt2) Part 2, which addresses the issue for properties with a static list of valid values. --- clients/common/nm-meta-setting-desc.c | 97 ++++++++------------------- 1 file changed, 28 insertions(+), 69 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index e7d836e593..be2cb28ce8 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -1714,6 +1714,16 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) return TRUE; \ }) +#define DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING_STATIC(def_func, s_macro, num_func, rem_func_idx, values_static, rem_func_val) \ + _DEFINE_REMOVER_INDEX_OR_VALUE (def_func, s_macro, num_func, rem_func_idx, { \ + value = nmc_string_is_valid (value, values_static, error); \ + if (!value) \ + return FALSE; \ + \ + rem_func_val (s_macro (setting), value); \ + return TRUE; \ + }) + #define DEFINE_REMOVER_OPTION(def_func, s_macro, rem_func) \ static gboolean \ def_func (ARGS_REMOVE_FCN) \ @@ -4549,29 +4559,12 @@ _set_fcn_wireless_security_proto (ARGS_SET_FCN) return check_and_add_wifi_sec_proto (setting, property_info->property_name, value, wifi_sec_valid_protos, error); } -static gboolean -_validate_and_remove_wifi_sec_proto (NMSettingWirelessSecurity *setting, - const char *proto, - GError **error) -{ - gboolean ret; - const char *valid; - - valid = nmc_string_is_valid (proto, wifi_sec_valid_protos, error); - if (!valid) - return FALSE; - - ret = nm_setting_wireless_security_remove_proto_by_value (setting, proto); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain protocol '%s'"), proto); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_wireless_security_proto, - NM_SETTING_WIRELESS_SECURITY, - nm_setting_wireless_security_get_num_protos, - nm_setting_wireless_security_remove_proto, - _validate_and_remove_wifi_sec_proto) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING_STATIC (_remove_fcn_wireless_security_proto, + NM_SETTING_WIRELESS_SECURITY, + nm_setting_wireless_security_get_num_protos, + nm_setting_wireless_security_remove_proto, + wifi_sec_valid_protos, + nm_setting_wireless_security_remove_proto_by_value) static const char *wifi_sec_valid_pairwises[] = { "tkip", "ccmp", NULL }; @@ -4585,29 +4578,12 @@ _set_fcn_wireless_security_pairwise (ARGS_SET_FCN) return check_and_add_wifi_sec_pairwise (setting, property_info->property_name, value, wifi_sec_valid_pairwises, error); } -static gboolean -_validate_and_remove_wifi_sec_pairwise (NMSettingWirelessSecurity *setting, - const char *pairwise, - GError **error) -{ - gboolean ret; - const char *valid; - - valid = nmc_string_is_valid (pairwise, wifi_sec_valid_pairwises, error); - if (!valid) - return FALSE; - - ret = nm_setting_wireless_security_remove_pairwise_by_value (setting, pairwise); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain protocol '%s'"), pairwise); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_wireless_security_pairwise, - NM_SETTING_WIRELESS_SECURITY, - nm_setting_wireless_security_get_num_pairwise, - nm_setting_wireless_security_remove_pairwise, - _validate_and_remove_wifi_sec_pairwise) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING_STATIC (_remove_fcn_wireless_security_pairwise, + NM_SETTING_WIRELESS_SECURITY, + nm_setting_wireless_security_get_num_pairwise, + nm_setting_wireless_security_remove_pairwise, + wifi_sec_valid_pairwises, + nm_setting_wireless_security_remove_pairwise_by_value) static const char *wifi_sec_valid_groups[] = { "wep40", "wep104", "tkip", "ccmp", NULL }; @@ -4621,29 +4597,12 @@ _set_fcn_wireless_security_group (ARGS_SET_FCN) return check_and_add_wifi_sec_group (setting, property_info->property_name, value, wifi_sec_valid_groups, error); } -static gboolean -_validate_and_remove_wifi_sec_group (NMSettingWirelessSecurity *setting, - const char *group, - GError **error) -{ - gboolean ret; - const char *valid; - - valid = nmc_string_is_valid (group, wifi_sec_valid_groups, error); - if (!valid) - return FALSE; - - ret = nm_setting_wireless_security_remove_group_by_value (setting, group); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain protocol '%s'"), group); - return ret; -} -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_wireless_security_group, - NM_SETTING_WIRELESS_SECURITY, - nm_setting_wireless_security_get_num_groups, - nm_setting_wireless_security_remove_group, - _validate_and_remove_wifi_sec_group) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING_STATIC (_remove_fcn_wireless_security_group, + NM_SETTING_WIRELESS_SECURITY, + nm_setting_wireless_security_get_num_groups, + nm_setting_wireless_security_remove_group, + wifi_sec_valid_groups, + nm_setting_wireless_security_remove_group_by_value) static gboolean _set_fcn_wireless_wep_key (ARGS_SET_FCN) From 936912cad0ac338077ea57261bde8df9180e6546 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 09:00:24 +0100 Subject: [PATCH 14/98] cli: don't fail removing non-existing option (pt3) Part 3, which addresses the issue for properties that have a simple validation function. --- clients/common/nm-meta-setting-desc.c | 124 ++++++++++++-------------- 1 file changed, 58 insertions(+), 66 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index be2cb28ce8..a44e3aa478 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -1714,6 +1714,15 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) return TRUE; \ }) +#define DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING(def_func, s_macro, num_func, rem_func_idx, rem_func_val) \ + _DEFINE_REMOVER_INDEX_OR_VALUE (def_func, s_macro, num_func, rem_func_idx, { \ + gs_free char *value_to_free = NULL; \ + \ + return rem_func_val (s_macro (setting), \ + nm_strstrip_avoid_copy (value, &value_to_free), \ + error); \ + }) + #define DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING_STATIC(def_func, s_macro, num_func, rem_func_idx, values_static, rem_func_val) \ _DEFINE_REMOVER_INDEX_OR_VALUE (def_func, s_macro, num_func, rem_func_idx, { \ value = nmc_string_is_valid (value, values_static, error); \ @@ -2669,25 +2678,20 @@ _validate_and_remove_connection_secondary (NMSettingConnection *setting, const char *secondary_uuid, GError **error) { - gboolean ret; - if (!nm_utils_is_uuid (secondary_uuid)) { - g_set_error (error, 1, 0, - _("the value '%s' is not a valid UUID"), secondary_uuid); + nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT, + _("the value '%s' is not a valid UUID"), secondary_uuid); return FALSE; } - ret = nm_setting_connection_remove_secondary_by_value (setting, secondary_uuid); - if (!ret) - g_set_error (error, 1, 0, - _("the property doesn't contain UUID '%s'"), secondary_uuid); - return ret; + nm_setting_connection_remove_secondary_by_value (setting, secondary_uuid); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_connection_secondaries, - NM_SETTING_CONNECTION, - nm_setting_connection_get_num_secondaries, - nm_setting_connection_remove_secondary, - _validate_and_remove_connection_secondary) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (_remove_fcn_connection_secondaries, + NM_SETTING_CONNECTION, + nm_setting_connection_get_num_secondaries, + nm_setting_connection_remove_secondary, + _validate_and_remove_connection_secondary) static gconstpointer _get_fcn_connection_metered (ARGS_GET_FCN) @@ -3283,24 +3287,20 @@ _validate_and_remove_ipv4_dns (NMSettingIPConfig *setting, const char *dns, GError **error) { - guint32 ip4_addr; - gboolean ret; - - if (inet_pton (AF_INET, dns, &ip4_addr) < 1) { - g_set_error (error, 1, 0, _("invalid IPv4 address '%s'"), dns); + if (!nm_utils_parse_inaddr (AF_INET, dns, NULL)) { + nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT, + _("invalid IPv4 address '%s'"), dns); return FALSE; } - ret = nm_setting_ip_config_remove_dns_by_value (setting, dns); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain DNS server '%s'"), dns); - return ret; + nm_setting_ip_config_remove_dns_by_value (setting, dns); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv4_config_dns, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_dns, - nm_setting_ip_config_remove_dns, - _validate_and_remove_ipv4_dns) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (_remove_fcn_ipv4_config_dns, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_dns, + nm_setting_ip_config_remove_dns, + _validate_and_remove_ipv4_dns) static gboolean _set_fcn_ip_config_dns_search (ARGS_SET_FCN) @@ -3502,24 +3502,20 @@ _validate_and_remove_ipv6_dns (NMSettingIPConfig *setting, const char *dns, GError **error) { - struct in6_addr ip6_addr; - gboolean ret; - - if (inet_pton (AF_INET6, dns, &ip6_addr) < 1) { - g_set_error (error, 1, 0, _("invalid IPv6 address '%s'"), dns); + if (!nm_utils_parse_inaddr (AF_INET6, dns, NULL)) { + nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT, + _("invalid IPv6 address '%s'"), dns); return FALSE; } - ret = nm_setting_ip_config_remove_dns_by_value (setting, dns); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain DNS server '%s'"), dns); - return ret; + nm_setting_ip_config_remove_dns_by_value (setting, dns); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv6_config_dns, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_dns, - nm_setting_ip_config_remove_dns, - _validate_and_remove_ipv6_dns) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (_remove_fcn_ipv6_config_dns, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_dns, + nm_setting_ip_config_remove_dns, + _validate_and_remove_ipv6_dns) static gboolean _dns_options_is_default (NMSettingIPConfig *setting) @@ -4377,24 +4373,22 @@ _validate_and_remove_wired_mac_blacklist_item (NMSettingWired *setting, const char *mac, GError **error) { - gboolean ret; guint8 buf[32]; if (!nm_utils_hwaddr_aton (mac, buf, ETH_ALEN)) { - g_set_error (error, 1, 0, _("'%s' is not a valid MAC address"), mac); - return FALSE; + nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT, + _("'%s' is not a valid MAC address"), mac); + return FALSE; } - ret = nm_setting_wired_remove_mac_blacklist_item_by_value (setting, mac); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain MAC address '%s'"), mac); - return ret; + nm_setting_wired_remove_mac_blacklist_item_by_value (setting, mac); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_wired_mac_address_blacklist, - NM_SETTING_WIRED, - nm_setting_wired_get_num_mac_blacklist_items, - nm_setting_wired_remove_mac_blacklist_item, - _validate_and_remove_wired_mac_blacklist_item) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (_remove_fcn_wired_mac_address_blacklist, + NM_SETTING_WIRED, + nm_setting_wired_get_num_mac_blacklist_items, + nm_setting_wired_remove_mac_blacklist_item, + _validate_and_remove_wired_mac_blacklist_item) static gboolean _set_fcn_wired_s390_subchannels (ARGS_SET_FCN) @@ -4508,24 +4502,22 @@ _validate_and_remove_wifi_mac_blacklist_item (NMSettingWireless *setting, const char *mac, GError **error) { - gboolean ret; guint8 buf[32]; if (!nm_utils_hwaddr_aton (mac, buf, ETH_ALEN)) { - g_set_error (error, 1, 0, _("'%s' is not a valid MAC address"), mac); - return FALSE; + nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT, + _("'%s' is not a valid MAC address"), mac); + return FALSE; } - ret = nm_setting_wireless_remove_mac_blacklist_item_by_value (setting, mac); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain MAC address '%s'"), mac); - return ret; + nm_setting_wireless_remove_mac_blacklist_item_by_value (setting, mac); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_wireless_mac_address_blacklist, - NM_SETTING_WIRELESS, - nm_setting_wireless_get_num_mac_blacklist_items, - nm_setting_wireless_remove_mac_blacklist_item, - _validate_and_remove_wifi_mac_blacklist_item) +DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (_remove_fcn_wireless_mac_address_blacklist, + NM_SETTING_WIRELESS, + nm_setting_wireless_get_num_mac_blacklist_items, + nm_setting_wireless_remove_mac_blacklist_item, + _validate_and_remove_wifi_mac_blacklist_item) static gconstpointer _get_fcn_wireless_security_wep_key (ARGS_GET_FCN) From 6baaa207639ec18f3a43898c32c2a8cac7c716cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 09:00:24 +0100 Subject: [PATCH 15/98] cli: don't fail removing non-existing option (pt4) Part 4, which addresses the issue for properties that have complex values. --- clients/common/nm-meta-setting-desc.c | 248 ++++++++++---------------- 1 file changed, 90 insertions(+), 158 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index a44e3aa478..5d35db652f 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -1665,31 +1665,6 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) return TRUE; \ } -#define DEFINE_REMOVER_INDEX_OR_VALUE(def_func, s_macro, num_func, rem_func_idx, rem_func_val) \ - static gboolean \ - def_func (ARGS_REMOVE_FCN) \ - { \ - guint32 num; \ - if (value) { \ - gboolean ret; \ - char *value_stripped = g_strstrip (g_strdup (value)); \ - ret = rem_func_val (s_macro (setting), value_stripped, error); \ - g_free (value_stripped); \ - return ret; \ - } \ - num = num_func (s_macro (setting)); \ - if (num == 0) { \ - g_set_error_literal (error, 1, 0, _("no item to remove")); \ - return FALSE; \ - } \ - if (idx >= num) { \ - g_set_error (error, 1, 0, _("index '%d' is not in range <0-%d>"), idx, num - 1); \ - return FALSE; \ - } \ - rem_func_idx (s_macro (setting), idx); \ - return TRUE; \ - } - #define _DEFINE_REMOVER_INDEX_OR_VALUE(def_func, s_macro, num_func, rem_func_idx, rem_func_cmd) \ static gboolean \ def_func (ARGS_REMOVE_FCN) \ @@ -1733,6 +1708,9 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) return TRUE; \ }) +#define DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX(def_func, s_macro, num_func, rem_func_idx, rem_func_val) \ + DEFINE_REMOVER_INDEX_OR_VALUE_VALIDATING (def_func, s_macro, num_func, rem_func_idx, rem_func_val) + #define DEFINE_REMOVER_OPTION(def_func, s_macro, rem_func) \ static gboolean \ def_func (ARGS_REMOVE_FCN) \ @@ -3376,26 +3354,20 @@ _validate_and_remove_ipv4_address (NMSettingIPConfig *setting, const char *address, GError **error) { - NMIPAddress *ip4addr; - gboolean ret; + nm_auto_unref_ip_address NMIPAddress *ip4addr = NULL; ip4addr = _parse_ip_address (AF_INET, address, error); if (!ip4addr) return FALSE; - ret = nm_setting_ip_config_remove_address_by_value (setting, ip4addr); - if (!ret) { - g_set_error (error, 1, 0, - _("the property doesn't contain IP address '%s'"), address); - } - nm_ip_address_unref (ip4addr); - return ret; + nm_setting_ip_config_remove_address_by_value (setting, ip4addr); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv4_config_addresses, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_addresses, - nm_setting_ip_config_remove_address, - _validate_and_remove_ipv4_address) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_ipv4_config_addresses, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_addresses, + nm_setting_ip_config_remove_address, + _validate_and_remove_ipv4_address) static gboolean _set_fcn_ip4_config_gateway (ARGS_SET_FCN) @@ -3433,27 +3405,23 @@ _set_fcn_ip4_config_routes (ARGS_SET_FCN) static gboolean _validate_and_remove_ipv4_route (NMSettingIPConfig *setting, - const char *route, + const char *value, GError **error) { - NMIPRoute *ip4route; - gboolean ret; + nm_auto_unref_ip_route NMIPRoute *route = NULL; - ip4route = _parse_ip_route (AF_INET, route, error); - if (!ip4route) + route = _parse_ip_route (AF_INET, value, error); + if (!route) return FALSE; - ret = nm_setting_ip_config_remove_route_by_value (setting, ip4route); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain route '%s'"), route); - nm_ip_route_unref (ip4route); - return ret; + nm_setting_ip_config_remove_route_by_value (setting, route); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv4_config_routes, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_routes, - nm_setting_ip_config_remove_route, - _validate_and_remove_ipv4_route) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_ipv4_config_routes, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_routes, + nm_setting_ip_config_remove_route, + _validate_and_remove_ipv4_route) static const char *ipv6_valid_methods[] = { NM_SETTING_IP6_CONFIG_METHOD_IGNORE, @@ -3544,27 +3512,23 @@ _set_fcn_ip6_config_addresses (ARGS_SET_FCN) static gboolean _validate_and_remove_ipv6_address (NMSettingIPConfig *setting, - const char *address, + const char *value, GError **error) { - NMIPAddress *ip6addr; - gboolean ret; + nm_auto_unref_ip_address NMIPAddress *addr = NULL; - ip6addr = _parse_ip_address (AF_INET6, address, error); - if (!ip6addr) + addr = _parse_ip_address (AF_INET6, value, error); + if (!addr) return FALSE; - ret = nm_setting_ip_config_remove_address_by_value (setting, ip6addr); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain IP address '%s'"), address); - nm_ip_address_unref (ip6addr); - return ret; + nm_setting_ip_config_remove_address_by_value (setting, addr); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv6_config_addresses, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_addresses, - nm_setting_ip_config_remove_address, - _validate_and_remove_ipv6_address) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_ipv6_config_addresses, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_addresses, + nm_setting_ip_config_remove_address, + _validate_and_remove_ipv6_address) static gboolean _set_fcn_ip6_config_gateway (ARGS_SET_FCN) @@ -3604,27 +3568,23 @@ _set_fcn_ip6_config_routes (ARGS_SET_FCN) static gboolean _validate_and_remove_ipv6_route (NMSettingIPConfig *setting, - const char *route, + const char *value, GError **error) { - NMIPRoute *ip6route; - gboolean ret; + nm_auto_unref_ip_route NMIPRoute *route = NULL; - ip6route = _parse_ip_route (AF_INET6, route, error); - if (!ip6route) + route = _parse_ip_route (AF_INET6, value, error); + if (!route) return FALSE; - ret = nm_setting_ip_config_remove_route_by_value (setting, ip6route); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain route '%s'"), route); - nm_ip_route_unref (ip6route); - return ret; + nm_setting_ip_config_remove_route_by_value (setting, route); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_ipv6_config_routes, - NM_SETTING_IP_CONFIG, - nm_setting_ip_config_get_num_routes, - nm_setting_ip_config_remove_route, - _validate_and_remove_ipv6_route) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_ipv6_config_routes, + NM_SETTING_IP_CONFIG, + nm_setting_ip_config_get_num_routes, + nm_setting_ip_config_remove_route, + _validate_and_remove_ipv6_route) static gconstpointer _get_fcn_match_interface_name (ARGS_GET_FCN) @@ -3807,11 +3767,12 @@ _set_fcn_tc_config_qdiscs (ARGS_SET_FCN) { gs_free const char **strv = NULL; const char *const*iter; - NMTCQdisc *tc_qdisc; gs_free_error GError *local = NULL; strv = nm_utils_strsplit_set (value, ",", FALSE); for (iter = strv; strv && *iter; iter++) { + nm_auto_unref_tc_qdisc NMTCQdisc *tc_qdisc = NULL; + tc_qdisc = nm_utils_tc_qdisc_from_str (*iter, &local); if (!tc_qdisc) { g_set_error (error, 1, 0, "%s %s", local->message, @@ -3819,7 +3780,6 @@ _set_fcn_tc_config_qdiscs (ARGS_SET_FCN) return FALSE; } nm_setting_tc_config_add_qdisc (NM_SETTING_TC_CONFIG (setting), tc_qdisc); - nm_tc_qdisc_unref (tc_qdisc); } return TRUE; } @@ -3829,52 +3789,40 @@ _validate_and_remove_sriov_vf (NMSettingSriov *setting, const char *value, GError **error) { - NMSriovVF *vf; - gboolean ret; + nm_auto_unref_sriov_vf NMSriovVF *vf = NULL; vf = nm_utils_sriov_vf_from_str (value, error); if (!vf) return FALSE; - ret = nm_setting_sriov_remove_vf_by_index (setting, nm_sriov_vf_get_index (vf)); - if (!ret) { - g_set_error (error, 1, 0, - _("the property doesn't contain vf with index %u"), - nm_sriov_vf_get_index (vf)); - } - nm_sriov_vf_unref (vf); - return ret; + nm_setting_sriov_remove_vf_by_index (setting, nm_sriov_vf_get_index (vf)); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_sriov_vfs, - NM_SETTING_SRIOV, - nm_setting_sriov_get_num_vfs, - nm_setting_sriov_remove_vf, - _validate_and_remove_sriov_vf) - +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_sriov_vfs, + NM_SETTING_SRIOV, + nm_setting_sriov_get_num_vfs, + nm_setting_sriov_remove_vf, + _validate_and_remove_sriov_vf) static gboolean _validate_and_remove_tc_qdisc (NMSettingTCConfig *setting, const char *value, GError **error) { - NMTCQdisc *qdisc; - gboolean ret; + nm_auto_unref_tc_qdisc NMTCQdisc *qdisc = NULL; qdisc = nm_utils_tc_qdisc_from_str (value, error); if (!qdisc) return FALSE; - ret = nm_setting_tc_config_remove_qdisc_by_value (setting, qdisc); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain qdisc '%s'"), value); - nm_tc_qdisc_unref (qdisc); - return ret; + nm_setting_tc_config_remove_qdisc_by_value (setting, qdisc); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_tc_config_qdiscs, - NM_SETTING_TC_CONFIG, - nm_setting_tc_config_get_num_qdiscs, - nm_setting_tc_config_remove_qdisc, - _validate_and_remove_tc_qdisc) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_tc_config_qdiscs, + NM_SETTING_TC_CONFIG, + nm_setting_tc_config_get_num_qdiscs, + nm_setting_tc_config_remove_qdisc, + _validate_and_remove_tc_qdisc) static gconstpointer _get_fcn_tc_config_tfilters (ARGS_GET_FCN) @@ -3934,24 +3882,20 @@ _validate_and_remove_tc_tfilter (NMSettingTCConfig *setting, const char *value, GError **error) { - NMTCTfilter *tfilter; - gboolean ret; + nm_auto_unref_tc_tfilter NMTCTfilter *tfilter = NULL; tfilter = nm_utils_tc_tfilter_from_str (value, error); if (!tfilter) return FALSE; - ret = nm_setting_tc_config_remove_tfilter_by_value (setting, tfilter); - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain tfilter '%s'"), value); - nm_tc_tfilter_unref (tfilter); - return ret; + nm_setting_tc_config_remove_tfilter_by_value (setting, tfilter); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_tc_config_tfilters, - NM_SETTING_TC_CONFIG, - nm_setting_tc_config_get_num_tfilters, - nm_setting_tc_config_remove_tfilter, - _validate_and_remove_tc_tfilter) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_tc_config_tfilters, + NM_SETTING_TC_CONFIG, + nm_setting_tc_config_get_num_tfilters, + nm_setting_tc_config_remove_tfilter, + _validate_and_remove_tc_tfilter) static const char * _validate_fcn_team_config (const char *value, char **out_to_free, GError **error) @@ -4058,29 +4002,23 @@ _set_fcn_team_link_watchers (ARGS_SET_FCN) static gboolean _validate_and_remove_team_link_watcher (NMSettingTeam *setting, - const char *watcher_str, + const char *value, GError **error) { - NMTeamLinkWatcher *watcher; - gboolean ret; + nm_auto_unref_team_link_watcher NMTeamLinkWatcher *watcher = NULL; - watcher = _parse_team_link_watcher (watcher_str, error); + watcher = _parse_team_link_watcher (value, error); if (!watcher) return FALSE; - ret = nm_setting_team_remove_link_watcher_by_value (setting, watcher); - if (!ret) { - g_set_error (error, 1, 0, _("the property doesn't contain link watcher '%s'"), - watcher_str); - } - nm_team_link_watcher_unref (watcher); - return ret; + nm_setting_team_remove_link_watcher_by_value (setting, watcher); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_team_link_watchers, - NM_SETTING_TEAM, - nm_setting_team_get_num_link_watchers, - nm_setting_team_remove_link_watcher, - _validate_and_remove_team_link_watcher) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_team_link_watchers, + NM_SETTING_TEAM, + nm_setting_team_get_num_link_watchers, + nm_setting_team_remove_link_watcher, + _validate_and_remove_team_link_watcher) static gconstpointer _get_fcn_team_port_link_watchers (ARGS_GET_FCN) @@ -4131,29 +4069,23 @@ _set_fcn_team_port_link_watchers (ARGS_SET_FCN) static gboolean _validate_and_remove_team_port_link_watcher (NMSettingTeamPort *setting, - const char *watcher_str, + const char *value, GError **error) { - NMTeamLinkWatcher *watcher; - gboolean ret; + nm_auto_unref_team_link_watcher NMTeamLinkWatcher *watcher = NULL; - watcher = _parse_team_link_watcher (watcher_str, error); + watcher = _parse_team_link_watcher (value, error); if (!watcher) return FALSE; - ret = nm_setting_team_port_remove_link_watcher_by_value (setting, watcher); - if (!ret) { - g_set_error (error, 1, 0, _("the property doesn't contain link watcher '%s'"), - watcher_str); - } - nm_team_link_watcher_unref (watcher); - return ret; + nm_setting_team_port_remove_link_watcher_by_value (setting, watcher); + return TRUE; } -DEFINE_REMOVER_INDEX_OR_VALUE (_remove_fcn_team_port_link_watchers, - NM_SETTING_TEAM_PORT, - nm_setting_team_port_get_num_link_watchers, - nm_setting_team_port_remove_link_watcher, - _validate_and_remove_team_port_link_watcher) +DEFINE_REMOVER_INDEX_OR_VALUE_COMPLEX (_remove_fcn_team_port_link_watchers, + NM_SETTING_TEAM_PORT, + nm_setting_team_port_get_num_link_watchers, + nm_setting_team_port_remove_link_watcher, + _validate_and_remove_team_port_link_watcher) static gconstpointer _get_fcn_vlan_flags (ARGS_GET_FCN) From eff01cd2568907576b2051afab5920bfc1cd224e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 09:00:24 +0100 Subject: [PATCH 16/98] cli: don't fail removing non-existing option (pt5) Part 5, which addresses the issue for DEFINE_REMOVER_OPTION(), which are simple properties. --- clients/common/nm-meta-setting-desc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 5d35db652f..4777929bab 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -1715,14 +1715,9 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) static gboolean \ def_func (ARGS_REMOVE_FCN) \ { \ - gboolean success = FALSE; \ - if (value && *value) { \ - success = rem_func (s_macro (setting), value); \ - if (!success) \ - g_set_error (error, 1, 0, _("invalid option '%s'"), value); \ - } else \ - g_set_error_literal (error, 1, 0, _("missing option")); \ - return success; \ + if (value && *value) \ + rem_func (s_macro (setting), value); \ + return TRUE; \ } #define DEFINE_ALLOWED_VAL_FUNC(def_func, valid_values) \ From 626aed64e75ee5d401b6dc5adc4d9c0bfbecdd97 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 09:10:40 +0100 Subject: [PATCH 17/98] cli: don't fail removing non-existing option (pt6) Part 6, which addresses the issue for VLAN priority maps. --- clients/common/nm-meta-setting-desc.c | 33 +++++++-------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 4777929bab..f1e59ecd06 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -4164,13 +4164,10 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment, { guint32 num; - /* If value != NULL, remove by value */ if (value) { - gboolean ret; - char **prio_map; - gs_free char *v = g_strdup (value); + gs_strfreev char **prio_map = NULL; - prio_map = _parse_vlan_priority_maps (v, map_type, TRUE, error); + prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error); if (!prio_map) return FALSE; if (prio_map[1]) { @@ -4179,29 +4176,15 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment, N_("only one mapping at a time is supported; taking the first one (%s)"), prio_map[0]); } - ret = nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), - map_type, - prio_map[0]); - - if (!ret) - g_set_error (error, 1, 0, _("the property doesn't contain mapping '%s'"), prio_map[0]); - g_strfreev (prio_map); - return ret; + nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), + map_type, + prio_map[0]); + return TRUE; } - /* Else remove by index */ num = nm_setting_vlan_get_num_priorities (NM_SETTING_VLAN (setting), map_type); - if (num == 0) { - g_set_error_literal (error, 1, 0, _("no priority to remove")); - return FALSE; - } - if (idx >= num) { - g_set_error (error, 1, 0, _("index '%d' is not in the range of <0-%d>"), - idx, num - 1); - return FALSE; - } - - nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx); + if (idx < num) + nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx); return TRUE; } From d3cfe20598fee6f8afbd7a750edbc14876e5e4eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 16 Mar 2019 13:15:14 +0100 Subject: [PATCH 18/98] cli: support removing multiple vlan ingress/egress priority mappings $ nmcli connection add type vlan autoconnect no con-name v dev vlan.1 id 1 $ nmcli connection modify v +vlan.ingress-priority-map 1:2,2:3 $ nmcli connection modify v +vlan.ingress-priority-map 2:3,4:5 $ nmcli connection modify v -vlan.ingress-priority-map 1:2,4:5 Warning: only one mapping at a time is supported; taking the first one (1:2) --- clients/common/nm-meta-setting-desc.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index f1e59ecd06..e0de0bf184 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -4166,19 +4166,17 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment, if (value) { gs_strfreev char **prio_map = NULL; + gsize i; prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error); if (!prio_map) return FALSE; - if (prio_map[1]) { - _env_warn_fcn (environment, environment_user_data, - NM_META_ENV_WARN_LEVEL_WARN, - N_("only one mapping at a time is supported; taking the first one (%s)"), - prio_map[0]); + + for (i = 0; prio_map[i]; i++) { + nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), + map_type, + prio_map[i]); } - nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), - map_type, - prio_map[0]); return TRUE; } From cb5a81399a3d109e6e386762b3fdb83385c38269 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 09:23:37 +0100 Subject: [PATCH 19/98] cli: don't interpret value as index too early for nmc_setting_remove_property_option() Not all implementations support having the value being an index. For example, the implementations that are done via DEFINE_REMOVER_OPTION() macro. The meaning of the "value" string must not be determined by nmc_setting_remove_property_option(). It's up to the implementation to decide whether to allow an index and how to interpret it. --- clients/cli/connections.c | 21 +++--------- clients/cli/settings.c | 18 +++------- clients/cli/settings.h | 3 +- clients/common/nm-meta-setting-desc.c | 49 +++++++++++++++------------ clients/common/nm-meta-setting-desc.h | 3 +- 5 files changed, 37 insertions(+), 57 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 6db44f8760..c9a6d2be4a 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -4004,12 +4004,7 @@ set_property (NMClient *client, * - or option name: remove item with the option name */ if (value) { - unsigned long idx; - - if (nmc_string_to_uint (value, TRUE, 0, G_MAXUINT32, &idx)) - nmc_setting_remove_property_option (setting, property_name, NULL, idx, &local); - else - nmc_setting_remove_property_option (setting, property_name, value, 0, &local); + nmc_setting_remove_property_option (setting, property_name, value, &local); if (local) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, _("Error: failed to remove a value from %s.%s: %s."), @@ -6996,17 +6991,9 @@ property_edit_submenu (NmCli *nmc, case NMC_EDITOR_SUB_CMD_REMOVE: if (cmd_property_arg) { - unsigned long val_int = G_MAXUINT32; - gs_free char *option = NULL; - - if (!nmc_string_to_uint (cmd_property_arg, TRUE, 0, G_MAXUINT32, &val_int)) { - option = g_strdup (cmd_property_arg); - g_strstrip (option); - } - - if (!nmc_setting_remove_property_option (curr_setting, prop_name, - option, - (guint32) val_int, + if (!nmc_setting_remove_property_option (curr_setting, + prop_name, + cmd_property_arg, &tmp_err)) { g_print (_("Error: %s\n"), tmp_err->message); g_clear_error (&tmp_err); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index a04c8eb604..840eb96201 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -618,26 +618,17 @@ nmc_setting_reset_property (NMSetting *setting, const char *prop, GError **error return FALSE; } -/* - * Generic function for removing items for collection-type properties. - * - * If 'option' is not NULL, it tries to remove it, otherwise 'idx' is used. - * For single-value properties (not having specialized remove function) this - * function does nothing and just returns TRUE. - * - * Returns: TRUE on success; FALSE on failure and sets error - */ gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *prop, - const char *option, - guint32 idx, + const char *value, GError **error) { const NMMetaPropertyInfo *property_info; g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + g_return_val_if_fail (!error || !*error, FALSE); + g_return_val_if_fail (value, FALSE); if ((property_info = nm_meta_property_info_find_by_setting (setting, prop))) { if (property_info->property_type->remove_fcn) { @@ -645,8 +636,7 @@ nmc_setting_remove_property_option (NMSetting *setting, nmc_meta_environment, nmc_meta_environment_arg, setting, - option, - idx, + value, error); } } diff --git a/clients/cli/settings.h b/clients/cli/settings.h index 4e7e38df85..8bf2dfc6b4 100644 --- a/clients/cli/settings.h +++ b/clients/cli/settings.h @@ -52,8 +52,7 @@ gboolean nmc_setting_reset_property (NMSetting *setting, GError **error); gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *prop, - const char *option, - guint32 idx, + const char *value, GError **error); void nmc_property_set_default_value (NMSetting *setting, const char *prop); diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index e0de0bf184..5293268ca7 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -628,7 +628,7 @@ _env_warn_fcn (const NMMetaEnvironment *environment, const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, GError **error #define ARGS_REMOVE_FCN \ - const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, guint32 idx, GError **error + const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, GError **error #define ARGS_COMPLETE_FCN \ const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, const NMMetaOperationContext *operation_context, const char *text, char ***out_to_free @@ -1669,9 +1669,11 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) static gboolean \ def_func (ARGS_REMOVE_FCN) \ { \ - guint32 num; \ + gint64 idx; \ + gint64 num; \ \ - if (!value) { \ + idx = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT64, -1); \ + if (idx != -1) { \ num = num_func (s_macro (setting)); \ if (idx < num) \ rem_func_idx (s_macro (setting), idx); \ @@ -4158,31 +4160,36 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment, NMSetting *setting, const NMMetaPropertyInfo *property_info, const char *value, - guint32 idx, NMVlanPriorityMap map_type, GError **error) { + gs_strfreev char **prio_map = NULL; guint32 num; + gint64 idx; + gsize i; - if (value) { - gs_strfreev char **prio_map = NULL; - gsize i; - - prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error); - if (!prio_map) - return FALSE; - - for (i = 0; prio_map[i]; i++) { - nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), - map_type, - prio_map[i]); - } + idx = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT64, -1); + if (idx != -1) { + /* FIXME: this is ugly, if the caller only provides one number, we accept it as + * index to remove. + * The ugly-ness comes from the fact, that + * " -vlan.ingress-priority-map 0,1,2" + * cannot be used to remove the first 3 elements. */ + num = nm_setting_vlan_get_num_priorities (NM_SETTING_VLAN (setting), map_type); + if (idx < (gint64) num) + nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx); return TRUE; } - num = nm_setting_vlan_get_num_priorities (NM_SETTING_VLAN (setting), map_type); - if (idx < num) - nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx); + prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error); + if (!prio_map) + return FALSE; + + for (i = 0; prio_map[i]; i++) { + nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting), + map_type, + prio_map[i]); + } return TRUE; } @@ -4194,7 +4201,6 @@ _remove_fcn_vlan_ingress_priority_map (ARGS_REMOVE_FCN) setting, property_info, value, - idx, NM_VLAN_INGRESS_MAP, error); } @@ -4207,7 +4213,6 @@ _remove_fcn_vlan_egress_priority_map (ARGS_REMOVE_FCN) setting, property_info, value, - idx, NM_VLAN_EGRESS_MAP, error); } diff --git a/clients/common/nm-meta-setting-desc.h b/clients/common/nm-meta-setting-desc.h index e3f9d230ee..1aee7a8d75 100644 --- a/clients/common/nm-meta-setting-desc.h +++ b/clients/common/nm-meta-setting-desc.h @@ -215,8 +215,7 @@ struct _NMMetaPropertyType { const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, - const char *option, - guint32 idx, + const char *value, GError **error); const char *const*(*values_fcn) (const NMMetaPropertyInfo *property_info, From 7b5d514aef756bf1b0d77e73bc242f841a8142bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 10:09:24 +0100 Subject: [PATCH 20/98] cli: implement nmc_setting_reset_property() based on nmc_setting_set_property() "reset" is just a special case of "set". We can keep nmc_setting_reset_property() as a convenience function, but it must be implemented based on nmc_setting_set_property(). Also, reset only used nmc_property_set_default_value(), which only works with GObject based properties. It's wrong to assume that all properties are GObject based. By implementing it based via nmc_setting_set_property() we can fix this (later). --- clients/cli/connections.c | 12 +++--- clients/cli/settings.c | 89 +++++++++++++-------------------------- clients/cli/settings.h | 13 ++++-- 3 files changed, 45 insertions(+), 69 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index c9a6d2be4a..7994305f40 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -3988,7 +3988,7 @@ set_property (NMClient *client, * so make a copy if we are going to free it. */ value = value_free = g_strdup (value); - nmc_setting_reset_property (setting, property_name, NULL); + nmc_setting_reset_property (client, setting, property_name, NULL); } if (!nmc_setting_set_property (client, setting, property_name, value, &local)) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, @@ -4013,7 +4013,7 @@ set_property (NMClient *client, return FALSE; } } else - nmc_setting_reset_property (setting, property_name, NULL); + nmc_setting_reset_property (client, setting, property_name, NULL); } /* Don't ask for this property in interactive mode. */ @@ -6999,7 +6999,7 @@ property_edit_submenu (NmCli *nmc, g_clear_error (&tmp_err); } } else { - if (!nmc_setting_reset_property (curr_setting, prop_name, &tmp_err)) { + if (!nmc_setting_reset_property (nmc->client, curr_setting, prop_name, &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); @@ -7513,8 +7513,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (!prop_name) break; - /* Delete property value */ - if (!nmc_setting_reset_property (menu_ctx.curr_setting, prop_name, &tmp_err)) { + if (!nmc_setting_reset_property (nmc->client, menu_ctx.curr_setting, prop_name, &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); @@ -7564,8 +7563,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err); if (prop_name) { - /* Delete property value */ - if (!nmc_setting_reset_property (ss, prop_name, &tmp_err)) { + if (!nmc_setting_reset_property (nmc->client, ss, prop_name, &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 840eb96201..5dbfab82a2 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -539,40 +539,39 @@ nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - if ((property_info = nm_meta_property_info_find_by_setting (setting, prop))) { + if (!(property_info = nm_meta_property_info_find_by_setting (setting, prop))) + goto out_fail_read_only; - if (!value) { - /* No value argument sets default value */ - nmc_property_set_default_value (setting, prop); - return TRUE; - } - - if (property_info->property_type->set_fcn) { - switch (property_info->setting_info->general->meta_type) { - case NM_META_SETTING_TYPE_CONNECTION: - if (nm_streq (property_info->property_name, NM_SETTING_CONNECTION_SECONDARIES)) { - gs_free char *value_coerced = NULL; - - if (!_set_fcn_precheck_connection_secondaries (client, value, &value_coerced, error)) - return FALSE; - - return _set_fcn_call (property_info, - setting, - value_coerced ?: value, - error); - } - break; - default: - break; - } - return _set_fcn_call (property_info, - setting, - value, - error); - } + if (!value) { + /* No value argument sets default value */ + nmc_property_set_default_value (setting, prop); + return TRUE; } - g_set_error_literal (error, 1, 0, _("the property can't be changed")); + if (property_info->property_type->set_fcn) { + gs_free char *value_to_free = NULL; + + switch (property_info->setting_info->general->meta_type) { + case NM_META_SETTING_TYPE_CONNECTION: + if (nm_streq (property_info->property_name, NM_SETTING_CONNECTION_SECONDARIES)) { + if (!_set_fcn_precheck_connection_secondaries (client, value, &value_to_free, error)) + return FALSE; + if (value_to_free) + value = value_to_free; + } + break; + default: + break; + } + + return _set_fcn_call (property_info, + setting, + value, + error); + } + +out_fail_read_only: + nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN, _("the property can't be changed")); return FALSE; } @@ -590,34 +589,6 @@ nmc_property_set_default_value (NMSetting *setting, const char *prop) } } -/* - * Generic function for resetting (single value) properties. - * - * The function resets the property value to the default one. It respects - * nmcli restrictions for changing properties. So if 'set_func' is NULL, - * resetting the value is denied. - * - * Returns: TRUE on success; FALSE on failure and sets error - */ -gboolean -nmc_setting_reset_property (NMSetting *setting, const char *prop, GError **error) -{ - const NMMetaPropertyInfo *property_info; - - g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - - if ((property_info = nm_meta_property_info_find_by_setting (setting, prop))) { - if (property_info->property_type->set_fcn) { - nmc_property_set_default_value (setting, prop); - return TRUE; - } - } - - g_set_error_literal (error, 1, 0, _("the property can't be changed")); - return FALSE; -} - gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *prop, diff --git a/clients/cli/settings.h b/clients/cli/settings.h index 8bf2dfc6b4..d3ea8f780f 100644 --- a/clients/cli/settings.h +++ b/clients/cli/settings.h @@ -47,13 +47,20 @@ gboolean nmc_setting_set_property (NMClient *client, const char *prop, const char *val, GError **error); -gboolean nmc_setting_reset_property (NMSetting *setting, - const char *prop, - GError **error); +static inline gboolean +nmc_setting_reset_property (NMClient *client, + NMSetting *setting, + const char *prop, + GError **error) +{ + return nmc_setting_set_property (client, setting, prop, NULL, error); +} + gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *prop, const char *value, GError **error); + void nmc_property_set_default_value (NMSetting *setting, const char *prop); gboolean nmc_property_get_gvalue (NMSetting *setting, const char *prop, GValue *value); From f24ada398ac9282a7de75303ef5bea78bf693cde Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 10:14:36 +0100 Subject: [PATCH 21/98] cli: drop nmc_property_set_default_value() It's wrong to pretend that all properties are GObject based. Drop nmc_property_set_default_value() and use nmc_setting_reset_property() instead. --- clients/cli/connections.c | 4 ++-- clients/cli/settings.c | 26 +++++++++++--------------- clients/cli/settings.h | 2 -- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 7994305f40..fe8aff39cf 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -6954,7 +6954,7 @@ property_edit_submenu (NmCli *nmc, */ if (cmdsub == NMC_EDITOR_SUB_CMD_SET) { nmc_property_get_gvalue (curr_setting, prop_name, &prop_g_value); - nmc_property_set_default_value (curr_setting, prop_name); + nmc_setting_reset_property (nmc->client, curr_setting, prop_name, NULL); } set_result = nmc_setting_set_property (nmc->client, curr_setting, prop_name, prop_val_user, &tmp_err); @@ -6978,7 +6978,7 @@ property_edit_submenu (NmCli *nmc, prop_name); nmc_property_get_gvalue (curr_setting, prop_name, &prop_g_value); - nmc_property_set_default_value (curr_setting, prop_name); + nmc_setting_reset_property (nmc->client, curr_setting, prop_name, NULL); if (!nmc_setting_set_property (nmc->client, curr_setting, prop_name, prop_val_user, &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 5dbfab82a2..78a61832bb 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -543,8 +543,18 @@ nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop goto out_fail_read_only; if (!value) { + nm_auto_unset_gvalue GValue gvalue = G_VALUE_INIT; + GParamSpec *param_spec; + /* No value argument sets default value */ - nmc_property_set_default_value (setting, prop); + + /* FIXME: this only works with GObject based properties. */ + param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); + if (param_spec) { + g_value_init (&gvalue, G_PARAM_SPEC_VALUE_TYPE (param_spec)); + g_param_value_set_default (param_spec, &gvalue); + g_object_set_property (G_OBJECT (setting), prop, &gvalue); + } return TRUE; } @@ -575,20 +585,6 @@ out_fail_read_only: return FALSE; } -void -nmc_property_set_default_value (NMSetting *setting, const char *prop) -{ - GValue value = G_VALUE_INIT; - GParamSpec *param_spec; - - param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); - if (param_spec) { - g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (param_spec)); - g_param_value_set_default (param_spec, &value); - g_object_set_property (G_OBJECT (setting), prop, &value); - } -} - gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *prop, diff --git a/clients/cli/settings.h b/clients/cli/settings.h index d3ea8f780f..931ecdfe08 100644 --- a/clients/cli/settings.h +++ b/clients/cli/settings.h @@ -61,8 +61,6 @@ gboolean nmc_setting_remove_property_option (NMSetting *setting, const char *value, GError **error); -void nmc_property_set_default_value (NMSetting *setting, const char *prop); - gboolean nmc_property_get_gvalue (NMSetting *setting, const char *prop, GValue *value); gboolean nmc_property_set_gvalue (NMSetting *setting, const char *prop, GValue *value); From de93bc0712dd455849edeb85050faf012297a057 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 11:19:05 +0100 Subject: [PATCH 22/98] cli: don't use nmc_property_get_gvalue() in ipv4_method_changed_cb() ipv4_method_changed_cb() and ipv6_method_changed_cb() are horrible hacks, as they use a static variable to cache the previous value. Anyway, don't fix that now. We are going to drop nmc_property_get_gvalue()/nmc_property_set_gvalue() because that works only with GObject based properties -- and having this API around wrongly suggests it works in general. Use the native types directly. --- clients/cli/settings.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 78a61832bb..34fb03704a 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -89,7 +89,7 @@ ipv4_addresses_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_dat static void ipv4_method_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) { - static GValue value = G_VALUE_INIT; + static GPtrArray *old_value = NULL; static gboolean answered = FALSE; static gboolean answer = FALSE; @@ -103,17 +103,17 @@ ipv4_method_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) answer = get_answer ("ipv4.addresses", NULL); } if (answer) { - if (G_IS_VALUE (&value)) - g_value_unset (&value); - nmc_property_get_gvalue (NM_SETTING (object), NM_SETTING_IP_CONFIG_ADDRESSES, &value); + nm_clear_pointer (&old_value, g_ptr_array_unref); + g_object_get (object, NM_SETTING_IP_CONFIG_ADDRESSES, &old_value, NULL); g_object_set (object, NM_SETTING_IP_CONFIG_ADDRESSES, NULL, NULL); } } } else { answered = FALSE; - if (G_IS_VALUE (&value)) { - nmc_property_set_gvalue (NM_SETTING (object), NM_SETTING_IP_CONFIG_ADDRESSES, &value); - g_value_unset (&value); + if (old_value) { + gs_unref_ptrarray GPtrArray *v = g_steal_pointer (&old_value); + + g_object_set (object, NM_SETTING_IP_CONFIG_ADDRESSES, v, NULL); } } @@ -152,7 +152,7 @@ ipv6_addresses_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_dat static void ipv6_method_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) { - static GValue value = G_VALUE_INIT; + static GPtrArray *old_value = NULL; static gboolean answered = FALSE; static gboolean answer = FALSE; @@ -166,17 +166,17 @@ ipv6_method_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) answer = get_answer ("ipv6.addresses", NULL); } if (answer) { - if (G_IS_VALUE (&value)) - g_value_unset (&value); - nmc_property_get_gvalue (NM_SETTING (object), NM_SETTING_IP_CONFIG_ADDRESSES, &value); + nm_clear_pointer (&old_value, g_ptr_array_unref); + g_object_get (object, NM_SETTING_IP_CONFIG_ADDRESSES, &old_value, NULL); g_object_set (object, NM_SETTING_IP_CONFIG_ADDRESSES, NULL, NULL); } } } else { answered = FALSE; - if (G_IS_VALUE (&value)) { - nmc_property_set_gvalue (NM_SETTING (object), NM_SETTING_IP_CONFIG_ADDRESSES, &value); - g_value_unset (&value); + if (old_value) { + gs_unref_ptrarray GPtrArray *v = g_steal_pointer (&old_value); + + g_object_set (object, NM_SETTING_IP_CONFIG_ADDRESSES, v, NULL); } } From 649968632ea065d7b4cbf213ec1cc04a0736bc33 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 11:08:41 +0100 Subject: [PATCH 23/98] cli: merge remove-property, reset-property and set-property It's fundamentally wrong to have separate "remove_fcn" and "set_fcn" implementations. Set, reset, add, and remove are all similar, and should be implemented in a similar manner. Merge the implementations all in set-property, which now can: - reset the value (value == NULL && modifier == '\0') - set a value (value != NULL && modifier == '\0') - add a value (value != NULL && modifier == '+') - remove a value (value != NULL && modifier == '-') The real problem is that remove_fcn() behaves fundamentally different from set_fcn(). You can do "+setting.property value1,value2" but you cannot remove them the same way. That is because most remove_fcn() implementations don't expect a list of values. But it's also because of the unnatural split between set_fcn() and remove_fcn(). The next commit will merge set_fcn(), remove_fcn() and reset property. This commit just merges them all in nmc_setting_set_property(). --- clients/cli/connections.c | 129 +++++++++++------------------ clients/cli/settings.c | 166 +++++++++++++++----------------------- clients/cli/settings.h | 17 +--- 3 files changed, 115 insertions(+), 197 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index fe8aff39cf..0235de8132 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -3959,11 +3959,12 @@ set_property (NMClient *client, char modifier, GError **error) { - gs_free char *property_name = NULL, *value_free = NULL; + gs_free char *property_name = NULL; + gs_free_error GError *local = NULL; NMSetting *setting; - GError *local = NULL; - g_assert (setting_name && setting_name[0]); + nm_assert (setting_name && setting_name[0]); + nm_assert (NM_IN_SET (modifier, '\0', '+', '-')); setting = nm_connection_get_setting_by_name (connection, setting_name); if (!setting) { @@ -3977,43 +3978,26 @@ set_property (NMClient *client, g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, _("Error: invalid property '%s': %s."), property, local->message); - g_clear_error (&local); return FALSE; } - if (modifier != '-') { - /* Set/add value */ - if (modifier != '+') { - /* We allow the existing property value to be passed as parameter, - * so make a copy if we are going to free it. - */ - value = value_free = g_strdup (value); - nmc_setting_reset_property (client, setting, property_name, NULL); - } - if (!nmc_setting_set_property (client, setting, property_name, value, &local)) { - g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, - _("Error: failed to modify %s.%s: %s."), - setting_name, property, local->message); - g_clear_error (&local); - return FALSE; - } - } else { - /* Remove value - * - either empty: remove whole value - * - or specified by index <0-n>: remove item at the index - * - or option name: remove item with the option name - */ - if (value) { - nmc_setting_remove_property_option (setting, property_name, value, &local); - if (local) { - g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, - _("Error: failed to remove a value from %s.%s: %s."), - setting_name, property, local->message); - g_clear_error (&local); - return FALSE; - } - } else - nmc_setting_reset_property (client, setting, property_name, NULL); + if (!nmc_setting_set_property (client, + setting, + property_name, + ( (modifier == '-' && !value) + ? '\0' + : modifier), + value, + &local)) { + g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("Error: failed to %s %s.%s: %s."), + ( modifier != '-' + ? "modify" + : "remove a value from"), + setting_name, + property, + local->message); + return FALSE; } /* Don't ask for this property in interactive mode. */ @@ -6899,7 +6883,6 @@ property_edit_submenu (NmCli *nmc, gs_free char *cmd_property_user = NULL; gs_free char *cmd_property_arg = NULL; gs_free char *prop_val_user = NULL; - nm_auto_unset_gvalue GValue prop_g_value = G_VALUE_INIT; gboolean removed; gboolean dirty; @@ -6949,24 +6932,17 @@ property_edit_submenu (NmCli *nmc, } else prop_val_user = g_strdup (cmd_property_arg); - /* nmc_setting_set_property() only adds new value, thus we have to - * remove the original value and save it for error cases. - */ - if (cmdsub == NMC_EDITOR_SUB_CMD_SET) { - nmc_property_get_gvalue (curr_setting, prop_name, &prop_g_value); - nmc_setting_reset_property (nmc->client, curr_setting, prop_name, NULL); - } - - set_result = nmc_setting_set_property (nmc->client, curr_setting, prop_name, prop_val_user, &tmp_err); + set_result = nmc_setting_set_property (nmc->client, + curr_setting, + prop_name, + (cmdsub == NMC_EDITOR_SUB_CMD_SET) + ? '\0' + : '+', + prop_val_user, + &tmp_err); if (!set_result) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); - if (cmdsub == NMC_EDITOR_SUB_CMD_SET) { - /* Block change signals and restore original value */ - g_signal_handlers_block_matched (curr_setting, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL); - nmc_property_set_gvalue (curr_setting, prop_name, &prop_g_value); - g_signal_handlers_unblock_matched (curr_setting, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL); - } } break; @@ -6977,33 +6953,23 @@ property_edit_submenu (NmCli *nmc, _("Edit '%s' value: "), prop_name); - nmc_property_get_gvalue (curr_setting, prop_name, &prop_g_value); - nmc_setting_reset_property (nmc->client, curr_setting, prop_name, NULL); - - if (!nmc_setting_set_property (nmc->client, curr_setting, prop_name, prop_val_user, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, curr_setting, prop_name, '\0', prop_val_user, &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); - g_signal_handlers_block_matched (curr_setting, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL); - nmc_property_set_gvalue (curr_setting, prop_name, &prop_g_value); - g_signal_handlers_unblock_matched (curr_setting, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL); } break; case NMC_EDITOR_SUB_CMD_REMOVE: - if (cmd_property_arg) { - if (!nmc_setting_remove_property_option (curr_setting, - prop_name, - cmd_property_arg, - &tmp_err)) { - g_print (_("Error: %s\n"), tmp_err->message); - g_clear_error (&tmp_err); - } - } else { - if (!nmc_setting_reset_property (nmc->client, curr_setting, prop_name, &tmp_err)) { - g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, - tmp_err->message); - g_clear_error (&tmp_err); - } + if (!nmc_setting_set_property (nmc->client, + curr_setting, + prop_name, + ( cmd_property_arg + ? '-' + : '\0'), + cmd_property_arg, + &tmp_err)) { + g_print (_("Error: %s\n"), tmp_err->message); + g_clear_error (&tmp_err); } break; @@ -7354,8 +7320,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t _("Enter '%s' value: "), prop_name); - /* Set property value */ - if (!nmc_setting_set_property (nmc->client, menu_ctx.curr_setting, prop_name, prop_val_user, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, menu_ctx.curr_setting, prop_name, '+', prop_val_user, &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); } @@ -7415,8 +7380,12 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t prop_name); } - /* Set property value */ - if (!nmc_setting_set_property (nmc->client, ss, prop_name, cmd_arg_v, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, + ss, + prop_name, + cmd_arg_v ? '+' : '\0', + cmd_arg_v, + &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); @@ -7513,7 +7482,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (!prop_name) break; - if (!nmc_setting_reset_property (nmc->client, menu_ctx.curr_setting, prop_name, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, menu_ctx.curr_setting, prop_name, '\0', NULL, &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); @@ -7563,7 +7532,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err); if (prop_name) { - if (!nmc_setting_reset_property (nmc->client, ss, prop_name, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, ss, prop_name, '\0', NULL, &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 34fb03704a..416f5a0c12 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -509,46 +509,35 @@ nmc_setting_get_property_parsable (NMSetting *setting, const char *prop, GError return get_property_val (setting, prop, NM_META_ACCESSOR_GET_TYPE_PARSABLE, TRUE, error); } -static gboolean -_set_fcn_call (const NMMetaPropertyInfo *property_info, - NMSetting *setting, - const char *value, - GError **error) -{ - return property_info->property_type->set_fcn (property_info, - nmc_meta_environment, - nmc_meta_environment_arg, - setting, - value, - error); -} - -/* - * Generic function for setting property value. - * - * Sets property=value in setting by calling specialized functions. - * If value is NULL then default property value is set. - * - * Returns: TRUE on success; FALSE on failure and sets error - */ gboolean -nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop, const char *value, GError **error) +nmc_setting_set_property (NMClient *client, + NMSetting *setting, + const char *prop, + char modifier, + const char *value, + GError **error) { + nm_auto_unset_gvalue GValue gvalue_old = G_VALUE_INIT; const NMMetaPropertyInfo *property_info; + gs_free char *value_to_free = NULL; + /* FIXME: any mentioning of GParamSpec only works for GObject base properties. That is + * wrong, the property meta-data must handle all properties. */ + GParamSpec *param_spec = NULL; g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + g_return_val_if_fail (NM_IN_SET (modifier, '\0', '-', '+'), FALSE); if (!(property_info = nm_meta_property_info_find_by_setting (setting, prop))) goto out_fail_read_only; + if (!property_info->property_type->set_fcn) + goto out_fail_read_only; if (!value) { nm_auto_unset_gvalue GValue gvalue = G_VALUE_INIT; - GParamSpec *param_spec; - /* No value argument sets default value */ + g_return_val_if_fail (modifier == '\0', TRUE); - /* FIXME: this only works with GObject based properties. */ param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); if (param_spec) { g_value_init (&gvalue, G_PARAM_SPEC_VALUE_TYPE (param_spec)); @@ -558,46 +547,7 @@ nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop return TRUE; } - if (property_info->property_type->set_fcn) { - gs_free char *value_to_free = NULL; - - switch (property_info->setting_info->general->meta_type) { - case NM_META_SETTING_TYPE_CONNECTION: - if (nm_streq (property_info->property_name, NM_SETTING_CONNECTION_SECONDARIES)) { - if (!_set_fcn_precheck_connection_secondaries (client, value, &value_to_free, error)) - return FALSE; - if (value_to_free) - value = value_to_free; - } - break; - default: - break; - } - - return _set_fcn_call (property_info, - setting, - value, - error); - } - -out_fail_read_only: - nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN, _("the property can't be changed")); - return FALSE; -} - -gboolean -nmc_setting_remove_property_option (NMSetting *setting, - const char *prop, - const char *value, - GError **error) -{ - const NMMetaPropertyInfo *property_info; - - g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - g_return_val_if_fail (!error || !*error, FALSE); - g_return_val_if_fail (value, FALSE); - - if ((property_info = nm_meta_property_info_find_by_setting (setting, prop))) { + if (modifier == '-') { if (property_info->property_type->remove_fcn) { return property_info->property_type->remove_fcn (property_info, nmc_meta_environment, @@ -606,9 +556,58 @@ nmc_setting_remove_property_option (NMSetting *setting, value, error); } + return TRUE; + } + + switch (property_info->setting_info->general->meta_type) { + case NM_META_SETTING_TYPE_CONNECTION: + if (nm_streq (property_info->property_name, NM_SETTING_CONNECTION_SECONDARIES)) { + if (!_set_fcn_precheck_connection_secondaries (client, value, &value_to_free, error)) + return FALSE; + if (value_to_free) + value = value_to_free; + } + break; + default: + break; + } + + if (modifier == '\0') { + /* FIXME: reset the value. By default, "set_fcn" adds values (don't ask). */ + param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); + if (param_spec) { + nm_auto_unset_gvalue GValue gvalue = G_VALUE_INIT; + + /* get the current value, to restore it on failure below. */ + g_value_init (&gvalue_old, G_PARAM_SPEC_VALUE_TYPE (param_spec)); + g_object_get_property (G_OBJECT (setting), prop, &gvalue_old); + + g_value_init (&gvalue, G_PARAM_SPEC_VALUE_TYPE (param_spec)); + g_param_value_set_default (param_spec, &gvalue); + g_object_set_property (G_OBJECT (setting), prop, &gvalue); + } + } + + if (!property_info->property_type->set_fcn (property_info, + nmc_meta_environment, + nmc_meta_environment_arg, + setting, + value, + error)) { + if ( modifier == '\0' + && param_spec) { + /* restore the previous value. */ + g_object_set_property (G_OBJECT (setting), prop, &gvalue_old); + } + + return FALSE; } return TRUE; + +out_fail_read_only: + nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN, _("the property can't be changed")); + return FALSE; } /* @@ -706,41 +705,6 @@ nmc_setting_get_property_desc (NMSetting *setting, const char *prop) nmcli_desc ?: ""); } -/* - * Gets setting:prop property value and returns it in 'value'. - * Caller is responsible for freeing the GValue resources using g_value_unset() - */ -gboolean -nmc_property_get_gvalue (NMSetting *setting, const char *prop, GValue *value) -{ - GParamSpec *param_spec; - - param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); - if (param_spec) { - memset (value, 0, sizeof (GValue)); - g_value_init (value, G_PARAM_SPEC_VALUE_TYPE (param_spec)); - g_object_get_property (G_OBJECT (setting), prop, value); - return TRUE; - } - return FALSE; -} - -/* - * Sets setting:prop property value from 'value'. - */ -gboolean -nmc_property_set_gvalue (NMSetting *setting, const char *prop, GValue *value) -{ - GParamSpec *param_spec; - - param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (setting)), prop); - if (param_spec && G_VALUE_TYPE (value) == G_PARAM_SPEC_VALUE_TYPE (param_spec)) { - g_object_set_property (G_OBJECT (setting), prop, value); - return TRUE; - } - return FALSE; -} - /*****************************************************************************/ gboolean diff --git a/clients/cli/settings.h b/clients/cli/settings.h index 931ecdfe08..1ff936859c 100644 --- a/clients/cli/settings.h +++ b/clients/cli/settings.h @@ -45,24 +45,9 @@ char *nmc_setting_get_property_parsable (NMSetting *setting, gboolean nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop, + char modifier, const char *val, GError **error); -static inline gboolean -nmc_setting_reset_property (NMClient *client, - NMSetting *setting, - const char *prop, - GError **error) -{ - return nmc_setting_set_property (client, setting, prop, NULL, error); -} - -gboolean nmc_setting_remove_property_option (NMSetting *setting, - const char *prop, - const char *value, - GError **error); - -gboolean nmc_property_get_gvalue (NMSetting *setting, const char *prop, GValue *value); -gboolean nmc_property_set_gvalue (NMSetting *setting, const char *prop, GValue *value); gboolean setting_details (const NmcConfig *nmc_config, NMSetting *setting, const char *one_prop); From 2bc29453bdaa7e07e8b0965f68fcea663fffaddf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 13:14:33 +0100 Subject: [PATCH 24/98] cli: avoid unnecessary string clones for stripping whitespace for property values --- clients/common/nm-meta-setting-desc.c | 76 +++++++++++++-------------- shared/nm-utils/nm-shared-utils.c | 2 + 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 5293268ca7..d595e70e48 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -124,7 +124,8 @@ _parse_ip_route (int family, gint64 metric = -1; guint i; gs_free const char **routev = NULL; - gs_free char *str_clean = NULL; + gs_free char *str_clean_free = NULL; + const char *str_clean; gs_free char *dest_clone = NULL; const char *dest; const char *plen; @@ -136,7 +137,7 @@ _parse_ip_route (int family, nm_assert (str); nm_assert (!error || !*error); - str_clean = g_strstrip (g_strdup (str)); + str_clean = nm_strstrip_avoid_copy (str, &str_clean_free); routev = nm_utils_strsplit_set (str_clean, " \t", FALSE); if (!routev) { g_set_error (error, 1, 0, @@ -299,7 +300,8 @@ _parse_team_link_watcher (const char *str, GError **error) { gs_free const char **watcherv = NULL; - gs_free char *str_clean = NULL; + gs_free char *str_clean_free = NULL; + const char *str_clean; guint i; gs_free const char *name = NULL; int val1 = 0, val2 = 0, val3 = 3, val4 = -1; @@ -310,7 +312,7 @@ _parse_team_link_watcher (const char *str, nm_assert (str); nm_assert (!error || !*error); - str_clean = g_strstrip (g_strdup (str)); + str_clean = nm_strstrip_avoid_copy (str, &str_clean_free); watcherv = nm_utils_strsplit_set (str_clean, " \t", FALSE); if (!watcherv) { g_set_error (error, 1, 0, "'%s' is not valid", str); @@ -1064,14 +1066,10 @@ _set_fcn_gobject_int (ARGS_SET_FCN) if (property_info->property_typ_data) { if ( value && (value_infos = property_info->property_typ_data->subtype.gobject_int.value_infos)) { - gs_free char *vv_stripped = NULL; - const char *vv = nm_str_skip_leading_spaces (value); - - if (vv[0] && g_ascii_isspace (vv[strlen (vv) - 1])) { - vv_stripped = g_strstrip (g_strdup (vv)); - vv = vv_stripped; - } + gs_free char *vv_free = NULL; + const char *vv; + vv = nm_strstrip_avoid_copy (value, &vv_free); for (; value_infos->nick; value_infos++) { if (nm_streq (value_infos->nick, vv)) { v = value_infos->value; @@ -1636,9 +1634,12 @@ vpn_data_item (const char *key, const char *value, gpointer user_data) \ strv = nm_utils_strsplit_set (value, ",", FALSE); \ for (iter = strv; iter && *iter; iter++) { \ - gs_free char *left_clone = g_strstrip (g_strdup (*iter)); \ - char *left = left_clone; \ - char *right = strchr (left, '='); \ + char *left; \ + char *right; \ + \ + left = g_strstrip ((char *) *iter); \ + right = strchr (left, '='); \ + \ if (!right) { \ g_set_error (error, 1, 0, _("'%s' is not valid; use