From aa7a53bc67acbaa299565f5d64d33f0ca9584a3a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 25 Jun 2015 15:36:43 +0200 Subject: [PATCH 01/34] libnm-keyfile: ensure g_key_file_get_groups() sets the length argument Under certain cases, if g_key_file_get_groups() fails, it might not set the out argument @length. Play it safe and initialize it. --- libnm-core/nm-keyfile-reader.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 7e2a8c123d..9f2dbc15d6 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1576,6 +1576,8 @@ 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], VPN_SECRETS_GROUP)) { From f8c9863d55d494bcf8613ded58e28636b48466b5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 25 Jun 2015 13:02:10 +0200 Subject: [PATCH 02/34] config: fix order of processing [connection] sections in NMConfig We support the "NetworkManager.conf" sections '[connection]' and '[connection.\+]' (with arbitrary suffix). Fix the order of how we evaluate these section. Note that the literal '[connection]' section is always evaluated lastly after any other '[connection.\+]' section. Within one file, we want to evaluate the sections in top-to-bottom order. But accross multiple files, we want to order them later-files-first. That gives a reasonable behavior if the user looks at one file, and also if he wants to overwrite configuration via configuration snippets like "conf.d/99-last.conf". Note that if a later file extends/overwrites a section defined in an earlier file, the section is still considered with lower priority This is intentional, because the user ~extends~ a lower priority section. If he wants to add a higher priority section, he should choose a new suffix. Fixes: dc0193ac023e30a9a3794e78c66f8c5266695a60 --- man/NetworkManager.conf.xml.in | 7 +- src/nm-config-data.c | 89 ++++++++++++----------- src/nm-config.c | 58 +++++++++++++++ src/tests/config/NetworkManager.conf | 44 +++++++++++ src/tests/config/conf.d/00-overrides.conf | 22 ++++++ src/tests/config/conf.d/10-more.conf | 18 +++++ src/tests/config/test-config.c | 17 +++++ 7 files changed, 212 insertions(+), 43 deletions(-) diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index 800b0882f8..83143eecaf 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -482,7 +482,7 @@ ipv6.ip6-privacy=1 - The sections are considered in order of appearance, with the + The sections within one file are considered in order of appearance, with the exception that the [connection] section is always considered last. In the example above, this order is [connection-wifi-wlan0], [connection-wlan-other], and [connection]. @@ -495,6 +495,11 @@ ipv6.ip6-privacy=1 "[connection-wifi-wlan0]" matches the device, it does not contain that property and the search continues. + + When having different sections in multiple files, sections from files that are read + later have higher priority. So within one file the priority of the sections is + top-to-bottom. Across multiple files later definitions take precedence. + diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 73c36312f3..cd968ea6cc 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -225,58 +225,63 @@ nm_config_data_get_connection_default (const NMConfigData *self, return NULL; } +static void +_get_connection_info_init (ConnectionInfo *connection_info, GKeyFile *keyfile, char *group) +{ + char *value; + + /* pass ownership of @group on... */ + connection_info->group_name = group; + + value = g_key_file_get_value (keyfile, group, "match-device", NULL); + if (value) { + connection_info->match_device.has = TRUE; + connection_info->match_device.spec = nm_match_spec_split (value); + g_free (value); + } + connection_info->stop_match = nm_config_keyfile_get_boolean (keyfile, group, "stop-match", FALSE); +} + static ConnectionInfo * _get_connection_infos (GKeyFile *keyfile) { char **groups; - guint i; + gsize i, j, ngroups; char *connection_tag = NULL; - GSList *connection_groups = NULL; ConnectionInfo *connection_infos = NULL; /* get the list of existing [connection.\+] sections that we consider - * for nm_config_data_get_connection_default(). Also, get them - * in the right order. */ - groups = g_key_file_get_groups (keyfile, NULL); - for (i = 0; groups && groups[i]; i++) { - if (g_str_has_prefix (groups[i], "connection")) { - if (strlen (groups[i]) == STRLEN ("connection")) - connection_tag = groups[i]; - else - connection_groups = g_slist_prepend (connection_groups, groups[i]); - } else - g_free (groups[i]); + * for nm_config_data_get_connection_default(). + * + * We expect the sections in their right order, with lowest priority + * first. Only exception is the (literal) [connection] section, which + * we will always reorder to the end. */ + groups = g_key_file_get_groups (keyfile, &ngroups); + if (!groups) + ngroups = 0; + else if (ngroups > 0) { + for (i = 0, j = 0; i < ngroups; i++) { + if (g_str_has_prefix (groups[i], "connection")) { + if (groups[i][STRLEN ("connection")] == '\0') + connection_tag = groups[i]; + else + groups[j++] = groups[i]; + } else + g_free (groups[i]); + } + ngroups = j; + } + + connection_infos = g_new0 (ConnectionInfo, ngroups + 1 + (connection_tag ? 1 : 0)); + for (i = 0; i < ngroups; i++) { + /* pass ownership of @group on... */ + _get_connection_info_init (&connection_infos[i], keyfile, groups[ngroups - i - 1]); + } + if (connection_tag) { + /* pass ownership of @connection_tag on... */ + _get_connection_info_init (&connection_infos[i], keyfile, connection_tag); } g_free (groups); - if (connection_tag) { - /* We want the group "connection" checked at last, so that - * all other "connection.\+" have preference. Those other - * groups are checked in order of appearance. */ - connection_groups = g_slist_prepend (connection_groups, connection_tag); - } - if (connection_groups) { - guint len = g_slist_length (connection_groups); - GSList *iter; - - connection_infos = g_new0 (ConnectionInfo, len + 1); - for (iter = connection_groups; iter; iter = iter->next) { - ConnectionInfo *connection_info; - char *value; - - nm_assert (len >= 1); - connection_info = &connection_infos[--len]; - connection_info->group_name = iter->data; - - value = g_key_file_get_value (keyfile, iter->data, "match-device", NULL); - if (value) { - connection_info->match_device.has = TRUE; - connection_info->match_device.spec = nm_match_spec_split (value); - g_free (value); - } - connection_info->stop_match = nm_config_keyfile_get_boolean (keyfile, iter->data, "stop-match", FALSE); - } - g_slist_free (connection_groups); - } return connection_infos; } diff --git a/src/nm-config.c b/src/nm-config.c index ac5523d817..c297318dbc 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -408,6 +408,45 @@ nm_config_create_keyfile () return keyfile; } +static int +_sort_groups_cmp (const char **pa, const char **pb, gpointer dummy) +{ + const char *a, *b; + gboolean a_is_connection, b_is_connection; + + /* basic NULL checking... */ + if (pa == pb) + return 0; + if (!pa) + return -1; + if (!pb) + return 1; + + a = *pa; + b = *pb; + + a_is_connection = g_str_has_prefix (a, "connection"); + b_is_connection = g_str_has_prefix (b, "connection"); + + if (a_is_connection != b_is_connection) { + /* one is a [connection*] entry, the other not. We sort [connection*] entires + * after. */ + if (a_is_connection) + return 1; + return -1; + } + if (!a_is_connection) { + /* both are non-connection entries. Don't reorder. */ + return 0; + } + + /* both are [connection.\+] entires. Reverse their order. + * One of the sections might be literally [connection]. That section + * is special and it's order will be fixed later. It doesn't actually + * matter here how it compares with [connection.\+] sections. */ + return pa > pb ? -1 : 1; +} + static gboolean read_config (GKeyFile *keyfile, const char *path, GError **error) { @@ -435,6 +474,25 @@ read_config (GKeyFile *keyfile, const char *path, GError **error) /* Override the current settings with the new ones */ groups = g_key_file_get_groups (kf, &ngroups); + if (!groups) + ngroups = 0; + + /* Within one file we reverse the order of the '[connection.\+] sections. + * Here we merge the current file (@kf) into @keyfile. As we merge multiple + * files, earlier sections (with lower priority) will be added first. + * But within one file, we want a top-to-bottom order. This means we + * must reverse the order within each file. + * At the very end, we will revert the order of all sections again and + * get thus the right behavior. This final reversing is done in + * NMConfigData:_get_connection_infos(). */ + if (ngroups > 1) { + g_qsort_with_data (groups, + ngroups, + sizeof (char *), + (GCompareDataFunc) _sort_groups_cmp, + NULL); + } + for (g = 0; groups[g]; g++) { keys = g_key_file_get_keys (kf, groups[g], &nkeys, NULL); if (!keys) diff --git a/src/tests/config/NetworkManager.conf b/src/tests/config/NetworkManager.conf index 36113661f2..a750c801ee 100644 --- a/src/tests/config/NetworkManager.conf +++ b/src/tests/config/NetworkManager.conf @@ -22,6 +22,17 @@ ipv6.ip6_privacy=0 dummy.test1=no dummy.test2=no +ord.key00=A-0.0.00 +ord.key01=A-0.0.01 +ord.key02=A-0.0.02 +ord.key03=A-0.0.03 +ord.key04=A-0.0.04 +ord.key05=A-0.0.05 +ord.key06=A-0.0.06 +ord.key07=A-0.0.07 +ord.key08=A-0.0.08 +ord.key09=A-0.0.09 + [connection.dev51] match-device=mac:00:00:00:00:00:51 stop-match=yes @@ -37,3 +48,36 @@ match-device=interface-name:wlan1 # match-wifi is not yet implemented. Just an idea what could be useful. match-wifi=ssid:*[Ss]tarbucks*|*University* ipv6.ip6_privacy=2 + + +# the following sections are tested for their order across +# multiple files. +[connection.ord.0.1] +ord.key03=A-0.1.03 +ord.key04=A-0.1.04 +ord.key05=A-0.1.05 +ord.key06=A-0.1.06 +ord.key07=A-0.1.07 +ord.key08=A-0.1.08 +ord.key09=A-0.1.09 +ord.ovw01=A-0.1.ovw01 +[connection.ord.0.2] +ord.key02=A-0.2.02 +ord.key03=A-0.2.03 +ord.key04=A-0.2.04 +ord.key05=A-0.2.05 +ord.key06=A-0.2.06 +ord.key07=A-0.2.07 +ord.key08=A-0.2.08 +ord.key09=A-0.2.09 +[connection.ord.0.3] +ord.key01=A-0.3.01 +ord.key02=A-0.3.02 +ord.key03=A-0.3.03 +ord.key04=A-0.3.04 +ord.key05=A-0.3.05 +ord.key06=A-0.3.06 +ord.key07=A-0.3.07 +ord.key08=A-0.3.08 +ord.key09=A-0.3.09 +ord.ovw01=A-0.3.ovw01 diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf index 0aa19d484c..0c246a02a4 100644 --- a/src/tests/config/conf.d/00-overrides.conf +++ b/src/tests/config/conf.d/00-overrides.conf @@ -9,3 +9,25 @@ a=0 b=0 c=0 + +# the following sections are tested for their order across +# multiple files. +[connection.ord.1.1] +ord.key06=B-1.1.06 +ord.key07=B-1.1.07 +ord.key08=B-1.1.08 +ord.key09=B-1.1.09 +[connection.ord.1.2] +ord.key05=B-1.2.05 +ord.key06=B-1.2.06 +ord.key07=B-1.2.07 +ord.key08=B-1.2.08 +ord.key09=B-1.2.09 +[connection.ord.1.3] +ord.key04=B-1.3.04 +ord.key05=B-1.3.05 +ord.key06=B-1.3.06 +ord.key07=B-1.3.07 +ord.key08=B-1.3.08 +ord.key09=B-1.3.09 + diff --git a/src/tests/config/conf.d/10-more.conf b/src/tests/config/conf.d/10-more.conf index b1424a4bc8..08b73ddfdc 100644 --- a/src/tests/config/conf.d/10-more.conf +++ b/src/tests/config/conf.d/10-more.conf @@ -9,3 +9,21 @@ uri=http://example.net a=10 b=10 +# the following sections are tested for their order across +# multiple files. +[connection.ord.2.1] +ord.key09=C-2.1.09 +[connection.ord.2.2] +ord.key08=C-2.2.08 +ord.key09=C-2.2.09 +[connection.ord.2.3] +ord.key07=C-2.3.07 +ord.key08=C-2.3.08 +ord.key09=C-2.3.09 + +# you can overwrite individual settings in a file loaded +# previously. But note that this does not bump the priority +# of the section, i.e. [connection.ord.0.1] still has a pretty +# low priority and is shadowed by [connection.ord.2.1]. +[connection.ord.0.1] +ord.ovw01=C-0.1.ovw01 diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 4a23f23fe5..9057f26850 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -324,6 +324,23 @@ test_config_confdir (void) g_assert_cmpstr (value, ==, "0"); g_free (value); +#define ASSERT_GET_CONN_DEFAULT(xconfig, xname, xvalue) \ + G_STMT_START { \ + gs_free char *_value = nm_config_data_get_connection_default (nm_config_get_data_orig (xconfig), (xname), NULL); \ + g_assert_cmpstr (_value, ==, (xvalue)); \ + } G_STMT_END + ASSERT_GET_CONN_DEFAULT (config, "ord.key00", "A-0.0.00"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key01", "A-0.3.01"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key02", "A-0.2.02"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key03", "A-0.1.03"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key04", "B-1.3.04"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key05", "B-1.2.05"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key06", "B-1.1.06"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key07", "C-2.3.07"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key08", "C-2.2.08"); + ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); + ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); + g_object_unref (config); } From 69f2d22bfe355f89b6ce9622613ab0e9be7d1dd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Jun 2015 21:42:46 +0200 Subject: [PATCH 03/34] glib-compat: backport g_key_file_save_to_file() --- include/nm-glib-compat.h | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/include/nm-glib-compat.h b/include/nm-glib-compat.h index 741d610319..dc39d2b934 100644 --- a/include/nm-glib-compat.h +++ b/include/nm-glib-compat.h @@ -213,4 +213,41 @@ _nm_g_ptr_array_insert (GPtrArray *array, #endif +#if !GLIB_CHECK_VERSION (2, 40, 0) +inline static gboolean +_g_key_file_save_to_file (GKeyFile *key_file, + const gchar *filename, + GError **error) +{ + gchar *contents; + gboolean success; + gsize length; + + g_return_val_if_fail (key_file != NULL, FALSE); + g_return_val_if_fail (filename != NULL, FALSE); + g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + + contents = g_key_file_to_data (key_file, &length, NULL); + g_assert (contents != NULL); + + success = g_file_set_contents (filename, contents, length, error); + g_free (contents); + + return success; +} +#define g_key_file_save_to_file(key_file, filename, error) \ + _g_key_file_save_to_file (key_file, filename, error) +#else +#define g_key_file_save_to_file(key_file, filename, error) \ + ({ \ + gboolean _success; \ + \ + G_GNUC_BEGIN_IGNORE_DEPRECATIONS \ + _success = g_key_file_save_to_file (key_file, filename, error); \ + G_GNUC_END_IGNORE_DEPRECATIONS \ + _success; \ + }) +#endif + + #endif /* __NM_GLIB_COMPAT_H__ */ From 885d187d23d44de0a3b8696d03a9e8fa2cc29f54 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:46:17 +0200 Subject: [PATCH 04/34] libnm: add _nm_utils_strv_cleanup() function --- libnm-core/nm-core-internal.h | 5 +++++ libnm-core/nm-utils.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index f9ae42ecdd..d7cb47bf29 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -121,6 +121,11 @@ gboolean _nm_utils_string_in_list (const char *str, gssize _nm_utils_strv_find_first (char **list, gssize len, const char *needle); +char **_nm_utils_strv_cleanup (char **strv, + gboolean strip_whitespace, + gboolean skip_empty, + gboolean skip_repeated); + char ** _nm_utils_strsplit_set (const char *str, const char *delimiters, int max_tokens); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f9325e2a47..875882f132 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -483,6 +483,36 @@ _nm_utils_strv_find_first (char **list, gssize len, const char *needle) return -1; } +char ** +_nm_utils_strv_cleanup (char **strv, + gboolean strip_whitespace, + gboolean skip_empty, + gboolean skip_repeated) +{ + guint i, j; + + if (!strv || !*strv) + return strv; + + if (strip_whitespace) { + for (i = 0; strv[i]; i++) + g_strstrip (strv[i]); + } + if (!skip_empty && !skip_repeated) + return strv; + j = 0; + for (i = 0; strv[i]; i++) { + if ( (skip_empty && !*strv[i]) + || (skip_repeated && _nm_utils_strv_find_first (strv, j, strv[i]) >= 0)) + g_free (strv[i]); + else + strv[j++] = strv[i]; + } + strv[j] = NULL; + return strv; +} + + gboolean _nm_utils_string_slist_validate (GSList *list, const char **valid_values) { From d6a331bd8caf3aa21dda9f7a2931f73f78c41fa4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:55:19 +0200 Subject: [PATCH 05/34] macros: add nm_strstrip() util --- include/nm-macros-internal.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/nm-macros-internal.h b/include/nm-macros-internal.h index c65f9e0439..4f558558bf 100644 --- a/include/nm-macros-internal.h +++ b/include/nm-macros-internal.h @@ -250,4 +250,13 @@ _NM_BACKPORT_SYMBOL_IMPL(VERSION, RETURN_TYPE, FUNC, _##FUNC##_##VERSION, ARGS_T /*****************************************************************************/ +static inline char * +nm_strstrip (char *str) +{ + /* g_strstrip doesn't like NULL. */ + return str ? g_strstrip (str) : NULL; +} + +/*****************************************************************************/ + #endif /* __NM_MACROS_INTERNAL_H__ */ From f5177dbf7a06d691957f4aa506e0d7061ca3eec2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Jun 2015 21:53:49 +0200 Subject: [PATCH 06/34] test: add nmtst_assert_success() util --- include/nm-test-utils.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index 75485d3766..a2948b6aa1 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -137,6 +137,14 @@ nmtst_assert_error (GError *error, } \ } G_STMT_END +inline static void +_nmtst_assert_success (gboolean success, GError *error, const char *file, int line) +{ + if (!success || error) + g_error ("(%s:%d) FAILURE success=%d, error=%s", file, line, success, error && error->message ? error->message : "(no error)"); +} +#define nmtst_assert_success(success, error) _nmtst_assert_success ((success), (error), __FILE__, __LINE__) + /*******************************************************************************/ struct __nmtst_internal From 27bd7dc93846c0f403dc59a4d9f8a1f039c1382b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 6 Jun 2015 21:29:01 +0200 Subject: [PATCH 07/34] config: change examples for command line arguments to system default --- src/main.c | 4 ++-- src/nm-config.c | 12 ++++++------ src/nm-config.h | 1 + src/nm-connectivity.c | 10 ++++------ 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main.c b/src/main.c index bfc441f4cf..1a8014f053 100644 --- a/src/main.c +++ b/src/main.c @@ -237,8 +237,8 @@ do_early_setup (int *argc, char **argv[], NMConfigCmdLineOptions *config_cli) N_("Log domains separated by ',': any combination of [%s]"), "PLATFORM,RFKILL,WIFI" }, { "g-fatal-warnings", 0, 0, G_OPTION_ARG_NONE, &global_opt.g_fatal_warnings, N_("Make all warnings fatal"), NULL }, - { "pid-file", 'p', 0, G_OPTION_ARG_FILENAME, &global_opt.pidfile, N_("Specify the location of a PID file"), N_("filename") }, - { "state-file", 0, 0, G_OPTION_ARG_FILENAME, &global_opt.state_file, N_("State file location"), N_("/path/to/state.file") }, + { "pid-file", 'p', 0, G_OPTION_ARG_FILENAME, &global_opt.pidfile, N_("Specify the location of a PID file"), N_(NM_DEFAULT_PID_FILE) }, + { "state-file", 0, 0, G_OPTION_ARG_FILENAME, &global_opt.state_file, N_("State file location"), N_(NM_DEFAULT_SYSTEM_STATE_FILE) }, { "run-from-build-dir", 0, 0, G_OPTION_ARG_NONE, &global_opt.run_from_build_dir, "Run from build directory", NULL }, {NULL} }; diff --git a/src/nm-config.c b/src/nm-config.c index c297318dbc..fccc4914b1 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -379,16 +379,16 @@ nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli, { GOptionEntry config_options[] = { - { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_("/path/to/config.file") }, - { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_("/path/to/config/dir") }, - { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, "no-auto-default.state location", NULL }, - { "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_("plugin1,plugin2") }, + { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(NM_DEFAULT_SYSTEM_CONF_FILE) }, + { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(NM_DEFAULT_SYSTEM_CONF_DIR) }, + { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(NM_NO_AUTO_DEFAULT_STATE_FILE) }, + { "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_(CONFIG_PLUGINS_DEFAULT) }, { "configure-and-quit", 0, 0, G_OPTION_ARG_NONE, &cli->configure_and_quit, N_("Quit after initial configuration"), NULL }, /* These three are hidden for now, and should eventually just go away. */ { "connectivity-uri", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &cli->connectivity_uri, N_("An http(s) address for checking internet connectivity"), "http://example.com" }, - { "connectivity-interval", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_INT, &cli->connectivity_interval, N_("The interval between connectivity checks (in seconds)"), "60" }, - { "connectivity-response", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &cli->connectivity_response, N_("The expected start of the response"), N_("Bingo!") }, + { "connectivity-interval", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_INT, &cli->connectivity_interval, N_("The interval between connectivity checks (in seconds)"), G_STRINGIFY (NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL) }, + { "connectivity-response", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &cli->connectivity_response, N_("The expected start of the response"), N_(NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE) }, { 0 }, }; diff --git a/src/nm-config.h b/src/nm-config.h index d4f5e94f3c..4eeb8aa886 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -44,6 +44,7 @@ G_BEGIN_DECLS #define NM_CONFIG_SIGNAL_CONFIG_CHANGED "config-changed" #define NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL 300 +#define NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE "NetworkManager is online" /* NOT LOCALIZED */ typedef struct NMConfigCmdLineOptions NMConfigCmdLineOptions; diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index f7e7576c22..81ac8f9de4 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -37,8 +37,6 @@ G_DEFINE_TYPE (NMConnectivity, nm_connectivity, G_TYPE_OBJECT) #define NM_CONNECTIVITY_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_CONNECTIVITY, NMConnectivityPrivate)) -#define DEFAULT_RESPONSE "NetworkManager is online" /* NOT LOCALIZED */ - #define _LOG_DEFAULT_DOMAIN LOGD_CONCHECK #define _LOG(level, domain, ...) \ @@ -140,7 +138,7 @@ nm_connectivity_check_cb (SoupSession *session, SoupMessage *msg, gpointer user_ NMConnectivityState new_state; const char *nm_header; const char *uri = cb_data->uri; - const char *response = cb_data->response ? cb_data->response : DEFAULT_RESPONSE; + const char *response = cb_data->response ? cb_data->response : NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE; self = NM_CONNECTIVITY (g_async_result_get_source_object (G_ASYNC_RESULT (simple))); /* it is safe to unref @self here, @simple holds yet another reference. */ @@ -401,7 +399,7 @@ set_property (GObject *object, guint property_id, case PROP_RESPONSE: response = g_value_get_string (value); if (g_strcmp0 (response, priv->response) != 0) { - /* a response %NULL means, DEFAULT_RESPONSE. Any other response + /* a response %NULL means, NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE. Any other response * (including "") is accepted. */ g_free (priv->response); priv->response = g_strdup (response); @@ -432,7 +430,7 @@ get_property (GObject *object, guint property_id, if (priv->response) g_value_set_string (value, priv->response); else - g_value_set_static_string (value, DEFAULT_RESPONSE); + g_value_set_static_string (value, NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE); break; case PROP_STATE: g_value_set_uint (value, priv->state); @@ -510,7 +508,7 @@ nm_connectivity_class_init (NMConnectivityClass *klass) g_object_class_install_property (object_class, PROP_RESPONSE, g_param_spec_string (NM_CONNECTIVITY_RESPONSE, "", "", - DEFAULT_RESPONSE, + NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS)); From 3c8abc2d5bb2ad43a5ed9f4a90074fb97c31b538 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 6 Jun 2015 21:47:29 +0200 Subject: [PATCH 08/34] config/trivial: rename defines for default settings Make them match to the variable names that we assign them to. --- src/nm-config.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index fccc4914b1..6c4e09d297 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -37,10 +37,10 @@ #include #include -#define NM_DEFAULT_SYSTEM_CONF_FILE NMCONFDIR "/NetworkManager.conf" -#define NM_DEFAULT_SYSTEM_CONF_DIR NMCONFDIR "/conf.d" -#define NM_OLD_SYSTEM_CONF_FILE NMCONFDIR "/nm-system-settings.conf" -#define NM_NO_AUTO_DEFAULT_STATE_FILE NMSTATEDIR "/no-auto-default.state" +#define DEFAULT_CONFIG_MAIN_FILE NMCONFDIR "/NetworkManager.conf" +#define DEFAULT_CONFIG_DIR NMCONFDIR "/conf.d" +#define DEFAULT_CONFIG_MAIN_FILE_OLD NMCONFDIR "/nm-system-settings.conf" +#define DEFAULT_NO_AUTO_DEFAULT_FILE NMSTATEDIR "/no-auto-default.state" struct NMConfigCmdLineOptions { char *config_main_file; @@ -379,9 +379,9 @@ nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli, { GOptionEntry config_options[] = { - { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(NM_DEFAULT_SYSTEM_CONF_FILE) }, - { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(NM_DEFAULT_SYSTEM_CONF_DIR) }, - { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(NM_NO_AUTO_DEFAULT_STATE_FILE) }, + { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(DEFAULT_CONFIG_MAIN_FILE) }, + { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(DEFAULT_CONFIG_DIR) }, + { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(DEFAULT_NO_AUTO_DEFAULT_FILE) }, { "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_(CONFIG_PLUGINS_DEFAULT) }, { "configure-and-quit", 0, 0, G_OPTION_ARG_NONE, &cli->configure_and_quit, N_("Quit after initial configuration"), NULL }, @@ -562,27 +562,27 @@ read_base_config (GKeyFile *keyfile, */ /* Try deprecated nm-system-settings.conf first */ - if (read_config (keyfile, NM_OLD_SYSTEM_CONF_FILE, &my_error)) { - *out_config_main_file = g_strdup (NM_OLD_SYSTEM_CONF_FILE); + if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) { + *out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE_OLD); return TRUE; } if (!g_error_matches (my_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND)) { nm_log_warn (LOGD_CORE, "Old default config file %s invalid: %s\n", - NM_OLD_SYSTEM_CONF_FILE, + DEFAULT_CONFIG_MAIN_FILE_OLD, my_error->message); } g_clear_error (&my_error); /* Try the standard config file location next */ - if (read_config (keyfile, NM_DEFAULT_SYSTEM_CONF_FILE, &my_error)) { - *out_config_main_file = g_strdup (NM_DEFAULT_SYSTEM_CONF_FILE); + if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE, &my_error)) { + *out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE); return TRUE; } if (!g_error_matches (my_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND)) { nm_log_warn (LOGD_CORE, "Default config file %s invalid: %s\n", - NM_DEFAULT_SYSTEM_CONF_FILE, + DEFAULT_CONFIG_MAIN_FILE, my_error->message); g_propagate_error (error, my_error); return FALSE; @@ -592,9 +592,9 @@ read_base_config (GKeyFile *keyfile, /* If for some reason no config file exists, use the default * config file path. */ - *out_config_main_file = g_strdup (NM_DEFAULT_SYSTEM_CONF_FILE); + *out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE); nm_log_info (LOGD_CORE, "No config file found or given; using %s\n", - NM_DEFAULT_SYSTEM_CONF_FILE); + DEFAULT_CONFIG_MAIN_FILE); return TRUE; } @@ -915,7 +915,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) if (priv->cli.config_dir) priv->config_dir = g_strdup (priv->cli.config_dir); else - priv->config_dir = g_strdup (NM_DEFAULT_SYSTEM_CONF_DIR); + priv->config_dir = g_strdup (DEFAULT_CONFIG_DIR); keyfile = read_entire_config (&priv->cli, priv->config_dir, @@ -930,7 +930,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) if (priv->cli.no_auto_default_file) priv->no_auto_default_file = g_strdup (priv->cli.no_auto_default_file); else - priv->no_auto_default_file = g_strdup (NM_NO_AUTO_DEFAULT_STATE_FILE); + priv->no_auto_default_file = g_strdup (DEFAULT_NO_AUTO_DEFAULT_FILE); priv->plugins = g_key_file_get_string_list (keyfile, "main", "plugins", NULL, NULL); if (!priv->plugins) From a05e80913e6b21b38cc1e729de92bc2e1260574e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jul 2015 09:46:35 +0200 Subject: [PATCH 09/34] config: add NM_CONFIG_KEYFILE_LIST_SEPARATOR define --- src/nm-config.c | 2 +- src/nm-config.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nm-config.c b/src/nm-config.c index 6c4e09d297..e671ca14fe 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -404,7 +404,7 @@ nm_config_create_keyfile () GKeyFile *keyfile; keyfile = g_key_file_new (); - g_key_file_set_list_separator (keyfile, ','); + g_key_file_set_list_separator (keyfile, NM_CONFIG_KEYFILE_LIST_SEPARATOR); return keyfile; } diff --git a/src/nm-config.h b/src/nm-config.h index 4eeb8aa886..4cba2fd88f 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -46,6 +46,8 @@ G_BEGIN_DECLS #define NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL 300 #define NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE "NetworkManager is online" /* NOT LOCALIZED */ +#define NM_CONFIG_KEYFILE_LIST_SEPARATOR ',' + typedef struct NMConfigCmdLineOptions NMConfigCmdLineOptions; struct _NMConfig { From bb4ae800a1293aea047c113352cc5e87178bb0b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Jun 2015 12:30:44 +0200 Subject: [PATCH 10/34] config: add nm_config_keyfile_set_string_list() utils function --- src/nm-config.c | 33 +++++++++++++++++++++++++++++++++ src/nm-config.h | 5 +++++ 2 files changed, 38 insertions(+) diff --git a/src/nm-config.c b/src/nm-config.c index e671ca14fe..509ea992d7 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -141,6 +141,39 @@ nm_config_keyfile_get_boolean (GKeyFile *keyfile, return value; } +void +nm_config_keyfile_set_string_list (GKeyFile *keyfile, + const char *group, + const char *key, + const char *const* strv, + gssize len) +{ + gsize l; + char *new_value; + + if (len < 0) + len = strv ? g_strv_length ((char **) strv) : 0; + + g_key_file_set_string_list (keyfile, group, key, strv, len); + + /* g_key_file_set_string_list() appends a trailing separator to the value. + * We don't like that, get rid of it. */ + + new_value = g_key_file_get_value (keyfile, group, key, NULL); + if (!new_value) + return; + + l = strlen (new_value); + if (l > 0 && new_value[l - 1] == NM_CONFIG_KEYFILE_LIST_SEPARATOR) { + /* Maybe we should check that value doesn't end with "\\,", i.e. + * with an escaped separator. But the way g_key_file_set_string_list() + * is implemented (currently), it always adds a trailing separator. */ + new_value[l - 1] = '\0'; + g_key_file_set_value (keyfile, group, key, new_value); + } + g_free (new_value); +} + /************************************************************************/ NMConfigData * diff --git a/src/nm-config.h b/src/nm-config.h index 4cba2fd88f..8a08e0bb36 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -96,6 +96,11 @@ gboolean nm_config_keyfile_get_boolean (GKeyFile *keyfile, const char *section, const char *key, gboolean default_value); +void nm_config_keyfile_set_string_list (GKeyFile *keyfile, + const char *group, + const char *key, + const char *const* strv, + gssize len); GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key); G_END_DECLS From fab5c6a372092bd350460a27891136c861ea2852 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 12:03:19 +0200 Subject: [PATCH 11/34] config: refactor processing of 'option+' and 'option-' config settings We have a hack to extend GKeyFile to support specifying an 'option+' key. Also add support for 'option-'. Options that make use of these modifiers can only be string lists. So do the concatenation not based on plain strings, but by treating the values as string lists. Also, don't add duplicates. --- man/NetworkManager.conf.xml.in | 4 +- src/nm-config.c | 58 +++++++++++++++-------- src/tests/config/conf.d/00-overrides.conf | 17 +++++++ src/tests/config/conf.d/10-more.conf | 4 ++ src/tests/config/conf.d/90-last.conf | 3 ++ src/tests/config/test-config.c | 20 ++++++++ 6 files changed, 85 insertions(+), 21 deletions(-) diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index 83143eecaf..e2501c8464 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -62,7 +62,8 @@ Copyright 2010 - 2014 Red Hat, Inc. For keys that take a list of devices as their value, you can specify devices by their MAC addresses or interface names, or - "*" to specify all devices. + "*" to specify all devices. See + below. Minimal system settings configuration file looks like this: @@ -76,6 +77,7 @@ Copyright 2010 - 2014 Red Hat, Inc. append a value to a previously-set list-valued key by doing: plugins+=another-plugin + plugins-=remove-me diff --git a/src/nm-config.c b/src/nm-config.c index 509ea992d7..f2e112c7bd 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -527,35 +527,53 @@ read_config (GKeyFile *keyfile, const char *path, GError **error) } for (g = 0; groups[g]; g++) { - keys = g_key_file_get_keys (kf, groups[g], &nkeys, NULL); + const char *group = groups[g]; + + keys = g_key_file_get_keys (kf, group, &nkeys, NULL); if (!keys) continue; for (k = 0; keys[k]; k++) { - int len = strlen (keys[k]); - char *v; + const char *key; + char *new_value; + char last_char; + gsize key_len; - if (keys[k][len - 1] == '+') { - char *base_key = g_strndup (keys[k], len - 1); - char *old_val = g_key_file_get_value (keyfile, groups[g], base_key, NULL); - char *new_val = g_key_file_get_value (kf, groups[g], keys[k], NULL); + key = keys[k]; + g_assert (key && *key); + key_len = strlen (key); + last_char = key[key_len - 1]; + if ( key_len > 1 + && (last_char == '+' || last_char == '-')) { + gs_free char *base_key = g_strndup (key, key_len - 1); + gs_strfreev char **old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL); + gs_free char **new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL); + gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free); + char **iter_val; - if (old_val && *old_val) { - char *combined = g_strconcat (old_val, ",", new_val, NULL); + for (iter_val = old_val; iter_val && *iter_val; iter_val++) { + if ( last_char != '-' + || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0) + g_ptr_array_add (new, g_strdup (*iter_val)); + } + for (iter_val = new_val; iter_val && *iter_val; iter_val++) { + /* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */ + if ( last_char == '+' + && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0) + g_ptr_array_add (new, *iter_val); + else + g_free (*iter_val); + } - g_key_file_set_value (keyfile, groups[g], base_key, combined); - g_free (combined); - } else - g_key_file_set_value (keyfile, groups[g], base_key, new_val); - - g_free (base_key); - g_free (old_val); - g_free (new_val); + if (new->len > 0) + nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len); + else + g_key_file_remove_key (keyfile, group, base_key, NULL); continue; } - g_key_file_set_value (keyfile, groups[g], keys[k], - v = g_key_file_get_value (kf, groups[g], keys[k], NULL)); - g_free (v); + new_value = g_key_file_get_value (kf, group, key, NULL); + g_key_file_set_value (keyfile, group, key, new_value); + g_free (new_value); } g_strfreev (keys); } diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf index 0c246a02a4..cb0116edd8 100644 --- a/src/tests/config/conf.d/00-overrides.conf +++ b/src/tests/config/conf.d/00-overrides.conf @@ -31,3 +31,20 @@ ord.key07=B-1.3.07 ord.key08=B-1.3.08 ord.key09=B-1.3.09 + +[append] +val1=a,b + +val2-=VAL2 +val2=VAL2 + +val3=VAL3 +val3-=VAL3 + +val4=VAL4 +val4+=VAL4,va,vb,va,vb +val4-=VAL4,va + +val5=VAL5 +val5-=VAL5 +val5+=VAL5 diff --git a/src/tests/config/conf.d/10-more.conf b/src/tests/config/conf.d/10-more.conf index 08b73ddfdc..a1959c1948 100644 --- a/src/tests/config/conf.d/10-more.conf +++ b/src/tests/config/conf.d/10-more.conf @@ -27,3 +27,7 @@ ord.key09=C-2.3.09 # low priority and is shadowed by [connection.ord.2.1]. [connection.ord.0.1] ord.ovw01=C-0.1.ovw01 + + +[append] +val1-=b diff --git a/src/tests/config/conf.d/90-last.conf b/src/tests/config/conf.d/90-last.conf index dc1de394f1..c75dcc4710 100644 --- a/src/tests/config/conf.d/90-last.conf +++ b/src/tests/config/conf.d/90-last.conf @@ -3,3 +3,6 @@ plugins+=one,two [order] a=90 + +[append] +val1+=c,a diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 9057f26850..8c4043cf51 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -341,6 +341,26 @@ test_config_confdir (void) ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NULL); + g_assert_cmpstr (value, ==, "a,c"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NULL); + g_assert_cmpstr (value, ==, "VAL2"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NULL); + g_assert_cmpstr (value, ==, NULL); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NULL); + g_assert_cmpstr (value, ==, "vb,vb"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NULL); + g_assert_cmpstr (value, ==, "VAL5"); + g_free (value); + g_object_unref (config); } From 3e4458659b6bbed6441aca0c2a1786bfc33488a9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 23:27:01 +0200 Subject: [PATCH 12/34] config: fix evaluation of no-auto-default setting We used to merge the spec list for no-auto-default from keyfile with the content of the state file. Since the addition of the "except:" spec this is wrong. For example, if the user configured: no-auto-default=except:mac:11:11:11:11:11 and statefile contained "11:11:11:11:11" and "22:22:22:22:22", we would wrongly not match "11:11:11:11:11". The two lists must be kept separate, so that devices that are blocked by internal decision always match. This separation is also clearer. Now the spec list is devided into a part that comes from user configuration, and a part that comes from internal decision. --- src/nm-config-data.c | 69 +++++++++++++++++---------- src/nm-config-data.h | 2 +- src/nm-config.c | 108 ++++++++++++++++++++----------------------- 3 files changed, 97 insertions(+), 82 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index cd968ea6cc..9e6ac78733 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -60,6 +60,7 @@ typedef struct { struct { char **arr; GSList *specs; + GSList *specs_config; } no_auto_default; GSList *ignore_carrier; @@ -145,12 +146,17 @@ nm_config_data_get_no_auto_default (const NMConfigData *self) return (const char *const*) NM_CONFIG_DATA_GET_PRIVATE (self)->no_auto_default.arr; } -const GSList * -nm_config_data_get_no_auto_default_list (const NMConfigData *self) +gboolean +nm_config_data_get_no_auto_default_for_device (const NMConfigData *self, NMDevice *device) { - g_return_val_if_fail (self, NULL); + NMConfigDataPrivate *priv; - return NM_CONFIG_DATA_GET_PRIVATE (self)->no_auto_default.specs; + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), FALSE); + g_return_val_if_fail (NM_IS_DEVICE (device), FALSE); + + priv = NM_CONFIG_DATA_GET_PRIVATE (self); + return nm_device_spec_match_list (device, priv->no_auto_default.specs) + || nm_device_spec_match_list (device, priv->no_auto_default.specs_config); } const char * @@ -315,12 +321,21 @@ _keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b) return TRUE; } +static gboolean +_slist_str_equals (GSList *a, GSList *b) +{ + while (a && b && g_strcmp0 (a->data, b->data) == 0) { + a = a->next; + b = b->next; + } + return !a && !b; +} + NMConfigChangeFlags nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) { NMConfigChangeFlags changes = NM_CONFIG_CHANGE_NONE; NMConfigDataPrivate *priv_old, *priv_new; - GSList *spec_old, *spec_new; g_return_val_if_fail (NM_IS_CONFIG_DATA (old_data), NM_CONFIG_CHANGE_NONE); g_return_val_if_fail (NM_IS_CONFIG_DATA (new_data), NM_CONFIG_CHANGE_NONE); @@ -341,13 +356,8 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) || g_strcmp0 (nm_config_data_get_connectivity_response (old_data), nm_config_data_get_connectivity_response (new_data))) changes |= NM_CONFIG_CHANGE_CONNECTIVITY; - spec_old = priv_old->no_auto_default.specs; - spec_new = priv_new->no_auto_default.specs; - while (spec_old && spec_new && strcmp (spec_old->data, spec_new->data) == 0) { - spec_old = spec_old->next; - spec_new = spec_new->next; - } - if (spec_old || spec_new) + if ( !_slist_str_equals (priv_old->no_auto_default.specs, priv_new->no_auto_default.specs) + || !_slist_str_equals (priv_old->no_auto_default.specs_config, priv_new->no_auto_default.specs_config)) changes |= NM_CONFIG_CHANGE_NO_AUTO_DEFAULT; if (g_strcmp0 (nm_config_data_get_dns_mode (old_data), nm_config_data_get_dns_mode (new_data))) @@ -376,9 +386,6 @@ get_property (GObject *object, case PROP_CONFIG_DESCRIPTION: g_value_set_string (value, nm_config_data_get_config_description (self)); break; - case PROP_NO_AUTO_DEFAULT: - g_value_take_boxed (value, g_strdupv ((char **) nm_config_data_get_no_auto_default (self))); - break; case PROP_CONNECTIVITY_URI: g_value_set_string (value, nm_config_data_get_connectivity_uri (self)); break; @@ -402,7 +409,6 @@ set_property (GObject *object, { NMConfigData *self = NM_CONFIG_DATA (object); NMConfigDataPrivate *priv = NM_CONFIG_DATA_GET_PRIVATE (self); - guint i; /* This type is immutable. All properties are construct only. */ switch (prop_id) { @@ -418,12 +424,24 @@ set_property (GObject *object, priv->keyfile = nm_config_create_keyfile (); break; case PROP_NO_AUTO_DEFAULT: - priv->no_auto_default.arr = g_strdupv (g_value_get_boxed (value)); - if (!priv->no_auto_default.arr) - priv->no_auto_default.arr = g_new0 (char *, 1); - for (i = 0; priv->no_auto_default.arr[i]; i++) - priv->no_auto_default.specs = g_slist_prepend (priv->no_auto_default.specs, priv->no_auto_default.arr[i]); - priv->no_auto_default.specs = g_slist_reverse (priv->no_auto_default.specs); + { + char **value_arr = g_value_get_boxed (value); + guint i, j = 0; + + priv->no_auto_default.arr = g_new (char *, g_strv_length (value_arr) + 1); + priv->no_auto_default.specs = NULL; + + for (i = 0; value_arr && value_arr[i]; i++) { + if ( *value_arr[i] + && nm_utils_hwaddr_valid (value_arr[i], -1) + && _nm_utils_strv_find_first (value_arr, i, value_arr[i]) < 0) { + priv->no_auto_default.arr[j++] = g_strdup (value_arr[i]); + priv->no_auto_default.specs = g_slist_prepend (priv->no_auto_default.specs, g_strdup_printf ("mac:%s", value_arr[i])); + } + } + priv->no_auto_default.arr[j++] = NULL; + priv->no_auto_default.specs = g_slist_reverse (priv->no_auto_default.specs); + } break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -448,7 +466,8 @@ finalize (GObject *gobject) g_free (priv->connectivity.uri); g_free (priv->connectivity.response); - g_slist_free (priv->no_auto_default.specs); + g_slist_free_full (priv->no_auto_default.specs, g_free); + g_slist_free_full (priv->no_auto_default.specs_config, g_free); g_strfreev (priv->no_auto_default.arr); g_free (priv->dns_mode); @@ -501,6 +520,8 @@ constructed (GObject *object) priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, "main", "ignore-carrier"); priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, "main", "assume-ipv6ll-only"); + priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, "main", "no-auto-default"); + G_OBJECT_CLASS (nm_config_data_parent_class)->constructed (object); } @@ -594,7 +615,7 @@ nm_config_data_class_init (NMConfigDataClass *config_class) (object_class, PROP_NO_AUTO_DEFAULT, g_param_spec_boxed (NM_CONFIG_DATA_NO_AUTO_DEFAULT, "", "", G_TYPE_STRV, - G_PARAM_READWRITE | + G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 4ac089bf6b..56d11a01a9 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -91,7 +91,7 @@ const guint nm_config_data_get_connectivity_interval (const NMConfigData *config const char *nm_config_data_get_connectivity_response (const NMConfigData *config_data); const char *const*nm_config_data_get_no_auto_default (const NMConfigData *config_data); -const GSList * nm_config_data_get_no_auto_default_list (const NMConfigData *config_data); +gboolean nm_config_data_get_no_auto_default_for_device (const NMConfigData *self, NMDevice *device); const char *nm_config_data_get_dns_mode (const NMConfigData *self); const char *nm_config_data_get_rc_manager (const NMConfigData *self); diff --git a/src/nm-config.c b/src/nm-config.c index f2e112c7bd..39af527794 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -260,94 +260,99 @@ nm_config_get_configure_and_quit (NMConfig *config) /************************************************************************/ static char ** -no_auto_default_merge_from_file (const char *no_auto_default_file, const char *const* no_auto_default) +no_auto_default_from_file (const char *no_auto_default_file) { - GPtrArray *updated; + GPtrArray *no_auto_default_new; char **list; - int i, j; + guint i; char *data; - updated = g_ptr_array_new (); - if (no_auto_default) { - for (i = 0; no_auto_default[i]; i++) - g_ptr_array_add (updated, g_strdup (no_auto_default[i])); - } + no_auto_default_new = g_ptr_array_new (); if ( no_auto_default_file && g_file_get_contents (no_auto_default_file, &data, NULL, NULL)) { list = g_strsplit (data, "\n", -1); for (i = 0; list[i]; i++) { - if (!*list[i]) + if ( *list[i] + && nm_utils_hwaddr_valid (list[i], -1) + && _nm_utils_strv_find_first (list, i, list[i]) < 0) + g_ptr_array_add (no_auto_default_new, list[i]); + else g_free (list[i]); - else { - for (j = 0; j < updated->len; j++) { - if (!strcmp (list[i], updated->pdata[j])) - break; - } - if (j == updated->len) - g_ptr_array_add (updated, list[i]); - else - g_free (list[i]); - } } g_free (list); g_free (data); } - g_ptr_array_add (updated, NULL); - return (char **) g_ptr_array_free (updated, FALSE); + g_ptr_array_add (no_auto_default_new, NULL); + return (char **) g_ptr_array_free (no_auto_default_new, FALSE); +} + +static gboolean +no_auto_default_to_file (const char *no_auto_default_file, const char *const*no_auto_default, GError **error) +{ + GString *data; + gboolean success; + guint i; + + data = g_string_new (""); + for (i = 0; no_auto_default && no_auto_default[i]; i++) { + g_string_append (data, no_auto_default[i]); + g_string_append_c (data, '\n'); + } + success = g_file_set_contents (no_auto_default_file, data->str, data->len, error); + g_string_free (data, TRUE); + return success; } gboolean nm_config_get_no_auto_default_for_device (NMConfig *self, NMDevice *device) { - NMConfigData *config_data; - g_return_val_if_fail (NM_IS_CONFIG (self), FALSE); - g_return_val_if_fail (NM_IS_DEVICE (device), FALSE); - config_data = NM_CONFIG_GET_PRIVATE (self)->config_data; - return nm_device_spec_match_list (device, nm_config_data_get_no_auto_default_list (config_data)); + return nm_config_data_get_no_auto_default_for_device (NM_CONFIG_GET_PRIVATE (self)->config_data, device); } void nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device) { NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (self); - char *current; - GString *updated; GError *error = NULL; - char **no_auto_default; NMConfigData *new_data = NULL; + const char *hw_address; + const char *const*no_auto_default_current; + GPtrArray *no_auto_default_new = NULL; + guint i; g_return_if_fail (NM_IS_CONFIG (self)); g_return_if_fail (NM_IS_DEVICE (device)); - if (nm_config_get_no_auto_default_for_device (self, device)) - return; + hw_address = nm_device_get_hw_address (device); - updated = g_string_new (NULL); - if (g_file_get_contents (priv->no_auto_default_file, ¤t, NULL, NULL)) { - g_string_append (updated, current); - g_free (current); - if (updated->str[updated->len - 1] != '\n') - g_string_append_c (updated, '\n'); + no_auto_default_current = nm_config_data_get_no_auto_default (priv->config_data); + + if (_nm_utils_strv_find_first ((char **) no_auto_default_current, -1, hw_address) >= 0) { + /* @hw_address is already blocked. We don't have to update our in-memory representation. + * Maybe we should write to no_auto_default_file anew, but let's save that too. */ + return; } - g_string_append (updated, nm_device_get_hw_address (device)); - g_string_append_c (updated, '\n'); + no_auto_default_new = g_ptr_array_new (); + for (i = 0; no_auto_default_current && no_auto_default_current[i]; i++) + g_ptr_array_add (no_auto_default_new, (char *) no_auto_default_current[i]); + g_ptr_array_add (no_auto_default_new, (char *) hw_address); + g_ptr_array_add (no_auto_default_new, NULL); - if (!g_file_set_contents (priv->no_auto_default_file, updated->str, updated->len, &error)) { + if (!no_auto_default_to_file (priv->no_auto_default_file, (const char *const*) no_auto_default_new->pdata, &error)) { nm_log_warn (LOGD_SETTINGS, "Could not update no-auto-default.state file: %s", error->message); g_error_free (error); } - g_string_free (updated, TRUE); + new_data = nm_config_data_new_update_no_auto_default (priv->config_data, (const char *const*) no_auto_default_new->pdata); - no_auto_default = no_auto_default_merge_from_file (priv->no_auto_default_file, nm_config_data_get_no_auto_default (priv->config_data)); - new_data = nm_config_data_new_update_no_auto_default (priv->config_data, (const char *const*) no_auto_default); - g_strfreev (no_auto_default); + /* unref no_auto_default_set here. Note that _set_config_data() probably invalidates the content of the array. */ + g_ptr_array_unref (no_auto_default_new); _set_config_data (self, new_data, 0); } @@ -951,9 +956,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) GKeyFile *keyfile; char *config_main_file = NULL; char *config_description = NULL; - char **no_auto_default; - GSList *no_auto_default_orig_list; - GPtrArray *no_auto_default_orig; + gs_strfreev char **no_auto_default = NULL; if (priv->config_dir) { /* Object is already initialized. */ @@ -1000,19 +1003,10 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->configure_and_quit = nm_config_keyfile_get_boolean (keyfile, "main", "configure-and-quit", FALSE); - no_auto_default_orig_list = nm_config_get_device_match_spec (keyfile, "main", "no-auto-default"); - - no_auto_default_orig = _nm_utils_copy_slist_to_array (no_auto_default_orig_list, NULL, NULL); - g_ptr_array_add (no_auto_default_orig, NULL); - no_auto_default = no_auto_default_merge_from_file (priv->no_auto_default_file, (const char *const *) no_auto_default_orig->pdata); - g_ptr_array_unref (no_auto_default_orig); - - g_slist_free_full (no_auto_default_orig_list, g_free); + no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); priv->config_data_orig = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile); - g_strfreev (no_auto_default); - priv->config_data = g_object_ref (priv->config_data_orig); g_free (config_main_file); From 4a8a0b09180f7f3e9bc63f9cf45e6875dc402b50 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 16:58:22 +0200 Subject: [PATCH 13/34] config: reload also no-auto-default state The content of the no-auto-default state file is part of NMConfig. During a reload, also reload that. This way, a user could edit the no-auto-default file and it would be properly reloaded. --- src/nm-config.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nm-config.c b/src/nm-config.c index 39af527794..32f2883306 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -799,6 +799,7 @@ nm_config_reload (NMConfig *self, int signal) NMConfigData *new_data = NULL; char *config_main_file = NULL; char *config_description = NULL; + gs_strfreev char **no_auto_default = NULL; g_return_if_fail (NM_IS_CONFIG (self)); @@ -824,7 +825,9 @@ nm_config_reload (NMConfig *self, int signal) _set_config_data (self, NULL, signal); return; } - new_data = nm_config_data_new (config_main_file, config_description, nm_config_data_get_no_auto_default (priv->config_data), keyfile); + no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); + + new_data = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile); g_free (config_main_file); g_free (config_description); g_key_file_unref (keyfile); From 71323122c6b755cfc50ce93c07d6758b45c19cba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 13:30:30 +0200 Subject: [PATCH 14/34] libnm: add keyfile utility functions --- libnm-core/nm-keyfile-internal.h | 5 ++ libnm-core/nm-keyfile-utils.c | 79 ++++++++++++++++++++++++++++++++ libnm-core/tests/test-keyfile.c | 37 +-------------- src/nm-config-data.c | 31 +------------ 4 files changed, 88 insertions(+), 64 deletions(-) diff --git a/libnm-core/nm-keyfile-internal.h b/libnm-core/nm-keyfile-internal.h index 90af562cdf..bce5e8726a 100644 --- a/libnm-core/nm-keyfile-internal.h +++ b/libnm-core/nm-keyfile-internal.h @@ -162,5 +162,10 @@ GKeyFile *nm_keyfile_write (NMConnection *connection, char *nm_keyfile_plugin_kf_get_string (GKeyFile *kf, const char *group, const char *key, GError **error); void nm_keyfile_plugin_kf_set_string (GKeyFile *kf, const char *group, const char *key, const char *value); +void _nm_keyfile_copy (GKeyFile *dst, GKeyFile *src); +gboolean _nm_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b); +gboolean _nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b); +gboolean _nm_keyfile_has_values (GKeyFile *keyfile); + #endif /* __NM_KEYFILE_INTERNAL_H__ */ diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c index 61b30ab9c8..1ddc1b59e6 100644 --- a/libnm-core/nm-keyfile-utils.c +++ b/libnm-core/nm-keyfile-utils.c @@ -24,6 +24,7 @@ #include #include +#include "gsystem-local-alloc.h" #include "nm-keyfile-utils.h" #include "nm-keyfile-internal.h" #include "nm-setting-wired.h" @@ -204,4 +205,82 @@ nm_keyfile_plugin_kf_has_key (GKeyFile *kf, return has; } +/************************************************************************/ +void +_nm_keyfile_copy (GKeyFile *dst, GKeyFile *src) +{ + gs_strfreev char **groups = NULL; + guint g, k; + + groups = g_key_file_get_groups (src, NULL); + for (g = 0; groups && groups[g]; g++) { + const char *group = groups[g]; + gs_strfreev char **keys = NULL; + + keys = g_key_file_get_keys (src, group, NULL, NULL); + if (!keys) + continue; + + for (k = 0; keys[k]; k++) { + const char *key = keys[k]; + gs_free char *value = NULL; + + value = g_key_file_get_value (src, group, key, NULL); + if (value) + g_key_file_set_value (dst, group, key, value); + else + g_key_file_remove_key (dst, group, key, NULL); + } + } +} + +/************************************************************************/ + +gboolean +_nm_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b) +{ + gs_strfreev char **groups = NULL; + guint i, j; + + if (kf_a == kf_b) + return TRUE; + if (!kf_a || !kf_b) + return FALSE; + + groups = g_key_file_get_groups (kf_a, NULL); + for (i = 0; groups && groups[i]; i++) { + gs_strfreev char **keys = NULL; + + keys = g_key_file_get_keys (kf_a, groups[i], NULL, NULL); + if (!keys) + continue; + + for (j = 0; keys[j]; j++) { + gs_free char *key_a = g_key_file_get_value (kf_a, groups[i], keys[j], NULL); + gs_free char *key_b = g_key_file_get_value (kf_b, groups[i], keys[j], NULL); + + if (g_strcmp0 (key_a, key_b) != 0) + return FALSE; + } + } + return TRUE; +} + +gboolean +_nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b) +{ + return _nm_keyfile_a_contains_all_in_b (kf_a, kf_b) + && _nm_keyfile_a_contains_all_in_b (kf_b, kf_a); +} + +gboolean +_nm_keyfile_has_values (GKeyFile *keyfile) +{ + gs_strfreev char **groups; + + g_return_val_if_fail (keyfile, FALSE); + + groups = g_key_file_get_groups (keyfile, NULL); + return groups && groups[0]; +} diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 28d659f985..f9eb574b0c 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -81,39 +81,6 @@ _keyfile_load_from_data (const char *str) return keyfile; } -static gboolean -_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b) -{ - gs_strfreev char **groups = NULL; - guint i, j; - - if (kf_a == kf_b) - return TRUE; - - groups = g_key_file_get_groups (kf_a, NULL); - for (i = 0; groups && groups[i]; i++) { - gs_strfreev char **keys = NULL; - - keys = g_key_file_get_keys (kf_a, groups[i], NULL, NULL); - if (keys) { - for (j = 0; keys[j]; j++) { - gs_free char *key_a = g_key_file_get_value (kf_a, groups[i], keys[j], NULL); - gs_free char *key_b = g_key_file_get_value (kf_b, groups[i], keys[j], NULL); - - if (g_strcmp0 (key_a, key_b) != 0) - return FALSE; - } - } - } - return TRUE; -} - -static gboolean -_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b) -{ - return _keyfile_a_contains_all_in_b (kf_a, kf_b) && _keyfile_a_contains_all_in_b (kf_b, kf_a); -} - static GKeyFile * _nm_keyfile_write (NMConnection *connection, NMKeyfileWriteHandler handler, @@ -185,7 +152,7 @@ _keyfile_convert (NMConnection **con, c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, base_dir, read_handler, read_data, FALSE); c0_k1_c2_k3 = _nm_keyfile_write (c0_k1_c2, write_handler, write_data); - _keyfile_equals (c0_k1, c0_k1_c2_k3); + _nm_keyfile_equals (c0_k1, c0_k1_c2_k3); } if (k0) { NMSetting8021x *s1, *s2; @@ -247,7 +214,7 @@ _keyfile_convert (NMConnection **con, else { /* finally, if both a keyfile and a connection are given, assert that they are equal * after a round of conversion. */ - _keyfile_equals (c0_k1, k0_c1_k2); + _nm_keyfile_equals (c0_k1, k0_c1_k2); nmtst_assert_connection_equals (k0_c1, FALSE, c0_k1_c2, FALSE); } } diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 9e6ac78733..c8376132f7 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -27,6 +27,7 @@ #include "gsystem-local-alloc.h" #include "nm-device.h" #include "nm-core-internal.h" +#include "nm-keyfile-internal.h" #include "nm-macros-internal.h" typedef struct { @@ -294,33 +295,6 @@ _get_connection_infos (GKeyFile *keyfile) /************************************************************************/ -static gboolean -_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b) -{ - gs_strfreev char **groups = NULL; - guint i, j; - - if (kf_a == kf_b) - return TRUE; - - groups = g_key_file_get_groups (kf_a, NULL); - for (i = 0; groups && groups[i]; i++) { - gs_strfreev char **keys = NULL; - - keys = g_key_file_get_keys (kf_a, groups[i], NULL, NULL); - if (keys) { - for (j = 0; keys[j]; j++) { - gs_free char *key_a = g_key_file_get_value (kf_a, groups[i], keys[j], NULL); - gs_free char *key_b = g_key_file_get_value (kf_b, groups[i], keys[j], NULL); - - if (g_strcmp0 (key_a, key_b) != 0) - return FALSE; - } - } - } - return TRUE; -} - static gboolean _slist_str_equals (GSList *a, GSList *b) { @@ -343,8 +317,7 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) priv_old = NM_CONFIG_DATA_GET_PRIVATE (old_data); priv_new = NM_CONFIG_DATA_GET_PRIVATE (new_data); - if ( !_keyfile_a_contains_all_in_b (priv_old->keyfile, priv_new->keyfile) - || !_keyfile_a_contains_all_in_b (priv_new->keyfile, priv_old->keyfile)) + if (!_nm_keyfile_equals (priv_old->keyfile, priv_new->keyfile)) changes |= NM_CONFIG_CHANGE_VALUES; if ( g_strcmp0 (nm_config_data_get_config_main_file (old_data), nm_config_data_get_config_main_file (new_data)) != 0 From e1b0195c67d16f6fd44770828e912d190f2cd888 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 24 Jun 2015 17:16:22 +0200 Subject: [PATCH 15/34] libnm-keyfile/test: fix missing assertion in test --- libnm-core/tests/test-keyfile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index f9eb574b0c..c2e0ad6e93 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -152,7 +152,7 @@ _keyfile_convert (NMConnection **con, c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, base_dir, read_handler, read_data, FALSE); c0_k1_c2_k3 = _nm_keyfile_write (c0_k1_c2, write_handler, write_data); - _nm_keyfile_equals (c0_k1, c0_k1_c2_k3); + g_assert (_nm_keyfile_equals (c0_k1, c0_k1_c2_k3)); } if (k0) { NMSetting8021x *s1, *s2; @@ -214,7 +214,7 @@ _keyfile_convert (NMConnection **con, else { /* finally, if both a keyfile and a connection are given, assert that they are equal * after a round of conversion. */ - _nm_keyfile_equals (c0_k1, k0_c1_k2); + g_assert (_nm_keyfile_equals (c0_k1, k0_c1_k2)); nmtst_assert_connection_equals (k0_c1, FALSE, c0_k1_c2, FALSE); } } From 7fbfaf567df0562e6b9c39954087dc897c5e05b6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Jun 2015 21:12:00 +0200 Subject: [PATCH 16/34] libnm: consider ordering for _nm_keyfile_equals() GKeyFile considers the order of the files, so add a possibility to check whether to keyfiles are equal -- also with respect to the order of the elements. --- libnm-core/nm-keyfile-internal.h | 2 +- libnm-core/nm-keyfile-utils.c | 64 ++++++++++++++++++++++++++++++-- libnm-core/tests/test-keyfile.c | 4 +- src/nm-config-data.c | 2 +- 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/libnm-core/nm-keyfile-internal.h b/libnm-core/nm-keyfile-internal.h index bce5e8726a..f4bb079637 100644 --- a/libnm-core/nm-keyfile-internal.h +++ b/libnm-core/nm-keyfile-internal.h @@ -164,7 +164,7 @@ void nm_keyfile_plugin_kf_set_string (GKeyFile *kf, const char *group, const cha void _nm_keyfile_copy (GKeyFile *dst, GKeyFile *src); gboolean _nm_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b); -gboolean _nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b); +gboolean _nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b, gboolean consider_order); gboolean _nm_keyfile_has_values (GKeyFile *keyfile); diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c index 1ddc1b59e6..c05f4cd26f 100644 --- a/libnm-core/nm-keyfile-utils.c +++ b/libnm-core/nm-keyfile-utils.c @@ -267,11 +267,67 @@ _nm_keyfile_a_contains_all_in_b (GKeyFile *kf_a, GKeyFile *kf_b) return TRUE; } -gboolean -_nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b) + +static gboolean +_nm_keyfile_equals_ordered (GKeyFile *kf_a, GKeyFile *kf_b) { - return _nm_keyfile_a_contains_all_in_b (kf_a, kf_b) - && _nm_keyfile_a_contains_all_in_b (kf_b, kf_a); + gs_strfreev char **groups = NULL; + gs_strfreev char **groups_b = NULL; + guint i, j; + + if (kf_a == kf_b) + return TRUE; + if (!kf_a || !kf_b) + return FALSE; + + groups = g_key_file_get_groups (kf_a, NULL); + groups_b = g_key_file_get_groups (kf_b, NULL); + if (!groups && !groups_b) + return TRUE; + if (!groups || !groups_b) + return FALSE; + for (i = 0; groups[i] && groups_b[i] && !strcmp (groups[i], groups_b[i]); i++) + ; + if (groups[i] || groups_b[i]) + return FALSE; + + for (i = 0; groups[i]; i++) { + gs_strfreev char **keys = NULL; + gs_strfreev char **keys_b = NULL; + + keys = g_key_file_get_keys (kf_a, groups[i], NULL, NULL); + keys_b = g_key_file_get_keys (kf_b, groups[i], NULL, NULL); + + if ((!keys) != (!keys_b)) + return FALSE; + if (!keys) + continue; + + for (j = 0; keys[j] && keys_b[j] && !strcmp (keys[j], keys_b[j]); j++) + ; + if (keys[j] || keys_b[j]) + return FALSE; + + for (j = 0; keys[j]; j++) { + gs_free char *key_a = g_key_file_get_value (kf_a, groups[i], keys[j], NULL); + gs_free char *key_b = g_key_file_get_value (kf_b, groups[i], keys[j], NULL); + + if (g_strcmp0 (key_a, key_b) != 0) + return FALSE; + } + } + return TRUE; +} + +gboolean +_nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b, gboolean consider_order) +{ + if (!consider_order) { + return _nm_keyfile_a_contains_all_in_b (kf_a, kf_b) + && _nm_keyfile_a_contains_all_in_b (kf_b, kf_a); + } else { + return _nm_keyfile_equals_ordered (kf_a, kf_b); + } } gboolean diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index c2e0ad6e93..99f88ac543 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -152,7 +152,7 @@ _keyfile_convert (NMConnection **con, c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, base_dir, read_handler, read_data, FALSE); c0_k1_c2_k3 = _nm_keyfile_write (c0_k1_c2, write_handler, write_data); - g_assert (_nm_keyfile_equals (c0_k1, c0_k1_c2_k3)); + g_assert (_nm_keyfile_equals (c0_k1, c0_k1_c2_k3, TRUE)); } if (k0) { NMSetting8021x *s1, *s2; @@ -214,7 +214,7 @@ _keyfile_convert (NMConnection **con, else { /* finally, if both a keyfile and a connection are given, assert that they are equal * after a round of conversion. */ - g_assert (_nm_keyfile_equals (c0_k1, k0_c1_k2)); + g_assert (_nm_keyfile_equals (c0_k1, k0_c1_k2, TRUE)); nmtst_assert_connection_equals (k0_c1, FALSE, c0_k1_c2, FALSE); } } diff --git a/src/nm-config-data.c b/src/nm-config-data.c index c8376132f7..d5e8314656 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -317,7 +317,7 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) priv_old = NM_CONFIG_DATA_GET_PRIVATE (old_data); priv_new = NM_CONFIG_DATA_GET_PRIVATE (new_data); - if (!_nm_keyfile_equals (priv_old->keyfile, priv_new->keyfile)) + if (!_nm_keyfile_equals (priv_old->keyfile, priv_new->keyfile, TRUE)) changes |= NM_CONFIG_CHANGE_VALUES; if ( g_strcmp0 (nm_config_data_get_config_main_file (old_data), nm_config_data_get_config_main_file (new_data)) != 0 From a5f7abb842b037cbb21b1456c7681d0c93e91d36 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 08:17:28 +0200 Subject: [PATCH 17/34] config: get rid of @error argument to nm_config_data_get_value() We don't use this argument. A failure to retrieve a key is (for every practical purpose) the same as no such key. --- src/nm-config-data.c | 6 +++-- src/nm-config-data.h | 2 +- src/settings/plugins/ifnet/plugin.c | 6 ++--- src/settings/plugins/ifupdown/plugin.c | 13 +++------- src/settings/plugins/keyfile/plugin.c | 8 +++--- src/tests/config/test-config.c | 35 +++++++++++--------------- 6 files changed, 29 insertions(+), 41 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index d5e8314656..7266ffe967 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -108,11 +108,13 @@ nm_config_data_get_config_description (const NMConfigData *self) } char * -nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key, GError **error) +nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key) { g_return_val_if_fail (self, NULL); + g_return_val_if_fail (group && *group, NULL); + g_return_val_if_fail (key && *key, NULL); - return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, error); + return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); } const char * diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 56d11a01a9..5708190c24 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -84,7 +84,7 @@ NMConfigChangeFlags nm_config_data_diff (NMConfigData *old_data, NMConfigData *n const char *nm_config_data_get_config_main_file (const NMConfigData *config_data); const char *nm_config_data_get_config_description (const NMConfigData *config_data); -char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key, GError **error); +char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key); const char *nm_config_data_get_connectivity_uri (const NMConfigData *config_data); const guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); diff --git a/src/settings/plugins/ifnet/plugin.c b/src/settings/plugins/ifnet/plugin.c index 95de868e6e..814fd238b8 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -85,8 +85,7 @@ is_managed_plugin (void) char *result = NULL; result = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), - IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED, - NULL); + IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED); if (result) { gboolean ret = is_true (result); g_free (result); @@ -219,8 +218,7 @@ reload_connections (NMSystemConfigInterface *config) nm_log_info (LOGD_SETTINGS, "Loading connections"); str_auto_refresh = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), - IFNET_KEY_FILE_GROUP, "auto_refresh", - NULL); + IFNET_KEY_FILE_GROUP, "auto_refresh"); if (str_auto_refresh && is_true (str_auto_refresh)) auto_refresh = TRUE; g_free (str_auto_refresh); diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c index 2c30da0256..fc828bd312 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -303,7 +303,6 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) GHashTable *auto_ifaces; if_block *block = NULL; char *value; - GError *error = NULL; GList *keys, *iter; GHashTableIter con_iter; const char *block_name; @@ -421,17 +420,11 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) /* Check the config file to find out whether to manage interfaces */ value = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), - IFUPDOWN_KEY_FILE_GROUP, IFUPDOWN_KEY_FILE_KEY_MANAGED, - &error); - if (error) { - nm_log_info (LOGD_SETTINGS, "loading system config file (%s) caused error: %s", - nm_config_data_get_config_main_file (nm_config_get_data (nm_config_get ())), - error->message); - } else { + IFUPDOWN_KEY_FILE_GROUP, IFUPDOWN_KEY_FILE_KEY_MANAGED); + if (value) { gboolean manage_well_known; - error = NULL; - manage_well_known = !g_strcmp0 (value, "true") || !g_strcmp0 (value, "1"); + manage_well_known = !strcmp (value, "true") || !strcmp (value, "1"); priv->unmanage_well_known = !manage_well_known; g_free (value); } diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index ee7da79630..5ca0374b3d 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -323,8 +323,8 @@ config_changed_cb (NMConfig *config, { gs_free char *old_value = NULL, *new_value = NULL; - old_value = nm_config_data_get_value (old_data, "keyfile", "unmanaged-devices", NULL); - new_value = nm_config_data_get_value (config_data, "keyfile", "unmanaged-devices", NULL); + old_value = nm_config_data_get_value (old_data, "keyfile", "unmanaged-devices"); + new_value = nm_config_data_get_value (config_data, "keyfile", "unmanaged-devices"); if (g_strcmp0 (old_value, new_value) != 0) g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); @@ -526,7 +526,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (config); gs_free char *value = NULL; - value = nm_config_data_get_value (nm_config_get_data (priv->config), "keyfile", "unmanaged-devices", NULL); + value = nm_config_data_get_value (nm_config_get_data (priv->config), "keyfile", "unmanaged-devices"); return nm_match_spec_split (value); } @@ -647,7 +647,7 @@ nm_settings_keyfile_plugin_new (void) priv->config = g_object_ref (nm_config_get ()); value = nm_config_data_get_value (nm_config_get_data (priv->config), - "keyfile", "hostname", NULL); + "keyfile", "hostname"); if (value) { nm_log_warn (LOGD_SETTINGS, "keyfile: 'hostname' option is deprecated and has no effect"); g_free (value); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 8c4043cf51..995511f519 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -91,7 +91,6 @@ static void test_config_simple (void) { NMConfig *config; - GError *error = NULL; const char **plugins; char *value; gs_unref_object NMDevice *dev50 = nm_test_device_new ("00:00:00:00:00:50"); @@ -111,25 +110,21 @@ test_config_simple (void) g_assert_cmpstr (plugins[1], ==, "bar"); g_assert_cmpstr (plugins[2], ==, "baz"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "extra-key", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "extra-key"); g_assert_cmpstr (value, ==, "some value"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "no-key", &error); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "no-key"); g_assert (!value); - g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND); - g_clear_error (&error); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "no-section", "no-key", &error); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "no-section", "no-key"); g_assert (!value); - g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND); - g_clear_error (&error); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection", "ipv6.ip6_privacy", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection", "ipv6.ip6_privacy"); g_assert_cmpstr (value, ==, "0"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection.dev51", "ipv4.route-metric", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection.dev51", "ipv4.route-metric"); g_assert_cmpstr (value, ==, "51"); g_free (value); @@ -306,21 +301,21 @@ test_config_confdir (void) g_assert_cmpstr (plugins[3], ==, "one"); g_assert_cmpstr (plugins[4], ==, "two"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "extra", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "extra"); g_assert_cmpstr (value, ==, "hello"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new"); g_assert_cmpstr (value, ==, "something"); /* not ",something" */ g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "a", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "a"); g_assert_cmpstr (value, ==, "90"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "b", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "b"); g_assert_cmpstr (value, ==, "10"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "c", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "c"); g_assert_cmpstr (value, ==, "0"); g_free (value); @@ -341,23 +336,23 @@ test_config_confdir (void) ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1"); g_assert_cmpstr (value, ==, "a,c"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2"); g_assert_cmpstr (value, ==, "VAL2"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3"); g_assert_cmpstr (value, ==, NULL); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4"); g_assert_cmpstr (value, ==, "vb,vb"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NULL); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5"); g_assert_cmpstr (value, ==, "VAL5"); g_free (value); From 2c46003e99b472595d983a03c70dd28d254fad5c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 08:47:41 +0200 Subject: [PATCH 18/34] config: add macros NM_CONFIG_GET_DATA and NM_CONFIG_GET_DATA_ORIG --- src/devices/nm-device.c | 8 ++++---- src/nm-config.h | 4 ++++ src/nm-manager.c | 2 +- src/settings/plugins/ifnet/plugin.c | 4 ++-- src/settings/plugins/ifupdown/plugin.c | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 430c6f28c3..dcdd29e440 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -805,7 +805,7 @@ _get_ipx_route_metric (NMDevice *self, /* use the current NMConfigData, which makes this configuration reloadable. * Note that that means that the route-metric might change between SIGHUP. * You must cache the returned value if that is a problem. */ - value = nm_config_data_get_connection_default (nm_config_get_data (nm_config_get ()), + value = nm_config_data_get_connection_default (NM_CONFIG_GET_DATA, is_v4 ? "ipv4.route-metric" : "ipv6.route-metric", self); if (value) { route_metric = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXUINT32, -1); @@ -2329,7 +2329,7 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master) && g_strcmp0 (ip6_method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL) == 0 && !nm_setting_connection_get_master (NM_SETTING_CONNECTION (s_con)) && !priv->slaves - && !nm_config_data_get_assume_ipv6ll_only (nm_config_get_data (nm_config_get ()), self)) { + && !nm_config_data_get_assume_ipv6ll_only (NM_CONFIG_GET_DATA, self)) { _LOGD (LOGD_DEVICE, "ignoring generated connection (IPv6LL-only and not in master-slave relationship)"); g_object_unref (connection); connection = NULL; @@ -4902,7 +4902,7 @@ _ip6_privacy_get (NMDevice *self) } } - value = nm_config_data_get_connection_default (nm_config_get_data (nm_config_get ()), + value = nm_config_data_get_connection_default (NM_CONFIG_GET_DATA, "ipv6.ip6-privacy", self); /* 2.) use the default value from the configuration. */ @@ -8454,7 +8454,7 @@ _set_state_full (NMDevice *self, if (nm_device_has_capability (self, NM_DEVICE_CAP_CARRIER_DETECT)) { /* We cache the ignore_carrier state to not react on config-reloads while the connection * is active. But on deactivating, reset the ignore-carrier flag to the current state. */ - priv->ignore_carrier = nm_config_data_get_ignore_carrier (nm_config_get_data (nm_config_get ()), self); + priv->ignore_carrier = nm_config_data_get_ignore_carrier (NM_CONFIG_GET_DATA, self); } if (quitting) { diff --git a/src/nm-config.h b/src/nm-config.h index 8a08e0bb36..0dadf2e4b4 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -69,6 +69,10 @@ char *nm_config_change_flags_to_string (NMConfigChangeFlags flags); NMConfigData *nm_config_get_data (NMConfig *config); NMConfigData *nm_config_get_data_orig (NMConfig *config); + +#define NM_CONFIG_GET_DATA (nm_config_get_data (nm_config_get ())) +#define NM_CONFIG_GET_DATA_ORIG (nm_config_get_data_orig (nm_config_get ())) + const char **nm_config_get_plugins (NMConfig *config); gboolean nm_config_get_monitor_connection_files (NMConfig *config); gboolean nm_config_get_auth_polkit (NMConfig *config); diff --git a/src/nm-manager.c b/src/nm-manager.c index d32ab2167f..cd73125213 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2472,7 +2472,7 @@ should_connect_slaves (NMConnection *connection, NMDevice *device) goto out; /* Check configuration default for autoconnect-slaves property */ - value = nm_config_data_get_connection_default (nm_config_get_data (nm_config_get ()), + value = nm_config_data_get_connection_default (NM_CONFIG_GET_DATA, "connection.autoconnect-slaves", device); if (value) autoconnect_slaves = _nm_utils_ascii_str_to_int64 (value, 10, 0, 1, -1); diff --git a/src/settings/plugins/ifnet/plugin.c b/src/settings/plugins/ifnet/plugin.c index 814fd238b8..3f7a7db9e9 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -84,7 +84,7 @@ is_managed_plugin (void) { char *result = NULL; - result = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), + result = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED); if (result) { gboolean ret = is_true (result); @@ -217,7 +217,7 @@ reload_connections (NMSystemConfigInterface *config) nm_log_info (LOGD_SETTINGS, "Loading connections"); - str_auto_refresh = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), + str_auto_refresh = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, IFNET_KEY_FILE_GROUP, "auto_refresh"); if (str_auto_refresh && is_true (str_auto_refresh)) auto_refresh = TRUE; diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c index fc828bd312..081de24a57 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -419,7 +419,7 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) g_hash_table_destroy (auto_ifaces); /* Check the config file to find out whether to manage interfaces */ - value = nm_config_data_get_value (nm_config_get_data_orig (nm_config_get ()), + value = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, IFUPDOWN_KEY_FILE_GROUP, IFUPDOWN_KEY_FILE_KEY_MANAGED); if (value) { gboolean manage_well_known; From 35d2981546911f7f6e29c845eaca95a8973d7004 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:02:32 +0200 Subject: [PATCH 19/34] config: add nm_config_parse_boolean() function Add function to parse as boolean according our NMConfig convention. Split this out from nm_config_keyfile_get_boolean() so that we can use it independently. Also, change the return type to gint, so that one might pass -1 to indicate an invalid/missing boolean value. Thereby also don't log a warning in nm_config_keyfile_get_boolean() We don't want to log a warning every time we access a keyfile value. If we want to warn about invalid values, we should do it once after the configuration is loaded. And then we should not only do it for booleans, but for other types as well. --- src/nm-config.c | 57 +++++++++++++++++++++++++++++++------------------ src/nm-config.h | 18 +++++++++------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 32f2883306..d92d4628ec 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -108,37 +108,52 @@ static void _set_config_data (NMConfig *self, NMConfigData *new_data, int signal /************************************************************************/ -gboolean +gint +nm_config_parse_boolean (const char *str, + gint default_value) +{ + gsize len; + char *s = NULL; + + if (!str) + return default_value; + + while (str[0] && g_ascii_isspace (str[0])) + str++; + + if (!str[0]) + return default_value; + + len = strlen (str); + if (g_ascii_isspace (str[len - 1])) { + s = g_strdup (str); + g_strchomp (s); + str = s; + } + + if (!g_ascii_strcasecmp (str, "true") || !g_ascii_strcasecmp (str, "yes") || !g_ascii_strcasecmp (str, "on") || !g_ascii_strcasecmp (str, "1")) + default_value = TRUE; + else if (!g_ascii_strcasecmp (str, "false") || !g_ascii_strcasecmp (str, "no") || !g_ascii_strcasecmp (str, "off") || !g_ascii_strcasecmp (str, "0")) + default_value = FALSE; + if (s) + g_free (s); + return default_value; +} + +gint nm_config_keyfile_get_boolean (GKeyFile *keyfile, const char *section, const char *key, - gboolean default_value) + gint default_value) { - gboolean value = default_value; - char *str; + gs_free char *str = NULL; g_return_val_if_fail (keyfile != NULL, default_value); g_return_val_if_fail (section != NULL, default_value); g_return_val_if_fail (key != NULL, default_value); str = g_key_file_get_value (keyfile, section, key, NULL); - if (!str) - return default_value; - - g_strstrip (str); - if (str[0]) { - if (!g_ascii_strcasecmp (str, "true") || !g_ascii_strcasecmp (str, "yes") || !g_ascii_strcasecmp (str, "on") || !g_ascii_strcasecmp (str, "1")) - value = TRUE; - else if (!g_ascii_strcasecmp (str, "false") || !g_ascii_strcasecmp (str, "no") || !g_ascii_strcasecmp (str, "off") || !g_ascii_strcasecmp (str, "0")) - value = FALSE; - else { - nm_log_warn (LOGD_CORE, "Unrecognized value for %s.%s: '%s'. Assuming '%s'", - section, key, str, default_value ? "true" : "false"); - } - } - - g_free (str); - return value; + return nm_config_parse_boolean (str, default_value); } void diff --git a/src/nm-config.h b/src/nm-config.h index 0dadf2e4b4..13c4eeca18 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -95,16 +95,18 @@ NMConfig *nm_config_new (const NMConfigCmdLineOptions *cli, GError **error); NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, GError **error); void nm_config_reload (NMConfig *config, int signal); +gint nm_config_parse_boolean (const char *str, gint default_value); + GKeyFile *nm_config_create_keyfile (void); -gboolean nm_config_keyfile_get_boolean (GKeyFile *keyfile, - const char *section, +gint nm_config_keyfile_get_boolean (GKeyFile *keyfile, + const char *section, + const char *key, + gint default_value); +void nm_config_keyfile_set_string_list (GKeyFile *keyfile, + const char *group, const char *key, - gboolean default_value); -void nm_config_keyfile_set_string_list (GKeyFile *keyfile, - const char *group, - const char *key, - const char *const* strv, - gssize len); + const char *const* strv, + gssize len); GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key); G_END_DECLS From a0e92799af0b21c698cd9c4147b7d8870018437f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:06:53 +0200 Subject: [PATCH 20/34] config: add nm_config_data_get_value_boolean() --- src/nm-config-data.c | 14 ++++++++++++++ src/nm-config-data.h | 1 + 2 files changed, 15 insertions(+) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 7266ffe967..0b94fecc2e 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -117,6 +117,20 @@ nm_config_data_get_value (const NMConfigData *self, const char *group, const cha return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); } +gint +nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, const char *key, gint default_value) +{ + char *str; + gint value = default_value; + + str = nm_config_data_get_value (self, group, key); + if (str) { + value = nm_config_parse_boolean (str, default_value); + g_free (str); + } + return value; +} + const char * nm_config_data_get_connectivity_uri (const NMConfigData *self) { diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 5708190c24..59f3c413e1 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -85,6 +85,7 @@ const char *nm_config_data_get_config_main_file (const NMConfigData *config_data const char *nm_config_data_get_config_description (const NMConfigData *config_data); char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key); +gint nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, const char *key, gint default_value); const char *nm_config_data_get_connectivity_uri (const NMConfigData *config_data); const guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); From 1b0ab2129c2789c476b2376da4c8e3d45c05b548 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:12:01 +0200 Subject: [PATCH 21/34] config: use nm_config_data_get_value_boolean() This removes duplicate parsing, but also makes all places use the same str-to-boolean convention. --- src/settings/plugins/ifnet/plugin.c | 24 +++++++----------------- src/settings/plugins/ifupdown/plugin.c | 18 +++++------------- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/settings/plugins/ifnet/plugin.c b/src/settings/plugins/ifnet/plugin.c index 3f7a7db9e9..552a480994 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -82,16 +82,9 @@ ignore_cb(NMSettingsConnectionInterface * connection, static gboolean is_managed_plugin (void) { - char *result = NULL; - - result = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, - IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED); - if (result) { - gboolean ret = is_true (result); - g_free (result); - return ret; - } - return IFNET_MANAGE_WELL_KNOWN_DEFAULT; + return nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, + IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED, + IFNET_MANAGE_WELL_KNOWN_DEFAULT); } static void @@ -199,8 +192,7 @@ reload_connections (NMSystemConfigInterface *config) SCPluginIfnet *self = SC_PLUGIN_IFNET (config); SCPluginIfnetPrivate *priv = SC_PLUGIN_IFNET_GET_PRIVATE (self); GList *conn_names = NULL, *n_iter = NULL; - gboolean auto_refresh = FALSE; - char *str_auto_refresh; + gboolean auto_refresh; GError *error = NULL; /* save names for removing unused connections */ @@ -217,11 +209,9 @@ reload_connections (NMSystemConfigInterface *config) nm_log_info (LOGD_SETTINGS, "Loading connections"); - str_auto_refresh = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, - IFNET_KEY_FILE_GROUP, "auto_refresh"); - if (str_auto_refresh && is_true (str_auto_refresh)) - auto_refresh = TRUE; - g_free (str_auto_refresh); + auto_refresh = nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, + IFNET_KEY_FILE_GROUP, "auto_refresh", + FALSE); new_connections = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c index 081de24a57..0252a1ba47 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -302,7 +302,6 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) SCPluginIfupdownPrivate *priv = SC_PLUGIN_IFUPDOWN_GET_PRIVATE (self); GHashTable *auto_ifaces; if_block *block = NULL; - char *value; GList *keys, *iter; GHashTableIter con_iter; const char *block_name; @@ -328,8 +327,6 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) } else g_signal_connect (priv->client, "uevent", G_CALLBACK (handle_uevent), self); - priv->unmanage_well_known = IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT; - /* Read in all the interfaces */ ifparser_init (ENI_INTERFACES_FILE, 0); block = ifparser_getfirst (); @@ -419,15 +416,10 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) g_hash_table_destroy (auto_ifaces); /* Check the config file to find out whether to manage interfaces */ - value = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, - IFUPDOWN_KEY_FILE_GROUP, IFUPDOWN_KEY_FILE_KEY_MANAGED); - if (value) { - gboolean manage_well_known; - - manage_well_known = !strcmp (value, "true") || !strcmp (value, "1"); - priv->unmanage_well_known = !manage_well_known; - g_free (value); - } + priv->unmanage_well_known = !nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, + IFUPDOWN_KEY_FILE_GROUP, + IFUPDOWN_KEY_FILE_KEY_MANAGED, + !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); nm_log_info (LOGD_SETTINGS, "management mode: %s", priv->unmanage_well_known ? "unmanaged" : "managed"); /* Add well-known interfaces */ @@ -517,7 +509,7 @@ sc_plugin_ifupdown_init (SCPluginIfupdown *plugin) static void GObject__get_property (GObject *object, guint prop_id, - GValue *value, GParamSpec *pspec) + GValue *value, GParamSpec *pspec) { switch (prop_id) { case NM_SYSTEM_CONFIG_INTERFACE_PROP_NAME: From 6d6ab20be01cdda8d3126df78504edabc27bf8bd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:24:28 +0200 Subject: [PATCH 22/34] config: add defines for keyfile groups to "nm-config.h" Some plugins had their local defines for the name of the sections and keys in NMConfig. Move those defines to "nm-config.h". Usually plugins make use of code in core, but not the other way round. Defining the names inside "nm-config.h" is no violation of that because the config section names are anyway not local to the plugin, but global in the shared name-space with other settings. For example, another plugins shouldn't reuse the section "ifnet". For that reason, it is correct and consistent to move these defines to "nm-config.h". We don't use those names in core, we merely signal their existance. --- src/nm-config-data.c | 20 +++++++-------- src/nm-config.c | 34 ++++++++++++------------- src/nm-config.h | 14 ++++++++++ src/settings/plugins/ifnet/net_parser.h | 1 - src/settings/plugins/ifnet/plugin.c | 5 ++-- src/settings/plugins/ifupdown/plugin.c | 6 ++--- src/settings/plugins/keyfile/plugin.c | 8 +++--- 7 files changed, 49 insertions(+), 39 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 0b94fecc2e..6e51c05251 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -284,8 +284,8 @@ _get_connection_infos (GKeyFile *keyfile) ngroups = 0; else if (ngroups > 0) { for (i = 0, j = 0; i < ngroups; i++) { - if (g_str_has_prefix (groups[i], "connection")) { - if (groups[i][STRLEN ("connection")] == '\0') + if (g_str_has_prefix (groups[i], NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION)) { + if (groups[i][STRLEN (NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION)] == '\0') connection_tag = groups[i]; else groups[j++] = groups[i]; @@ -492,24 +492,24 @@ constructed (GObject *object) priv->connection_infos = _get_connection_infos (priv->keyfile); - priv->connectivity.uri = g_key_file_get_value (priv->keyfile, "connectivity", "uri", NULL); - priv->connectivity.response = g_key_file_get_value (priv->keyfile, "connectivity", "response", NULL); + priv->connectivity.uri = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); + priv->connectivity.response = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); /* On missing config value, fallback to 300. On invalid value, disable connectivity checking by setting * the interval to zero. */ - interval = g_key_file_get_value (priv->keyfile, "connectivity", "interval", NULL); + interval = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); priv->connectivity.interval = interval ? _nm_utils_ascii_str_to_int64 (interval, 10, 0, G_MAXUINT, 0) : NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL; g_free (interval); - priv->dns_mode = g_key_file_get_value (priv->keyfile, "main", "dns", NULL); - priv->rc_manager = g_key_file_get_value (priv->keyfile, "main", "rc-manager", NULL); + priv->dns_mode = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); + priv->rc_manager = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL); - priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, "main", "ignore-carrier"); - priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, "main", "assume-ipv6ll-only"); + priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier"); + priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only"); - priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, "main", "no-auto-default"); + priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default"); G_OBJECT_CLASS (nm_config_data_parent_class)->constructed (object); } diff --git a/src/nm-config.c b/src/nm-config.c index d92d4628ec..8a948f64a9 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -478,8 +478,8 @@ _sort_groups_cmp (const char **pa, const char **pb, gpointer dummy) a = *pa; b = *pb; - a_is_connection = g_str_has_prefix (a, "connection"); - b_is_connection = g_str_has_prefix (b, "connection"); + a_is_connection = g_str_has_prefix (a, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); + b_is_connection = g_str_has_prefix (b, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); if (a_is_connection != b_is_connection) { /* one is a [connection*] entry, the other not. We sort [connection*] entires @@ -771,23 +771,23 @@ read_entire_config (const NMConfigCmdLineOptions *cli, * config files. */ if (cli && cli->plugins && cli->plugins[0]) - g_key_file_set_value (keyfile, "main", "plugins", cli->plugins); - plugins_tmp = g_key_file_get_string_list (keyfile, "main", "plugins", NULL, NULL); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", cli->plugins); + plugins_tmp = g_key_file_get_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NULL, NULL); if (!plugins_tmp) { if (STRLEN (CONFIG_PLUGINS_DEFAULT) > 0) - g_key_file_set_value (keyfile, "main", "plugins", CONFIG_PLUGINS_DEFAULT); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", CONFIG_PLUGINS_DEFAULT); } else g_strfreev (plugins_tmp); if (cli && cli->configure_and_quit) - g_key_file_set_value (keyfile, "main", "configure-and-quit", "true"); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", "true"); if (cli && cli->connectivity_uri && cli->connectivity_uri[0]) - g_key_file_set_value (keyfile, "connectivity", "uri", cli->connectivity_uri); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); if (cli && cli->connectivity_interval >= 0) - g_key_file_set_integer (keyfile, "connectivity", "interval", cli->connectivity_interval); + g_key_file_set_integer (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", cli->connectivity_interval); if (cli && cli->connectivity_response && cli->connectivity_response[0]) - g_key_file_set_value (keyfile, "connectivity", "response", cli->connectivity_response); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); *out_config_main_file = o_config_main_file; *out_config_description = o_config_description; @@ -1004,22 +1004,22 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) else priv->no_auto_default_file = g_strdup (DEFAULT_NO_AUTO_DEFAULT_FILE); - priv->plugins = g_key_file_get_string_list (keyfile, "main", "plugins", NULL, NULL); + priv->plugins = g_key_file_get_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NULL, NULL); if (!priv->plugins) priv->plugins = g_new0 (char *, 1); - priv->monitor_connection_files = nm_config_keyfile_get_boolean (keyfile, "main", "monitor-connection-files", FALSE); + priv->monitor_connection_files = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "monitor-connection-files", FALSE); - priv->auth_polkit = nm_config_keyfile_get_boolean (keyfile, "main", "auth-polkit", NM_CONFIG_DEFAULT_AUTH_POLKIT); + priv->auth_polkit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "auth-polkit", NM_CONFIG_DEFAULT_AUTH_POLKIT); - priv->dhcp_client = g_key_file_get_value (keyfile, "main", "dhcp", NULL); + priv->dhcp_client = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); - priv->log_level = g_key_file_get_value (keyfile, "logging", "level", NULL); - priv->log_domains = g_key_file_get_value (keyfile, "logging", "domains", NULL); + priv->log_level = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); + priv->log_domains = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); - priv->debug = g_key_file_get_value (keyfile, "main", "debug", NULL); + priv->debug = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); - priv->configure_and_quit = nm_config_keyfile_get_boolean (keyfile, "main", "configure-and-quit", FALSE); + priv->configure_and_quit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", FALSE); no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); diff --git a/src/nm-config.h b/src/nm-config.h index 13c4eeca18..67d375c141 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -48,6 +48,20 @@ G_BEGIN_DECLS #define NM_CONFIG_KEYFILE_LIST_SEPARATOR ',' +#define NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION "connection" + +#define NM_CONFIG_KEYFILE_GROUP_MAIN "main" +#define NM_CONFIG_KEYFILE_GROUP_LOGGING "logging" +#define NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY "connectivity" + +#define NM_CONFIG_KEYFILE_GROUP_KEYFILE "keyfile" +#define NM_CONFIG_KEYFILE_GROUP_IFUPDOWN "ifupdown" +#define NM_CONFIG_KEYFILE_GROUP_IFNET "ifnet" + +#define NM_CONFIG_KEYFILE_KEY_IFNET_AUTO_REFRESH "auto_refresh" +#define NM_CONFIG_KEYFILE_KEY_IFNET_MANAGED "managed" +#define NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED "managed" + typedef struct NMConfigCmdLineOptions NMConfigCmdLineOptions; struct _NMConfig { diff --git a/src/settings/plugins/ifnet/net_parser.h b/src/settings/plugins/ifnet/net_parser.h index 005207adfe..d10979cca6 100644 --- a/src/settings/plugins/ifnet/net_parser.h +++ b/src/settings/plugins/ifnet/net_parser.h @@ -25,7 +25,6 @@ #include #define CONF_NET_FILE SYSCONFDIR "/conf.d/net" -#define IFNET_KEY_FILE_GROUP "ifnet" gboolean ifnet_init (gchar * config_file); void ifnet_destroy (void); diff --git a/src/settings/plugins/ifnet/plugin.c b/src/settings/plugins/ifnet/plugin.c index 552a480994..550f31a5fc 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -46,7 +46,6 @@ #define IFNET_PLUGIN_NAME_PRINT "ifnet" #define IFNET_PLUGIN_INFO "(C) 1999-2010 Gentoo Foundation, Inc. To report bugs please use bugs.gentoo.org with [networkmanager] or [qiaomuf] prefix." #define IFNET_MANAGE_WELL_KNOWN_DEFAULT TRUE -#define IFNET_KEY_FILE_KEY_MANAGED "managed" typedef struct { GHashTable *connections; /* uuid::connection */ @@ -83,7 +82,7 @@ static gboolean is_managed_plugin (void) { return nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, - IFNET_KEY_FILE_GROUP, IFNET_KEY_FILE_KEY_MANAGED, + NM_CONFIG_KEYFILE_GROUP_IFNET, NM_CONFIG_KEYFILE_KEY_IFNET_MANAGED, IFNET_MANAGE_WELL_KNOWN_DEFAULT); } @@ -210,7 +209,7 @@ reload_connections (NMSystemConfigInterface *config) nm_log_info (LOGD_SETTINGS, "Loading connections"); auto_refresh = nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, - IFNET_KEY_FILE_GROUP, "auto_refresh", + NM_CONFIG_KEYFILE_GROUP_IFNET, NM_CONFIG_KEYFILE_KEY_IFNET_AUTO_REFRESH, FALSE); new_connections = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c index 0252a1ba47..c3cb180b58 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -59,8 +59,6 @@ #define IFUPDOWN_PLUGIN_NAME "ifupdown" #define IFUPDOWN_PLUGIN_INFO "(C) 2008 Canonical Ltd. To report bugs please use the NetworkManager mailing list." -#define IFUPDOWN_KEY_FILE_GROUP "ifupdown" -#define IFUPDOWN_KEY_FILE_KEY_MANAGED "managed" #define IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT TRUE /* #define ALWAYS_UNMANAGE TRUE */ @@ -417,8 +415,8 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) /* Check the config file to find out whether to manage interfaces */ priv->unmanage_well_known = !nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, - IFUPDOWN_KEY_FILE_GROUP, - IFUPDOWN_KEY_FILE_KEY_MANAGED, + NM_CONFIG_KEYFILE_GROUP_IFUPDOWN, + NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED, !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); nm_log_info (LOGD_SETTINGS, "management mode: %s", priv->unmanage_well_known ? "unmanaged" : "managed"); diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index 5ca0374b3d..f12ffd90e1 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -323,8 +323,8 @@ config_changed_cb (NMConfig *config, { gs_free char *old_value = NULL, *new_value = NULL; - old_value = nm_config_data_get_value (old_data, "keyfile", "unmanaged-devices"); - new_value = nm_config_data_get_value (config_data, "keyfile", "unmanaged-devices"); + old_value = nm_config_data_get_value (old_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); + new_value = nm_config_data_get_value (config_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); if (g_strcmp0 (old_value, new_value) != 0) g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); @@ -526,7 +526,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (config); gs_free char *value = NULL; - value = nm_config_data_get_value (nm_config_get_data (priv->config), "keyfile", "unmanaged-devices"); + value = nm_config_data_get_value (nm_config_get_data (priv->config), NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); return nm_match_spec_split (value); } @@ -647,7 +647,7 @@ nm_settings_keyfile_plugin_new (void) priv->config = g_object_ref (nm_config_get ()); value = nm_config_data_get_value (nm_config_get_data (priv->config), - "keyfile", "hostname"); + NM_CONFIG_KEYFILE_GROUP_KEYFILE, "hostname"); if (value) { nm_log_warn (LOGD_SETTINGS, "keyfile: 'hostname' option is deprecated and has no effect"); g_free (value); From b506c29fe1374c00b2886f2263a632da0bc17b6b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:59:18 +0200 Subject: [PATCH 23/34] config: log configuration at startup and on reload --- src/main.c | 1 + src/nm-config-data.c | 89 ++++++++++++++++++++++++++++++++++ src/nm-config-data.h | 2 + src/nm-config.c | 1 + src/tests/config/test-config.c | 2 + 5 files changed, 95 insertions(+) diff --git a/src/main.c b/src/main.c index 1a8014f053..dc6973446f 100644 --- a/src/main.c +++ b/src/main.c @@ -412,6 +412,7 @@ main (int argc, char *argv[]) dbus_glib_global_set_disable_legacy_property_access (); nm_log_info (LOGD_CORE, "Read config: %s", nm_config_data_get_config_description (nm_config_get_data (config))); + nm_config_data_log (nm_config_get_data (config), "CONFIG: "); nm_log_dbg (LOGD_CORE, "WEXT support is %s", #if HAVE_WEXT "enabled" diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 6e51c05251..b528612fa4 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -29,6 +29,7 @@ #include "nm-core-internal.h" #include "nm-keyfile-internal.h" #include "nm-macros-internal.h" +#include "nm-logging.h" typedef struct { char *group_name; @@ -212,6 +213,94 @@ nm_config_data_get_assume_ipv6ll_only (const NMConfigData *self, NMDevice *devic /************************************************************************/ +static int +_nm_config_data_log_sort (const char **pa, const char **pb, gpointer dummy) +{ + gboolean a_is_connection, b_is_connection; + const char *a = *pa; + const char *b = *pb; + + /* we sort connection groups before intern groups (to the end). */ + a_is_connection = a && g_str_has_prefix (a, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); + b_is_connection = b && g_str_has_prefix (b, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); + + if (a_is_connection && b_is_connection) { + /* if both are connection groups, we want the explicit [connection] group first. */ + a_is_connection = a[STRLEN (NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION)] == '\0'; + b_is_connection = b[STRLEN (NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION)] == '\0'; + + if (a_is_connection != b_is_connection) { + if (a_is_connection) + return -1; + return 1; + } + /* the sections are ordered lowest-priority first. Reverse their order. */ + return pa < pb ? 1 : -1; + } + if (a_is_connection && !b_is_connection) + return 1; + if (b_is_connection && !a_is_connection) + return -1; + + /* no reordering. */ + return 0; +} + +void +nm_config_data_log (const NMConfigData *self, const char *prefix) +{ + NMConfigDataPrivate *priv; + gs_strfreev char **groups = NULL; + gsize ngroups; + guint g, k; + + g_return_if_fail (NM_IS_CONFIG_DATA (self)); + + if (!nm_logging_enabled (LOGL_DEBUG, LOGD_CORE)) + return; + + if (!prefix) + prefix = ""; + +#define _LOG(...) _nm_log (LOGL_DEBUG, LOGD_CORE, 0, "%s"_NM_UTILS_MACRO_FIRST(__VA_ARGS__), prefix _NM_UTILS_MACRO_REST (__VA_ARGS__)) + + priv = NM_CONFIG_DATA_GET_PRIVATE (self); + + groups = g_key_file_get_groups (priv->keyfile, &ngroups); + if (!groups) + ngroups = 0; + + if (groups && groups[0]) { + g_qsort_with_data (groups, ngroups, + sizeof (char *), + (GCompareDataFunc) _nm_config_data_log_sort, + NULL); + } + + _LOG ("config-data[%p]: %lu groups", self, (unsigned long) ngroups); + + for (g = 0; g < ngroups; g++) { + const char *group = groups[g]; + gs_strfreev char **keys = NULL; + + _LOG (""); + _LOG ("[%s]", group); + + keys = g_key_file_get_keys (priv->keyfile, group, NULL, NULL); + for (k = 0; keys && keys[k]; k++) { + const char *key = keys[k]; + gs_free char *value = NULL; + + value = g_key_file_get_value (priv->keyfile, group, key, NULL); + _LOG (" %s=%s", key, value); + } + } + +#undef _LOG +} + +/************************************************************************/ + char * nm_config_data_get_connection_default (const NMConfigData *self, const char *property, diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 59f3c413e1..ada1d3c931 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -81,6 +81,8 @@ NMConfigData *nm_config_data_new_update_no_auto_default (const NMConfigData *bas NMConfigChangeFlags nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data); +void nm_config_data_log (const NMConfigData *config_data, const char *prefix); + const char *nm_config_data_get_config_main_file (const NMConfigData *config_data); const char *nm_config_data_get_config_description (const NMConfigData *config_data); diff --git a/src/nm-config.c b/src/nm-config.c index 8a948f64a9..dcf2ab207c 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -933,6 +933,7 @@ _set_config_data (NMConfig *self, NMConfigData *new_data, int signal) if (new_data) { nm_log_info (LOGD_CORE, "config: update %s (%s)", nm_config_data_get_config_description (new_data), (log_str = nm_config_change_flags_to_string (changes))); + nm_config_data_log (new_data, "CONFIG: "); priv->config_data = new_data; } else if (had_new_data) nm_log_info (LOGD_CORE, "config: signal %s (no changes from disk)", (log_str = nm_config_change_flags_to_string (changes))); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 995511f519..aca6e83124 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -356,6 +356,8 @@ test_config_confdir (void) g_assert_cmpstr (value, ==, "VAL5"); g_free (value); + nm_config_data_log (nm_config_get_data_orig (config), ">>> TEST: "); + g_object_unref (config); } From 0abb502ff389704e5979c6b16f88129ec5609be6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 12:06:14 +0200 Subject: [PATCH 24/34] config: fix usage of g_key_file_get_value() vs. g_key_file_get_string() g_key_file_get_value() returns the raw value as stored in the file. When accessing a string value, in most cases it is correct to use g_key_file_get_string() instead. When working with internals, such as comparing two keyfiles for equality, g_key_file_get_value() is correct. When parsing booleans, we parse it based on the raw value. Fix the usages. This is a change in behavior if the config file contained unusual strings. --- src/nm-config-data.c | 41 ++++++++++++++++++++++------------------- src/nm-config.c | 23 ++++++++++++++--------- src/nm-config.h | 2 +- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index b528612fa4..93ef6bd677 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -111,10 +111,12 @@ nm_config_data_get_config_description (const NMConfigData *self) char * nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key) { - g_return_val_if_fail (self, NULL); + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), NULL); g_return_val_if_fail (group && *group, NULL); g_return_val_if_fail (key && *key, NULL); + /* nm_config_data_get_value() translates to g_key_file_get_string(), because we want + * to use the string representation, not the (raw) GKeyFile value. */ return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); } @@ -124,7 +126,12 @@ nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, c char *str; gint value = default_value; - str = nm_config_data_get_value (self, group, key); + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), default_value); + g_return_val_if_fail (group && *group, default_value); + g_return_val_if_fail (key && *key, default_value); + + /* when parsing the boolean, base it on the raw value from g_key_file_get_value(). */ + str = g_key_file_get_value (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); if (str) { value = nm_config_parse_boolean (str, default_value); g_free (str); @@ -322,7 +329,7 @@ nm_config_data_get_connection_default (const NMConfigData *self, char *value; gboolean match; - value = g_key_file_get_value (priv->keyfile, connection_info->group_name, property, NULL); + value = g_key_file_get_string (priv->keyfile, connection_info->group_name, property, NULL); if (!value && !connection_info->stop_match) continue; @@ -340,17 +347,13 @@ nm_config_data_get_connection_default (const NMConfigData *self, static void _get_connection_info_init (ConnectionInfo *connection_info, GKeyFile *keyfile, char *group) { - char *value; - /* pass ownership of @group on... */ connection_info->group_name = group; - value = g_key_file_get_value (keyfile, group, "match-device", NULL); - if (value) { - connection_info->match_device.has = TRUE; - connection_info->match_device.spec = nm_match_spec_split (value); - g_free (value); - } + connection_info->match_device.spec = nm_config_get_device_match_spec (keyfile, + group, + "match-device", + &connection_info->match_device.has); connection_info->stop_match = nm_config_keyfile_get_boolean (keyfile, group, "stop-match", FALSE); } @@ -581,24 +584,24 @@ constructed (GObject *object) priv->connection_infos = _get_connection_infos (priv->keyfile); - priv->connectivity.uri = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); - priv->connectivity.response = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); + priv->connectivity.uri = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); + priv->connectivity.response = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); /* On missing config value, fallback to 300. On invalid value, disable connectivity checking by setting * the interval to zero. */ - interval = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); + interval = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); priv->connectivity.interval = interval ? _nm_utils_ascii_str_to_int64 (interval, 10, 0, G_MAXUINT, 0) : NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL; g_free (interval); - priv->dns_mode = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); - priv->rc_manager = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL); + priv->dns_mode = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); + priv->rc_manager = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL); - priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier"); - priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only"); + priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier", NULL); + priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only", NULL); - priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default"); + priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default", NULL); G_OBJECT_CLASS (nm_config_data_parent_class)->constructed (object); } diff --git a/src/nm-config.c b/src/nm-config.c index dcf2ab207c..29b6cd6221 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -780,14 +780,14 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_strfreev (plugins_tmp); if (cli && cli->configure_and_quit) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", "true"); + g_key_file_set_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", TRUE); if (cli && cli->connectivity_uri && cli->connectivity_uri[0]) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); if (cli && cli->connectivity_interval >= 0) g_key_file_set_integer (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", cli->connectivity_interval); if (cli && cli->connectivity_response && cli->connectivity_response[0]) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); *out_config_main_file = o_config_main_file; *out_config_description = o_config_description; @@ -795,11 +795,16 @@ read_entire_config (const NMConfigCmdLineOptions *cli, } GSList * -nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key) +nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key) { gs_free char *value = NULL; - value = g_key_file_get_string ((GKeyFile *) keyfile, group, key, NULL); + /* nm_match_spec_split() already supports full escaping and is basically + * a modified version of g_key_file_parse_value_as_string(). So we first read + * the raw value (g_key_file_get_value()), and do the parsing ourselves. */ + value = g_key_file_get_value ((GKeyFile *) keyfile, group, key, NULL); + if (out_has_key) + *out_has_key = !!value; return nm_match_spec_split (value); } @@ -1013,12 +1018,12 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->auth_polkit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "auth-polkit", NM_CONFIG_DEFAULT_AUTH_POLKIT); - priv->dhcp_client = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); + priv->dhcp_client = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); - priv->log_level = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); - priv->log_domains = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); + priv->log_level = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); + priv->log_domains = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); - priv->debug = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); + priv->debug = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); priv->configure_and_quit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", FALSE); diff --git a/src/nm-config.h b/src/nm-config.h index 67d375c141..448e2f4e2d 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -121,7 +121,7 @@ void nm_config_keyfile_set_string_list (GKeyFile *keyfile, const char *key, const char *const* strv, gssize len); -GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key); +GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key); G_END_DECLS From 7e94785f28a822bfd95acf2f2551814f84dd3d79 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Jun 2015 17:27:15 +0200 Subject: [PATCH 25/34] config: ensure nm_config_get_plugins() to return stripped values --- src/nm-config.c | 3 ++- src/settings/nm-settings.c | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 29b6cd6221..b59bf5c203 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1010,7 +1010,8 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) else priv->no_auto_default_file = g_strdup (DEFAULT_NO_AUTO_DEFAULT_FILE); - priv->plugins = g_key_file_get_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NULL, NULL); + priv->plugins = _nm_utils_strv_cleanup (g_key_file_get_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NULL, NULL), + TRUE, TRUE, TRUE); if (!priv->plugins) priv->plugins = g_new0 (char *, 1); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 68b9dd2659..9f7b47d9bc 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -769,17 +769,13 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) GModule *plugin; gs_free char *full_name = NULL; gs_free char *path = NULL; - gs_free char *pname = NULL; + const char *pname; GObject *obj; GObject * (*factory_func) (void); struct stat st; int errsv; - pname = g_strdup (*iter); - g_strstrip (pname); - - if (!*pname) - continue; + pname = *iter; if (!*pname || strchr (pname, '/')) { LOG (LOGL_WARN, "ignore invalid plugin \"%s\"", pname); From 11c0e107b9b903b13c741448eae68b443e98ad2e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Jun 2015 23:43:29 +0200 Subject: [PATCH 26/34] config: add config utility accessors --- src/nm-config-data.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/nm-config-data.h | 5 +++++ 2 files changed, 56 insertions(+) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 93ef6bd677..424fe3b7fb 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -108,6 +108,15 @@ nm_config_data_get_config_description (const NMConfigData *self) return NM_CONFIG_DATA_GET_PRIVATE (self)->config_description; } +gboolean +nm_config_data_has_group (const NMConfigData *self, const char *group) +{ + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), FALSE); + g_return_val_if_fail (group && *group, FALSE); + + return g_key_file_has_group (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group); +} + char * nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key) { @@ -120,6 +129,21 @@ nm_config_data_get_value (const NMConfigData *self, const char *group, const cha return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); } +gboolean +nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key) +{ + gs_free char *value = NULL; + + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), FALSE); + g_return_val_if_fail (group && *group, FALSE); + g_return_val_if_fail (key && *key, FALSE); + + /* nm_config_data_get_value() translates to g_key_file_get_string(), because we want + * to use the string representation, not the (raw) GKeyFile value. */ + value = g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); + return !!value; +} + gint nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, const char *key, gint default_value) { @@ -220,6 +244,33 @@ nm_config_data_get_assume_ipv6ll_only (const NMConfigData *self, NMDevice *devic /************************************************************************/ +/** + * nm_config_data_get_groups: + * @self: the #NMConfigData instance + * + * Returns: (transfer-full): the list of groups in the configuration. The order + * of the section is undefined, as the configuration gets merged from multiple + * sources. + */ +char ** +nm_config_data_get_groups (const NMConfigData *self) +{ + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), NULL); + + return g_key_file_get_groups (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, NULL); +} + +char ** +nm_config_data_get_keys (const NMConfigData *self, const char *group) +{ + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), NULL); + g_return_val_if_fail (group && *group, NULL); + + return g_key_file_get_keys (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, NULL, NULL); +} + +/************************************************************************/ + static int _nm_config_data_log_sort (const char **pa, const char **pb, gpointer dummy) { diff --git a/src/nm-config-data.h b/src/nm-config-data.h index ada1d3c931..54a20e20ff 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -86,6 +86,8 @@ void nm_config_data_log (const NMConfigData *config_data, const char *prefix); const char *nm_config_data_get_config_main_file (const NMConfigData *config_data); const char *nm_config_data_get_config_description (const NMConfigData *config_data); +gboolean nm_config_data_has_group (const NMConfigData *self, const char *group); +gboolean nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key); char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key); gint nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, const char *key, gint default_value); @@ -106,6 +108,9 @@ char *nm_config_data_get_connection_default (const NMConfigData *self, const char *property, NMDevice *device); +char **nm_config_data_get_groups (const NMConfigData *self); +char **nm_config_data_get_keys (const NMConfigData *self, const char *group); + G_END_DECLS #endif /* NM_CONFIG_DATA_H */ From 7498b670a828d335e177003c9d00d68802f52561 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:06:48 +0200 Subject: [PATCH 27/34] utils: strip whitespace for device spec in nm_match_spec_split() Via escape sequences, the user still can specify trailing and leading white spaces: such as "\s \s" will result in 3 spaces. --- src/NetworkManagerUtils.c | 56 ++++++++++++++++++++++++++++++++------- src/tests/test-general.c | 8 +++++- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 5a129215f7..24a6f952cb 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1313,11 +1313,32 @@ nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels) return match; } +/** + * nm_match_spec_split: + * @value: the string of device specs + * + * Splits the specs from the string and returns them as individual + * entires in a #GSList. + * + * It does not validate any specs, it basically just does a special + * strsplit with ',' or ';' as separators and supporting '\\' as + * escape character. + * + * Leading and trailing spaces of each entry are removed. But the user + * can preserve them by specifying "\\s has 2 leading" or "has 2 trailing \\s". + * + * Specs can have a qualifier like "interface-name:". We still don't strip + * any whitespace after the colon, so "interface-name: X" matches an interface + * named " X". + * + * Returns: (transfer-full): the list of device specs. + */ GSList * nm_match_spec_split (const char *value) { char *string_value, *p, *q0, *q; GSList *pieces = NULL; + int trailing_ws; if (!value || !*value) return NULL; @@ -1328,7 +1349,13 @@ nm_match_spec_split (const char *value) string_value = g_new (gchar, strlen (value) + 1); p = (gchar *) value; + + /* skip over leading whitespace */ + while (g_ascii_isspace (*p)) + p++; + q0 = q = string_value; + trailing_ws = 0; while (*p) { if (*p == '\\') { p++; @@ -1360,23 +1387,34 @@ nm_match_spec_split (const char *value) } break; } + if (*p == '\0') + break; + p++; + trailing_ws = 0; } else { *q = *p; - if (NM_IN_SET (*p, ',', ';')) { - if (q0 < q) - pieces = g_slist_prepend (pieces, g_strndup (q0, q - q0)); + if (*p == '\0') + break; + if (g_ascii_isspace (*p)) { + trailing_ws++; + p++; + } else if (NM_IN_SET (*p, ',', ';')) { + if (q0 < q - trailing_ws) + pieces = g_slist_prepend (pieces, g_strndup (q0, (q - q0) - trailing_ws)); q0 = q + 1; - } + p++; + trailing_ws = 0; + while (g_ascii_isspace (*p)) + p++; + } else + p++; } - if (*p == '\0') - break; q++; - p++; } *q = '\0'; - if (q0 < q) - pieces = g_slist_prepend (pieces, g_strndup (q0, q - q0)); + if (q0 < q - trailing_ws) + pieces = g_slist_prepend (pieces, g_strndup (q0, (q - q0) - trailing_ws)); g_free (string_value); return g_slist_reverse (pieces); } diff --git a/src/tests/test-general.c b/src/tests/test-general.c index baea1d54e2..3397dd9715 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -680,6 +680,9 @@ test_match_spec_ifname (const char *spec_str, const char **matches, const char * g_assert (spec_str); specs = nm_match_spec_split (spec_str); + + /* also check the matches in the reverse order. They must yield the same result because + * matches are inclusive -- except "except:" which always wins. */ specs_reverse = g_slist_reverse (g_slist_copy (specs)); for (i = 0; matches && matches[i]; i++) { @@ -746,9 +749,12 @@ test_nm_match_spec_interface_name (void) test_match_spec_ifname ("interface-name:em\\;1,em\\,2,\\,,\\\\,,em\\\\x", S ("em;1", "em,2", ",", "\\", "em\\x"), NULL); - test_match_spec_ifname (" , interface-name:a, ,", + test_match_spec_ifname ("\\s\\s,\\sinterface-name:a,\\s,", S (" ", " ", " interface-name:a"), NULL); + test_match_spec_ifname (" aa ; bb ; cc\\;dd ;e , ; \t\\t , ", + S ("aa", "bb", "cc;dd", "e", "\t"), + NULL); #undef S } From 076ffda6f395b37b497929de0838688db8eba84a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:59:29 +0200 Subject: [PATCH 28/34] config: strip white space from configuration values https://bugzilla.gnome.org/show_bug.cgi?id=750659 https://bugzilla.redhat.com/show_bug.cgi?id=1229861 --- src/nm-config-data.c | 6 +++--- src/nm-config.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 424fe3b7fb..36fc388138 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -635,7 +635,7 @@ constructed (GObject *object) priv->connection_infos = _get_connection_infos (priv->keyfile); - priv->connectivity.uri = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); + priv->connectivity.uri = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL)); priv->connectivity.response = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); /* On missing config value, fallback to 300. On invalid value, disable connectivity checking by setting @@ -646,8 +646,8 @@ constructed (GObject *object) : NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL; g_free (interval); - priv->dns_mode = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); - priv->rc_manager = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL); + priv->dns_mode = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL)); + priv->rc_manager = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL)); priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier", NULL); priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only", NULL); diff --git a/src/nm-config.c b/src/nm-config.c index b59bf5c203..94993dabe1 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1019,10 +1019,10 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->auth_polkit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "auth-polkit", NM_CONFIG_DEFAULT_AUTH_POLKIT); - priv->dhcp_client = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); + priv->dhcp_client = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL)); - priv->log_level = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); - priv->log_domains = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); + priv->log_level = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL)); + priv->log_domains = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL)); priv->debug = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); From d3e21937834d94fa844a06b1158cf5f0411636b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 15:13:26 +0200 Subject: [PATCH 29/34] config: add NMConfigGetValueFlags argument to nm_config_data_get_value() In some cases we want the returned value to be stripped. In some cases, we want to read the raw value instead of the string parsed by GKeyFile. Add an flags argument to nm_config_data_get_value(). It is up to the caller to determine the exact meaning (and whether to strip). By adding the flags argument, the caller can get the desired behavior easier without having to workaround it afterwards. But more importantly, it becomes apparent that there are different ways to retrieve the value and the caller should decide on the details. --- src/nm-config-data.c | 12 ++++------- src/nm-config-data.h | 21 +++++++++++++++++-- src/nm-config.c | 28 +++++++++++++++++++++++++ src/nm-config.h | 4 ++++ src/settings/plugins/keyfile/plugin.c | 15 ++++++-------- src/tests/config/test-config.c | 30 +++++++++++++-------------- 6 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 36fc388138..bc68015770 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -118,19 +118,17 @@ nm_config_data_has_group (const NMConfigData *self, const char *group) } char * -nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key) +nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key, NMConfigGetValueFlags flags) { g_return_val_if_fail (NM_IS_CONFIG_DATA (self), NULL); g_return_val_if_fail (group && *group, NULL); g_return_val_if_fail (key && *key, NULL); - /* nm_config_data_get_value() translates to g_key_file_get_string(), because we want - * to use the string representation, not the (raw) GKeyFile value. */ - return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); + return nm_config_keyfile_get_value (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, flags); } gboolean -nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key) +nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key, NMConfigGetValueFlags flags) { gs_free char *value = NULL; @@ -138,9 +136,7 @@ nm_config_data_has_value (const NMConfigData *self, const char *group, const cha g_return_val_if_fail (group && *group, FALSE); g_return_val_if_fail (key && *key, FALSE); - /* nm_config_data_get_value() translates to g_key_file_get_string(), because we want - * to use the string representation, not the (raw) GKeyFile value. */ - value = g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); + value = nm_config_keyfile_get_value (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, flags); return !!value; } diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 54a20e20ff..bd26b9dfbb 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -45,6 +45,23 @@ G_BEGIN_DECLS #define NM_CONFIG_DATA_NO_AUTO_DEFAULT "no-auto-default" #define NM_CONFIG_DATA_DNS_MODE "dns" +typedef enum { /**/ + NM_CONFIG_GET_VALUE_NONE = 0, + + /* use g_key_file_get_value() instead of g_key_file_get_string(). */ + NM_CONFIG_GET_VALUE_RAW = (1LL << 0), + + /* strip whitespaces */ + NM_CONFIG_GET_VALUE_STRIP = (1LL << 1), + + /* if the returned string would be the empty word, return NULL. */ + NM_CONFIG_GET_VALUE_NO_EMPTY = (1LL << 2), + + /* special flag to read device spec. You want to use this before passing the + * value to nm_match_spec_split(). */ + NM_CONFIG_GET_VALUE_TYPE_SPEC = NM_CONFIG_GET_VALUE_RAW, +} NMConfigGetValueFlags; + typedef enum { /*< flags >*/ NM_CONFIG_CHANGE_NONE = 0, @@ -87,8 +104,8 @@ const char *nm_config_data_get_config_main_file (const NMConfigData *config_data const char *nm_config_data_get_config_description (const NMConfigData *config_data); gboolean nm_config_data_has_group (const NMConfigData *self, const char *group); -gboolean nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key); -char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key); +gboolean nm_config_data_has_value (const NMConfigData *self, const char *group, const char *key, NMConfigGetValueFlags flags); +char *nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key, NMConfigGetValueFlags flags); gint nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, const char *key, gint default_value); const char *nm_config_data_get_connectivity_uri (const NMConfigData *config_data); diff --git a/src/nm-config.c b/src/nm-config.c index 94993dabe1..8655239287 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -156,6 +156,34 @@ nm_config_keyfile_get_boolean (GKeyFile *keyfile, return nm_config_parse_boolean (str, default_value); } +char * +nm_config_keyfile_get_value (GKeyFile *keyfile, + const char *section, + const char *key, + NMConfigGetValueFlags flags) +{ + char *value; + + if (NM_FLAGS_HAS (flags, NM_CONFIG_GET_VALUE_RAW)) + value = g_key_file_get_value (keyfile, section, key, NULL); + else + value = g_key_file_get_string (keyfile, section, key, NULL); + + if (!value) + return NULL; + + if (NM_FLAGS_HAS (flags, NM_CONFIG_GET_VALUE_STRIP)) + g_strstrip (value); + + if ( NM_FLAGS_HAS (flags, NM_CONFIG_GET_VALUE_NO_EMPTY) + && !*value) { + g_free (value); + return NULL; + } + + return value; +} + void nm_config_keyfile_set_string_list (GKeyFile *keyfile, const char *group, diff --git a/src/nm-config.h b/src/nm-config.h index 448e2f4e2d..20dbab388f 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -116,6 +116,10 @@ gint nm_config_keyfile_get_boolean (GKeyFile *keyfile, const char *section, const char *key, gint default_value); +char *nm_config_keyfile_get_value (GKeyFile *keyfile, + const char *section, + const char *key, + NMConfigGetValueFlags flags); void nm_config_keyfile_set_string_list (GKeyFile *keyfile, const char *group, const char *key, diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index f12ffd90e1..57b0d094fb 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -323,8 +323,8 @@ config_changed_cb (NMConfig *config, { gs_free char *old_value = NULL, *new_value = NULL; - old_value = nm_config_data_get_value (old_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); - new_value = nm_config_data_get_value (config_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); + old_value = nm_config_data_get_value (old_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices", NM_CONFIG_GET_VALUE_TYPE_SPEC); + new_value = nm_config_data_get_value (config_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices", NM_CONFIG_GET_VALUE_TYPE_SPEC); if (g_strcmp0 (old_value, new_value) != 0) g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); @@ -526,7 +526,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (config); gs_free char *value = NULL; - value = nm_config_data_get_value (nm_config_get_data (priv->config), NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); + value = nm_config_data_get_value (nm_config_get_data (priv->config), NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices", NM_CONFIG_GET_VALUE_TYPE_SPEC); return nm_match_spec_split (value); } @@ -639,19 +639,16 @@ nm_settings_keyfile_plugin_new (void) { static SCPluginKeyfile *singleton = NULL; SCPluginKeyfilePrivate *priv; - char *value; if (!singleton) { singleton = SC_PLUGIN_KEYFILE (g_object_new (SC_TYPE_PLUGIN_KEYFILE, NULL)); priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (singleton); priv->config = g_object_ref (nm_config_get ()); - value = nm_config_data_get_value (nm_config_get_data (priv->config), - NM_CONFIG_KEYFILE_GROUP_KEYFILE, "hostname"); - if (value) { + if (nm_config_data_has_value (nm_config_get_data_orig (priv->config), + NM_CONFIG_KEYFILE_GROUP_KEYFILE, "hostname", + NM_CONFIG_GET_VALUE_RAW)) nm_log_warn (LOGD_SETTINGS, "keyfile: 'hostname' option is deprecated and has no effect"); - g_free (value); - } } else g_object_ref (singleton); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index aca6e83124..97995afa7b 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -110,21 +110,21 @@ test_config_simple (void) g_assert_cmpstr (plugins[1], ==, "bar"); g_assert_cmpstr (plugins[2], ==, "baz"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "extra-key"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "extra-key", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "some value"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "no-key"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "extra-section", "no-key", NM_CONFIG_GET_VALUE_NONE); g_assert (!value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "no-section", "no-key"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "no-section", "no-key", NM_CONFIG_GET_VALUE_NONE); g_assert (!value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection", "ipv6.ip6_privacy"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection", "ipv6.ip6_privacy", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "0"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection.dev51", "ipv4.route-metric"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "connection.dev51", "ipv4.route-metric", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "51"); g_free (value); @@ -301,21 +301,21 @@ test_config_confdir (void) g_assert_cmpstr (plugins[3], ==, "one"); g_assert_cmpstr (plugins[4], ==, "two"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "extra"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "extra", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "hello"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "something"); /* not ",something" */ g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "a"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "a", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "90"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "b"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "b", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "10"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "c"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "order", "c", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "0"); g_free (value); @@ -336,23 +336,23 @@ test_config_confdir (void) ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "a,c"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "VAL2"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, NULL); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "vb,vb"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "VAL5"); g_free (value); From 0c6a011e344612e607758626e51077b6edda66f5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 15:33:13 +0200 Subject: [PATCH 30/34] config/trivial: add code comment --- src/nm-config-data.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index bc68015770..82c1deab11 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -376,6 +376,13 @@ nm_config_data_get_connection_default (const NMConfigData *self, char *value; gboolean match; + /* FIXME: Here we use g_key_file_get_string(). This should be in sync with what keyfile-reader + * does. + * + * Unfortunately that is currently not possible because keyfile-reader does the two steps + * string_to_value(keyfile_to_string(keyfile)) in one. Optimally, keyfile library would + * expose both functions, and we would return here keyfile_to_string(keyfile). + * The caller then could convert the string to the proper value via string_to_value(value). */ value = g_key_file_get_string (priv->keyfile, connection_info->group_name, property, NULL); if (!value && !connection_info->stop_match) continue; From 93e4a8d10219fe431abe976a054b5a63c552aadf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 14:02:31 +0200 Subject: [PATCH 31/34] libnm: expose strv utils function in internal header nm-core-internal.h --- libnm-core/nm-core-internal.h | 6 ++++++ libnm-core/nm-utils-private.h | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index d7cb47bf29..47a0dc53d1 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -130,6 +130,12 @@ char ** _nm_utils_strsplit_set (const char *str, const char *delimiters, int max_tokens); +GSList * _nm_utils_strv_to_slist (char **strv); +char ** _nm_utils_slist_to_strv (GSList *slist); + +GPtrArray * _nm_utils_strv_to_ptrarray (char **strv); +char ** _nm_utils_ptrarray_to_strv (GPtrArray *ptrarray); + #define NM_UTILS_UUID_TYPE_LEGACY 0 #define NM_UTILS_UUID_TYPE_VARIANT3 1 diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h index a337a8ad07..68aaaa1c80 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -45,12 +45,6 @@ GVariant * _nm_utils_bytes_to_dbus (const GValue *prop_value); void _nm_utils_bytes_from_dbus (GVariant *dbus_value, GValue *prop_value); -GSList * _nm_utils_strv_to_slist (char **strv); -char ** _nm_utils_slist_to_strv (GSList *slist); - -GPtrArray * _nm_utils_strv_to_ptrarray (char **strv); -char ** _nm_utils_ptrarray_to_strv (GPtrArray *ptrarray); - char * _nm_utils_hwaddr_canonical_or_invalid (const char *mac, gssize length); #endif From ed632207cd7623061a292af73e35659d31a7949d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 14:08:51 +0200 Subject: [PATCH 32/34] libnm: add @deep_copy argument to _nm_utils_strv_to_slist() and _nm_utils_slist_to_strv() --- libnm-core/nm-core-internal.h | 4 ++-- libnm-core/nm-setting-8021x.c | 12 ++++++------ libnm-core/nm-setting-connection.c | 4 ++-- libnm-core/nm-setting-wireless-security.c | 12 ++++++------ libnm-core/nm-setting-wireless.c | 4 ++-- libnm-core/nm-utils.c | 22 ++++++++++++++++------ 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 47a0dc53d1..f05528b3e4 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -130,8 +130,8 @@ char ** _nm_utils_strsplit_set (const char *str, const char *delimiters, int max_tokens); -GSList * _nm_utils_strv_to_slist (char **strv); -char ** _nm_utils_slist_to_strv (GSList *slist); +GSList * _nm_utils_strv_to_slist (char **strv, gboolean deep_copy); +char ** _nm_utils_slist_to_strv (GSList *slist, gboolean deep_copy); GPtrArray * _nm_utils_strv_to_ptrarray (char **strv); char ** _nm_utils_ptrarray_to_strv (GPtrArray *ptrarray); diff --git a/libnm-core/nm-setting-8021x.c b/libnm-core/nm-setting-8021x.c index 8024624a2d..244cf552d8 100644 --- a/libnm-core/nm-setting-8021x.c +++ b/libnm-core/nm-setting-8021x.c @@ -2890,7 +2890,7 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_EAP: g_slist_free_full (priv->eap, g_free); - priv->eap = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->eap = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_IDENTITY: g_free (priv->identity); @@ -2924,7 +2924,7 @@ set_property (GObject *object, guint prop_id, break; case PROP_ALTSUBJECT_MATCHES: g_slist_free_full (priv->altsubject_matches, g_free); - priv->altsubject_matches = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->altsubject_matches = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_CLIENT_CERT: if (priv->client_cert) @@ -2976,7 +2976,7 @@ set_property (GObject *object, guint prop_id, break; case PROP_PHASE2_ALTSUBJECT_MATCHES: g_slist_free_full (priv->phase2_altsubject_matches, g_free); - priv->phase2_altsubject_matches = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->phase2_altsubject_matches = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_PHASE2_CLIENT_CERT: if (priv->phase2_client_cert) @@ -3062,7 +3062,7 @@ get_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_EAP: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->eap)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->eap, TRUE)); break; case PROP_IDENTITY: g_value_set_string (value, priv->identity); @@ -3083,7 +3083,7 @@ get_property (GObject *object, guint prop_id, g_value_set_string (value, priv->subject_match); break; case PROP_ALTSUBJECT_MATCHES: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->altsubject_matches)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->altsubject_matches, TRUE)); break; case PROP_CLIENT_CERT: g_value_set_boxed (value, priv->client_cert); @@ -3113,7 +3113,7 @@ get_property (GObject *object, guint prop_id, g_value_set_string (value, priv->phase2_subject_match); break; case PROP_PHASE2_ALTSUBJECT_MATCHES: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->phase2_altsubject_matches)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->phase2_altsubject_matches, TRUE)); break; case PROP_PHASE2_CLIENT_CERT: g_value_set_boxed (value, priv->phase2_client_cert); diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 03f97661ce..5945f9c246 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -1184,7 +1184,7 @@ set_property (GObject *object, guint prop_id, break; case PROP_SECONDARIES: g_slist_free_full (priv->secondaries, g_free); - priv->secondaries = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->secondaries = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_GATEWAY_PING_TIMEOUT: priv->gateway_ping_timeout = g_value_get_uint (value); @@ -1260,7 +1260,7 @@ get_property (GObject *object, guint prop_id, g_value_set_enum (value, nm_setting_connection_get_autoconnect_slaves (setting)); break; case PROP_SECONDARIES: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->secondaries)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->secondaries, TRUE)); break; case PROP_GATEWAY_PING_TIMEOUT: g_value_set_uint (value, priv->gateway_ping_timeout); diff --git a/libnm-core/nm-setting-wireless-security.c b/libnm-core/nm-setting-wireless-security.c index a56a8e9757..6df5b46176 100644 --- a/libnm-core/nm-setting-wireless-security.c +++ b/libnm-core/nm-setting-wireless-security.c @@ -1132,15 +1132,15 @@ set_property (GObject *object, guint prop_id, break; case PROP_PROTO: g_slist_free_full (priv->proto, g_free); - priv->proto = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->proto = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_PAIRWISE: g_slist_free_full (priv->pairwise, g_free); - priv->pairwise = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->pairwise = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_GROUP: g_slist_free_full (priv->group, g_free); - priv->group = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->group = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_LEAP_USERNAME: g_free (priv->leap_username); @@ -1206,13 +1206,13 @@ get_property (GObject *object, guint prop_id, g_value_set_string (value, priv->auth_alg); break; case PROP_PROTO: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->proto)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->proto, TRUE)); break; case PROP_PAIRWISE: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->pairwise)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->pairwise, TRUE)); break; case PROP_GROUP: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->group)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->group, TRUE)); break; case PROP_LEAP_USERNAME: g_value_set_string (value, priv->leap_username); diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c index 5e6bb249d2..c5365c9ab7 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -929,7 +929,7 @@ set_property (GObject *object, guint prop_id, break; case PROP_SEEN_BSSIDS: g_slist_free_full (priv->seen_bssids, g_free); - priv->seen_bssids = _nm_utils_strv_to_slist (g_value_get_boxed (value)); + priv->seen_bssids = _nm_utils_strv_to_slist (g_value_get_boxed (value), TRUE); break; case PROP_HIDDEN: priv->hidden = g_value_get_boolean (value); @@ -985,7 +985,7 @@ get_property (GObject *object, guint prop_id, g_value_set_uint (value, nm_setting_wireless_get_mtu (setting)); break; case PROP_SEEN_BSSIDS: - g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->seen_bssids)); + g_value_take_boxed (value, _nm_utils_slist_to_strv (priv->seen_bssids, TRUE)); break; case PROP_HIDDEN: g_value_set_boolean (value, nm_setting_wireless_get_hidden (setting)); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 875882f132..7ec67cffc5 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -755,21 +755,26 @@ _nm_utils_bytes_from_dbus (GVariant *dbus_value, } GSList * -_nm_utils_strv_to_slist (char **strv) +_nm_utils_strv_to_slist (char **strv, gboolean deep_copy) { int i; GSList *list = NULL; if (strv) { - for (i = 0; strv[i]; i++) - list = g_slist_prepend (list, g_strdup (strv[i])); + if (deep_copy) { + for (i = 0; strv[i]; i++) + list = g_slist_prepend (list, g_strdup (strv[i])); + } else { + for (i = 0; strv[i]; i++) + list = g_slist_prepend (list, strv[i]); + } } return g_slist_reverse (list); } char ** -_nm_utils_slist_to_strv (GSList *slist) +_nm_utils_slist_to_strv (GSList *slist, gboolean deep_copy) { GSList *iter; char **strv; @@ -778,8 +783,13 @@ _nm_utils_slist_to_strv (GSList *slist) len = g_slist_length (slist); strv = g_new (char *, len + 1); - for (i = 0, iter = slist; iter; iter = iter->next, i++) - strv[i] = g_strdup (iter->data); + if (deep_copy) { + for (i = 0, iter = slist; iter; iter = iter->next, i++) + strv[i] = g_strdup (iter->data); + } else { + for (i = 0, iter = slist; iter; iter = iter->next, i++) + strv[i] = iter->data; + } strv[i] = NULL; return strv; From bd57d76af863f2e3ac314084b331b37797a072d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 17:01:44 +0200 Subject: [PATCH 33/34] core: add nm_match_spec_join() function We have a special implemenation nm_match_spec_split() to split a string. We also need the reverse operation to be able to convert a list of specs to string without loss. --- src/NetworkManagerUtils.c | 79 +++++++++++++++++++++++++++++++++++++++ src/NetworkManagerUtils.h | 1 + src/tests/test-general.c | 16 +++++++- 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 24a6f952cb..4ca8bc89b6 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1419,6 +1419,85 @@ nm_match_spec_split (const char *value) return g_slist_reverse (pieces); } +/** + * nm_match_spec_join: + * @specs: the device specs to join + * + * This is based on g_key_file_parse_string_as_value(), analog to + * nm_match_spec_split() which is based on g_key_file_parse_value_as_string(). + * + * Returns: (transfer-full): a joined list of device specs that can be + * split again with nm_match_spec_split(). Note that + * nm_match_spec_split (nm_match_spec_join (specs)) yields the original + * result (which is not true the other way around because there are multiple + * ways to encode the same joined specs string). + */ +char * +nm_match_spec_join (GSList *specs) +{ + const char *p; + GString *str; + + str = g_string_new (""); + + for (; specs; specs = specs->next) { + p = specs->data; + + if (!p || !*p) + continue; + + if (str->len > 0) + g_string_append_c (str, ','); + + /* escape leading whitespace */ + switch (*p) { + case ' ': + g_string_append (str, "\\s"); + p++; + break; + case '\t': + g_string_append (str, "\\t"); + p++; + break; + } + + for (; *p; p++) { + switch (*p) { + case '\n': + g_string_append (str, "\\n"); + break; + case '\r': + g_string_append (str, "\\r"); + break; + case '\\': + g_string_append (str, "\\\\"); + break; + case ',': + g_string_append (str, "\\,"); + break; + case ';': + g_string_append (str, "\\;"); + break; + default: + g_string_append_c (str, *p); + break; + } + } + + /* escape trailing whitespaces */ + switch (str->str[str->len - 1]) { + case ' ': + g_string_overwrite (str, str->len - 1, "\\s"); + break; + case '\t': + g_string_overwrite (str, str->len - 1, "\\t"); + break; + } + } + + return g_string_free (str, FALSE); +} + const char * nm_utils_get_shared_wifi_permission (NMConnection *connection) { diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 4c4f55b96e..d9adfc0975 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -98,6 +98,7 @@ NMMatchSpecMatchType nm_match_spec_hwaddr (const GSList *specs, const char *hwad NMMatchSpecMatchType nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); NMMatchSpecMatchType nm_match_spec_interface_name (const GSList *specs, const char *interface_name); GSList *nm_match_spec_split (const char *value); +char *nm_match_spec_join (GSList *specs); const char *nm_utils_get_shared_wifi_permission (NMConnection *connection); diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 3397dd9715..eda7a1f892 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -674,13 +674,27 @@ static void test_match_spec_ifname (const char *spec_str, const char **matches, const char **neg_matches) { const char *m; - GSList *specs, *specs_reverse = NULL; + GSList *specs, *specs_reverse = NULL, *specs_resplit, *specs_i, *specs_j; guint i; + gs_free char *specs_joined = NULL; g_assert (spec_str); specs = nm_match_spec_split (spec_str); + /* assert that split(join(specs)) == specs */ + specs_joined = nm_match_spec_join (specs); + specs_resplit = nm_match_spec_split (specs_joined); + specs_i = specs; + specs_j = specs_resplit; + while (specs_i && specs_j && g_strcmp0 (specs_i->data, specs_j->data) == 0) { + specs_i = specs_i->next; + specs_j = specs_j->next; + } + g_assert (!specs_i); + g_assert (!specs_j); + g_slist_free_full (specs_resplit, g_free); + /* also check the matches in the reverse order. They must yield the same result because * matches are inclusive -- except "except:" which always wins. */ specs_reverse = g_slist_reverse (g_slist_copy (specs)); From a1ea678f7801188f3a1850a319dcc249a4c3bd37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 13:59:06 +0200 Subject: [PATCH 34/34] config: only handle 'option+' and 'option-' keys for known settings It is wrong to blindly merge keys that have an 'option+' or 'option-'. Merging options is only possibly when we understand what the option means and how to merge it. No longer handle every setting but only those that are explicitly known to be string-lists (or device-specs). --- src/nm-config.c | 106 +++++++++++++++++----- src/nm-config.h | 1 + src/tests/config/conf.d/00-overrides.conf | 9 +- src/tests/config/conf.d/10-more.conf | 9 +- src/tests/config/conf.d/90-last.conf | 2 +- src/tests/config/test-config.c | 36 ++++++-- 6 files changed, 133 insertions(+), 30 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 8655239287..add9b4d696 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -528,6 +528,27 @@ _sort_groups_cmp (const char **pa, const char **pb, gpointer dummy) return pa > pb ? -1 : 1; } +static gboolean +_setting_is_device_spec (const char *group, const char *key) +{ +#define _IS(group_v, key_v) (strcmp (group, (""group_v)) == 0 && strcmp (key, (""key_v)) == 0) + return _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default") + || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier") + || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only") + || _IS (NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices") + || (g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION) && !strcmp (key, "match-device")); +} + +static gboolean +_setting_is_string_list (const char *group, const char *key) +{ + return _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins") + || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "debug") + || _IS (NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains") + || g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST); +#undef _IS +} + static gboolean read_config (GKeyFile *keyfile, const char *path, GError **error) { @@ -593,29 +614,72 @@ read_config (GKeyFile *keyfile, const char *path, GError **error) if ( key_len > 1 && (last_char == '+' || last_char == '-')) { gs_free char *base_key = g_strndup (key, key_len - 1); - gs_strfreev char **old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL); - gs_free char **new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL); - gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free); - char **iter_val; + gboolean is_string_list; - for (iter_val = old_val; iter_val && *iter_val; iter_val++) { - if ( last_char != '-' - || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0) - g_ptr_array_add (new, g_strdup (*iter_val)); - } - for (iter_val = new_val; iter_val && *iter_val; iter_val++) { - /* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */ - if ( last_char == '+' - && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0) - g_ptr_array_add (new, *iter_val); - else - g_free (*iter_val); - } + is_string_list = _setting_is_string_list (group, base_key); - if (new->len > 0) - nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len); - else - g_key_file_remove_key (keyfile, group, base_key, NULL); + if ( is_string_list + || _setting_is_device_spec (group, base_key)) { + gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free); + char **iter_val; + gs_strfreev char **old_val = NULL; + gs_free char **new_val = NULL; + + if (is_string_list) { + old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL); + new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL); + } else { + gs_free char *old_sval = nm_config_keyfile_get_value (keyfile, group, base_key, NM_CONFIG_GET_VALUE_TYPE_SPEC); + gs_free char *new_sval = nm_config_keyfile_get_value (kf, group, key, NM_CONFIG_GET_VALUE_TYPE_SPEC); + gs_free_slist GSList *old_specs = nm_match_spec_split (old_sval); + gs_free_slist GSList *new_specs = nm_match_spec_split (new_sval); + + /* the key is a device spec. This is a special kind of string-list, that + * we must split differently. */ + old_val = _nm_utils_slist_to_strv (old_specs, FALSE); + new_val = _nm_utils_slist_to_strv (new_specs, FALSE); + } + + /* merge the string lists, by omiting duplicates. */ + + for (iter_val = old_val; iter_val && *iter_val; iter_val++) { + if ( last_char != '-' + || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0) + g_ptr_array_add (new, g_strdup (*iter_val)); + } + for (iter_val = new_val; iter_val && *iter_val; iter_val++) { + /* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */ + if ( last_char == '+' + && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0) + g_ptr_array_add (new, *iter_val); + else + g_free (*iter_val); + } + + if (new->len > 0) { + if (is_string_list) + nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len); + else { + gs_free_slist GSList *specs = NULL; + gs_free char *specs_joined = NULL; + + g_ptr_array_add (new, NULL); + specs = _nm_utils_strv_to_slist ((char **) new->pdata, FALSE); + + specs_joined = nm_match_spec_join (specs); + + g_key_file_set_value (keyfile, group, base_key, specs_joined); + } + } else { + if (is_string_list) + g_key_file_remove_key (keyfile, group, base_key, NULL); + else + g_key_file_set_value (keyfile, group, base_key, ""); + } + } else { + /* For any other settings we don't support extending the option with +/-. + * Just drop the key. */ + } continue; } diff --git a/src/nm-config.h b/src/nm-config.h index 20dbab388f..5b096f165d 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -49,6 +49,7 @@ G_BEGIN_DECLS #define NM_CONFIG_KEYFILE_LIST_SEPARATOR ',' #define NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION "connection" +#define NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST ".test-append-stringlist" #define NM_CONFIG_KEYFILE_GROUP_MAIN "main" #define NM_CONFIG_KEYFILE_GROUP_LOGGING "logging" diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf index cb0116edd8..f26ed93b9a 100644 --- a/src/tests/config/conf.d/00-overrides.conf +++ b/src/tests/config/conf.d/00-overrides.conf @@ -1,9 +1,16 @@ [main] dhcp=dhcpcd +no-auto-default=spec1,spec2 +ignore-carrier=\s space1 \s + [logging] domains=PLATFORM,DNS,WIFI +[appendable-test] +non-appendable-key1+=i-will-be-dropped +non-appendable-key2-=i-will-be-dropped + [order] a=0 b=0 @@ -32,7 +39,7 @@ ord.key08=B-1.3.08 ord.key09=B-1.3.09 -[append] +[.test-append-stringlist.1] val1=a,b val2-=VAL2 diff --git a/src/tests/config/conf.d/10-more.conf b/src/tests/config/conf.d/10-more.conf index a1959c1948..eadb7f96f6 100644 --- a/src/tests/config/conf.d/10-more.conf +++ b/src/tests/config/conf.d/10-more.conf @@ -1,5 +1,12 @@ [main] extra=hello + +no-auto-default-=spec1 +no-auto-default+=spec3 + +ignore-carrier+=\sspace2\t + +[.test-append-stringlist.0] new+=something [connectivity] @@ -29,5 +36,5 @@ ord.key09=C-2.3.09 ord.ovw01=C-0.1.ovw01 -[append] +[.test-append-stringlist.1] val1-=b diff --git a/src/tests/config/conf.d/90-last.conf b/src/tests/config/conf.d/90-last.conf index c75dcc4710..7d078788de 100644 --- a/src/tests/config/conf.d/90-last.conf +++ b/src/tests/config/conf.d/90-last.conf @@ -4,5 +4,5 @@ plugins+=one,two [order] a=90 -[append] +[.test-append-stringlist.1] val1+=c,a diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 97995afa7b..ca115fd1ab 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -283,6 +283,7 @@ test_config_confdir (void) NMConfig *config; const char **plugins; char *value; + GSList *specs; config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", NULL); @@ -305,7 +306,23 @@ test_config_confdir (void) g_assert_cmpstr (value, ==, "hello"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "no-auto-default", NM_CONFIG_GET_VALUE_TYPE_SPEC); + specs = nm_match_spec_split (value); + g_free (value); + g_assert_cmpint (g_slist_length (specs), ==, 2); + g_assert_cmpstr (g_slist_nth_data (specs, 0), ==, "spec2"); + g_assert_cmpstr (g_slist_nth_data (specs, 1), ==, "spec3"); + g_slist_free_full (specs, g_free); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "ignore-carrier", NM_CONFIG_GET_VALUE_TYPE_SPEC); + specs = nm_match_spec_split (value); + g_free (value); + g_assert_cmpint (g_slist_length (specs), ==, 2); + g_assert_cmpstr (g_slist_nth_data (specs, 0), ==, " space1 "); + g_assert_cmpstr (g_slist_nth_data (specs, 1), ==, " space2\t"); + g_slist_free_full (specs, g_free); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".0", "new", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "something"); /* not ",something" */ g_free (value); @@ -319,6 +336,13 @@ test_config_confdir (void) g_assert_cmpstr (value, ==, "0"); g_free (value); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1", NM_CONFIG_GET_VALUE_RAW)); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1+", NM_CONFIG_GET_VALUE_RAW)); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1-", NM_CONFIG_GET_VALUE_RAW)); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2", NM_CONFIG_GET_VALUE_RAW)); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2+", NM_CONFIG_GET_VALUE_RAW)); + g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2-", NM_CONFIG_GET_VALUE_RAW)); + #define ASSERT_GET_CONN_DEFAULT(xconfig, xname, xvalue) \ G_STMT_START { \ gs_free char *_value = nm_config_data_get_connection_default (nm_config_get_data_orig (xconfig), (xname), NULL); \ @@ -336,23 +360,23 @@ test_config_confdir (void) ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val1", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "a,c"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val2", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "VAL2"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val3", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, NULL); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val4", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "vb,vb"); g_free (value); - value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NM_CONFIG_GET_VALUE_NONE); + value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val5", NM_CONFIG_GET_VALUE_NONE); g_assert_cmpstr (value, ==, "VAL5"); g_free (value);