From 6ff613c21f13ed8416c2cbb269f1be4cc41f0b63 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 14:56:55 +0200 Subject: [PATCH 01/22] keyfile/tests: add test reading VPN profile --- libnm-core/tests/test-keyfile.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index c7d638244d..543ca629ab 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -719,6 +719,33 @@ test_user_1 (void) /*****************************************************************************/ +static void +test_vpn_1 (void) +{ + gs_unref_keyfile GKeyFile *keyfile = NULL; + gs_unref_object NMConnection *con = NULL; + NMSettingVpn *s_vpn; + + con = nmtst_create_connection_from_keyfile ( + "[connection]\n" + "id=t\n" + "type=vpn\n" + "\n" + "[vpn]\n" + "service-type=a.b.c\n" + "vpn-key-1=value1\n" + "", + "/test_vpn_1/invalid", NULL); + g_assert (con); + s_vpn = NM_SETTING_VPN (nm_connection_get_setting (con, NM_TYPE_SETTING_VPN)); + g_assert (s_vpn); + g_assert_cmpstr (nm_setting_vpn_get_data_item (s_vpn, "vpn-key-1"), ==, "value1"); + + CLEAR (&con, &keyfile); +} + +/*****************************************************************************/ + NMTST_DEFINE (); int main (int argc, char **argv) @@ -731,6 +758,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/keyfile/test_team_conf_read/valid", test_team_conf_read_valid); g_test_add_func ("/core/keyfile/test_team_conf_read/invalid", test_team_conf_read_invalid); g_test_add_func ("/core/keyfile/test_user/1", test_user_1); + g_test_add_func ("/core/keyfile/test_vpn/1", test_vpn_1); return g_test_run (); } From 1636e6411bef902710bbaaa2d8c1aa2605869c0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 12:56:59 +0200 Subject: [PATCH 02/22] keyfile: fix freeing connection in error path of nm_keyfile_read() Fixes: 04df4edf48e55478d0f360ea566f5f398aa76268 --- libnm-core/nm-keyfile-reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 1e75cbf922..b6cd8d8cd0 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1940,6 +1940,6 @@ nm_keyfile_read (GKeyFile *keyfile, return connection; out_error: g_propagate_error (error, info.error); - g_free (connection); + g_object_unref (connection); return NULL; } From 8f967d02814cb3c43d191c0e3b6561e7f21c3a7f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 14:07:33 +0200 Subject: [PATCH 03/22] keyfile: minor cleanup parsing IP addresses/routes --- libnm-core/nm-keyfile-reader.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index b6cd8d8cd0..acb9a7b9d6 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -432,8 +432,8 @@ read_one_ip_address_or_route (KeyfileReaderInfo *info, address_str, plen, property_name); if (!result) return NULL; - if (out_gateway && gateway_str) - *out_gateway = g_strdup (gateway_str); + if (gateway_str) + NM_SET_OUT (out_gateway, g_strdup (gateway_str)); } #undef VALUE_ORIG @@ -475,16 +475,13 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c static const char *key_names_routes[] = { "route", "routes", NULL }; static const char *key_names_addresses[] = { "address", "addresses", NULL }; const char **key_names = routes ? key_names_routes : key_names_addresses; - char *gateway = NULL; - GPtrArray *list; - GDestroyNotify free_func; + gs_free char *gateway = NULL; + gs_unref_ptrarray GPtrArray *list = NULL; int i; - if (routes) - free_func = (GDestroyNotify) nm_ip_route_unref; - else - free_func = (GDestroyNotify) nm_ip_address_unref; - list = g_ptr_array_new_with_free_func (free_func); + list = g_ptr_array_new_with_free_func (routes + ? (GDestroyNotify) nm_ip_route_unref + : (GDestroyNotify) nm_ip_address_unref); for (i = -1; i < 1000; i++) { const char **key_basename; @@ -509,26 +506,19 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c fill_route_attributes (info->keyfile, item, setting_name, options_key, ipv6 ? AF_INET6 : AF_INET); } - if (info->error) { - g_ptr_array_unref (list); - g_free (gateway); + if (info->error) return; - } + if (item) g_ptr_array_add (list, item); - } } if (list->len >= 1) g_object_set (setting, key, list, NULL); - if (gateway) { + if (gateway) g_object_set (setting, "gateway", gateway, NULL); - g_free (gateway); - } - - g_ptr_array_unref (list); } static void From 8d93017b1659ba950a2e33e9a103ebfaaa6809b4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 14:07:33 +0200 Subject: [PATCH 04/22] keyfile/tests: extend test for parsing routes/addresses Keyfile supports both route*/address* and routes*/addresses* fields at the same time. Extend the tests, that they are read all as expected. --- .../keyfile/tests/keyfiles/Test_Wired_Connection | 8 +++++++- src/settings/plugins/keyfile/tests/test-keyfile.c | 15 ++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection index 5cb4d7267b..a4dfaa50b6 100644 --- a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection @@ -26,15 +26,21 @@ routes1=1.2.3.0/24,2.3.4.8,99 route=5.6.7.8/32 routes2=1.1.1.2/12, routes3=1.1.1.3/13,, +routes7=1.1.1.7/17,0.0.0.0 routes4=1.1.1.4/14,2.2.2.4 +address30=1.2.3.130/24 routes5=1.1.1.5/15,2.2.2.5, routes6=1.1.1.6/16,2.2.2.6,0 -routes7=1.1.1.7/17,0.0.0.0 routes8=1.1.1.8/18,0.0.0.0, routes9=1.1.1.9/19,0.0.0.0,0 +route10=1.1.1.10/21,,0 routes10=1.1.1.10/20,,0 routes11=1.1.1.11/21,,21 routes11_options=cwnd=10,lock-cwnd=true,mtu=1430,src=7.7.7.7 +address30=1.2.3.30/24 +addresses30=1.2.3.30/25 +addresses31=1.2.3.31/25 +address31=1.2.3.31/24 ignore-auto-routes=false ignore-auto-dns=false diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 15140a7339..ef266be2ed 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -280,19 +280,23 @@ test_read_valid_wired_connection (void) g_assert_cmpstr (nm_setting_ip_config_get_dns (s_ip4, 1), ==, "4.2.2.2"); /* IPv4 addresses */ - g_assert_cmpint (nm_setting_ip_config_get_num_addresses (s_ip4), ==, 6); + g_assert_cmpint (nm_setting_ip_config_get_num_addresses (s_ip4), ==, 10); check_ip_address (s_ip4, 0, "2.3.4.5", 24); check_ip_address (s_ip4, 1, "192.168.0.5", 24); check_ip_address (s_ip4, 2, "1.2.3.4", 16); check_ip_address (s_ip4, 3, "3.4.5.6", 16); check_ip_address (s_ip4, 4, "4.5.6.7", 24); check_ip_address (s_ip4, 5, "5.6.7.8", 24); + check_ip_address (s_ip4, 6, "1.2.3.30", 24); + check_ip_address (s_ip4, 7, "1.2.3.30", 25); + check_ip_address (s_ip4, 8, "1.2.3.31", 24); + check_ip_address (s_ip4, 9, "1.2.3.31", 25); /* IPv4 gateway */ g_assert_cmpstr (nm_setting_ip_config_get_gateway (s_ip4), ==, "2.3.4.6"); /* IPv4 routes */ - g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 12); + g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 13); check_ip_route (s_ip4, 0, "5.6.7.8", 32, NULL, -1); check_ip_route (s_ip4, 1, "1.2.3.0", 24, "2.3.4.8", 99); check_ip_route (s_ip4, 2, "1.1.1.2", 12, NULL, -1); @@ -303,11 +307,12 @@ test_read_valid_wired_connection (void) check_ip_route (s_ip4, 7, "1.1.1.7", 17, NULL, -1); check_ip_route (s_ip4, 8, "1.1.1.8", 18, NULL, -1); check_ip_route (s_ip4, 9, "1.1.1.9", 19, NULL, 0); - check_ip_route (s_ip4, 10, "1.1.1.10", 20, NULL, 0); - check_ip_route (s_ip4, 11, "1.1.1.11", 21, NULL, 21); + check_ip_route (s_ip4, 10, "1.1.1.10", 21, NULL, 0); + check_ip_route (s_ip4, 11, "1.1.1.10", 20, NULL, 0); + check_ip_route (s_ip4, 12, "1.1.1.11", 21, NULL, 21); /* Route attributes */ - route = nm_setting_ip_config_get_route (s_ip4, 11); + route = nm_setting_ip_config_get_route (s_ip4, 12); g_assert (route); nmtst_assert_route_attribute_uint32 (route, NM_IP_ROUTE_ATTRIBUTE_CWND, 10); From 584a06e4e8e001c23c6590553bc01e313699ac39 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 14:54:23 +0200 Subject: [PATCH 05/22] keyfile: optimize parsing of addresses/routes in keyfile reader With this, parsing the properties address/route (for both IPv4/IPv6) has a runtime complexity of O(n*ln(n)). Previously, parsing these properties was O(1), but the constant factor was very high because for each address/route x ipv4/ipv6 combination we would search about 2*1001 times whether there is a matching value. Now the runtime complexity is O(n*ln(n)) for each of these 4 properties where n is the number of entries in the keyfile. Also note, that we only have 4 properties for which the parsing has this complexity. Hence, parsing the entire keyfile is still O(n) + 4*O(n*ln(n)) which reduces to O(n*ln(n)). So, parsing the entire keyfile is still benign and the logarithmic factor comes merely from sorting (which is fast). Now, the number of supported addresses/routes is no longer limited to 1000 (as before). Now we would accept all keys up from 0 up to G_MAXINT32. Like before, indexes will be automatically adjusted and gaps in the numbering are accepted. That is convenient, if the user edits the keyfile manually and deletes some lines. And we anyway must not change behavior. $ multitime -n 200 -s 0 -q ./src/settings/plugins/keyfile/tests/test-keyfile # build with -O2 --without-more-asserts # before: Mean Std.Dev. Min Median Max real 0.290+/-0.0000 0.013 0.275 0.289 0.418 user 0.284+/-0.0000 0.010 0.267 0.284 0.331 # after: Mean Std.Dev. Min Median Max real 0.101+/-0.0000 0.002 0.099 0.100 0.118 user 0.096+/-0.0000 0.003 0.091 0.096 0.113 sys 0.004+/-0.0000 0.002 0.001 0.004 0.009 --- libnm-core/nm-keyfile-reader.c | 203 +++++++++++++++++++++++++++------ libnm-core/nm-keyfile-utils.c | 2 +- 2 files changed, 170 insertions(+), 35 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index acb9a7b9d6..9274061113 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -466,56 +466,191 @@ fill_route_attributes (GKeyFile *kf, NMIPRoute *route, const char *setting, cons } } +typedef struct { + const char *s_key; + gint32 key_idx; + gint8 key_type; +} IPAddrRouteBuildListData; + +static int +_ip_addrroute_build_lst_data_cmp (gconstpointer p_a, gconstpointer p_b, gpointer user_data) +{ + const IPAddrRouteBuildListData *a = p_a; + const IPAddrRouteBuildListData *b = p_b; + + NM_CMP_FIELD (a, b, key_idx); + NM_CMP_FIELD (a, b, key_type); + NM_CMP_FIELD_STR (a, b, s_key); + return 0; +} + +static gboolean +ip_addrroute_match_key_w_name_ (const char *key, + const char *base_name, + gsize base_name_l, + gint32 *out_key_idx) +{ + gint64 v; + + /* some very strict parsing. */ + + /* the key must start with base_name. */ + if (strncmp (key, base_name, base_name_l) != 0) + return FALSE; + + key += base_name_l; + if (key[0] == '\0') { + /* if key is identical to base_name, that's good. */ + NM_SET_OUT (out_key_idx, -1); + return TRUE; + } + + /* if base_name is followed by a zero, then it must be + * only a zero, nothing else. */ + if (key[0] == '0') { + if (key[1] != '\0') + return FALSE; + NM_SET_OUT (out_key_idx, 0); + return TRUE; + } + + /* otherwise, it can only be followed by a non-zero decimal. */ + if (!(key[0] >= '1' && key[0] <= '9')) + return FALSE; + /* and all remaining chars must be decimals too. */ + if (!NM_STRCHAR_ALL (&key[1], ch, g_ascii_isdigit (ch))) + return FALSE; + + /* and it must be convertible to a (positive) int. */ + v = _nm_utils_ascii_str_to_int64 (key, 10, 0, G_MAXINT32, -1); + if (v < 0) + return FALSE; + + /* good */ + NM_SET_OUT (out_key_idx, v); + return TRUE; +} + +static gboolean +ip_addrroute_match_key (const char *key, + gboolean is_routes, + gint32 *out_key_idx, + gint8 *out_key_type) +{ +#define ip_addrroute_match_key_w_name(key, base_name, out_key_idx) \ + ip_addrroute_match_key_w_name_ (key, base_name, NM_STRLEN (base_name), out_key_idx) + + if (is_routes) { + if (ip_addrroute_match_key_w_name (key, "route", out_key_idx)) + NM_SET_OUT (out_key_type, 0); + else if (ip_addrroute_match_key_w_name (key, "routes", out_key_idx)) + NM_SET_OUT (out_key_type, 1); + else + return FALSE; + } else { + if (ip_addrroute_match_key_w_name (key, "address", out_key_idx)) + NM_SET_OUT (out_key_type, 0); + else if (ip_addrroute_match_key_w_name (key, "addresses", out_key_idx)) + NM_SET_OUT (out_key_type, 1); + else + return FALSE; + } + return TRUE; +} + static void -ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) +ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *setting_key) { const char *setting_name = nm_setting_get_name (setting); - gboolean ipv6 = !strcmp (setting_name, "ipv6"); - gboolean routes = !strcmp (key, "routes"); - static const char *key_names_routes[] = { "route", "routes", NULL }; - static const char *key_names_addresses[] = { "address", "addresses", NULL }; - const char **key_names = routes ? key_names_routes : key_names_addresses; + gboolean is_ipv6 = nm_streq (setting_name, "ipv6"); + gboolean is_routes = nm_streq (setting_key, "routes"); gs_free char *gateway = NULL; gs_unref_ptrarray GPtrArray *list = NULL; - int i; + gs_strfreev char **keys = NULL; + gsize i_keys, keys_len; + gs_free IPAddrRouteBuildListData *build_list = NULL; + gsize i_build_list, build_list_len = 0; - list = g_ptr_array_new_with_free_func (routes + + keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &keys_len, NULL); + + if (keys_len == 0) + return; + + /* first create a list of all relevant keys, and sort them. */ + for (i_keys = 0; i_keys < keys_len; i_keys++) { + const char *s_key = keys[i_keys]; + gint32 key_idx; + gint8 key_type; + + if (!ip_addrroute_match_key (s_key, is_routes, &key_idx, &key_type)) + continue; + + if (G_UNLIKELY (!build_list)) + build_list = g_new (IPAddrRouteBuildListData, keys_len - i_keys); + + build_list[build_list_len].s_key = s_key; + build_list[build_list_len].key_idx = key_idx; + build_list[build_list_len].key_type = key_type; + build_list_len++; + } + + if (build_list_len == 0) + return; + + g_qsort_with_data (build_list, + build_list_len, + sizeof (IPAddrRouteBuildListData), + _ip_addrroute_build_lst_data_cmp, + NULL); + + list = g_ptr_array_new_with_free_func (is_routes ? (GDestroyNotify) nm_ip_route_unref : (GDestroyNotify) nm_ip_address_unref); - for (i = -1; i < 1000; i++) { - const char **key_basename; + for (i_build_list = 0; i_build_list < build_list_len; i_build_list++) { + const IPAddrRouteBuildListData *build_data = &build_list[i_build_list]; + gpointer item; - for (key_basename = key_names; *key_basename; key_basename++) { - char key_name_buf[100]; - const char *key_name; - gpointer item; + if ( i_build_list + 1 < build_list_len + && build_data->key_idx == build_data[1].key_idx + && build_data->key_type == build_data[1].key_type + && nm_streq (build_data->s_key, build_data[1].s_key)) { + /* the keyfile contains duplicate keys, which are both returned + * by g_key_file_get_keys() (WHY??). + * + * Skip the earlier one. */ + continue; + } + + item = read_one_ip_address_or_route (info, + setting_key, + setting_name, + build_data->s_key, + is_ipv6, + is_routes, + gateway ? NULL : &gateway, + setting); + if (item && is_routes) { char options_key[128]; - /* -1 means no suffix */ - key_name = *key_basename; - if (i >= 0) { - nm_sprintf_buf (key_name_buf, "%s%d", key_name, i); - key_name = key_name_buf; - } - - item = read_one_ip_address_or_route (info, key, setting_name, key_name, ipv6, routes, - gateway ? NULL : &gateway, setting); - if (item && routes) { - nm_sprintf_buf (options_key, "%s_options", key_name); - fill_route_attributes (info->keyfile, item, setting_name, options_key, ipv6 ? AF_INET6 : AF_INET); - } - - if (info->error) - return; - - if (item) - g_ptr_array_add (list, item); + nm_sprintf_buf (options_key, "%s_options", build_data->s_key); + fill_route_attributes (info->keyfile, + item, + setting_name, + options_key, + is_ipv6 ? AF_INET6 : AF_INET); } + + if (info->error) + return; + + if (item) + g_ptr_array_add (list, item); } if (list->len >= 1) - g_object_set (setting, key, list, NULL); + g_object_set (setting, setting_key, list, NULL); if (gateway) g_object_set (setting, "gateway", gateway, NULL); diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c index 8858331949..dc24a5e9f7 100644 --- a/libnm-core/nm-keyfile-utils.c +++ b/libnm-core/nm-keyfile-utils.c @@ -195,7 +195,7 @@ nm_keyfile_plugin_kf_get_keys (GKeyFile *kf, alias = nm_keyfile_plugin_get_alias_for_setting_name (group); if (alias) { g_clear_error (&local); - keys = g_key_file_get_keys (kf, alias, out_length, &local); + keys = g_key_file_get_keys (kf, alias, out_length, error ? &local : NULL); } } if (local) From 3b8e9a3ea676a94821a9fe63ad2e2755fd1f8920 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 16:23:13 +0200 Subject: [PATCH 06/22] keyfile: fix memleak parsing dns values --- libnm-core/nm-keyfile-reader.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 9274061113..2e87b4184d 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -662,7 +662,8 @@ ip4_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) const char *setting_name = nm_setting_get_name (setting); GPtrArray *array; gsize length; - char **list, **iter; + gs_strfreev char **list = NULL; + char **iter; int ret; list = nm_keyfile_plugin_kf_get_string_list (info->keyfile, setting_name, key, &length, NULL); @@ -679,7 +680,6 @@ ip4_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) _("ignoring invalid DNS server IPv4 address '%s'"), *iter)) { g_ptr_array_unref (array); - g_strfreev (list); return; } continue; @@ -691,7 +691,6 @@ ip4_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) g_object_set (setting, key, array->pdata, NULL); g_ptr_array_unref (array); - g_strfreev (list); } static void @@ -700,7 +699,8 @@ ip6_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) const char *setting_name = nm_setting_get_name (setting); GPtrArray *array = NULL; gsize length; - char **list, **iter; + gs_strfreev char **list = NULL; + char **iter; int ret; list = nm_keyfile_plugin_kf_get_string_list (info->keyfile, setting_name, key, &length, NULL); @@ -718,7 +718,6 @@ ip6_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) _("ignoring invalid DNS server IPv6 address '%s'"), *iter)) { g_ptr_array_unref (array); - g_strfreev (list); return; } continue; @@ -730,7 +729,6 @@ ip6_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) g_object_set (setting, key, array->pdata, NULL); g_ptr_array_unref (array); - g_strfreev (list); } static void From 23b0655ceba1f343c533435e5ffc5a92f05f54bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 16:43:43 +0200 Subject: [PATCH 07/22] keyfile: merge IPv4 and IPv6 version of DNS parser --- libnm-core/nm-keyfile-reader.c | 51 ++++++---------------------------- 1 file changed, 8 insertions(+), 43 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 2e87b4184d..19a6f33e5b 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -657,46 +657,10 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c } static void -ip4_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) -{ - const char *setting_name = nm_setting_get_name (setting); - GPtrArray *array; - gsize length; - gs_strfreev char **list = NULL; - char **iter; - int ret; - - list = nm_keyfile_plugin_kf_get_string_list (info->keyfile, setting_name, key, &length, NULL); - if (!list || !g_strv_length (list)) - return; - - array = g_ptr_array_sized_new (length + 1); - for (iter = list; *iter; iter++) { - guint32 addr; - - ret = inet_pton (AF_INET, *iter, &addr); - if (ret <= 0) { - if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("ignoring invalid DNS server IPv4 address '%s'"), - *iter)) { - g_ptr_array_unref (array); - return; - } - continue; - } - - g_ptr_array_add (array, *iter); - } - g_ptr_array_add (array, NULL); - - g_object_set (setting, key, array->pdata, NULL); - g_ptr_array_unref (array); -} - -static void -ip6_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) +ip_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) { const char *setting_name = nm_setting_get_name (setting); + int addr_family = nm_streq (setting_name, NM_SETTING_IP6_CONFIG_SETTING_NAME) ? AF_INET6 : AF_INET; GPtrArray *array = NULL; gsize length; gs_strfreev char **list = NULL; @@ -710,12 +674,13 @@ ip6_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) array = g_ptr_array_sized_new (length + 1); for (iter = list; *iter; iter++) { - struct in6_addr addr; + NMIPAddr addr; - ret = inet_pton (AF_INET6, *iter, &addr); + ret = inet_pton (addr_family, *iter, &addr); if (ret <= 0) { if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("ignoring invalid DNS server IPv6 address '%s'"), + _("ignoring invalid DNS server IPv%c address '%s'"), + nm_utils_addr_family_to_char (addr_family), *iter)) { g_ptr_array_unref (array); return; @@ -1550,11 +1515,11 @@ static KeyParser key_parsers[] = { { NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP_CONFIG_DNS, FALSE, - ip4_dns_parser }, + ip_dns_parser }, { NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_SETTING_IP_CONFIG_DNS, FALSE, - ip6_dns_parser }, + ip_dns_parser }, { NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, FALSE, From c858f9d35144a3f39a72b70c7bd9fb8224a0835b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 16:36:41 +0200 Subject: [PATCH 08/22] keyfile: avoid cloning the array while parsing DNS entries --- libnm-core/nm-keyfile-reader.c | 42 +++++++++++-------- .../tests/keyfiles/Test_Wired_Connection | 2 +- .../plugins/keyfile/tests/test-keyfile.c | 1 + 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 19a6f33e5b..3c93b32d70 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -659,41 +659,47 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c static void ip_dns_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) { - const char *setting_name = nm_setting_get_name (setting); - int addr_family = nm_streq (setting_name, NM_SETTING_IP6_CONFIG_SETTING_NAME) ? AF_INET6 : AF_INET; - GPtrArray *array = NULL; - gsize length; + int addr_family; gs_strfreev char **list = NULL; - char **iter; - int ret; + gsize i, n, length; - list = nm_keyfile_plugin_kf_get_string_list (info->keyfile, setting_name, key, &length, NULL); - if (!list || !g_strv_length (list)) + nm_assert (NM_IS_SETTING_IP4_CONFIG (setting) || NM_IS_SETTING_IP6_CONFIG (setting)); + + list = nm_keyfile_plugin_kf_get_string_list (info->keyfile, + nm_setting_get_name (setting), + key, + &length, + NULL); + nm_assert (length == NM_PTRARRAY_LEN (list)); + if (length == 0) return; - array = g_ptr_array_sized_new (length + 1); + addr_family = NM_IS_SETTING_IP4_CONFIG (setting) ? AF_INET : AF_INET6; - for (iter = list; *iter; iter++) { + n = 0; + for (i = 0; i < length; i++) { NMIPAddr addr; - ret = inet_pton (addr_family, *iter, &addr); - if (ret <= 0) { + if (inet_pton (addr_family, list[i], &addr) <= 0) { if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("ignoring invalid DNS server IPv%c address '%s'"), nm_utils_addr_family_to_char (addr_family), - *iter)) { - g_ptr_array_unref (array); + list[i])) { + do { + nm_clear_g_free (&list[i]); + } while (++i < length); return; } + nm_clear_g_free (&list[i]); continue; } - g_ptr_array_add (array, *iter); + if (n != i) + list[n] = g_steal_pointer (&list[i]); + n++; } - g_ptr_array_add (array, NULL); - g_object_set (setting, key, array->pdata, NULL); - g_ptr_array_unref (array); + g_object_set (setting, key, list, NULL); } static void diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection index a4dfaa50b6..1e62f4b3ef 100644 --- a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection @@ -15,7 +15,7 @@ mtu=1400 [ipv4] method=manual -dns=4.2.2.1;4.2.2.2; +dns=4.2.2.1;bogus;4.2.2.2; addresses1=192.168.0.5;24;192.168.0.1; addresses2=1.2.3.4;16;1.2.1.1; address=2.3.4.5/24,2.3.4.6 diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index ef266be2ed..b46475e8a8 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -235,6 +235,7 @@ test_read_valid_wired_connection (void) NMTST_EXPECT_NM_INFO ("*ipv4.addresses:*semicolon at the end*addresses2*"); NMTST_EXPECT_NM_WARN ("*missing prefix length*address4*"); NMTST_EXPECT_NM_WARN ("*missing prefix length*address5*"); + NMTST_EXPECT_NM_WARN ("*ipv4.dns: ignoring invalid DNS server IPv4 address 'bogus'*"); NMTST_EXPECT_NM_INFO ("*ipv4.routes*semicolon at the end*routes2*"); NMTST_EXPECT_NM_INFO ("*ipv4.routes*semicolon at the end*routes3*"); NMTST_EXPECT_NM_INFO ("*ipv4.routes*semicolon at the end*routes5*"); From 1ed8bdd3b1d498606287a1d3782d08d299420c9a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 16:52:58 +0200 Subject: [PATCH 09/22] keyfile/trivial: fix indention --- libnm-core/nm-keyfile-reader.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 3c93b32d70..0769b200d1 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1606,14 +1606,14 @@ static KeyParser key_parsers[] = { NM_SETTING_TEAM_CONFIG, TRUE, team_config_parser }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_QDISCS, + { NM_SETTING_TC_CONFIG_SETTING_NAME, + NM_SETTING_TC_CONFIG_QDISCS, FALSE, - qdisc_parser }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_TFILTERS, + qdisc_parser }, + { NM_SETTING_TC_CONFIG_SETTING_NAME, + NM_SETTING_TC_CONFIG_TFILTERS, FALSE, - tfilter_parser }, + tfilter_parser }, { NULL, NULL, FALSE } }; From 22578e5fd332e7007f4c9cddf3526be906b00c98 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 11:33:19 +0200 Subject: [PATCH 10/22] keyfile: drop unused handling of non-existing "address-lables" The key_writers array is searched by matching the @key during write_setting_value(). Note how write_setting_value() is called by nm_connection_for_each_setting_value(), thus, @key is the name of a GObject property for NMSettingIP4Config. But NMSettingIP4Config has no property names "address-labels". Hence, this was unused since introducing libnm-core (which never had this internal property). --- libnm-core/nm-keyfile-writer.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c index f92faef5bb..4c51578c17 100644 --- a/libnm-core/nm-keyfile-writer.c +++ b/libnm-core/nm-keyfile-writer.c @@ -221,15 +221,6 @@ addr_writer (KeyfileWriterInfo *info, write_ip_values (info->keyfile, setting_name, array, gateway, FALSE); } -static void -ip4_addr_label_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - /* skip */ -} - static void gateway_writer (KeyfileWriterInfo *info, NMSetting *setting, @@ -596,9 +587,6 @@ static KeyWriter key_writers[] = { { NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP_CONFIG_ADDRESSES, addr_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - "address-labels", - ip4_addr_label_writer }, { NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_SETTING_IP_CONFIG_ADDRESSES, addr_writer }, From f99dc6b936eeb352c0a79a3d7208753eba8e7695 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 22:32:59 +0200 Subject: [PATCH 11/22] libnm/keyfile: merge keyfile sources (pt1, rename nm-keyfile-reader.c) I am going to merge the files for keyfile handling in libnm-core. There is a reason for that, I'll tell you next. --- Makefile.am | 2 +- libnm-core/meson.build | 2 +- libnm-core/{nm-keyfile-reader.c => nm-keyfile.c} | 0 po/POTFILES.in | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename libnm-core/{nm-keyfile-reader.c => nm-keyfile.c} (100%) diff --git a/Makefile.am b/Makefile.am index b488c37e80..697fa881a9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -502,7 +502,7 @@ libnm_core_lib_c_real = \ libnm-core/nm-connection.c \ libnm-core/nm-dbus-utils.c \ libnm-core/nm-errors.c \ - libnm-core/nm-keyfile-reader.c \ + libnm-core/nm-keyfile.c \ libnm-core/nm-keyfile-utils.c \ libnm-core/nm-keyfile-writer.c \ libnm-core/nm-property-compare.c \ diff --git a/libnm-core/meson.build b/libnm-core/meson.build index a011e38608..db00433cb4 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -103,7 +103,7 @@ libnm_core_sources = libnm_core_settings_sources + files( 'nm-connection.c', 'nm-dbus-utils.c', 'nm-errors.c', - 'nm-keyfile-reader.c', + 'nm-keyfile.c', 'nm-keyfile-utils.c', 'nm-keyfile-writer.c', 'nm-property-compare.c', diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile.c similarity index 100% rename from libnm-core/nm-keyfile-reader.c rename to libnm-core/nm-keyfile.c diff --git a/po/POTFILES.in b/po/POTFILES.in index d41376b5c7..6451228191 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -55,7 +55,7 @@ libnm-core/crypto_gnutls.c libnm-core/crypto_nss.c libnm-core/nm-connection.c libnm-core/nm-dbus-utils.c -libnm-core/nm-keyfile-reader.c +libnm-core/nm-keyfile.c libnm-core/nm-keyfile-writer.c libnm-core/nm-setting-8021x.c libnm-core/nm-setting-adsl.c From 21f6058cfee6e1eee94b770bf78b690054b2928a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Apr 2018 22:32:59 +0200 Subject: [PATCH 12/22] libnm/keyfile: merge keyfile sources (pt2, merge nm-keyfile-writer.c) Splitting keyfile handling in two "reader.c" and "writer.c" files is not helpful. What is most interesting, is to see how property XYZ is serialized to keyfile, and to verify that the parser does the inverse. For that, it's easier if both the write_xzy() and parse_xyz() function are beside each other, and not split accross files. The more important reason is, that both reader and writer have their separate handler arrays, for special handling of certain properties: @key_parsers and @key_writers. These two should not be separate but will be merged. Since they reference static functions, these functions must all be in the same source file (unless, we put them into headers, which would be unnecessary complex). No code was changed, only moved. --- Makefile.am | 1 - libnm-core/meson.build | 1 - libnm-core/nm-keyfile-writer.c | 866 --------------------------------- libnm-core/nm-keyfile.c | 839 ++++++++++++++++++++++++++++++++ po/POTFILES.in | 1 - 5 files changed, 839 insertions(+), 869 deletions(-) delete mode 100644 libnm-core/nm-keyfile-writer.c diff --git a/Makefile.am b/Makefile.am index 697fa881a9..eb4afde8ca 100644 --- a/Makefile.am +++ b/Makefile.am @@ -504,7 +504,6 @@ libnm_core_lib_c_real = \ libnm-core/nm-errors.c \ libnm-core/nm-keyfile.c \ libnm-core/nm-keyfile-utils.c \ - libnm-core/nm-keyfile-writer.c \ libnm-core/nm-property-compare.c \ libnm-core/nm-setting.c \ libnm-core/nm-simple-connection.c \ diff --git a/libnm-core/meson.build b/libnm-core/meson.build index db00433cb4..bba844069c 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -105,7 +105,6 @@ libnm_core_sources = libnm_core_settings_sources + files( 'nm-errors.c', 'nm-keyfile.c', 'nm-keyfile-utils.c', - 'nm-keyfile-writer.c', 'nm-property-compare.c', 'nm-setting.c', 'nm-simple-connection.c', diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c deleted file mode 100644 index 4c51578c17..0000000000 --- a/libnm-core/nm-keyfile-writer.c +++ /dev/null @@ -1,866 +0,0 @@ -/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ -/* NetworkManager system settings service - keyfile plugin - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program 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 General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Copyright (C) 2008 Novell, Inc. - * Copyright (C) 2008 - 2017 Red Hat, Inc. - */ - -#include "nm-default.h" - -#include "nm-keyfile-internal.h" - -#include -#include -#include -#include -#include -#include -#include - -#include "nm-core-internal.h" -#include "nm-keyfile-utils.h" - -typedef struct { - NMConnection *connection; - GKeyFile *keyfile; - GError *error; - NMKeyfileWriteHandler handler; - void *user_data; -} KeyfileWriterInfo; - - -/* Some setting properties also contain setting names, such as - * NMSettingConnection's 'type' property (which specifies the base type of the - * connection, eg ethernet or wifi) or the 802-11-wireless setting's - * 'security' property which specifies whether or not the AP requires - * encryption. This function handles translating those properties' values - * from the real setting name to the more-readable alias. - */ -static void -setting_alias_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - const char *str, *alias; - - str = g_value_get_string (value); - alias = nm_keyfile_plugin_get_alias_for_setting_name (str); - nm_keyfile_plugin_kf_set_string (info->keyfile, - nm_setting_get_name (setting), - key, - alias ? alias : str); -} - -static void -write_array_of_uint (GKeyFile *file, - NMSetting *setting, - const char *key, - const GValue *value) -{ - GArray *array; - guint i; - gs_free int *tmp_array = NULL; - - array = (GArray *) g_value_get_boxed (value); - if (!array || !array->len) - return; - - g_return_if_fail (g_array_get_element_size (array) == sizeof (guint)); - - tmp_array = g_new (gint, array->len); - for (i = 0; i < array->len; i++) { - guint v = g_array_index (array, guint, i); - - if (v > G_MAXINT) - g_return_if_reached (); - tmp_array[i] = (int) v; - } - - nm_keyfile_plugin_kf_set_integer_list (file, nm_setting_get_name (setting), key, tmp_array, array->len); -} - -static void -dns_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - char **list; - - list = g_value_get_boxed (value); - if (list && list[0]) { - nm_keyfile_plugin_kf_set_string_list (info->keyfile, nm_setting_get_name (setting), key, - (const char **) list, g_strv_length (list)); - } -} - -static void -ip6_addr_gen_mode_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - NMSettingIP6ConfigAddrGenMode addr_gen_mode; - gs_free char *str = NULL; - - addr_gen_mode = (NMSettingIP6ConfigAddrGenMode) g_value_get_int (value); - str = nm_utils_enum_to_str (nm_setting_ip6_config_addr_gen_mode_get_type (), - addr_gen_mode); - nm_keyfile_plugin_kf_set_string (info->keyfile, - nm_setting_get_name (setting), - key, - str); -} - -static void -write_ip_values (GKeyFile *file, - const char *setting_name, - GPtrArray *array, - const char *gateway, - gboolean is_route) -{ - GString *output; - int family, i; - const char *addr, *gw; - guint32 plen; - char key_name[64], *key_name_idx; - - if (!array->len) - return; - - family = !strcmp (setting_name, NM_SETTING_IP4_CONFIG_SETTING_NAME) ? AF_INET : AF_INET6; - - strcpy (key_name, is_route ? "route" : "address"); - key_name_idx = key_name + strlen (key_name); - - output = g_string_sized_new (2*INET_ADDRSTRLEN + 10); - for (i = 0; i < array->len; i++) { - gint64 metric = -1; - - if (is_route) { - NMIPRoute *route = array->pdata[i]; - - addr = nm_ip_route_get_dest (route); - plen = nm_ip_route_get_prefix (route); - gw = nm_ip_route_get_next_hop (route); - metric = nm_ip_route_get_metric (route); - } else { - NMIPAddress *address = array->pdata[i]; - - addr = nm_ip_address_get_address (address); - plen = nm_ip_address_get_prefix (address); - gw = i == 0 ? gateway : NULL; - } - - g_string_set_size (output, 0); - g_string_append_printf (output, "%s/%u", addr, plen); - if ( metric != -1 - || gw) { - /* Older versions of the plugin do not support the form - * "a.b.c.d/plen,,metric", so, we always have to write the - * gateway, even if there isn't one. - * The current version supports reading of the above form. - */ - if (!gw) { - if (family == AF_INET) - gw = "0.0.0.0"; - else - gw = "::"; - } - - g_string_append_printf (output, ",%s", gw); - if (is_route && metric != -1) - g_string_append_printf (output, ",%lu", (unsigned long) metric); - } - - sprintf (key_name_idx, "%d", i + 1); - nm_keyfile_plugin_kf_set_string (file, setting_name, key_name, output->str); - - if (is_route) { - gs_free char *attributes = NULL; - GHashTable *hash; - - hash = _nm_ip_route_get_attributes_direct (array->pdata[i]); - attributes = nm_utils_format_variant_attributes (hash, ',', '='); - if (attributes) { - g_strlcat (key_name, "_options", sizeof (key_name)); - nm_keyfile_plugin_kf_set_string (file, setting_name, key_name, attributes); - } - } - } - g_string_free (output, TRUE); -} - -static void -addr_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - GPtrArray *array; - const char *setting_name = nm_setting_get_name (setting); - const char *gateway = nm_setting_ip_config_get_gateway (NM_SETTING_IP_CONFIG (setting)); - - array = (GPtrArray *) g_value_get_boxed (value); - if (array && array->len) - write_ip_values (info->keyfile, setting_name, array, gateway, FALSE); -} - -static void -gateway_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - /* skip */ -} - -static void -route_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - GPtrArray *array; - const char *setting_name = nm_setting_get_name (setting); - - array = (GPtrArray *) g_value_get_boxed (value); - if (array && array->len) - write_ip_values (info->keyfile, setting_name, array, NULL, TRUE); -} - -static void -qdisc_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - gsize i; - GPtrArray *array; - - array = (GPtrArray *) g_value_get_boxed (value); - if (!array || !array->len) - return; - - for (i = 0; i < array->len; i++) { - NMTCQdisc *qdisc = array->pdata[i]; - GString *key_name = g_string_sized_new (16); - GString *value_str = g_string_sized_new (60); - - g_string_append (key_name, "qdisc."); - _nm_utils_string_append_tc_parent (key_name, NULL, - nm_tc_qdisc_get_parent (qdisc)); - _nm_utils_string_append_tc_qdisc_rest (value_str, qdisc); - - nm_keyfile_plugin_kf_set_string (info->keyfile, - NM_SETTING_TC_CONFIG_SETTING_NAME, - key_name->str, - value_str->str); - - g_string_free (key_name, TRUE); - g_string_free (value_str, TRUE); - } -} - -static void -tfilter_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - gsize i; - GPtrArray *array; - - array = (GPtrArray *) g_value_get_boxed (value); - if (!array || !array->len) - return; - - for (i = 0; i < array->len; i++) { - NMTCTfilter *tfilter = array->pdata[i]; - GString *key_name = g_string_sized_new (16); - GString *value_str = g_string_sized_new (60); - - g_string_append (key_name, "tfilter."); - _nm_utils_string_append_tc_parent (key_name, NULL, - nm_tc_tfilter_get_parent (tfilter)); - _nm_utils_string_append_tc_tfilter_rest (value_str, tfilter, NULL); - - nm_keyfile_plugin_kf_set_string (info->keyfile, - NM_SETTING_TC_CONFIG_SETTING_NAME, - key_name->str, - value_str->str); - - g_string_free (key_name, TRUE); - g_string_free (value_str, TRUE); - } -} - -static void -write_hash_of_string (GKeyFile *file, - NMSetting *setting, - const char *key, - const GValue *value) -{ - GHashTable *hash; - const char *group_name = nm_setting_get_name (setting); - gboolean vpn_secrets = FALSE; - gs_free const char **keys = NULL; - guint i, l; - - /* Write VPN secrets out to a different group to keep them separate */ - if (NM_IS_SETTING_VPN (setting) && !strcmp (key, NM_SETTING_VPN_SECRETS)) { - group_name = NM_KEYFILE_GROUP_VPN_SECRETS; - vpn_secrets = TRUE; - } - - hash = g_value_get_boxed (value); - - keys = nm_utils_strdict_get_keys (hash, TRUE, &l); - for (i = 0; i < l; i++) { - const char *property, *data; - gboolean write_item = TRUE; - - property = keys[i]; - - /* Handle VPN secrets specially; they are nested in the property's hash; - * we don't want to write them if the secret is not saved, not required, - * or owned by a user's secret agent. - */ - if (vpn_secrets) { - NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - - nm_setting_get_secret_flags (setting, property, &secret_flags, NULL); - if (secret_flags != NM_SETTING_SECRET_FLAG_NONE) - write_item = FALSE; - } - - if (write_item) { - gs_free char *to_free = NULL; - - data = g_hash_table_lookup (hash, property); - nm_keyfile_plugin_kf_set_string (file, group_name, - nm_keyfile_key_encode (property, &to_free), - data); - } - } -} - -static void -ssid_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - GBytes *bytes; - const guint8 *ssid_data; - gsize ssid_len; - const char *setting_name = nm_setting_get_name (setting); - gboolean new_format = TRUE; - gsize semicolons = 0; - gsize i; - - g_return_if_fail (G_VALUE_HOLDS (value, G_TYPE_BYTES)); - - bytes = g_value_get_boxed (value); - if (!bytes) - return; - ssid_data = g_bytes_get_data (bytes, &ssid_len); - if (!ssid_data || !ssid_len) { - nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, ""); - return; - } - - /* Check whether each byte is printable. If not, we have to use an - * integer list, otherwise we can just use a string. - */ - for (i = 0; i < ssid_len; i++) { - const char c = ssid_data[i]; - - if (!g_ascii_isprint (c)) { - new_format = FALSE; - break; - } - if (c == ';') - semicolons++; - } - - if (new_format) { - gs_free char *ssid = NULL; - - if (semicolons == 0) - ssid = g_strndup ((char *) ssid_data, ssid_len); - else { - /* Escape semicolons with backslashes to make strings - * containing ';', such as '16;17;' unambiguous */ - gsize j = 0; - - ssid = g_malloc (ssid_len + semicolons + 1); - for (i = 0; i < ssid_len; i++) { - if (ssid_data[i] == ';') - ssid[j++] = '\\'; - ssid[j++] = ssid_data[i]; - } - ssid[j] = '\0'; - } - nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, ssid); - } else - nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, ssid_data, ssid_len); -} - -static void -password_raw_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - const char *setting_name = nm_setting_get_name (setting); - GBytes *array; - gsize len; - const guint8 *data; - - g_return_if_fail (G_VALUE_HOLDS (value, G_TYPE_BYTES)); - - array = (GBytes *) g_value_get_boxed (value); - if (!array) - return; - data = g_bytes_get_data (array, &len); - if (!data) - len = 0; - nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, data, len); -} - -/*****************************************************************************/ - -static void -cert_writer_default (NMConnection *connection, - GKeyFile *file, - NMKeyfileWriteTypeDataCert *cert_data) -{ - const char *setting_name = nm_setting_get_name (NM_SETTING (cert_data->setting)); - NMSetting8021xCKScheme scheme; - - scheme = cert_data->vtable->scheme_func (cert_data->setting); - if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - const char *path; - char *path_free = NULL, *tmp; - gs_free char *base_dir = NULL; - - path = cert_data->vtable->path_func (cert_data->setting); - g_assert (path); - - /* If the path is relative, make it an absolute path. - * Relative paths make a keyfile not easily usable in another - * context. */ - if (path[0] && path[0] != '/') { - base_dir = g_get_current_dir (); - path = path_free = g_strconcat (base_dir, "/", path, NULL); - } else - base_dir = g_path_get_dirname (path); - - /* path cannot start with "file://" or "data:;base64,", because it is an absolute path. - * Still, make sure that a prefix-less path will be recognized. This can happen - * for example if the path is longer then 500 chars. */ - tmp = nm_keyfile_detect_unqualified_path_scheme (base_dir, path, -1, FALSE, NULL); - if (tmp) - g_clear_pointer (&tmp, g_free); - else - path = tmp = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH, path, NULL); - - /* Path contains at least a '/', hence it cannot be recognized as the old - * binary format consisting of a list of integers. */ - - nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, path); - g_free (tmp); - g_free (path_free); - } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { - GBytes *blob; - const guint8 *blob_data; - gsize blob_len; - char *blob_base64, *val; - - blob = cert_data->vtable->blob_func (cert_data->setting); - g_assert (blob); - blob_data = g_bytes_get_data (blob, &blob_len); - - blob_base64 = g_base64_encode (blob_data, blob_len); - val = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB, blob_base64, NULL); - - nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, val); - g_free (val); - g_free (blob_base64); - } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { - nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, - cert_data->vtable->uri_func (cert_data->setting)); - } else { - /* scheme_func() returns UNKNOWN in all other cases. The only valid case - * where a scheme is allowed to be UNKNOWN, is unsetting the value. In this - * case, we don't expect the writer to be called, because the default value - * will not be serialized. - * The only other reason for the scheme to be UNKNOWN is an invalid cert. - * But our connection verifies, so that cannot happen either. */ - g_return_if_reached (); - } -} - -static void -cert_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - const NMSetting8021xSchemeVtable *objtype = NULL; - guint i; - NMKeyfileWriteTypeDataCert type_data = { 0 }; - - for (i = 0; nm_setting_8021x_scheme_vtable[i].setting_key; i++) { - if (g_strcmp0 (nm_setting_8021x_scheme_vtable[i].setting_key, key) == 0) { - objtype = &nm_setting_8021x_scheme_vtable[i]; - break; - } - } - if (!objtype) - g_return_if_reached (); - - type_data.setting = NM_SETTING_802_1X (setting); - type_data.vtable = objtype; - - if (info->handler) { - if (info->handler (info->connection, - info->keyfile, - NM_KEYFILE_WRITE_TYPE_CERT, - &type_data, - info->user_data, - &info->error)) - return; - if (info->error) - return; - } - - cert_writer_default (info->connection, info->keyfile, &type_data); -} - -static void -null_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - /* skip */ -} - -/*****************************************************************************/ - -typedef struct { - const char *setting_name; - const char *key; - void (*writer) (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value); -} KeyWriter; - -/* A table of keys that require further parsing/conversion because they are - * stored in a format that can't be automatically read using the key's type. - * i.e. IPv4 addresses, which are stored in NetworkManager as guint32, but are - * stored in keyfiles as strings, eg "10.1.1.2" or IPv6 addresses stored - * in struct in6_addr internally, but as string in keyfiles. - */ -static KeyWriter key_writers[] = { - { NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_TYPE, - setting_alias_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - addr_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - addr_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_GATEWAY, - gateway_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_GATEWAY, - gateway_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - route_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - route_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - dns_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - dns_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, - ip6_addr_gen_mode_writer }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_SSID, - ssid_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PASSWORD_RAW, - password_raw_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CA_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CLIENT_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PRIVATE_KEY, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CA_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CLIENT_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, - cert_writer }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_QDISCS, - qdisc_writer }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_TFILTERS, - tfilter_writer }, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_MCAST_REJOIN_COUNT, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_HASH, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_BALANCER, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_ACTIVE, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_FAST_RATE, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_SYS_PRIO, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_MIN_PORTS, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_LINK_WATCHERS, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_QUEUE_ID, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_PRIO, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_STICKY, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LACP_PRIO, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LACP_KEY, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LINK_WATCHERS, - null_writer}, - { NULL, NULL, NULL } -}; - -static gboolean -can_omit_default_value (NMSetting *setting, const char *property) -{ - if (NM_IS_SETTING_VLAN (setting)) { - if (!strcmp (property, NM_SETTING_VLAN_FLAGS)) - return FALSE; - } else if (NM_IS_SETTING_IP6_CONFIG (setting)) { - if (!strcmp (property, NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE)) - return FALSE; - } - - return TRUE; -} - -static void -write_setting_value (NMSetting *setting, - const char *key, - const GValue *value, - GParamFlags flag, - gpointer user_data) -{ - KeyfileWriterInfo *info = user_data; - const char *setting_name; - GType type = G_VALUE_TYPE (value); - KeyWriter *writer = &key_writers[0]; - GParamSpec *pspec; - - if (info->error) - return; - - /* Setting name gets picked up from the keyfile's section name instead */ - if (!strcmp (key, NM_SETTING_NAME)) - return; - - /* Don't write the NMSettingConnection object's 'read-only' property */ - if ( NM_IS_SETTING_CONNECTION (setting) - && !strcmp (key, NM_SETTING_CONNECTION_READ_ONLY)) - return; - - setting_name = nm_setting_get_name (setting); - - /* If the value is the default value, remove the item from the keyfile */ - pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); - if (pspec) { - if ( can_omit_default_value (setting, key) - && g_param_value_defaults (pspec, (GValue *) value)) { - g_key_file_remove_key (info->keyfile, setting_name, key, NULL); - return; - } - } - - /* Don't write secrets that are owned by user secret agents or aren't - * supposed to be saved. VPN secrets are handled specially though since - * the secret flags there are in a third-level hash in the 'secrets' - * property. - */ - if (pspec && (pspec->flags & NM_SETTING_PARAM_SECRET) && !NM_IS_SETTING_VPN (setting)) { - NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - - if (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL)) - g_assert_not_reached (); - if (secret_flags != NM_SETTING_SECRET_FLAG_NONE) - return; - } - - /* Look through the list of handlers for non-standard format key values */ - while (writer->setting_name) { - if (!strcmp (writer->setting_name, setting_name) && !strcmp (writer->key, key)) { - (*writer->writer) (info, setting, key, value); - return; - } - writer++; - } - - if (type == G_TYPE_STRING) { - const char *str; - - str = g_value_get_string (value); - if (str) - nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, str); - } else if (type == G_TYPE_UINT) - nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (int) g_value_get_uint (value)); - else if (type == G_TYPE_INT) - nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, g_value_get_int (value)); - else if (type == G_TYPE_UINT64) { - char *numstr; - - numstr = g_strdup_printf ("%" G_GUINT64_FORMAT, g_value_get_uint64 (value)); - nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); - g_free (numstr); - } else if (type == G_TYPE_INT64) { - char *numstr; - - numstr = g_strdup_printf ("%" G_GINT64_FORMAT, g_value_get_int64 (value)); - nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); - g_free (numstr); - } else if (type == G_TYPE_BOOLEAN) { - nm_keyfile_plugin_kf_set_boolean (info->keyfile, setting_name, key, g_value_get_boolean (value)); - } else if (type == G_TYPE_CHAR) { - nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (int) g_value_get_schar (value)); - } else if (type == G_TYPE_BYTES) { - GBytes *bytes; - const guint8 *data; - gsize len = 0; - - bytes = g_value_get_boxed (value); - data = bytes ? g_bytes_get_data (bytes, &len) : NULL; - - if (data != NULL && len > 0) - nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, data, len); - } else if (type == G_TYPE_STRV) { - char **array; - - array = (char **) g_value_get_boxed (value); - nm_keyfile_plugin_kf_set_string_list (info->keyfile, setting_name, key, (const gchar **const) array, g_strv_length (array)); - } else if (type == G_TYPE_HASH_TABLE) { - write_hash_of_string (info->keyfile, setting, key, value); - } else if (type == G_TYPE_ARRAY) { - write_array_of_uint (info->keyfile, setting, key, value); - } else if (G_VALUE_HOLDS_FLAGS (value)) { - /* Flags are guint but GKeyFile has no uint reader, just uint64 */ - nm_keyfile_plugin_kf_set_uint64 (info->keyfile, setting_name, key, (guint64) g_value_get_flags (value)); - } else if (G_VALUE_HOLDS_ENUM (value)) - nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (gint) g_value_get_enum (value)); - else - g_warn_if_reached (); -} - -GKeyFile * -nm_keyfile_write (NMConnection *connection, - NMKeyfileWriteHandler handler, - void *user_data, - GError **error) -{ - KeyfileWriterInfo info = { 0 }; - - g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); - g_return_val_if_fail (!error || !*error, NULL); - - if (!nm_connection_verify (connection, error)) - return NULL; - - info.connection = connection; - info.keyfile = g_key_file_new (); - info.error = NULL; - info.handler = handler; - info.user_data = user_data; - nm_connection_for_each_setting_value (connection, write_setting_value, &info); - - if (info.error) { - g_propagate_error (error, info.error); - g_key_file_unref (info.keyfile); - return NULL; - } - return info.keyfile; -} - diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 0769b200d1..3a47cf1ea9 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -38,6 +39,8 @@ #include "nm-setting-user.h" +/*****************************************************************************/ + typedef struct { NMConnection *connection; GKeyFile *keyfile; @@ -49,6 +52,15 @@ typedef struct { NMSetting *setting; } KeyfileReaderInfo; +typedef struct { + NMConnection *connection; + GKeyFile *keyfile; + GError *error; + NMKeyfileWriteHandler handler; + void *user_data; +} KeyfileWriterInfo; + +/*****************************************************************************/ static void _handle_warn (KeyfileReaderInfo *info, @@ -1480,6 +1492,530 @@ tfilter_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) g_ptr_array_unref (tfilters); } +/*****************************************************************************/ + +/* Some setting properties also contain setting names, such as + * NMSettingConnection's 'type' property (which specifies the base type of the + * connection, eg ethernet or wifi) or the 802-11-wireless setting's + * 'security' property which specifies whether or not the AP requires + * encryption. This function handles translating those properties' values + * from the real setting name to the more-readable alias. + */ +static void +setting_alias_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + const char *str, *alias; + + str = g_value_get_string (value); + alias = nm_keyfile_plugin_get_alias_for_setting_name (str); + nm_keyfile_plugin_kf_set_string (info->keyfile, + nm_setting_get_name (setting), + key, + alias ? alias : str); +} + +static void +write_array_of_uint (GKeyFile *file, + NMSetting *setting, + const char *key, + const GValue *value) +{ + GArray *array; + guint i; + gs_free int *tmp_array = NULL; + + array = (GArray *) g_value_get_boxed (value); + if (!array || !array->len) + return; + + g_return_if_fail (g_array_get_element_size (array) == sizeof (guint)); + + tmp_array = g_new (gint, array->len); + for (i = 0; i < array->len; i++) { + guint v = g_array_index (array, guint, i); + + if (v > G_MAXINT) + g_return_if_reached (); + tmp_array[i] = (int) v; + } + + nm_keyfile_plugin_kf_set_integer_list (file, nm_setting_get_name (setting), key, tmp_array, array->len); +} + +static void +dns_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + char **list; + + list = g_value_get_boxed (value); + if (list && list[0]) { + nm_keyfile_plugin_kf_set_string_list (info->keyfile, nm_setting_get_name (setting), key, + (const char **) list, g_strv_length (list)); + } +} + +static void +ip6_addr_gen_mode_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + NMSettingIP6ConfigAddrGenMode addr_gen_mode; + gs_free char *str = NULL; + + addr_gen_mode = (NMSettingIP6ConfigAddrGenMode) g_value_get_int (value); + str = nm_utils_enum_to_str (nm_setting_ip6_config_addr_gen_mode_get_type (), + addr_gen_mode); + nm_keyfile_plugin_kf_set_string (info->keyfile, + nm_setting_get_name (setting), + key, + str); +} + +static void +write_ip_values (GKeyFile *file, + const char *setting_name, + GPtrArray *array, + const char *gateway, + gboolean is_route) +{ + GString *output; + int family, i; + const char *addr, *gw; + guint32 plen; + char key_name[64], *key_name_idx; + + if (!array->len) + return; + + family = !strcmp (setting_name, NM_SETTING_IP4_CONFIG_SETTING_NAME) ? AF_INET : AF_INET6; + + strcpy (key_name, is_route ? "route" : "address"); + key_name_idx = key_name + strlen (key_name); + + output = g_string_sized_new (2*INET_ADDRSTRLEN + 10); + for (i = 0; i < array->len; i++) { + gint64 metric = -1; + + if (is_route) { + NMIPRoute *route = array->pdata[i]; + + addr = nm_ip_route_get_dest (route); + plen = nm_ip_route_get_prefix (route); + gw = nm_ip_route_get_next_hop (route); + metric = nm_ip_route_get_metric (route); + } else { + NMIPAddress *address = array->pdata[i]; + + addr = nm_ip_address_get_address (address); + plen = nm_ip_address_get_prefix (address); + gw = i == 0 ? gateway : NULL; + } + + g_string_set_size (output, 0); + g_string_append_printf (output, "%s/%u", addr, plen); + if ( metric != -1 + || gw) { + /* Older versions of the plugin do not support the form + * "a.b.c.d/plen,,metric", so, we always have to write the + * gateway, even if there isn't one. + * The current version supports reading of the above form. + */ + if (!gw) { + if (family == AF_INET) + gw = "0.0.0.0"; + else + gw = "::"; + } + + g_string_append_printf (output, ",%s", gw); + if (is_route && metric != -1) + g_string_append_printf (output, ",%lu", (unsigned long) metric); + } + + sprintf (key_name_idx, "%d", i + 1); + nm_keyfile_plugin_kf_set_string (file, setting_name, key_name, output->str); + + if (is_route) { + gs_free char *attributes = NULL; + GHashTable *hash; + + hash = _nm_ip_route_get_attributes_direct (array->pdata[i]); + attributes = nm_utils_format_variant_attributes (hash, ',', '='); + if (attributes) { + g_strlcat (key_name, "_options", sizeof (key_name)); + nm_keyfile_plugin_kf_set_string (file, setting_name, key_name, attributes); + } + } + } + g_string_free (output, TRUE); +} + +static void +addr_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + GPtrArray *array; + const char *setting_name = nm_setting_get_name (setting); + const char *gateway = nm_setting_ip_config_get_gateway (NM_SETTING_IP_CONFIG (setting)); + + array = (GPtrArray *) g_value_get_boxed (value); + if (array && array->len) + write_ip_values (info->keyfile, setting_name, array, gateway, FALSE); +} + +static void +gateway_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + /* skip */ +} + +static void +route_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + GPtrArray *array; + const char *setting_name = nm_setting_get_name (setting); + + array = (GPtrArray *) g_value_get_boxed (value); + if (array && array->len) + write_ip_values (info->keyfile, setting_name, array, NULL, TRUE); +} + +static void +qdisc_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + gsize i; + GPtrArray *array; + + array = (GPtrArray *) g_value_get_boxed (value); + if (!array || !array->len) + return; + + for (i = 0; i < array->len; i++) { + NMTCQdisc *qdisc = array->pdata[i]; + GString *key_name = g_string_sized_new (16); + GString *value_str = g_string_sized_new (60); + + g_string_append (key_name, "qdisc."); + _nm_utils_string_append_tc_parent (key_name, NULL, + nm_tc_qdisc_get_parent (qdisc)); + _nm_utils_string_append_tc_qdisc_rest (value_str, qdisc); + + nm_keyfile_plugin_kf_set_string (info->keyfile, + NM_SETTING_TC_CONFIG_SETTING_NAME, + key_name->str, + value_str->str); + + g_string_free (key_name, TRUE); + g_string_free (value_str, TRUE); + } +} + +static void +tfilter_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + gsize i; + GPtrArray *array; + + array = (GPtrArray *) g_value_get_boxed (value); + if (!array || !array->len) + return; + + for (i = 0; i < array->len; i++) { + NMTCTfilter *tfilter = array->pdata[i]; + GString *key_name = g_string_sized_new (16); + GString *value_str = g_string_sized_new (60); + + g_string_append (key_name, "tfilter."); + _nm_utils_string_append_tc_parent (key_name, NULL, + nm_tc_tfilter_get_parent (tfilter)); + _nm_utils_string_append_tc_tfilter_rest (value_str, tfilter, NULL); + + nm_keyfile_plugin_kf_set_string (info->keyfile, + NM_SETTING_TC_CONFIG_SETTING_NAME, + key_name->str, + value_str->str); + + g_string_free (key_name, TRUE); + g_string_free (value_str, TRUE); + } +} + +static void +write_hash_of_string (GKeyFile *file, + NMSetting *setting, + const char *key, + const GValue *value) +{ + GHashTable *hash; + const char *group_name = nm_setting_get_name (setting); + gboolean vpn_secrets = FALSE; + gs_free const char **keys = NULL; + guint i, l; + + /* Write VPN secrets out to a different group to keep them separate */ + if (NM_IS_SETTING_VPN (setting) && !strcmp (key, NM_SETTING_VPN_SECRETS)) { + group_name = NM_KEYFILE_GROUP_VPN_SECRETS; + vpn_secrets = TRUE; + } + + hash = g_value_get_boxed (value); + + keys = nm_utils_strdict_get_keys (hash, TRUE, &l); + for (i = 0; i < l; i++) { + const char *property, *data; + gboolean write_item = TRUE; + + property = keys[i]; + + /* Handle VPN secrets specially; they are nested in the property's hash; + * we don't want to write them if the secret is not saved, not required, + * or owned by a user's secret agent. + */ + if (vpn_secrets) { + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + nm_setting_get_secret_flags (setting, property, &secret_flags, NULL); + if (secret_flags != NM_SETTING_SECRET_FLAG_NONE) + write_item = FALSE; + } + + if (write_item) { + gs_free char *to_free = NULL; + + data = g_hash_table_lookup (hash, property); + nm_keyfile_plugin_kf_set_string (file, group_name, + nm_keyfile_key_encode (property, &to_free), + data); + } + } +} + +static void +ssid_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + GBytes *bytes; + const guint8 *ssid_data; + gsize ssid_len; + const char *setting_name = nm_setting_get_name (setting); + gboolean new_format = TRUE; + gsize semicolons = 0; + gsize i; + + g_return_if_fail (G_VALUE_HOLDS (value, G_TYPE_BYTES)); + + bytes = g_value_get_boxed (value); + if (!bytes) + return; + ssid_data = g_bytes_get_data (bytes, &ssid_len); + if (!ssid_data || !ssid_len) { + nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, ""); + return; + } + + /* Check whether each byte is printable. If not, we have to use an + * integer list, otherwise we can just use a string. + */ + for (i = 0; i < ssid_len; i++) { + const char c = ssid_data[i]; + + if (!g_ascii_isprint (c)) { + new_format = FALSE; + break; + } + if (c == ';') + semicolons++; + } + + if (new_format) { + gs_free char *ssid = NULL; + + if (semicolons == 0) + ssid = g_strndup ((char *) ssid_data, ssid_len); + else { + /* Escape semicolons with backslashes to make strings + * containing ';', such as '16;17;' unambiguous */ + gsize j = 0; + + ssid = g_malloc (ssid_len + semicolons + 1); + for (i = 0; i < ssid_len; i++) { + if (ssid_data[i] == ';') + ssid[j++] = '\\'; + ssid[j++] = ssid_data[i]; + } + ssid[j] = '\0'; + } + nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, ssid); + } else + nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, ssid_data, ssid_len); +} + +static void +password_raw_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + const char *setting_name = nm_setting_get_name (setting); + GBytes *array; + gsize len; + const guint8 *data; + + g_return_if_fail (G_VALUE_HOLDS (value, G_TYPE_BYTES)); + + array = (GBytes *) g_value_get_boxed (value); + if (!array) + return; + data = g_bytes_get_data (array, &len); + if (!data) + len = 0; + nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, data, len); +} + +/*****************************************************************************/ + +static void +cert_writer_default (NMConnection *connection, + GKeyFile *file, + NMKeyfileWriteTypeDataCert *cert_data) +{ + const char *setting_name = nm_setting_get_name (NM_SETTING (cert_data->setting)); + NMSetting8021xCKScheme scheme; + + scheme = cert_data->vtable->scheme_func (cert_data->setting); + if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { + const char *path; + char *path_free = NULL, *tmp; + gs_free char *base_dir = NULL; + + path = cert_data->vtable->path_func (cert_data->setting); + g_assert (path); + + /* If the path is relative, make it an absolute path. + * Relative paths make a keyfile not easily usable in another + * context. */ + if (path[0] && path[0] != '/') { + base_dir = g_get_current_dir (); + path = path_free = g_strconcat (base_dir, "/", path, NULL); + } else + base_dir = g_path_get_dirname (path); + + /* path cannot start with "file://" or "data:;base64,", because it is an absolute path. + * Still, make sure that a prefix-less path will be recognized. This can happen + * for example if the path is longer then 500 chars. */ + tmp = nm_keyfile_detect_unqualified_path_scheme (base_dir, path, -1, FALSE, NULL); + if (tmp) + g_clear_pointer (&tmp, g_free); + else + path = tmp = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH, path, NULL); + + /* Path contains at least a '/', hence it cannot be recognized as the old + * binary format consisting of a list of integers. */ + + nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, path); + g_free (tmp); + g_free (path_free); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { + GBytes *blob; + const guint8 *blob_data; + gsize blob_len; + char *blob_base64, *val; + + blob = cert_data->vtable->blob_func (cert_data->setting); + g_assert (blob); + blob_data = g_bytes_get_data (blob, &blob_len); + + blob_base64 = g_base64_encode (blob_data, blob_len); + val = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB, blob_base64, NULL); + + nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, val); + g_free (val); + g_free (blob_base64); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { + nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, + cert_data->vtable->uri_func (cert_data->setting)); + } else { + /* scheme_func() returns UNKNOWN in all other cases. The only valid case + * where a scheme is allowed to be UNKNOWN, is unsetting the value. In this + * case, we don't expect the writer to be called, because the default value + * will not be serialized. + * The only other reason for the scheme to be UNKNOWN is an invalid cert. + * But our connection verifies, so that cannot happen either. */ + g_return_if_reached (); + } +} + +static void +cert_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + const NMSetting8021xSchemeVtable *objtype = NULL; + guint i; + NMKeyfileWriteTypeDataCert type_data = { 0 }; + + for (i = 0; nm_setting_8021x_scheme_vtable[i].setting_key; i++) { + if (g_strcmp0 (nm_setting_8021x_scheme_vtable[i].setting_key, key) == 0) { + objtype = &nm_setting_8021x_scheme_vtable[i]; + break; + } + } + if (!objtype) + g_return_if_reached (); + + type_data.setting = NM_SETTING_802_1X (setting); + type_data.vtable = objtype; + + if (info->handler) { + if (info->handler (info->connection, + info->keyfile, + NM_KEYFILE_WRITE_TYPE_CERT, + &type_data, + info->user_data, + &info->error)) + return; + if (info->error) + return; + } + + cert_writer_default (info->connection, info->keyfile, &type_data); +} + +static void +null_writer (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value) +{ + /* skip */ +} + +/*****************************************************************************/ + typedef struct { const char *setting_name; const char *key; @@ -1487,6 +2023,15 @@ typedef struct { void (*parser) (KeyfileReaderInfo *info, NMSetting *setting, const char *key); } KeyParser; +typedef struct { + const char *setting_name; + const char *key; + void (*writer) (KeyfileWriterInfo *info, + NMSetting *setting, + const char *key, + const GValue *value); +} KeyWriter; + /* A table of keys that require further parsing/conversion because they are * stored in a format that can't be automatically read using the key's type. * i.e. IPv4 addresses, which are stored in NetworkManager as guint32, but are @@ -1617,6 +2162,141 @@ static KeyParser key_parsers[] = { { NULL, NULL, FALSE } }; +/* A table of keys that require further parsing/conversion because they are + * stored in a format that can't be automatically read using the key's type. + * i.e. IPv4 addresses, which are stored in NetworkManager as guint32, but are + * stored in keyfiles as strings, eg "10.1.1.2" or IPv6 addresses stored + * in struct in6_addr internally, but as string in keyfiles. + */ +static KeyWriter key_writers[] = { + { NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_TYPE, + setting_alias_writer }, + { NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_ADDRESSES, + addr_writer }, + { NM_SETTING_IP6_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_ADDRESSES, + addr_writer }, + { NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_GATEWAY, + gateway_writer }, + { NM_SETTING_IP6_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_GATEWAY, + gateway_writer }, + { NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_ROUTES, + route_writer }, + { NM_SETTING_IP6_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_ROUTES, + route_writer }, + { NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_DNS, + dns_writer }, + { NM_SETTING_IP6_CONFIG_SETTING_NAME, + NM_SETTING_IP_CONFIG_DNS, + dns_writer }, + { NM_SETTING_IP6_CONFIG_SETTING_NAME, + NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, + ip6_addr_gen_mode_writer }, + { NM_SETTING_WIRELESS_SETTING_NAME, + NM_SETTING_WIRELESS_SSID, + ssid_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PASSWORD_RAW, + password_raw_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_CA_CERT, + cert_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_CLIENT_CERT, + cert_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PRIVATE_KEY, + cert_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PHASE2_CA_CERT, + cert_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PHASE2_CLIENT_CERT, + cert_writer }, + { NM_SETTING_802_1X_SETTING_NAME, + NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, + cert_writer }, + { NM_SETTING_TC_CONFIG_SETTING_NAME, + NM_SETTING_TC_CONFIG_QDISCS, + qdisc_writer }, + { NM_SETTING_TC_CONFIG_SETTING_NAME, + NM_SETTING_TC_CONFIG_TFILTERS, + tfilter_writer }, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_MCAST_REJOIN_COUNT, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_TX_HASH, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_TX_BALANCER, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_ACTIVE, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_FAST_RATE, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_SYS_PRIO, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_MIN_PORTS, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, + null_writer}, + { NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_LINK_WATCHERS, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_QUEUE_ID, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_PRIO, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_STICKY, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_LACP_PRIO, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_LACP_KEY, + null_writer}, + { NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_LINK_WATCHERS, + null_writer}, + { NULL, NULL, NULL } +}; + +/*****************************************************************************/ + static void set_default_for_missing_key (NMSetting *setting, const char *property) { @@ -2037,3 +2717,162 @@ out_error: g_object_unref (connection); return NULL; } + +/*****************************************************************************/ + +static gboolean +can_omit_default_value (NMSetting *setting, const char *property) +{ + if (NM_IS_SETTING_VLAN (setting)) { + if (!strcmp (property, NM_SETTING_VLAN_FLAGS)) + return FALSE; + } else if (NM_IS_SETTING_IP6_CONFIG (setting)) { + if (!strcmp (property, NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE)) + return FALSE; + } + + return TRUE; +} + +static void +write_setting_value (NMSetting *setting, + const char *key, + const GValue *value, + GParamFlags flag, + gpointer user_data) +{ + KeyfileWriterInfo *info = user_data; + const char *setting_name; + GType type = G_VALUE_TYPE (value); + KeyWriter *writer = &key_writers[0]; + GParamSpec *pspec; + + if (info->error) + return; + + /* Setting name gets picked up from the keyfile's section name instead */ + if (!strcmp (key, NM_SETTING_NAME)) + return; + + /* Don't write the NMSettingConnection object's 'read-only' property */ + if ( NM_IS_SETTING_CONNECTION (setting) + && !strcmp (key, NM_SETTING_CONNECTION_READ_ONLY)) + return; + + setting_name = nm_setting_get_name (setting); + + /* If the value is the default value, remove the item from the keyfile */ + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); + if (pspec) { + if ( can_omit_default_value (setting, key) + && g_param_value_defaults (pspec, (GValue *) value)) { + g_key_file_remove_key (info->keyfile, setting_name, key, NULL); + return; + } + } + + /* Don't write secrets that are owned by user secret agents or aren't + * supposed to be saved. VPN secrets are handled specially though since + * the secret flags there are in a third-level hash in the 'secrets' + * property. + */ + if (pspec && (pspec->flags & NM_SETTING_PARAM_SECRET) && !NM_IS_SETTING_VPN (setting)) { + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + if (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL)) + g_assert_not_reached (); + if (secret_flags != NM_SETTING_SECRET_FLAG_NONE) + return; + } + + /* Look through the list of handlers for non-standard format key values */ + while (writer->setting_name) { + if (!strcmp (writer->setting_name, setting_name) && !strcmp (writer->key, key)) { + (*writer->writer) (info, setting, key, value); + return; + } + writer++; + } + + if (type == G_TYPE_STRING) { + const char *str; + + str = g_value_get_string (value); + if (str) + nm_keyfile_plugin_kf_set_string (info->keyfile, setting_name, key, str); + } else if (type == G_TYPE_UINT) + nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (int) g_value_get_uint (value)); + else if (type == G_TYPE_INT) + nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, g_value_get_int (value)); + else if (type == G_TYPE_UINT64) { + char *numstr; + + numstr = g_strdup_printf ("%" G_GUINT64_FORMAT, g_value_get_uint64 (value)); + nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); + g_free (numstr); + } else if (type == G_TYPE_INT64) { + char *numstr; + + numstr = g_strdup_printf ("%" G_GINT64_FORMAT, g_value_get_int64 (value)); + nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); + g_free (numstr); + } else if (type == G_TYPE_BOOLEAN) { + nm_keyfile_plugin_kf_set_boolean (info->keyfile, setting_name, key, g_value_get_boolean (value)); + } else if (type == G_TYPE_CHAR) { + nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (int) g_value_get_schar (value)); + } else if (type == G_TYPE_BYTES) { + GBytes *bytes; + const guint8 *data; + gsize len = 0; + + bytes = g_value_get_boxed (value); + data = bytes ? g_bytes_get_data (bytes, &len) : NULL; + + if (data != NULL && len > 0) + nm_keyfile_plugin_kf_set_integer_list_uint8 (info->keyfile, setting_name, key, data, len); + } else if (type == G_TYPE_STRV) { + char **array; + + array = (char **) g_value_get_boxed (value); + nm_keyfile_plugin_kf_set_string_list (info->keyfile, setting_name, key, (const gchar **const) array, g_strv_length (array)); + } else if (type == G_TYPE_HASH_TABLE) { + write_hash_of_string (info->keyfile, setting, key, value); + } else if (type == G_TYPE_ARRAY) { + write_array_of_uint (info->keyfile, setting, key, value); + } else if (G_VALUE_HOLDS_FLAGS (value)) { + /* Flags are guint but GKeyFile has no uint reader, just uint64 */ + nm_keyfile_plugin_kf_set_uint64 (info->keyfile, setting_name, key, (guint64) g_value_get_flags (value)); + } else if (G_VALUE_HOLDS_ENUM (value)) + nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, (gint) g_value_get_enum (value)); + else + g_warn_if_reached (); +} + +GKeyFile * +nm_keyfile_write (NMConnection *connection, + NMKeyfileWriteHandler handler, + void *user_data, + GError **error) +{ + KeyfileWriterInfo info = { 0 }; + + g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); + g_return_val_if_fail (!error || !*error, NULL); + + if (!nm_connection_verify (connection, error)) + return NULL; + + info.connection = connection; + info.keyfile = g_key_file_new (); + info.error = NULL; + info.handler = handler; + info.user_data = user_data; + nm_connection_for_each_setting_value (connection, write_setting_value, &info); + + if (info.error) { + g_propagate_error (error, info.error); + g_key_file_unref (info.keyfile); + return NULL; + } + return info.keyfile; +} diff --git a/po/POTFILES.in b/po/POTFILES.in index 6451228191..ed492ba11e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -56,7 +56,6 @@ libnm-core/crypto_nss.c libnm-core/nm-connection.c libnm-core/nm-dbus-utils.c libnm-core/nm-keyfile.c -libnm-core/nm-keyfile-writer.c libnm-core/nm-setting-8021x.c libnm-core/nm-setting-adsl.c libnm-core/nm-setting-bluetooth.c From bc1b15cf0585ed74f692ca8f21c9382ea2c3b58f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 08:28:26 +0200 Subject: [PATCH 13/22] shared: move cmp functions to nm-shared-utils.c For one, these functions are not often needed. No need to define them in the "nm-macros-internal.h" header, which is included everywhere. Move them to "nm-shared-utils.h", which must be explicitly included. Also, these functions are usually not called directly, but by passing their function pointer to a sort function or similar. There is no point in having defined in the header file. --- shared/nm-utils/nm-macros-internal.h | 48 -------------------------- shared/nm-utils/nm-shared-utils.c | 50 ++++++++++++++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 6 ++++ 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 63d0128739..cc7205a4c1 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -1092,54 +1092,6 @@ nm_strcmp_p (gconstpointer a, gconstpointer b) return strcmp (s1, s2); } -/* like nm_strcmp_p(), suitable for g_ptr_array_sort_with_data(). - * g_ptr_array_sort() just casts nm_strcmp_p() to a function of different - * signature. I guess, in glib there are knowledgeable people that ensure - * that this additional argument doesn't cause problems due to different ABI - * for every architecture that glib supports. - * For NetworkManager, we'd rather avoid such stunts. - **/ -static inline int -nm_strcmp_p_with_data (gconstpointer a, gconstpointer b, gpointer user_data) -{ - const char *s1 = *((const char **) a); - const char *s2 = *((const char **) b); - - return strcmp (s1, s2); -} - -static inline int -nm_cmp_uint32_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data) -{ - const guint32 a = *((const guint32 *) p_a); - const guint32 b = *((const guint32 *) p_b); - - if (a < b) - return -1; - if (a > b) - return 1; - return 0; -} - -static inline int -nm_cmp_int2ptr_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data) -{ - /* p_a and p_b are two pointers to a pointer, where the pointer is - * interpreted as a integer using GPOINTER_TO_INT(). - * - * That is the case of a hash-table that uses GINT_TO_POINTER() to - * convert integers as pointers, and the resulting keys-as-array - * array. */ - const int a = GPOINTER_TO_INT (*((gconstpointer *) p_a)); - const int b = GPOINTER_TO_INT (*((gconstpointer *) p_b)); - - if (a < b) - return -1; - if (a > b) - return 1; - return 0; -} - /*****************************************************************************/ /* Taken from systemd's UNIQ_T and UNIQ macros. */ diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 582ccfffcf..6937065c25 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -499,6 +499,56 @@ _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 ma /*****************************************************************************/ +/* like nm_strcmp_p(), suitable for g_ptr_array_sort_with_data(). + * g_ptr_array_sort() just casts nm_strcmp_p() to a function of different + * signature. I guess, in glib there are knowledgeable people that ensure + * that this additional argument doesn't cause problems due to different ABI + * for every architecture that glib supports. + * For NetworkManager, we'd rather avoid such stunts. + **/ +int +nm_strcmp_p_with_data (gconstpointer a, gconstpointer b, gpointer user_data) +{ + const char *s1 = *((const char **) a); + const char *s2 = *((const char **) b); + + return strcmp (s1, s2); +} + +int +nm_cmp_uint32_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data) +{ + const guint32 a = *((const guint32 *) p_a); + const guint32 b = *((const guint32 *) p_b); + + if (a < b) + return -1; + if (a > b) + return 1; + return 0; +} + +int +nm_cmp_int2ptr_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data) +{ + /* p_a and p_b are two pointers to a pointer, where the pointer is + * interpreted as a integer using GPOINTER_TO_INT(). + * + * That is the case of a hash-table that uses GINT_TO_POINTER() to + * convert integers as pointers, and the resulting keys-as-array + * array. */ + const int a = GPOINTER_TO_INT (*((gconstpointer *) p_a)); + const int b = GPOINTER_TO_INT (*((gconstpointer *) p_b)); + + if (a < b) + return -1; + if (a > b) + return 1; + return 0; +} + +/*****************************************************************************/ + /** * nm_utils_strsplit_set: * @str: the string to split. diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index f966d293d1..84325bb729 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -440,6 +440,12 @@ nm_g_variant_unref_floating (GVariant *var) /*****************************************************************************/ +int nm_strcmp_p_with_data (gconstpointer a, gconstpointer b, gpointer user_data); +int nm_cmp_uint32_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data); +int nm_cmp_int2ptr_p_with_data (gconstpointer p_a, gconstpointer p_b, gpointer user_data); + +/*****************************************************************************/ + typedef struct { const char *name; } NMUtilsNamedEntry; From 3695d5273ac4fca30a433d2e2eb733c47ed19162 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 14 Apr 2018 20:46:47 +0200 Subject: [PATCH 14/22] libnm/keyfile: merge parser/writer vtables for keyfile properties --- libnm-core/nm-keyfile.c | 645 ++++++++++++++++++++++------------------ 1 file changed, 354 insertions(+), 291 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 3a47cf1ea9..6642716b56 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2017,283 +2017,353 @@ null_writer (KeyfileWriterInfo *info, /*****************************************************************************/ typedef struct { - const char *setting_name; - const char *key; - gboolean check_for_key; - void (*parser) (KeyfileReaderInfo *info, NMSetting *setting, const char *key); -} KeyParser; - -typedef struct { - const char *setting_name; - const char *key; + const char *property_name; + void (*parser) (KeyfileReaderInfo *info, + NMSetting *setting, + const char *key); void (*writer) (KeyfileWriterInfo *info, NMSetting *setting, const char *key, const GValue *value); -} KeyWriter; + gboolean check_for_key; +} ParseInfoProperty; -/* A table of keys that require further parsing/conversion because they are - * stored in a format that can't be automatically read using the key's type. - * i.e. IPv4 addresses, which are stored in NetworkManager as guint32, but are - * stored in keyfiles as strings, eg "10.1.1.2" or IPv6 addresses stored - * in struct in6_addr internally, but as string in keyfiles. - */ -static KeyParser key_parsers[] = { - { NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_TYPE, - TRUE, - setting_alias_parser }, - { NM_SETTING_BRIDGE_SETTING_NAME, - NM_SETTING_BRIDGE_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - FALSE, - ip_address_or_route_parser }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - FALSE, - ip_address_or_route_parser }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - FALSE, - ip_address_or_route_parser }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - FALSE, - ip_address_or_route_parser }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - FALSE, - ip_dns_parser }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - FALSE, - ip_dns_parser }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, - FALSE, - ip6_addr_gen_mode_parser }, - { NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_WIRED_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_WIRED_CLONED_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER_cloned }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER_cloned }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_BSSID, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_BLUETOOTH_SETTING_NAME, - NM_SETTING_BLUETOOTH_BDADDR, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_INFINIBAND_SETTING_NAME, - NM_SETTING_INFINIBAND_MAC_ADDRESS, - TRUE, - mac_address_parser_INFINIBAND }, - { NM_SETTING_WIMAX_SETTING_NAME, - NM_SETTING_WIMAX_MAC_ADDRESS, - TRUE, - mac_address_parser_ETHER }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_SSID, - TRUE, - ssid_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PASSWORD_RAW, - TRUE, - password_raw_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CA_CERT, - TRUE, - cert_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CLIENT_CERT, - TRUE, - cert_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PRIVATE_KEY, - TRUE, - cert_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CA_CERT, - TRUE, - cert_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CLIENT_CERT, - TRUE, - cert_parser }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, - TRUE, - cert_parser }, - { NM_SETTING_SERIAL_SETTING_NAME, - NM_SETTING_SERIAL_PARITY, - TRUE, - parity_parser }, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_CONFIG, - TRUE, - team_config_parser }, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_CONFIG, - TRUE, - team_config_parser }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_QDISCS, - FALSE, - qdisc_parser }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_TFILTERS, - FALSE, - tfilter_parser }, - { NULL, NULL, FALSE } +#define PARSE_INFO_PROPERTY(_property_name, ...) \ + (&((const ParseInfoProperty) { \ + .property_name = _property_name, \ + __VA_ARGS__ \ + })) + +#define PARSE_INFO_PROPERTIES(...) \ + .properties = ((const ParseInfoProperty*const[]) { \ + __VA_ARGS__ \ + NULL, \ + }) + +typedef struct { + const char *setting_name; + const ParseInfoProperty*const*properties; +} ParseInfoSetting; + +#define PARSE_INFO_SETTING(_setting_name, ...) \ + { \ + .setting_name = _setting_name, \ + __VA_ARGS__ \ + } + +static const ParseInfoSetting parse_infos[] = { + PARSE_INFO_SETTING (NM_SETTING_WIRELESS_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_BSSID, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER_cloned, + ), + PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_SSID, + .check_for_key = TRUE, + .parser = ssid_parser, + .writer = ssid_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_802_1X_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_CA_CERT, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_CLIENT_CERT, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PASSWORD_RAW, + .check_for_key = TRUE, + .parser = password_raw_parser, + .writer = password_raw_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_CA_CERT, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_CLIENT_CERT, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PRIVATE_KEY, + .check_for_key = TRUE, + .parser = cert_parser, + .writer = cert_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_WIRED_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_WIRED_CLONED_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER_cloned, + ), + PARSE_INFO_PROPERTY (NM_SETTING_WIRED_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_BLUETOOTH_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_BLUETOOTH_BDADDR, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_BRIDGE_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_BRIDGE_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_CONNECTION_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_TYPE, + .check_for_key = TRUE, + .parser = setting_alias_parser, + .writer = setting_alias_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_INFINIBAND_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_INFINIBAND_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_INFINIBAND, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_IP4_CONFIG_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ADDRESSES, + .parser = ip_address_or_route_parser, + .writer = addr_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_DNS, + .parser = ip_dns_parser, + .writer = dns_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_GATEWAY, + .writer = gateway_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, + .parser = ip_address_or_route_parser, + .writer = route_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_IP6_CONFIG_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, + .parser = ip6_addr_gen_mode_parser, + .writer = ip6_addr_gen_mode_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ADDRESSES, + .parser = ip_address_or_route_parser, + .writer = addr_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_DNS, + .parser = ip_dns_parser, + .writer = dns_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_GATEWAY, + .writer = gateway_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, + .parser = ip_address_or_route_parser, + .writer = route_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_SERIAL_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_SERIAL_PARITY, + .check_for_key = TRUE, + .parser = parity_parser, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_TC_CONFIG_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_TC_CONFIG_QDISCS, + .parser = qdisc_parser, + .writer = qdisc_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TC_CONFIG_TFILTERS, + .parser = tfilter_parser, + .writer = tfilter_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_TEAM_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_CONFIG, + .check_for_key = TRUE, + .parser = team_config_parser, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_LINK_WATCHERS, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_COUNT, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_ACTIVE, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_FAST_RATE, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_MIN_PORTS, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_SYS_PRIO, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_HASH, + .writer = null_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_TEAM_PORT_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_CONFIG, + .check_for_key = TRUE, + .parser = team_config_parser, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_KEY, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_PRIO, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LINK_WATCHERS, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_PRIO, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_QUEUE_ID, + .writer = null_writer, + ), + PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_STICKY, + .writer = null_writer, + ), + ), + ), + PARSE_INFO_SETTING (NM_SETTING_WIMAX_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_WIMAX_MAC_ADDRESS, + .check_for_key = TRUE, + .parser = mac_address_parser_ETHER, + ), + ), + ), }; -/* A table of keys that require further parsing/conversion because they are - * stored in a format that can't be automatically read using the key's type. - * i.e. IPv4 addresses, which are stored in NetworkManager as guint32, but are - * stored in keyfiles as strings, eg "10.1.1.2" or IPv6 addresses stored - * in struct in6_addr internally, but as string in keyfiles. - */ -static KeyWriter key_writers[] = { - { NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_TYPE, - setting_alias_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - addr_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES, - addr_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_GATEWAY, - gateway_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_GATEWAY, - gateway_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - route_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ROUTES, - route_writer }, - { NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - dns_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_DNS, - dns_writer }, - { NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, - ip6_addr_gen_mode_writer }, - { NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_WIRELESS_SSID, - ssid_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PASSWORD_RAW, - password_raw_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CA_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_CLIENT_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PRIVATE_KEY, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CA_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_CLIENT_CERT, - cert_writer }, - { NM_SETTING_802_1X_SETTING_NAME, - NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, - cert_writer }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_QDISCS, - qdisc_writer }, - { NM_SETTING_TC_CONFIG_SETTING_NAME, - NM_SETTING_TC_CONFIG_TFILTERS, - tfilter_writer }, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_MCAST_REJOIN_COUNT, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_HASH, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_BALANCER, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_ACTIVE, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_FAST_RATE, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_SYS_PRIO, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_MIN_PORTS, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, - null_writer}, - { NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_TEAM_LINK_WATCHERS, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_QUEUE_ID, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_PRIO, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_STICKY, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LACP_PRIO, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LACP_KEY, - null_writer}, - { NM_SETTING_TEAM_PORT_SETTING_NAME, - NM_SETTING_TEAM_PORT_LINK_WATCHERS, - null_writer}, - { NULL, NULL, NULL } -}; +static const ParseInfoProperty * +_parse_info_find (const char *setting_name, const char *property_name) +{ + gssize idx; + +#if NM_MORE_ASSERTS > 5 + { + guint i, j; + + for (i = 0; i < G_N_ELEMENTS (parse_infos); i++) { + const ParseInfoSetting *pis = &parse_infos[i]; + + g_assert (pis->setting_name); + if ( i > 0 + && strcmp (pis[-1].setting_name, pis->setting_name) >= 0) + g_error ("Wrong order at index #%d: \"%s\" before \"%s\"", i - 1, pis[-1].setting_name, pis->setting_name); + g_assert (pis->properties); + g_assert (pis->properties[0]); + for (j = 0; pis->properties[j]; j++) { + const ParseInfoProperty *pip0; + const ParseInfoProperty *pip = pis->properties[j]; + + g_assert (pip->property_name); + if ( j > 0 + && (pip0 = pis->properties[j - 1]) + && strcmp (pip0->property_name, pip->property_name) >= 0) + g_error ("Wrong order at index #%d.%d: \"%s.%s\" before \"%s.%s\"", i, j - 1, pis->setting_name, pip0->property_name, pis->setting_name, pip->property_name); + } + } + } +#endif + + G_STATIC_ASSERT_EXPR (G_STRUCT_OFFSET (ParseInfoSetting, setting_name) == 0); + idx = _nm_utils_array_find_binary_search (parse_infos, + sizeof (ParseInfoSetting), + G_N_ELEMENTS (parse_infos), + &setting_name, + nm_strcmp_p_with_data, + NULL); + if (idx >= 0) { + const ParseInfoSetting *pis = &parse_infos[idx]; + + nm_assert (nm_streq (pis->setting_name, setting_name)); + idx = _nm_utils_ptrarray_find_binary_search ((gconstpointer *) pis->properties, + NM_PTRARRAY_LEN (pis->properties), + &property_name, + nm_strcmp_p_with_data, + NULL, + NULL, + NULL); + if (idx >= 0) + return pis->properties[idx]; + } + + return NULL; +} /*****************************************************************************/ @@ -2317,7 +2387,7 @@ read_one_setting_value (NMSetting *setting, GType type; gs_free_error GError *err = NULL; gboolean check_for_key = TRUE; - KeyParser *parser = &key_parsers[0]; + const ParseInfoProperty *pip; if (info->error) return; @@ -2344,13 +2414,12 @@ read_one_setting_value (NMSetting *setting, setting_name = nm_setting_get_name (setting); - /* Look through the list of handlers for non-standard format key values */ - while (parser->setting_name) { - if (!strcmp (parser->setting_name, setting_name) && !strcmp (parser->key, key)) { - check_for_key = parser->check_for_key; - break; - } - parser++; + pip = _parse_info_find (setting_name, key); + if (pip) { + if (pip->parser) + check_for_key = pip->check_for_key; + else + pip = NULL; } if (NM_IS_SETTING_VPN (setting)) @@ -2379,11 +2448,8 @@ read_one_setting_value (NMSetting *setting, return; } - /* If there's a custom parser for this key, handle that before the generic - * parsers below. - */ - if (parser->setting_name) { - (*parser->parser) (info, setting, key); + if (pip) { + pip->parser (info, setting, key); return; } @@ -2744,7 +2810,7 @@ write_setting_value (NMSetting *setting, KeyfileWriterInfo *info = user_data; const char *setting_name; GType type = G_VALUE_TYPE (value); - KeyWriter *writer = &key_writers[0]; + const ParseInfoProperty *pip; GParamSpec *pspec; if (info->error) @@ -2785,13 +2851,10 @@ write_setting_value (NMSetting *setting, return; } - /* Look through the list of handlers for non-standard format key values */ - while (writer->setting_name) { - if (!strcmp (writer->setting_name, setting_name) && !strcmp (writer->key, key)) { - (*writer->writer) (info, setting, key, value); - return; - } - writer++; + pip = _parse_info_find (setting_name, key); + if (pip && pip->writer) { + pip->writer (info, setting, key, value); + return; } if (type == G_TYPE_STRING) { From a5c026f90ea32e610a0a3a10e1b526da9f370ce0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 11:28:54 +0200 Subject: [PATCH 15/22] libnm/keyfile: replace dummy writer implementation with flag to skip writing --- libnm-core/nm-keyfile.c | 77 +++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 45 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 6642716b56..2dd99103b1 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -1672,15 +1672,6 @@ addr_writer (KeyfileWriterInfo *info, write_ip_values (info->keyfile, setting_name, array, gateway, FALSE); } -static void -gateway_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - /* skip */ -} - static void route_writer (KeyfileWriterInfo *info, NMSetting *setting, @@ -2005,15 +1996,6 @@ cert_writer (KeyfileWriterInfo *info, cert_writer_default (info->connection, info->keyfile, &type_data); } -static void -null_writer (KeyfileWriterInfo *info, - NMSetting *setting, - const char *key, - const GValue *value) -{ - /* skip */ -} - /*****************************************************************************/ typedef struct { @@ -2025,7 +2007,8 @@ typedef struct { NMSetting *setting, const char *key, const GValue *value); - gboolean check_for_key; + bool check_for_key:1; + bool writer_skip:1; } ParseInfoProperty; #define PARSE_INFO_PROPERTY(_property_name, ...) \ @@ -2168,7 +2151,7 @@ static const ParseInfoSetting parse_infos[] = { .writer = dns_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_GATEWAY, - .writer = gateway_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, .parser = ip_address_or_route_parser, @@ -2191,7 +2174,7 @@ static const ParseInfoSetting parse_infos[] = { .writer = dns_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_GATEWAY, - .writer = gateway_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, .parser = ip_address_or_route_parser, @@ -2226,49 +2209,49 @@ static const ParseInfoSetting parse_infos[] = { .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_LINK_WATCHERS, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_COUNT, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_ACTIVE, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_FAST_RATE, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_MIN_PORTS, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_SYS_PRIO, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_HASH, - .writer = null_writer, + .writer_skip = TRUE, ), ), ), @@ -2279,22 +2262,22 @@ static const ParseInfoSetting parse_infos[] = { .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_KEY, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_PRIO, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LINK_WATCHERS, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_PRIO, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_QUEUE_ID, - .writer = null_writer, + .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_STICKY, - .writer = null_writer, + .writer_skip = TRUE, ), ), ), @@ -2852,9 +2835,13 @@ write_setting_value (NMSetting *setting, } pip = _parse_info_find (setting_name, key); - if (pip && pip->writer) { - pip->writer (info, setting, key, value); - return; + if (pip) { + if (pip->writer_skip) + return; + if (pip->writer) { + pip->writer (info, setting, key, value); + return; + } } if (type == G_TYPE_STRING) { From 94a96b70d06d8a2dab1e67bad768875909877c6e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 12:20:59 +0200 Subject: [PATCH 16/22] keyfile: rework handling not skipping default-values in writer --- libnm-core/nm-keyfile.c | 58 ++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 2dd99103b1..ea8f3b34f2 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2009,6 +2009,11 @@ typedef struct { const GValue *value); bool check_for_key:1; bool writer_skip:1; + + /* usually, we skip to write values that have their + * default value. By setting this flag to TRUE, also + * default values are written. */ + bool writer_persist_default:1; } ParseInfoProperty; #define PARSE_INFO_PROPERTY(_property_name, ...) \ @@ -2164,6 +2169,7 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_PROPERTY (NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, .parser = ip6_addr_gen_mode_parser, .writer = ip6_addr_gen_mode_writer, + .writer_persist_default = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ADDRESSES, .parser = ip_address_or_route_parser, @@ -2281,6 +2287,13 @@ static const ParseInfoSetting parse_infos[] = { ), ), ), + PARSE_INFO_SETTING (NM_SETTING_VLAN_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_VLAN_FLAGS, + .writer_persist_default = TRUE, + ), + ), + ), PARSE_INFO_SETTING (NM_SETTING_WIMAX_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_WIMAX_MAC_ADDRESS, @@ -2769,20 +2782,6 @@ out_error: /*****************************************************************************/ -static gboolean -can_omit_default_value (NMSetting *setting, const char *property) -{ - if (NM_IS_SETTING_VLAN (setting)) { - if (!strcmp (property, NM_SETTING_VLAN_FLAGS)) - return FALSE; - } else if (NM_IS_SETTING_IP6_CONFIG (setting)) { - if (!strcmp (property, NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE)) - return FALSE; - } - - return TRUE; -} - static void write_setting_value (NMSetting *setting, const char *key, @@ -2810,22 +2809,16 @@ write_setting_value (NMSetting *setting, setting_name = nm_setting_get_name (setting); - /* If the value is the default value, remove the item from the keyfile */ pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); - if (pspec) { - if ( can_omit_default_value (setting, key) - && g_param_value_defaults (pspec, (GValue *) value)) { - g_key_file_remove_key (info->keyfile, setting_name, key, NULL); - return; - } - } + nm_assert (pspec); /* Don't write secrets that are owned by user secret agents or aren't * supposed to be saved. VPN secrets are handled specially though since * the secret flags there are in a third-level hash in the 'secrets' * property. */ - if (pspec && (pspec->flags & NM_SETTING_PARAM_SECRET) && !NM_IS_SETTING_VPN (setting)) { + if ( (pspec->flags & NM_SETTING_PARAM_SECRET) + && !NM_IS_SETTING_VPN (setting)) { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; if (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL)) @@ -2835,13 +2828,18 @@ write_setting_value (NMSetting *setting, } pip = _parse_info_find (setting_name, key); - if (pip) { - if (pip->writer_skip) - return; - if (pip->writer) { - pip->writer (info, setting, key, value); - return; - } + if (pip && pip->writer_skip) + return; + + if ( (!pip || !pip->writer_persist_default) + && g_param_value_defaults (pspec, (GValue *) value)) { + nm_assert (!g_key_file_has_key (info->keyfile, setting_name, key, NULL)); + return; + } + + if (pip && pip->writer) { + pip->writer (info, setting, key, value); + return; } if (type == G_TYPE_STRING) { From 4dc933174e853d56b03ca6ad1e6e5419f74a17e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 12:38:02 +0200 Subject: [PATCH 17/22] keyfile: don't special case skipping connection.read-only property in writer --- libnm-core/nm-keyfile.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index ea8f3b34f2..7ff19cd5d2 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2130,6 +2130,9 @@ static const ParseInfoSetting parse_infos[] = { ), PARSE_INFO_SETTING (NM_SETTING_CONNECTION_SETTING_NAME, PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_READ_ONLY, + .writer_skip = TRUE, + ), PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_TYPE, .check_for_key = TRUE, .parser = setting_alias_parser, @@ -2791,7 +2794,7 @@ write_setting_value (NMSetting *setting, { KeyfileWriterInfo *info = user_data; const char *setting_name; - GType type = G_VALUE_TYPE (value); + GType type; const ParseInfoProperty *pip; GParamSpec *pspec; @@ -2799,12 +2802,7 @@ write_setting_value (NMSetting *setting, return; /* Setting name gets picked up from the keyfile's section name instead */ - if (!strcmp (key, NM_SETTING_NAME)) - return; - - /* Don't write the NMSettingConnection object's 'read-only' property */ - if ( NM_IS_SETTING_CONNECTION (setting) - && !strcmp (key, NM_SETTING_CONNECTION_READ_ONLY)) + if (nm_streq (key, NM_SETTING_NAME)) return; setting_name = nm_setting_get_name (setting); @@ -2842,6 +2840,7 @@ write_setting_value (NMSetting *setting, return; } + type = G_VALUE_TYPE (value); if (type == G_TYPE_STRING) { const char *str; From 87cc3092493056ff888c95261ecf85c67f452908 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 12:54:04 +0200 Subject: [PATCH 18/22] keyfile: various cleanup of error paths in keyfile handling --- libnm-core/nm-keyfile.c | 98 +++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 57 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 7ff19cd5d2..f3d35aac32 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -1105,9 +1105,8 @@ nm_keyfile_detect_unqualified_path_scheme (const char *base_dir, { const char *data = pdata; gboolean exists = FALSE; - gboolean success = FALSE; gsize validate_len; - char *path; + gs_free char *path = NULL; GByteArray *tmp; g_return_val_if_fail (base_dir && base_dir[0] == '/', NULL); @@ -1139,35 +1138,30 @@ nm_keyfile_detect_unqualified_path_scheme (const char *base_dir, if ( !memchr (data, '/', data_len) && !has_cert_ext (path)) { if (!consider_exists) - goto out; + return NULL; exists = g_file_test (path, G_FILE_TEST_EXISTS); if (!exists) - goto out; + return NULL; } else if (out_exists) exists = g_file_test (path, G_FILE_TEST_EXISTS); - /* Construct the proper value as required for the PATH scheme */ + /* Construct the proper value as required for the PATH scheme. + * + * When returning TRUE, we must also be sure that @data_len does not look like + * the deprecated format of list of integers. With this implementation that is the + * case, as long as @consider_exists is FALSE. */ tmp = g_byte_array_sized_new (strlen (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) + strlen (path) + 1); g_byte_array_append (tmp, (const guint8 *) NM_KEYFILE_CERT_SCHEME_PREFIX_PATH, strlen (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)); g_byte_array_append (tmp, (const guint8 *) path, strlen (path) + 1); - if (nm_setting_802_1x_check_cert_scheme (tmp->data, tmp->len, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) { - g_free (path); - path = (char *) g_byte_array_free (tmp, FALSE); - /* when returning TRUE, we must also be sure that @data_len does not look like - * the deprecated format of list of integers. With this implementation that is the - * case, as long as @consider_exists is FALSE. */ - success = TRUE; - } else + if (nm_setting_802_1x_check_cert_scheme (tmp->data, tmp->len, NULL) != NM_SETTING_802_1X_CK_SCHEME_PATH) { g_byte_array_unref (tmp); - -out: - if (!success) { - g_free (path); return NULL; } - if (out_exists) - *out_exists = exists; - return path; + g_free (path); + path = (char *) g_byte_array_free (tmp, FALSE); + + NM_SET_OUT (out_exists, exists); + return g_steal_pointer (&path); } #define HAS_SCHEME_PREFIX(bin, bin_len, scheme) \ @@ -2439,7 +2433,7 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("error loading setting value: %s"), err->message)) - goto out_error; + return; } /* Allow default values different than in property spec */ @@ -2455,11 +2449,10 @@ read_one_setting_value (NMSetting *setting, type = G_VALUE_TYPE (value); if (type == G_TYPE_STRING) { - char *str_val; + gs_free char *str_val = NULL; str_val = nm_keyfile_plugin_kf_get_string (keyfile, setting_name, key, NULL); g_object_set (setting, key, str_val, NULL); - g_free (str_val); } else if (type == G_TYPE_UINT) { int int_val; @@ -2468,7 +2461,7 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid negative value (%i)"), int_val)) - goto out_error; + return; } g_object_set (setting, key, int_val, NULL); } else if (type == G_TYPE_INT) { @@ -2489,17 +2482,16 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid char value (%i)"), int_val)) - goto out_error; + return; } g_object_set (setting, key, int_val, NULL); } else if (type == G_TYPE_UINT64) { - char *tmp_str; + gs_free char *tmp_str = NULL; guint64 uint_val; tmp_str = nm_keyfile_plugin_kf_get_value (keyfile, setting_name, key, NULL); uint_val = g_ascii_strtoull (tmp_str, NULL, 10); - g_free (tmp_str); g_object_set (setting, key, uint_val, NULL); } else if (type == G_TYPE_INT64) { gs_free char *tmp_str = NULL; @@ -2512,7 +2504,7 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid int64 value (%s)"), tmp_str)) - goto out_error; + return; } else g_object_set (setting, key, int_val, NULL); } else if (type == G_TYPE_BYTES) { @@ -2537,7 +2529,7 @@ read_one_setting_value (NMSetting *setting, val)) { g_byte_array_unref (array); g_free (tmp); - goto out_error; + return; } already_warned = TRUE; } else @@ -2571,7 +2563,7 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("too large FLAGS property '%s' (%llu)"), G_VALUE_TYPE_NAME (value), (unsigned long long) uint_val)) - goto out_error; + return; } } } else if (G_VALUE_HOLDS_ENUM (value)) { @@ -2584,10 +2576,8 @@ read_one_setting_value (NMSetting *setting, if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("unhandled setting property type '%s'"), G_VALUE_TYPE_NAME (value))) - goto out_error; + return; } -out_error: - return; } static NMSetting * @@ -2622,7 +2612,8 @@ read_setting (KeyfileReaderInfo *info) static void read_vpn_secrets (KeyfileReaderInfo *info, NMSettingVpn *s_vpn) { - char **keys, **iter; + gs_strfreev char **keys = NULL; + char **iter; keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, NULL, NULL); for (iter = keys; *iter; iter++) { @@ -2634,7 +2625,6 @@ read_vpn_secrets (KeyfileReaderInfo *info, NMSettingVpn *s_vpn) g_free (secret); } } - g_strfreev (keys); } /** @@ -2668,7 +2658,7 @@ nm_keyfile_read (GKeyFile *keyfile, void *user_data, GError **error) { - NMConnection *connection = NULL; + gs_unref_object NMConnection *connection = NULL; NMSettingConnection *s_con; NMSetting *setting; gchar **groups; @@ -2702,8 +2692,6 @@ nm_keyfile_read (GKeyFile *keyfile, info.user_data = user_data; groups = g_key_file_get_groups (keyfile, &length); - if (!groups) - length = 0; for (i = 0; i < length; i++) { /* Only read out secrets when needed */ if (!strcmp (groups[i], NM_KEYFILE_GROUP_VPN_SECRETS)) { @@ -2714,8 +2702,10 @@ nm_keyfile_read (GKeyFile *keyfile, info.group = groups[i]; setting = read_setting (&info); info.group = NULL; - if (info.error) - goto out_error; + if (info.error) { + g_propagate_error (error, info.error); + return NULL; + } if (setting) nm_connection_add_setting (connection, setting); } @@ -2730,21 +2720,19 @@ nm_keyfile_read (GKeyFile *keyfile, /* Make sure that we have 'id' even if not explictly specified in the keyfile */ if ( keyfile_name && !nm_setting_connection_get_id (s_con)) { - char *base_name; + gs_free char *base_name = NULL; base_name = g_path_get_basename (keyfile_name); g_object_set (s_con, NM_SETTING_CONNECTION_ID, base_name, NULL); - g_free (base_name); } /* Make sure that we have 'uuid' even if not explictly specified in the keyfile */ if ( keyfile_name && !nm_setting_connection_get_uuid (s_con)) { - char *hashed_uuid; + gs_free char *hashed_uuid = NULL; hashed_uuid = _nm_utils_uuid_generate_from_strings ("keyfile", keyfile_name, NULL); g_object_set (s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); - g_free (hashed_uuid); } /* Make sure that we have 'interface-name' even if it was specified in the @@ -2771,16 +2759,14 @@ nm_keyfile_read (GKeyFile *keyfile, s_vpn = nm_connection_get_setting_vpn (connection); if (s_vpn) { read_vpn_secrets (&info, s_vpn); - if (info.error) - goto out_error; + if (info.error) { + g_propagate_error (error, info.error); + return NULL; + } } } - return connection; -out_error: - g_propagate_error (error, info.error); - g_object_unref (connection); - return NULL; + return g_steal_pointer (&connection); } /*****************************************************************************/ @@ -2852,17 +2838,15 @@ write_setting_value (NMSetting *setting, else if (type == G_TYPE_INT) nm_keyfile_plugin_kf_set_integer (info->keyfile, setting_name, key, g_value_get_int (value)); else if (type == G_TYPE_UINT64) { - char *numstr; + char numstr[30]; - numstr = g_strdup_printf ("%" G_GUINT64_FORMAT, g_value_get_uint64 (value)); + nm_sprintf_buf (numstr, "%" G_GUINT64_FORMAT, g_value_get_uint64 (value)); nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); - g_free (numstr); } else if (type == G_TYPE_INT64) { - char *numstr; + char numstr[30]; - numstr = g_strdup_printf ("%" G_GINT64_FORMAT, g_value_get_int64 (value)); + nm_sprintf_buf (numstr, "%" G_GINT64_FORMAT, g_value_get_int64 (value)); nm_keyfile_plugin_kf_set_value (info->keyfile, setting_name, key, numstr); - g_free (numstr); } else if (type == G_TYPE_BOOLEAN) { nm_keyfile_plugin_kf_set_boolean (info->keyfile, setting_name, key, g_value_get_boolean (value)); } else if (type == G_TYPE_CHAR) { From 7e3b7295a44b93c731ea2342d3ec7fb24610f56d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 13:27:18 +0200 Subject: [PATCH 19/22] keyfile: rework handling of checking for whether a key exists in reader Rework this to have a value "parser_no_check_key" so that: - the default value for this is FALSE, so that we don't need to explicitly set it in @parse_infos to only get the default. Contrary to check_for_key. - check_for_key only had meaning when also "parser" was set. That means, the value was really "pip->parser && pip->check_for_key". That came from the fact, that orginally this was tracked as key_parsers array, which had "parser" always set. That is confusing, don't do that. The field "parser_no_check_key" has it's meaning, regardless of whether "parser" is set. --- libnm-core/nm-keyfile.c | 55 ++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index f3d35aac32..3437cee652 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2001,7 +2001,7 @@ typedef struct { NMSetting *setting, const char *key, const GValue *value); - bool check_for_key:1; + bool parser_no_check_key:1; bool writer_skip:1; /* usually, we skip to write values that have their @@ -2037,19 +2037,15 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_WIRELESS_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_BSSID, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER_cloned, ), PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), PARSE_INFO_PROPERTY (NM_SETTING_WIRELESS_SSID, - .check_for_key = TRUE, .parser = ssid_parser, .writer = ssid_writer, ), @@ -2058,37 +2054,30 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_802_1X_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_802_1X_CA_CERT, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_CLIENT_CERT, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PASSWORD_RAW, - .check_for_key = TRUE, .parser = password_raw_parser, .writer = password_raw_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_CA_CERT, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_CLIENT_CERT, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_802_1X_PRIVATE_KEY, - .check_for_key = TRUE, .parser = cert_parser, .writer = cert_writer, ), @@ -2097,11 +2086,9 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_WIRED_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_WIRED_CLONED_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER_cloned, ), PARSE_INFO_PROPERTY (NM_SETTING_WIRED_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), ), @@ -2109,7 +2096,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_BLUETOOTH_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_BLUETOOTH_BDADDR, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), ), @@ -2117,7 +2103,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_BRIDGE_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_BRIDGE_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), ), @@ -2128,7 +2113,6 @@ static const ParseInfoSetting parse_infos[] = { .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_TYPE, - .check_for_key = TRUE, .parser = setting_alias_parser, .writer = setting_alias_writer, ), @@ -2137,7 +2121,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_INFINIBAND_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_INFINIBAND_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_INFINIBAND, ), ), @@ -2145,10 +2128,12 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_IP4_CONFIG_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ADDRESSES, + .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser, .writer = addr_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_DNS, + .parser_no_check_key = TRUE, .parser = ip_dns_parser, .writer = dns_writer, ), @@ -2156,6 +2141,7 @@ static const ParseInfoSetting parse_infos[] = { .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, + .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser, .writer = route_writer, ), @@ -2164,15 +2150,18 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_IP6_CONFIG_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, + .parser_no_check_key = TRUE, .parser = ip6_addr_gen_mode_parser, .writer = ip6_addr_gen_mode_writer, .writer_persist_default = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ADDRESSES, + .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser, .writer = addr_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_DNS, + .parser_no_check_key = TRUE, .parser = ip_dns_parser, .writer = dns_writer, ), @@ -2180,6 +2169,7 @@ static const ParseInfoSetting parse_infos[] = { .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_IP_CONFIG_ROUTES, + .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser, .writer = route_writer, ), @@ -2188,7 +2178,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_SERIAL_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_SERIAL_PARITY, - .check_for_key = TRUE, .parser = parity_parser, ), ), @@ -2196,10 +2185,12 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_TC_CONFIG_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_TC_CONFIG_QDISCS, + .parser_no_check_key = TRUE, .parser = qdisc_parser, .writer = qdisc_writer, ), PARSE_INFO_PROPERTY (NM_SETTING_TC_CONFIG_TFILTERS, + .parser_no_check_key = TRUE, .parser = tfilter_parser, .writer = tfilter_writer, ), @@ -2208,7 +2199,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_TEAM_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_TEAM_CONFIG, - .check_for_key = TRUE, .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_LINK_WATCHERS, @@ -2261,7 +2251,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_TEAM_PORT_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_TEAM_CONFIG, - .check_for_key = TRUE, .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_KEY, @@ -2294,7 +2283,6 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_WIMAX_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_WIMAX_MAC_ADDRESS, - .check_for_key = TRUE, .parser = mac_address_parser_ETHER, ), ), @@ -2379,7 +2367,7 @@ read_one_setting_value (NMSetting *setting, int errsv; GType type; gs_free_error GError *err = NULL; - gboolean check_for_key = TRUE; + gboolean parser_no_check_key; const ParseInfoProperty *pip; if (info->error) @@ -2408,26 +2396,25 @@ read_one_setting_value (NMSetting *setting, setting_name = nm_setting_get_name (setting); pip = _parse_info_find (setting_name, key); - if (pip) { - if (pip->parser) - check_for_key = pip->check_for_key; - else - pip = NULL; - } if (NM_IS_SETTING_VPN (setting)) - check_for_key = FALSE; + parser_no_check_key = TRUE; else if (NM_IS_SETTING_USER (setting)) - check_for_key = FALSE; + parser_no_check_key = TRUE; else if (NM_IS_SETTING_BOND (setting)) - check_for_key = FALSE; + parser_no_check_key = TRUE; + else if (pip) + parser_no_check_key = pip->parser_no_check_key; + else + parser_no_check_key = FALSE; /* Check for the exact key in the GKeyFile if required. Most setting * properties map 1:1 to a key in the GKeyFile, but for those properties * like IP addresses and routes where more than one value is actually * encoded by the setting property, this won't be true. */ - if (check_for_key && !nm_keyfile_plugin_kf_has_key (keyfile, setting_name, key, &err)) { + if ( !parser_no_check_key + && !nm_keyfile_plugin_kf_has_key (keyfile, setting_name, key, &err)) { /* Key doesn't exist or an error ocurred, thus nothing to do. */ if (err) { if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, @@ -2441,7 +2428,7 @@ read_one_setting_value (NMSetting *setting, return; } - if (pip) { + if (pip && pip->parser) { pip->parser (info, setting, key); return; } From 9c91d44667cb7e97aadc3627f9df7324027a3808 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 14:35:22 +0200 Subject: [PATCH 20/22] keyfile: drop unused set_default_for_missing_key() --- libnm-core/nm-keyfile.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 3437cee652..1c5d4e7431 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2348,12 +2348,6 @@ _parse_info_find (const char *setting_name, const char *property_name) /*****************************************************************************/ -static void -set_default_for_missing_key (NMSetting *setting, const char *property) -{ - /* Set a value different from the default value of the property's spec */ -} - static void read_one_setting_value (NMSetting *setting, const char *key, @@ -2422,9 +2416,6 @@ read_one_setting_value (NMSetting *setting, err->message)) return; } - - /* Allow default values different than in property spec */ - set_default_for_missing_key (setting, key); return; } From 8c4ce431a65fceeb711c9331cb6f4f846530763d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 14:44:58 +0200 Subject: [PATCH 21/22] keyfile: no special handling to set parser_no_check_key for certain settings Do not have multiple ways of expressing a certain thing. There is a way how to express that the parser shouldn't check for keys, and that is via the parse-information. No extra hacks. --- libnm-core/nm-keyfile.c | 50 ++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 1c5d4e7431..b102e2dd01 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2100,6 +2100,13 @@ static const ParseInfoSetting parse_infos[] = { ), ), ), + PARSE_INFO_SETTING (NM_SETTING_BOND_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_BOND_OPTIONS, + .parser_no_check_key = TRUE, + ), + ), + ), PARSE_INFO_SETTING (NM_SETTING_BRIDGE_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_BRIDGE_MAC_ADDRESS, @@ -2273,6 +2280,13 @@ static const ParseInfoSetting parse_infos[] = { ), ), ), + PARSE_INFO_SETTING (NM_SETTING_USER_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_USER_DATA, + .parser_no_check_key = TRUE, + ), + ), + ), PARSE_INFO_SETTING (NM_SETTING_VLAN_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_VLAN_FLAGS, @@ -2280,6 +2294,28 @@ static const ParseInfoSetting parse_infos[] = { ), ), ), + PARSE_INFO_SETTING (NM_SETTING_VPN_SETTING_NAME, + PARSE_INFO_PROPERTIES ( + PARSE_INFO_PROPERTY (NM_SETTING_VPN_DATA, + .parser_no_check_key = TRUE, + ), + PARSE_INFO_PROPERTY (NM_SETTING_VPN_PERSISTENT, + .parser_no_check_key = TRUE, + ), + PARSE_INFO_PROPERTY (NM_SETTING_VPN_SECRETS, + .parser_no_check_key = TRUE, + ), + PARSE_INFO_PROPERTY (NM_SETTING_VPN_SERVICE_TYPE, + .parser_no_check_key = TRUE, + ), + PARSE_INFO_PROPERTY (NM_SETTING_VPN_TIMEOUT, + .parser_no_check_key = TRUE, + ), + PARSE_INFO_PROPERTY (NM_SETTING_VPN_USER_NAME, + .parser_no_check_key = TRUE, + ), + ), + ), PARSE_INFO_SETTING (NM_SETTING_WIMAX_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_WIMAX_MAC_ADDRESS, @@ -2361,7 +2397,6 @@ read_one_setting_value (NMSetting *setting, int errsv; GType type; gs_free_error GError *err = NULL; - gboolean parser_no_check_key; const ParseInfoProperty *pip; if (info->error) @@ -2391,23 +2426,12 @@ read_one_setting_value (NMSetting *setting, pip = _parse_info_find (setting_name, key); - if (NM_IS_SETTING_VPN (setting)) - parser_no_check_key = TRUE; - else if (NM_IS_SETTING_USER (setting)) - parser_no_check_key = TRUE; - else if (NM_IS_SETTING_BOND (setting)) - parser_no_check_key = TRUE; - else if (pip) - parser_no_check_key = pip->parser_no_check_key; - else - parser_no_check_key = FALSE; - /* Check for the exact key in the GKeyFile if required. Most setting * properties map 1:1 to a key in the GKeyFile, but for those properties * like IP addresses and routes where more than one value is actually * encoded by the setting property, this won't be true. */ - if ( !parser_no_check_key + if ( (!pip || !pip->parser_no_check_key) && !nm_keyfile_plugin_kf_has_key (keyfile, setting_name, key, &err)) { /* Key doesn't exist or an error ocurred, thus nothing to do. */ if (err) { From 3b03b2caeebbed3d517919c918d1193933b3da9c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 16 Apr 2018 15:12:38 +0200 Subject: [PATCH 22/22] keyfile: don't hack certain properties to be skipped in reader For writer there is no such hack either. The property-info table should describe whether to skip a property or not. --- libnm-core/nm-keyfile.c | 64 +++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index b102e2dd01..40bf89daff 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2001,6 +2001,7 @@ typedef struct { NMSetting *setting, const char *key, const GValue *value); + bool parser_skip; bool parser_no_check_key:1; bool writer_skip:1; @@ -2117,6 +2118,7 @@ static const ParseInfoSetting parse_infos[] = { PARSE_INFO_SETTING (NM_SETTING_CONNECTION_SETTING_NAME, PARSE_INFO_PROPERTIES ( PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_READ_ONLY, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_CONNECTION_TYPE, @@ -2209,48 +2211,63 @@ static const ParseInfoSetting parse_infos[] = { .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_LINK_WATCHERS, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_COUNT, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_MCAST_REJOIN_INTERVAL, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_COUNT, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_NOTIFY_PEERS_INTERVAL, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_ACTIVE, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_FAST_RATE, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_HWADDR_POLICY, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_MIN_PORTS, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_SYS_PRIO, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_RUNNER_TX_HASH, + .parser_skip = TRUE, .writer_skip = TRUE, ), ), @@ -2261,21 +2278,27 @@ static const ParseInfoSetting parse_infos[] = { .parser = team_config_parser, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_KEY, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LACP_PRIO, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_LINK_WATCHERS, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_PRIO, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_QUEUE_ID, + .parser_skip = TRUE, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY (NM_SETTING_TEAM_PORT_STICKY, + .parser_skip = TRUE, .writer_skip = TRUE, ), ), @@ -2402,30 +2425,20 @@ read_one_setting_value (NMSetting *setting, if (info->error) return; - /* Property is not writable */ if (!(flags & G_PARAM_WRITABLE)) return; - /* Setting name gets picked up from the keyfile's section name instead */ - if (!strcmp (key, NM_SETTING_NAME)) - return; - - /* Don't read the NMSettingConnection object's 'read-only' property */ - if ( NM_IS_SETTING_CONNECTION (setting) - && !strcmp (key, NM_SETTING_CONNECTION_READ_ONLY)) - return; - - if ( ( NM_IS_SETTING_TEAM (setting) - || NM_IS_SETTING_TEAM_PORT (setting)) - && !NM_IN_STRSET (key, NM_SETTING_TEAM_CONFIG)) { - /* silently ignore all team properties (except "config"). */ - return; - } - setting_name = nm_setting_get_name (setting); pip = _parse_info_find (setting_name, key); + if ( !pip + && nm_streq (key, NM_SETTING_NAME)) + return; + + if (pip && pip->parser_skip) + return; + /* Check for the exact key in the GKeyFile if required. Most setting * properties map 1:1 to a key in the GKeyFile, but for those properties * like IP addresses and routes where more than one value is actually @@ -2789,15 +2802,20 @@ write_setting_value (NMSetting *setting, if (info->error) return; - /* Setting name gets picked up from the keyfile's section name instead */ - if (nm_streq (key, NM_SETTING_NAME)) - return; - setting_name = nm_setting_get_name (setting); pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); nm_assert (pspec); + pip = _parse_info_find (setting_name, key); + + if ( !pip + && nm_streq (key, NM_SETTING_NAME)) + return; + + if (pip && pip->writer_skip) + return; + /* Don't write secrets that are owned by user secret agents or aren't * supposed to be saved. VPN secrets are handled specially though since * the secret flags there are in a third-level hash in the 'secrets' @@ -2813,10 +2831,6 @@ write_setting_value (NMSetting *setting, return; } - pip = _parse_info_find (setting_name, key); - if (pip && pip->writer_skip) - return; - if ( (!pip || !pip->writer_persist_default) && g_param_value_defaults (pspec, (GValue *) value)) { nm_assert (!g_key_file_has_key (info->keyfile, setting_name, key, NULL));