From 519ea3f0d47de648743ed693d8bf649bd0768d14 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 25 Jun 2015 15:36:43 +0200 Subject: [PATCH 01/35] 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. (cherry picked from commit aa7a53bc67acbaa299565f5d64d33f0ca9584a3a) --- 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 df568a8ebd..353804f97c 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1577,6 +1577,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 14202e10b7bc577a1527cb82c66c9ad561016d3f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 25 Jun 2015 13:02:10 +0200 Subject: [PATCH 02/35] 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 (cherry picked from commit f8c9863d55d494bcf8613ded58e28636b48466b5) --- 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 bd187ff57c..f3011c4f73 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -465,7 +465,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]. @@ -478,6 +478,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 a4effbe0ce..3f2322527e 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -216,58 +216,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 7520c18e3b..17835b2737 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 7ce3c1ee7c..f12752847a 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 53223b484219532111ac7a9757a0a4676917e4bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Jun 2015 21:42:46 +0200 Subject: [PATCH 03/35] glib-compat: backport g_key_file_save_to_file() (cherry picked from commit 69f2d22bfe355f89b6ce9622613ab0e9be7d1dd1) --- 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 5cf8d6fd19c7ef04e09159258073281335e4161e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:46:17 +0200 Subject: [PATCH 04/35] libnm: add _nm_utils_strv_cleanup() function (cherry picked from commit 885d187d23d44de0a3b8696d03a9e8fa2cc29f54) --- 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 ab147b9f7a..c363016bf9 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -119,6 +119,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 16561175da..93f3316ac7 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 fa0f53b07b48707741fb696928a43a6cbf752610 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:55:19 +0200 Subject: [PATCH 05/35] macros: add nm_strstrip() util (cherry picked from commit d6a331bd8caf3aa21dda9f7a2931f73f78c41fa4) --- 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 3767a13397..405051f382 100644 --- a/include/nm-macros-internal.h +++ b/include/nm-macros-internal.h @@ -236,4 +236,13 @@ nm_clear_g_source (guint *id) /*****************************************************************************/ +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 0b10fb15a0f796e6cac3a655cf76b076a074bfe8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Jun 2015 21:53:49 +0200 Subject: [PATCH 06/35] test: add nmtst_assert_success() util (cherry picked from commit f5177dbf7a06d691957f4aa506e0d7061ca3eec2) --- 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 0cde9a4b91..4610ee62a6 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -123,6 +123,14 @@ nmtst_assert_error (GError *error, } } +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 0a2325ded0aa9d71952a7bddab17b8fae80a48ed Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 6 Jun 2015 21:29:01 +0200 Subject: [PATCH 07/35] config: change examples for command line arguments to system default (cherry picked from commit 27bd7dc93846c0f403dc59a4d9f8a1f039c1382b) --- 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 8a44db31d3..eb7004ecf8 100644 --- a/src/main.c +++ b/src/main.c @@ -240,8 +240,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 17835b2737..285110bd11 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 68a1c54fa2786fe772b1fd9077a496a2886dde03 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 6 Jun 2015 21:47:29 +0200 Subject: [PATCH 08/35] config/trivial: rename defines for default settings Make them match to the variable names that we assign them to. (cherry picked from commit 3c8abc2d5bb2ad43a5ed9f4a90074fb97c31b538) --- 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 285110bd11..dc585d292c 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; } @@ -913,7 +913,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, @@ -928,7 +928,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 bd2df64bac5b4fce816d2e76dca9e0ae3da0d3e4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jul 2015 09:46:35 +0200 Subject: [PATCH 09/35] config: add NM_CONFIG_KEYFILE_LIST_SEPARATOR define (cherry picked from commit a05e80913e6b21b38cc1e729de92bc2e1260574e) --- 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 dc585d292c..6b94207d15 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 4c45642b8bd9f737df88d920809573e1e7890619 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Jun 2015 12:30:44 +0200 Subject: [PATCH 10/35] config: add nm_config_keyfile_set_string_list() utils function (cherry picked from commit bb4ae800a1293aea047c113352cc5e87178bb0b2) --- 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 6b94207d15..51aff75db2 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 a325abc425e34d42aa3ef790a99a1ee9936a9a41 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 12:03:19 +0200 Subject: [PATCH 11/35] 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. (cherry picked from commit fab5c6a372092bd350460a27891136c861ea2852) --- 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 f3011c4f73..40fa49dd7a 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 51aff75db2..f018a91819 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 f12752847a..13a2aca9e1 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 4adecd466e7220bd8fe11ae5afd1cf49c6bcb983 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 23:27:01 +0200 Subject: [PATCH 12/35] 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. (cherry picked from commit 3e4458659b6bbed6441aca0c2a1786bfc33488a9) --- 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 3f2322527e..016d368910 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; @@ -144,12 +145,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 * @@ -306,12 +312,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); @@ -332,13 +347,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))) @@ -364,9 +374,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; @@ -390,7 +397,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) { @@ -406,12 +412,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); @@ -436,7 +454,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); @@ -487,6 +506,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); } @@ -580,7 +601,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 64ad372b3b..a9e1f3dd85 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -90,7 +90,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); diff --git a/src/nm-config.c b/src/nm-config.c index f018a91819..859d637550 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); } @@ -949,9 +954,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. */ @@ -998,19 +1001,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 bee9b7e327870b740ef645017bc1e8b3ef0daa41 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 16:58:22 +0200 Subject: [PATCH 13/35] 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. (cherry picked from commit 4a8a0b09180f7f3e9bc63f9cf45e6875dc402b50) --- 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 859d637550..3733a43a16 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 41d0902dc9b38d09d492a332f454df3196423572 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 13:30:30 +0200 Subject: [PATCH 14/35] libnm: add keyfile utility functions (cherry picked from commit 71323122c6b755cfc50ce93c07d6758b45c19cba) --- 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 016d368910..7b8921dea6 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -27,6 +27,7 @@ #include "nm-device.h" #include "gsystem-local-alloc.h" #include "nm-core-internal.h" +#include "nm-keyfile-internal.h" #include "nm-macros-internal.h" typedef struct { @@ -285,33 +286,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) { @@ -334,8 +308,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 582ef15d3e7f6383057f1be2d201b27827fa028d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Jul 2015 16:53:06 +0200 Subject: [PATCH 15/35] libnm/keyfile: fix compile warning about uninitialized variable Fixes: 71323122c6b755cfc50ce93c07d6758b45c19cba (cherry picked from commit 47551df352b7826a0396c001710e156d4d056a02) --- libnm-core/nm-keyfile-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c index 1ddc1b59e6..10981efcf4 100644 --- a/libnm-core/nm-keyfile-utils.c +++ b/libnm-core/nm-keyfile-utils.c @@ -277,7 +277,7 @@ _nm_keyfile_equals (GKeyFile *kf_a, GKeyFile *kf_b) gboolean _nm_keyfile_has_values (GKeyFile *keyfile) { - gs_strfreev char **groups; + gs_strfreev char **groups = NULL; g_return_val_if_fail (keyfile, FALSE); From 016d46265f1c92ba89391c34231bee65fd1b66ae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 24 Jun 2015 17:16:22 +0200 Subject: [PATCH 16/35] libnm-keyfile/test: fix missing assertion in test (cherry picked from commit e1b0195c67d16f6fd44770828e912d190f2cd888) --- 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 b1246efc606a5762668c59b7c0867a0b7b4b68e1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Jun 2015 21:12:00 +0200 Subject: [PATCH 17/35] 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. (cherry picked from commit 7fbfaf567df0562e6b9c39954087dc897c5e05b6) --- 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 10981efcf4..d04a3d1bef 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 7b8921dea6..33433a7c0e 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -308,7 +308,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 4ba8dd09aced06c618c868eca75aeadf61848fd6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 08:17:28 +0200 Subject: [PATCH 18/35] 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. (cherry picked from commit a5f7abb842b037cbb21b1456c7681d0c93e91d36) --- 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/tests/config/test-config.c | 35 +++++++++++--------------- 5 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 33433a7c0e..01550c387e 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -107,11 +107,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 a9e1f3dd85..d94c6c9441 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -83,7 +83,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 7f1eb4ff93..a4c6bb239e 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -124,8 +124,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); @@ -265,8 +264,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 1cbff29b3d..6fe431878b 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -328,7 +328,6 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) if_block *block = NULL; NMInotifyHelper *inotify_helper; char *value; - GError *error = NULL; GList *keys, *iter; GHashTableIter con_iter; const char *block_name; @@ -457,17 +456,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/tests/config/test-config.c b/src/tests/config/test-config.c index 13a2aca9e1..f7d4f7cd6e 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 a771e2ffcfd9b0a6361fe5a378524656e9a7bc90 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 08:47:41 +0200 Subject: [PATCH 19/35] config: add macros NM_CONFIG_GET_DATA and NM_CONFIG_GET_DATA_ORIG (cherry picked from commit 2c46003e99b472595d983a03c70dd28d254fad5c) --- 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 40802c6e72..f34ed7ef1f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -784,7 +784,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); @@ -2301,7 +2301,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; @@ -4859,7 +4859,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. */ @@ -8359,7 +8359,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 e6640ab3c9..080cdb8b24 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2487,7 +2487,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 a4c6bb239e..6b22ff2751 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -123,7 +123,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); @@ -263,7 +263,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 6fe431878b..ae5137de93 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -455,7 +455,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 5eeaf4df9126f13407917420cc96dbda96d45069 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:02:32 +0200 Subject: [PATCH 20/35] 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. (cherry picked from commit 35d2981546911f7f6e29c845eaca95a8973d7004) --- 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 3733a43a16..3b92bfb287 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 76ca6d86ec6655d6a554d65185a6f785e7ae1180 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:06:53 +0200 Subject: [PATCH 21/35] config: add nm_config_data_get_value_boolean() (cherry picked from commit a0e92799af0b21c698cd9c4147b7d8870018437f) --- 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 01550c387e..0e3c09c22a 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -116,6 +116,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 d94c6c9441..edffe20aad 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -84,6 +84,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 c4b1156b04733c3945477f5a818c01e94e024d29 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:12:01 +0200 Subject: [PATCH 22/35] config: use nm_config_data_get_value_boolean() This removes duplicate parsing, but also makes all places use the same str-to-boolean convention. (cherry picked from commit 1b0ab2129c2789c476b2376da4c8e3d45c05b548) --- 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 6b22ff2751..eed31e4483 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -121,16 +121,9 @@ write_system_hostname (NMSystemConfigInterface * config, 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 @@ -245,8 +238,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 */ @@ -263,11 +255,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 ae5137de93..d6d712803b 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -327,7 +327,6 @@ SCPluginIfupdown_init (NMSystemConfigInterface *config) GHashTable *auto_ifaces; if_block *block = NULL; NMInotifyHelper *inotify_helper; - char *value; GList *keys, *iter; GHashTableIter con_iter; const char *block_name; @@ -353,8 +352,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; - inotify_helper = nm_inotify_helper_get (); priv->inotify_event_id = g_signal_connect (inotify_helper, "event", @@ -455,15 +452,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 */ @@ -623,7 +615,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) { NMSystemConfigInterface *self = NM_SYSTEM_CONFIG_INTERFACE (object); From cbace6fe063388f7ffcd828ca0ae71ee31c13620 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:24:28 +0200 Subject: [PATCH 23/35] 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. (cherry picked from commit 6d6ab20be01cdda8d3126df78504edabc27bf8bd) --- src/nm-config-data.c | 18 ++++++------- 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 | 6 ++--- 7 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 0e3c09c22a..bcc833860b 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -275,8 +275,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]; @@ -479,23 +479,23 @@ 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->dns_mode = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", 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 3b92bfb287..b342b7aa48 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; @@ -1002,22 +1002,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 eed31e4483..5eab31abfd 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -47,7 +47,6 @@ #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_SYSTEM_HOSTNAME_FILE "/etc/conf.d/hostname" #define IFNET_MANAGE_WELL_KNOWN_DEFAULT TRUE -#define IFNET_KEY_FILE_KEY_MANAGED "managed" typedef struct { GHashTable *connections; /* uuid::connection */ @@ -122,7 +121,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); } @@ -256,7 +255,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 d6d712803b..8652dd8dd8 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -63,8 +63,6 @@ #define IFUPDOWN_PLUGIN_INFO "(C) 2008 Canonical Ltd. To report bugs please use the NetworkManager mailing list." #define IFUPDOWN_SYSTEM_HOSTNAME_FILE "/etc/hostname" -#define IFUPDOWN_KEY_FILE_GROUP "ifupdown" -#define IFUPDOWN_KEY_FILE_KEY_MANAGED "managed" #define IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT TRUE /* #define ALWAYS_UNMANAGE TRUE */ @@ -453,8 +451,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 611192baa5..cbfb9f500f 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -591,7 +591,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) key_file = nm_config_create_keyfile (); if (parse_key_file_allow_none (priv, key_file, &error)) - specs = nm_config_get_device_match_spec (key_file, "keyfile", "unmanaged-devices"); + specs = nm_config_get_device_match_spec (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); if (error) { nm_log_warn (LOGD_SETTINGS, "keyfile: error getting unmanaged specs: %s", error->message); @@ -617,7 +617,7 @@ plugin_get_hostname (SCPluginKeyfile *plugin) if (!parse_key_file_allow_none (priv, key_file, &error)) goto out; - hostname = g_key_file_get_value (key_file, "keyfile", "hostname", NULL); + hostname = g_key_file_get_value (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "hostname", NULL); out: if (error) { @@ -653,7 +653,7 @@ plugin_set_hostname (SCPluginKeyfile *plugin, const char *hostname) if (!parse_key_file_allow_none (priv, key_file, &error)) goto out; - g_key_file_set_string (key_file, "keyfile", "hostname", hostname); + g_key_file_set_string (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "hostname", hostname); data = g_key_file_to_data (key_file, &len, &error); if (!data) From bd83daf40860908a339c28026f0cbfcd0db84c83 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 09:59:18 +0200 Subject: [PATCH 24/35] config: log configuration at startup and on reload (cherry picked from commit b506c29fe1374c00b2886f2263a632da0bc17b6b) --- 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 eb7004ecf8..dfc307b5db 100644 --- a/src/main.c +++ b/src/main.c @@ -423,6 +423,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 bcc833860b..97cc36217b 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; @@ -203,6 +204,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 edffe20aad..769f2057d7 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -80,6 +80,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 b342b7aa48..b82a817b17 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -931,6 +931,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 f7d4f7cd6e..863fcd50a4 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 df1cd7312845772fe3ad7ce5ec751a1dee017db2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 12:06:14 +0200 Subject: [PATCH 25/35] 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. (cherry picked from commit 0abb502ff389704e5979c6b16f88129ec5609be6) --- src/nm-config-data.c | 39 ++++++++++++++------------- src/nm-config.c | 23 +++++++++------- src/nm-config.h | 2 +- src/settings/plugins/keyfile/plugin.c | 2 +- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 97cc36217b..e78d547201 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -110,10 +110,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); } @@ -123,7 +125,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); @@ -313,7 +320,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; @@ -331,17 +338,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); } @@ -568,23 +571,23 @@ 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->dns_mode = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", 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 b82a817b17..6cd5328ab7 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); } @@ -1011,12 +1016,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 diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index cbfb9f500f..7f21e71e67 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -591,7 +591,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) key_file = nm_config_create_keyfile (); if (parse_key_file_allow_none (priv, key_file, &error)) - specs = nm_config_get_device_match_spec (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); + specs = nm_config_get_device_match_spec (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices", NULL); if (error) { nm_log_warn (LOGD_SETTINGS, "keyfile: error getting unmanaged specs: %s", error->message); From e713fb5e99e8ffbe06c644ea059330d4fb9c322f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Jun 2015 17:27:15 +0200 Subject: [PATCH 26/35] config: ensure nm_config_get_plugins() to return stripped values (cherry picked from commit 7e94785f28a822bfd95acf2f2551814f84dd3d79) --- 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 6cd5328ab7..36dec1a1bc 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1008,7 +1008,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 9e4c441fbc..cb3b6b4386 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -685,17 +685,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 109f6756b8c3f355bbe97c2a39654d5d5c6e2d21 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Jun 2015 23:43:29 +0200 Subject: [PATCH 27/35] config: add config utility accessors (cherry picked from commit 11c0e107b9b903b13c741448eae68b443e98ad2e) --- 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 e78d547201..4bb8735000 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -107,6 +107,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) { @@ -119,6 +128,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) { @@ -211,6 +235,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 769f2057d7..c3879d88ee 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -85,6 +85,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); @@ -104,6 +106,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 fb71d7c2fdf2ecece3cc762b8236c78ab1f08386 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:06:48 +0200 Subject: [PATCH 28/35] 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. (cherry picked from commit 7498b670a828d335e177003c9d00d68802f52561) --- 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 6cd937da99..c8e3931ca1 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1310,11 +1310,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; @@ -1325,7 +1346,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++; @@ -1357,23 +1384,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 dae872c86d..c161fc1e4f 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -636,6 +636,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++) { @@ -702,9 +705,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 93ff88fff192a5725c0e2f686bfeac634c5baed8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 14:59:29 +0200 Subject: [PATCH 29/35] 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 (cherry picked from commit 076ffda6f395b37b497929de0838688db8eba84a) --- src/nm-config-data.c | 4 ++-- src/nm-config.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 4bb8735000..a9e66ee800 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -622,7 +622,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 @@ -633,7 +633,7 @@ 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->dns_mode = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", 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 36dec1a1bc..92bf86e6cb 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1017,10 +1017,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 1b66696f6a8ee512e47e51f6a8ed390a0ab80a0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 15:13:26 +0200 Subject: [PATCH 30/35] 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. (cherry picked from commit d3e21937834d94fa844a06b1158cf5f0411636b8) --- src/nm-config-data.c | 12 ++++-------- src/nm-config-data.h | 21 +++++++++++++++++++-- src/nm-config.c | 28 ++++++++++++++++++++++++++++ src/nm-config.h | 4 ++++ src/tests/config/test-config.c | 30 +++++++++++++++--------------- 5 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index a9e66ee800..206e9f5435 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -117,19 +117,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; @@ -137,9 +135,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 c3879d88ee..cc7f95e211 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, @@ -86,8 +103,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 92bf86e6cb..c5be85a85f 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/tests/config/test-config.c b/src/tests/config/test-config.c index 863fcd50a4..24592071ef 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 045841b66ccd67e54b6518fb2ee2b36692826701 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Jun 2015 15:33:13 +0200 Subject: [PATCH 31/35] config/trivial: add code comment (cherry picked from commit 0c6a011e344612e607758626e51077b6edda66f5) --- 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 206e9f5435..dc20064b19 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -367,6 +367,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 1dec33e2af47a8680420dfb224ea07c84eea6417 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 14:02:31 +0200 Subject: [PATCH 32/35] libnm: expose strv utils function in internal header nm-core-internal.h (cherry picked from commit 93e4a8d10219fe431abe976a054b5a63c552aadf) --- 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 c363016bf9..021f870a07 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -128,6 +128,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 3854d67951..ffdc631566 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -40,12 +40,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 0cbcf21e804b98301191d42af86f3ae3b7cfe2d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 14:08:51 +0200 Subject: [PATCH 33/35] libnm: add @deep_copy argument to _nm_utils_strv_to_slist() and _nm_utils_slist_to_strv() (cherry picked from commit ed632207cd7623061a292af73e35659d31a7949d) --- 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 021f870a07..dd5151e088 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -128,8 +128,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 02f9c9039d..e50ad4eae5 100644 --- a/libnm-core/nm-setting-8021x.c +++ b/libnm-core/nm-setting-8021x.c @@ -2888,7 +2888,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); @@ -2922,7 +2922,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) @@ -2974,7 +2974,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) @@ -3060,7 +3060,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); @@ -3081,7 +3081,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); @@ -3111,7 +3111,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 6f66e674a0..07b3401d1e 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -1149,7 +1149,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); @@ -1222,7 +1222,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 f8d22c2d00..ce3cf9a7e9 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -911,7 +911,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); @@ -964,7 +964,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 93f3316ac7..14686ed1fd 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 e896817d809eae4e8f736c3f0bf19f163357bc62 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 17:01:44 +0200 Subject: [PATCH 34/35] 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. (cherry picked from commit bd57d76af863f2e3ac314084b331b37797a072d2) --- 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 c8e3931ca1..26ec1b92d7 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1416,6 +1416,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 f8e759142e..1864547f56 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 c161fc1e4f..de65d50fd9 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -630,13 +630,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 500f590033858f565f35d553b21c822dc378efd2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Jul 2015 13:59:06 +0200 Subject: [PATCH 35/35] 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). (cherry picked from commit a1ea678f7801188f3a1850a319dcc249a4c3bd37) --- 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 c5be85a85f..c318eea790 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 24592071ef..8b8028e63a 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);