From ef32da01fa0b9e02ccae476bea412ff025091a7f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 27 Jul 2014 20:35:17 +0200 Subject: [PATCH 01/30] all: add nm-core-internal.h header Add a header file to expose private utility functions from libnm-core that can be used by NetworkManager (core) and libnm.so. The header is also used to give privileged access to libnm-core. Since NM links statically, these functions are not exported and not part of public ABI. This also removes the NM_UTILS_PRIVATE_CALL() macro and libnm.so no longer exports nm_utils_get_private(). Before, this functionality was partly declared in nm-utils-private.h. This was wrong because nm-utils-private.h is for functionality entirely private to libnm-core. Signed-off-by: Thomas Haller --- callouts/tests/test-dispatcher-envp.c | 1 - clients/tui/nmt-route-table.c | 1 - docs/libnm/Makefile.am | 1 + libnm-core/Makefile.libnm-core | 1 + libnm-core/nm-core-internal.h | 49 +++++++++++++++++++ libnm-core/nm-setting-ip4-config.c | 11 +++-- libnm-core/nm-setting-private.h | 5 -- libnm-core/nm-utils-private.h | 26 ---------- libnm-core/nm-utils.c | 19 ------- libnm-core/tests/test-general.c | 24 ++++----- libnm/libnm.ver | 1 - src/nm-ip4-config.c | 6 +-- src/settings/plugins/ifcfg-rh/reader.c | 4 +- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 10 ++-- src/settings/plugins/ifcfg-rh/writer.c | 6 +-- 15 files changed, 82 insertions(+), 83 deletions(-) create mode 100644 libnm-core/nm-core-internal.h diff --git a/callouts/tests/test-dispatcher-envp.c b/callouts/tests/test-dispatcher-envp.c index 1d9792fe9e..9d9ee52a4d 100644 --- a/callouts/tests/test-dispatcher-envp.c +++ b/callouts/tests/test-dispatcher-envp.c @@ -31,7 +31,6 @@ #include "nm-dbus-glib-types.h" #include "nm-dispatcher-api.h" #include "nm-utils.h" -#include "nm-utils-private.h" /*******************************************/ diff --git a/clients/tui/nmt-route-table.c b/clients/tui/nmt-route-table.c index c5a70761d1..82eefb457e 100644 --- a/clients/tui/nmt-route-table.c +++ b/clients/tui/nmt-route-table.c @@ -33,7 +33,6 @@ #include #include #include -#include #include "nmt-route-table.h" #include "nmt-route-entry.h" diff --git a/docs/libnm/Makefile.am b/docs/libnm/Makefile.am index 9d745b14c2..c99e242512 100644 --- a/docs/libnm/Makefile.am +++ b/docs/libnm/Makefile.am @@ -32,6 +32,7 @@ CFILE_GLOB=$(top_srcdir)/libnm-core/*.c $(top_srcdir)/libnm/*.c IGNORE_HFILES= \ crypto.h \ nm-dbus-helpers-private.h \ + nm-core-internal.h \ nm-device-private.h \ nm-object-cache.h \ nm-object-private.h \ diff --git a/libnm-core/Makefile.libnm-core b/libnm-core/Makefile.libnm-core index 49af47d7c3..b7924847e3 100644 --- a/libnm-core/Makefile.libnm-core +++ b/libnm-core/Makefile.libnm-core @@ -43,6 +43,7 @@ libnm_core_headers = \ libnm_core_private_headers = \ $(core)/crypto.h \ + $(core)/nm-core-internal.h \ $(core)/nm-param-spec-specialized.h \ $(core)/nm-setting-private.h \ $(core)/nm-utils-private.h diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h new file mode 100644 index 0000000000..41b51c2f69 --- /dev/null +++ b/libnm-core/nm-core-internal.h @@ -0,0 +1,49 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ + +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + * + * (C) Copyright 2014 Red Hat, Inc. + */ + +#ifndef NM_CORE_NM_INTERNAL_H +#define NM_CORE_NM_INTERNAL_H + +/* This header file contain functions that are provided as private API + * by libnm-core. It will contain functions to give privileged access to + * libnm-core. This can be useful for NetworkManager and libnm.so + * which both are special users of libnm-core. + * It also exposes some utility functions for reuse. + * + * These functions are not exported and are only available to components that link + * statically against libnm-core. This basically means libnm-core, libnm, NetworkManager + * and some test programs. + **/ + +#include + +#include "nm-utils.h" + +#include "nm-setting-ip4-config.h" + +const char *_nm_setting_ip4_config_get_address_label (NMSettingIP4Config *setting, + guint32 i); +gboolean _nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *setting, + NMIP4Address *address, + const char *label); + + +#endif diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index f3de89e98c..aab5546ec6 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -30,6 +30,7 @@ #include "nm-dbus-glib-types.h" #include "nm-glib-compat.h" #include "nm-setting-private.h" +#include "nm-core-internal.h" /** @@ -435,7 +436,7 @@ nm_setting_ip4_config_get_address (NMSettingIP4Config *setting, guint32 i) } const char * -nm_setting_ip4_config_get_address_label (NMSettingIP4Config *setting, guint32 i) +_nm_setting_ip4_config_get_address_label (NMSettingIP4Config *setting, guint32 i) { NMSettingIP4ConfigPrivate *priv; @@ -462,13 +463,13 @@ gboolean nm_setting_ip4_config_add_address (NMSettingIP4Config *setting, NMIP4Address *address) { - return nm_setting_ip4_config_add_address_with_label (setting, address, NULL); + return _nm_setting_ip4_config_add_address_with_label (setting, address, NULL); } gboolean -nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *setting, - NMIP4Address *address, - const char *label) +_nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *setting, + NMIP4Address *address, + const char *label) { NMSettingIP4ConfigPrivate *priv; NMIP4Address *copy; diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 338364cec4..a31d0d7006 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -101,11 +101,6 @@ static void __attribute__((constructor)) register_setting (void) \ NMSetting *nm_setting_find_in_list (GSList *settings_list, const char *setting_name); -/* Private NMSettingIP4Config methods */ -#include "nm-setting-ip4-config.h" -const char *nm_setting_ip4_config_get_address_label (NMSettingIP4Config *setting, guint32 i); -gboolean nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *setting, NMIP4Address *address, const char *label); - NMSettingVerifyResult _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, gboolean allow_missing, const char *setting_name, diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h index c91652b8a3..0e7be21e28 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -34,30 +34,4 @@ gboolean _nm_utils_gvalue_array_validate (GValueArray *elements, void _nm_value_transforms_register (void); -/***********************************************************/ - -typedef struct NMUtilsPrivateData { - const char * (*nm_setting_ip4_config_get_address_label) (NMSettingIP4Config *setting, - guint32 i); - gboolean (*nm_setting_ip4_config_add_address_with_label) (NMSettingIP4Config *setting, - NMIP4Address *address, - const char *label); -} NMUtilsPrivateData; - -const NMUtilsPrivateData *nm_utils_get_private (void); - -/** - * NM_UTILS_PRIVATE_CALL: - * @call: a call to a private libnm function - * - * Used to call private libnm functions. Eg, if there was a - * private function called nm_foo_get_bar(), you could call it like: - * - * bar = NM_UTILS_PRIVATE_CALL (nm_foo_get_bar (foo, x, y, z)); - * - * This macro only exists inside the NetworkManager source tree and - * is not part of the public API. - */ -#define NM_UTILS_PRIVATE_CALL(call) (nm_utils_get_private ()->call) - #endif diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 4304b5b683..4f91c1e1fa 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -2479,23 +2479,4 @@ nm_utils_check_virtual_device_compatibility (GType virtual_type, GType other_typ } } -/***********************************************************/ -static const NMUtilsPrivateData data = { - .nm_setting_ip4_config_get_address_label = nm_setting_ip4_config_get_address_label, - .nm_setting_ip4_config_add_address_with_label = nm_setting_ip4_config_add_address_with_label, -}; - -/** - * nm_utils_get_private: - * - * Entry point for NetworkManager-internal API. You should not use this - * function for any reason. - * - * Returns: Who knows? It's a mystery. - */ -const NMUtilsPrivateData * -nm_utils_get_private (void) -{ - return &data; -} diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 70effba4c0..0c660d0384 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -39,7 +39,7 @@ #include "nm-setting-vlan.h" #include "nm-setting-bond.h" #include "nm-utils.h" -#include "nm-utils-private.h" +#include "nm-core-internal.h" #include "nm-dbus-glib-types.h" #include "nm-test-utils.h" @@ -330,7 +330,7 @@ test_setting_ip4_config_labels (void) nm_setting_verify (NM_SETTING (s_ip4), NULL, &error); g_assert_no_error (error); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 0)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 0); g_assert_cmpstr (label, ==, NULL); /* addr 2 */ @@ -338,12 +338,12 @@ test_setting_ip4_config_labels (void) nm_ip4_address_set_address (addr, 0x02020202); nm_ip4_address_set_prefix (addr, 24); - NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_add_address_with_label (s_ip4, addr, "eth0:1")); + _nm_setting_ip4_config_add_address_with_label (s_ip4, addr, "eth0:1"); nm_ip4_address_unref (addr); nm_setting_verify (NM_SETTING (s_ip4), NULL, &error); g_assert_no_error (error); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 1)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 1); g_assert_cmpstr (label, ==, "eth0:1"); /* addr 3 */ @@ -351,12 +351,12 @@ test_setting_ip4_config_labels (void) nm_ip4_address_set_address (addr, 0x03030303); nm_ip4_address_set_prefix (addr, 24); - NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_add_address_with_label (s_ip4, addr, NULL)); + _nm_setting_ip4_config_add_address_with_label (s_ip4, addr, NULL); nm_ip4_address_unref (addr); nm_setting_verify (NM_SETTING (s_ip4), NULL, &error); g_assert_no_error (error); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 2)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 2); g_assert_cmpstr (label, ==, NULL); /* Remove addr 1 and re-verify remaining addresses */ @@ -366,12 +366,12 @@ test_setting_ip4_config_labels (void) addr = nm_setting_ip4_config_get_address (s_ip4, 0); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x02020202); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 0)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 0); g_assert_cmpstr (label, ==, "eth0:1"); addr = nm_setting_ip4_config_get_address (s_ip4, 1); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x03030303); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 1)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 1); g_assert_cmpstr (label, ==, NULL); @@ -395,12 +395,12 @@ test_setting_ip4_config_labels (void) addr = nm_setting_ip4_config_get_address (s_ip4, 0); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x02020202); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 0)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 0); g_assert_cmpstr (label, ==, NULL); addr = nm_setting_ip4_config_get_address (s_ip4, 1); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x03030303); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 1)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 1); g_assert_cmpstr (label, ==, NULL); /* Setting labels now will leave addresses untouched */ @@ -414,12 +414,12 @@ test_setting_ip4_config_labels (void) addr = nm_setting_ip4_config_get_address (s_ip4, 0); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x02020202); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 0)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 0); g_assert_cmpstr (label, ==, "eth0:1"); addr = nm_setting_ip4_config_get_address (s_ip4, 1); g_assert_cmpint (nm_ip4_address_get_address (addr), ==, 0x03030303); - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 1)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, 1); g_assert_cmpstr (label, ==, NULL); /* Setting labels to a value that's too short or too long will result in diff --git a/libnm/libnm.ver b/libnm/libnm.ver index e994ed974a..760bb42748 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -878,7 +878,6 @@ global: nm_utils_deinit; nm_utils_escape_ssid; nm_utils_file_is_pkcs12; - nm_utils_get_private; nm_utils_gvalue_hash_dup; nm_utils_hex2byte; nm_utils_hexstr2bin; diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index b286edb995..e1395ed31c 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -29,7 +29,7 @@ #include "nm-dbus-glib-types.h" #include "nm-ip4-config-glue.h" #include "NetworkManagerUtils.h" -#include "nm-utils-private.h" +#include "nm-core-internal.h" G_DEFINE_TYPE (NMIP4Config, nm_ip4_config, G_TYPE_OBJECT) @@ -328,7 +328,7 @@ nm_ip4_config_merge_setting (NMIP4Config *config, NMSettingIP4Config *setting, i /* Addresses */ for (i = 0; i < naddresses; i++) { NMIP4Address *s_addr = nm_setting_ip4_config_get_address (setting, i); - const char *label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (setting, i)); + const char *label = _nm_setting_ip4_config_get_address_label (setting, i); NMPlatformIP4Address address; memset (&address, 0, sizeof (address)); @@ -426,7 +426,7 @@ nm_ip4_config_create_setting (const NMIP4Config *config) nm_ip4_address_set_gateway (s_addr, gateway); if (*address->label) - NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_add_address_with_label (s_ip4, s_addr, address->label)); + _nm_setting_ip4_config_add_address_with_label (s_ip4, s_addr, address->label); else nm_setting_ip4_config_add_address (s_ip4, s_addr); nm_ip4_address_unref (s_addr); diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index ee8c620114..c49d052d7a 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -48,7 +48,7 @@ #include #include #include -#include +#include "nm-core-internal.h" #include #include "nm-platform.h" @@ -1574,7 +1574,7 @@ read_aliases (NMSettingIP4Config *s_ip4, const char *filename, const char *netwo ok = read_full_ip4_address (parsed, network_file, -1, addr, &err); svCloseFile (parsed); if (ok) { - if (!NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_add_address_with_label (s_ip4, addr, device))) + if (!_nm_setting_ip4_config_add_address_with_label (s_ip4, addr, device)) PARSE_WARNING ("duplicate IP4 address in alias file %s", item); } else { PARSE_WARNING ("error reading IP4 address from alias file '%s': %s", diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 8de6977fc9..4b9c7aa675 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -44,7 +44,7 @@ #include #include #include -#include +#include "nm-core-internal.h" #include "NetworkManagerUtils.h" @@ -2986,7 +2986,7 @@ test_read_wired_aliases_good (void) TEST_IFCFG_ALIASES_GOOD, i); - ASSERT (g_strcmp0 (NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, i)), expected_label[j]) == 0, + ASSERT (g_strcmp0 (_nm_setting_ip4_config_get_address_label (s_ip4, i), expected_label[j]) == 0, "aliases-good-verify-ip4", "failed to verify %s: unexpected IP4 address label #%d", TEST_IFCFG_ALIASES_GOOD, i); @@ -3113,7 +3113,7 @@ test_read_wired_aliases_bad (const char *base, const char *expected_id) "aliases-bad-verify-ip4", "failed to verify %s: unexpected IP4 address gateway", base); - ASSERT (g_strcmp0 (NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, 0)), expected_label) == 0, + ASSERT (g_strcmp0 (_nm_setting_ip4_config_get_address_label (s_ip4, 0), expected_label) == 0, "aliases-bad-verify-ip4", "failed to verify %s: unexpected IP4 address label", base); @@ -8185,7 +8185,7 @@ test_write_wired_aliases (void) nm_ip4_address_set_address (addr, ip[i]); nm_ip4_address_set_prefix (addr, prefix); nm_ip4_address_set_gateway (addr, gw); - NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_add_address_with_label (s_ip4, addr, label[i])); + _nm_setting_ip4_config_add_address_with_label (s_ip4, addr, label[i]); nm_ip4_address_unref (addr); } @@ -8293,7 +8293,7 @@ test_write_wired_aliases (void) testfile, i); - ASSERT (g_strcmp0 (NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, i)), label[j]) == 0, + ASSERT (g_strcmp0 (_nm_setting_ip4_config_get_address_label (s_ip4, i), label[j]) == 0, "wired-aliases-write-verify-ip4", "failed to verify %s: unexpected IP4 address label #%d", testfile, i); diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index ab12350837..e1c8f3678e 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -38,7 +38,7 @@ #include #include #include -#include +#include "nm-core-internal.h" #include #include "nm-logging.h" @@ -1927,7 +1927,7 @@ write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) NMIP4Address *addr; guint32 ip; - if (i > 0 && NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, i))) + if (i > 0 && _nm_setting_ip4_config_get_address_label (s_ip4, i)) continue; if (n == 0) { @@ -2202,7 +2202,7 @@ write_ip4_aliases (NMConnection *connection, char *base_ifcfg_path) guint32 ip; shvarFile *ifcfg; - label = NM_UTILS_PRIVATE_CALL (nm_setting_ip4_config_get_address_label (s_ip4, i)); + label = _nm_setting_ip4_config_get_address_label (s_ip4, i); if (!label) continue; if ( strncmp (label, base_name, base_name_len) != 0 From 8fe1b790126f433e8b9d05b3385e7075b6af68ae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 11 Aug 2014 18:10:43 +0200 Subject: [PATCH 02/30] libnm-core: declare NM_SETTING_COMPARE_FLAG_INFERRABLE flag in "nm-core-internal.h" As this flag is used by NM-core, move it to nm-core-internal.h header file. Signed-off-by: Thomas Haller --- libnm-core/nm-core-internal.h | 7 +++++++ libnm-core/nm-setting-private.h | 9 ++------- src/NetworkManagerUtils.c | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 41b51c2f69..9437bbd3a3 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -45,5 +45,12 @@ gboolean _nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *s NMIP4Address *address, const char *label); +/* NM_SETTING_COMPARE_FLAG_INFERRABLE: check whether a device-generated + * connection can be replaced by a already-defined connection. This flag only + * takes into account properties marked with the %NM_SETTING_PARAM_INFERRABLE + * flag. + */ +#define NM_SETTING_COMPARE_FLAG_INFERRABLE 0x80000000 + #endif diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index a31d0d7006..8fe798122a 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -24,6 +24,8 @@ #include "nm-setting.h" #include "nm-glib-compat.h" +#include "nm-core-internal.h" + #define NM_SETTING_SECRET_FLAGS_ALL \ (NM_SETTING_SECRET_FLAG_NONE | \ NM_SETTING_SECRET_FLAG_AGENT_OWNED | \ @@ -73,13 +75,6 @@ gboolean _nm_setting_clear_secrets_with_flags (NMSetting *setting, gpointer user_data); -/* NM_SETTING_COMPARE_FLAG_INFERRABLE: check whether a device-generated - * connection can be replaced by a already-defined connection. This flag only - * takes into account properties marked with the %NM_SETTING_PARAM_INFERRABLE - * flag. - */ -#define NM_SETTING_COMPARE_FLAG_INFERRABLE 0x80000000 - /* The property of the #NMSetting should be considered during comparisons that * use the %NM_SETTING_COMPARE_FLAG_INFERRABLE flag. Properties that don't have * this flag, are ignored when doing an infrerrable comparison. This flag should diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index fc85b0040c..6f3d8b6635 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -33,7 +33,7 @@ #include "NetworkManagerUtils.h" #include "nm-utils.h" -#include "nm-setting-private.h" +#include "nm-core-internal.h" #include "nm-logging.h" #include "nm-device.h" #include "nm-setting-connection.h" From d299c425d4495453743e0bc3884dbeb0a7405108 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 11 Aug 2014 18:21:53 +0200 Subject: [PATCH 03/30] libnm: move declaration of NM_SETTING_SECRET_FLAGS_ALL to nm-core-internal.h As NM_SETTING_SECRET_FLAGS_ALL is used in libnm/nm-vpn-plugin-utils.c, it is exposed as internal API and should be declared in nm-core-internal.h. Signed-off-by: Thomas Haller --- libnm-core/nm-core-internal.h | 8 ++++++++ libnm-core/nm-setting-private.h | 6 ------ libnm-core/nm-setting.h | 2 +- libnm/nm-vpn-plugin-utils.c | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 9437bbd3a3..fdf2397d6d 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -53,4 +53,12 @@ gboolean _nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *s #define NM_SETTING_COMPARE_FLAG_INFERRABLE 0x80000000 + +#define NM_SETTING_SECRET_FLAGS_ALL \ + (NM_SETTING_SECRET_FLAG_NONE | \ + NM_SETTING_SECRET_FLAG_AGENT_OWNED | \ + NM_SETTING_SECRET_FLAG_NOT_SAVED | \ + NM_SETTING_SECRET_FLAG_NOT_REQUIRED) + + #endif diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 8fe798122a..913b8a739e 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -26,12 +26,6 @@ #include "nm-core-internal.h" -#define NM_SETTING_SECRET_FLAGS_ALL \ - (NM_SETTING_SECRET_FLAG_NONE | \ - NM_SETTING_SECRET_FLAG_AGENT_OWNED | \ - NM_SETTING_SECRET_FLAG_NOT_SAVED | \ - NM_SETTING_SECRET_FLAG_NOT_REQUIRED) - /** * NMSettingVerifyResult: * @NM_SETTING_VERIFY_SUCCESS: the setting verifies successfully diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 793d24ed5b..2420cb49a8 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -108,7 +108,7 @@ typedef enum { NM_SETTING_SECRET_FLAG_NOT_SAVED = 0x00000002, NM_SETTING_SECRET_FLAG_NOT_REQUIRED = 0x00000004 - /* NOTE: if adding flags, update nm-setting-private.h as well */ + /* NOTE: if adding flags, update nm-core-internal.h as well */ } NMSettingSecretFlags; /** diff --git a/libnm/nm-vpn-plugin-utils.c b/libnm/nm-vpn-plugin-utils.c index 0e800e9c16..bfa939a15f 100644 --- a/libnm/nm-vpn-plugin-utils.c +++ b/libnm/nm-vpn-plugin-utils.c @@ -25,7 +25,7 @@ #include "nm-vpn-plugin-utils.h" #include "nm-vpn-plugin.h" -#include "nm-setting-private.h" +#include "nm-core-internal.h" #include "nm-dbus-glib-types.h" #define DATA_KEY_TAG "DATA_KEY=" From ff142ce780c8d524b54431db1a523ee36fc1867a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 27 Jul 2014 23:33:16 +0200 Subject: [PATCH 04/30] libnm-core: add _nm_utils_hash_values_to_slist() Signed-off-by: Thomas Haller --- libnm-core/nm-core-internal.h | 3 +++ libnm-core/nm-utils.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index fdf2397d6d..1e6dd63892 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -61,4 +61,7 @@ gboolean _nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *s NM_SETTING_SECRET_FLAG_NOT_REQUIRED) +GSList * _nm_utils_hash_values_to_slist (GHashTable *hash); + + #endif diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 4f91c1e1fa..fd99bfd206 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -536,6 +536,34 @@ done: return valid; } +/** + * _nm_utils_hash_values_to_slist: + * @hash: a #GHashTable + * + * Utility function to iterate over a hash table and return + * it's values as a #GSList. + * + * Returns: (element-type gpointer) (transfer container): a newly allocated #GSList + * containing the values of the hash table. The caller must free the + * returned list with g_slist_free(). The hash values are not owned + * by the returned list. + **/ +GSList * +_nm_utils_hash_values_to_slist (GHashTable *hash) +{ + GSList *list = NULL; + GHashTableIter iter; + void *value; + + g_return_val_if_fail (hash, NULL); + + g_hash_table_iter_init (&iter, hash); + while (g_hash_table_iter_next (&iter, NULL, &value)) + list = g_slist_prepend (list, value); + + return list; +} + static gboolean device_supports_ap_ciphers (guint32 dev_caps, guint32 ap_flags, From e478d1fb71634bbf49cb0bea5990e2ceb961c144 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 27 Jul 2014 23:55:55 +0200 Subject: [PATCH 05/30] all: use _nm_utils_hash_values_to_slist() Signed-off-by: Thomas Haller --- src/settings/nm-settings-connection.c | 18 +++++------------- src/settings/nm-settings.c | 8 ++------ src/settings/plugins/ifupdown/plugin.c | 9 +++------ src/settings/plugins/keyfile/plugin.c | 10 ++-------- 4 files changed, 12 insertions(+), 33 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index d0a4f7ddb1..3a2b5ac20e 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -29,6 +29,7 @@ #include #include #include +#include "nm-core-internal.h" #include "nm-settings-connection.h" #include "nm-session-monitor.h" @@ -1162,7 +1163,7 @@ get_settings_auth_cb (NMSettingsConnection *self, s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (dupl_con)); if (bssid_list && s_wifi) g_object_set (s_wifi, NM_SETTING_WIRELESS_SEEN_BSSIDS, bssid_list, NULL); - g_slist_free_full (bssid_list, g_free); + g_slist_free (bssid_list); /* 802-11-wireless.security property is deprecated. But we set it here so that * we don't disturb old clients that might expect it being properly set for @@ -1745,24 +1746,15 @@ mac_dup (const guint8 *old) * * Returns current list of seen BSSIDs for the connection. * - * Returns: (transfer full) list of seen BSSIDs (in the standard hex-digits-and-colons notation). - * The caller is responsible for freeing the list. + * Returns: (transfer container) list of seen BSSIDs (in the standard hex-digits-and-colons notation). + * The caller is responsible for freeing the list, but not the content. **/ GSList * nm_settings_connection_get_seen_bssids (NMSettingsConnection *connection) { - NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (connection); - GHashTableIter iter; - char *bssid_str; - GSList *bssid_list = NULL; - g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (connection), NULL); - g_hash_table_iter_init (&iter, priv->seen_bssids); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &bssid_str)) - bssid_list = g_slist_prepend (bssid_list, g_strdup (bssid_str)); - - return bssid_list; + return _nm_utils_hash_values_to_slist (NM_SETTINGS_CONNECTION_GET_PRIVATE (connection)->seen_bssids); } /** diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index edc4fa9fb4..3e186eaa52 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -52,6 +52,7 @@ #include #include #include +#include "nm-core-internal.h" #include "nm-device-ethernet.h" #include "nm-dbus-glib-types.h" @@ -1763,13 +1764,8 @@ get_connections (NMConnectionProvider *provider) GSList *list = NULL; NMSettings *self = NM_SETTINGS (provider); NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GHashTableIter iter; - NMSettingsConnection *connection; - g_hash_table_iter_init (&iter, priv->connections); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &connection)) - list = g_slist_prepend (list, connection); - list = g_slist_reverse (list); + list = _nm_utils_hash_values_to_slist (priv->connections); /* Cache the list every call so we can keep it 'const' for callers */ g_slist_free (priv->get_connections_cache); diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c index 7ef7c51b54..ba15c3e3fa 100644 --- a/src/settings/plugins/ifupdown/plugin.c +++ b/src/settings/plugins/ifupdown/plugin.c @@ -41,6 +41,7 @@ #include "nm-setting-wired.h" #include "nm-setting-ppp.h" #include "nm-utils.h" +#include "nm-core-internal.h" #include "nm-ifupdown-connection.h" #include "plugin.h" @@ -505,9 +506,7 @@ static GSList* SCPluginIfupdown_get_connections (NMSystemConfigInterface *config) { SCPluginIfupdownPrivate *priv = SC_PLUGIN_IFUPDOWN_GET_PRIVATE (config); - GSList *connections = NULL; - GHashTableIter iter; - gpointer value; + GSList *connections; nm_log_info (LOGD_SETTINGS, "(%d) ... get_connections.", GPOINTER_TO_UINT(config)); @@ -516,9 +515,7 @@ SCPluginIfupdown_get_connections (NMSystemConfigInterface *config) return NULL; } - g_hash_table_iter_init (&iter, priv->connections); - while (g_hash_table_iter_next (&iter, NULL, &value)) - connections = g_slist_prepend (connections, value); + connections = _nm_utils_hash_values_to_slist (priv->connections); nm_log_info (LOGD_SETTINGS, "(%d) connections count: %d", GPOINTER_TO_UINT(config), g_slist_length(connections)); return connections; diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index a1c776c0e0..562beeaeb1 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -36,6 +36,7 @@ #include #include #include +#include "nm-core-internal.h" #include "plugin.h" #include "nm-system-config-interface.h" @@ -372,20 +373,13 @@ static GSList * get_connections (NMSystemConfigInterface *config) { SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (config); - GHashTableIter iter; - gpointer data = NULL; - GSList *list = NULL; if (!priv->initialized) { setup_monitoring (config); read_connections (config); priv->initialized = TRUE; } - - g_hash_table_iter_init (&iter, priv->connections); - while (g_hash_table_iter_next (&iter, NULL, &data)) - list = g_slist_prepend (list, data); - return list; + return _nm_utils_hash_values_to_slist (priv->connections); } static gboolean From 0da0293f7ed60965fb0d7365972f654d21210ad7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 12:28:20 +0200 Subject: [PATCH 06/30] nmtst: add nmtst_connection_normalize() function Signed-off-by: Thomas Haller --- include/nm-test-utils.h | 65 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index ccf9cac970..b8db189445 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -724,6 +724,71 @@ nmtst_create_minimal_connection (const char *id, const char *uuid, const char *t return con; } +inline static gboolean +_nmtst_connection_normalize_v (NMConnection *connection, va_list args) +{ + GError *error = NULL; + gboolean success; + gboolean was_modified = FALSE; + GHashTable *parameters = NULL; + const char *p_name; + + g_assert (NM_IS_CONNECTION (connection)); + + while ((p_name = va_arg (args, const char *))) { + if (!parameters) + parameters = g_hash_table_new (g_str_hash, g_str_equal); + g_hash_table_insert (parameters, (gpointer *) p_name, va_arg (args, gpointer)); + } + + success = nm_connection_normalize (connection, + parameters, + &was_modified, + &error); + g_assert_no_error (error); + g_assert (success); + + if (parameters) + g_hash_table_destroy (parameters); + + return was_modified; +} + +inline static gboolean +_nmtst_connection_normalize (NMConnection *connection, ...) +{ + gboolean was_modified; + va_list args; + + va_start (args, connection); + was_modified = _nmtst_connection_normalize_v (connection, args); + va_end (args); + + return was_modified; +} +#define nmtst_connection_normalize(connection, ...) \ + _nmtst_connection_normalize(connection, ##__VA_ARGS__, NULL) + +inline static NMConnection * +_nmtst_connection_duplicate_and_normalize (NMConnection *connection, ...) +{ + gboolean was_modified; + va_list args; + + g_assert (NM_IS_CONNECTION (connection)); + + connection = nm_connection_duplicate (connection); + + va_start (args, connection); + was_modified = _nmtst_connection_normalize_v (connection, args); + va_end (args); + + return connection; +} +#define nmtst_connection_duplicate_and_normalize(connection, ...) \ + _nmtst_connection_duplicate_and_normalize(connection, ##__VA_ARGS__, NULL) + #endif + #endif /* __NM_TEST_UTILS_H__ */ From 80e1b05c316ffbde059d9478237f8683405e63a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 13:13:13 +0200 Subject: [PATCH 07/30] nmtst: add nmtst_assert_connection_equals() function Signed-off-by: Thomas Haller --- include/nm-test-utils.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index b8db189445..aeba15269e 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -788,6 +788,44 @@ _nmtst_connection_duplicate_and_normalize (NMConnection *connection, ...) #define nmtst_connection_duplicate_and_normalize(connection, ...) \ _nmtst_connection_duplicate_and_normalize(connection, ##__VA_ARGS__, NULL) +inline static void +nmtst_assert_connection_equals (NMConnection *a, gboolean normalize_a, NMConnection *b, gboolean normalize_b) +{ + gboolean compare; + gs_unref_object NMConnection *a2 = NULL; + gs_unref_object NMConnection *b2 = NULL; + GHashTable *out_settings = NULL; + + g_assert (NM_IS_CONNECTION (a)); + g_assert (NM_IS_CONNECTION (b)); + + if (normalize_a) + a = a2 = nmtst_connection_duplicate_and_normalize (a); + if (normalize_b) + b = b2 = nmtst_connection_duplicate_and_normalize (b); + + compare = nm_connection_diff (a, b, NM_SETTING_COMPARE_FLAG_EXACT, &out_settings); + if (!compare && out_settings) { + const char *name, *pname; + GHashTable *setting; + GHashTableIter iter, iter2; + + g_hash_table_iter_init (&iter, out_settings); + while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &setting)) { + __NMTST_LOG (g_message, ">>> differences in setting '%s':", name); + + g_hash_table_iter_init (&iter2, out_settings); + while (g_hash_table_iter_next (&iter2, (gpointer *) &pname, NULL)) + __NMTST_LOG (g_message, ">>> differences in setting '%s.%s':", name, pname); + } + } + g_assert (compare); + g_assert (!out_settings); + + compare = nm_connection_compare (a, b, NM_SETTING_COMPARE_FLAG_EXACT); + g_assert (compare); +} + #endif From 043ab29ca949bfaf9fcaa034f86bb67ead1a666c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 18 Jul 2014 23:35:22 +0200 Subject: [PATCH 08/30] nmtst: add assertion functions for verify() connection Signed-off-by: Thomas Haller --- include/nm-test-utils.h | 132 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index aeba15269e..9c8009355d 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -36,6 +36,30 @@ #include "gsystem-local-alloc.h" +/*******************************************************************************/ + +/* general purpose functions that have no dependency on other nmtst functions */ + +inline static void +nmtst_assert_error (GError *error, + GQuark expect_error_domain, + gint expect_error_code, + const char *expect_error_pattern) +{ + if (expect_error_domain) + g_assert_error (error, expect_error_domain, expect_error_code); + else + g_assert (error); + g_assert (error->message); + if ( expect_error_pattern + && !g_pattern_match_simple (expect_error_pattern, error->message)) { + g_error ("error message does not have expected pattern '%s'. Instead it is '%s' (%s, %d)", + expect_error_pattern, error->message, g_quark_to_string (error->domain), error->code); + } +} + +/*******************************************************************************/ + struct __nmtst_internal { GRand *rand0; @@ -777,7 +801,7 @@ _nmtst_connection_duplicate_and_normalize (NMConnection *connection, ...) g_assert (NM_IS_CONNECTION (connection)); - connection = nm_connection_duplicate (connection); + connection = nm_simple_connection_new_clone (connection); va_start (args, connection); was_modified = _nmtst_connection_normalize_v (connection, args); @@ -826,6 +850,112 @@ nmtst_assert_connection_equals (NMConnection *a, gboolean normalize_a, NMConnect g_assert (compare); } +inline static void +nmtst_assert_connection_verifies_without_normalization (NMConnection *con) +{ + /* assert that the connection verifies and does not need any normalization */ + + GError *error = NULL; + gboolean success; + gboolean was_modified = FALSE; + gs_unref_object NMConnection *clone = NULL; + + g_assert (NM_IS_CONNECTION (con)); + + clone = nm_simple_connection_new_clone (con); + + success = nm_connection_verify (con, &error); + g_assert_no_error (error); + g_assert (success); + + success = nm_connection_normalize (con, NULL, &was_modified, &error); + g_assert_no_error (error); + g_assert (success); + g_assert (!was_modified); + + nmtst_assert_connection_equals (con, FALSE, clone, FALSE); +} + +inline static void +nmtst_assert_connection_verifies_and_normalizable (NMConnection *con) +{ + /* assert that the connection does verify, but normalization still modifies it */ + GError *error = NULL; + gboolean success; + gboolean was_modified = FALSE; + + g_assert (NM_IS_CONNECTION (con)); + + success = nm_connection_verify (con, &error); + g_assert_no_error (error); + g_assert (success); + g_clear_error (&error); + + success = nm_connection_normalize (con, NULL, &was_modified, &error); + g_assert_no_error (error); + g_assert (success); + g_assert (was_modified); + + /* again! */ + nmtst_assert_connection_verifies_without_normalization (con); +} + +inline static void +nmtst_assert_connection_verifies_after_normalization (NMConnection *con, + GQuark expect_error_domain, + gint expect_error_code) +{ + /* assert that the connection does not verify, but normalization does fix it */ + GError *error = NULL; + gboolean success; + gboolean was_modified = FALSE; + + g_assert (NM_IS_CONNECTION (con)); + + success = nm_connection_verify (con, &error); + nmtst_assert_error (error, expect_error_domain, expect_error_code, NULL); + g_assert (!success); + g_clear_error (&error); + + success = nm_connection_normalize (con, NULL, &was_modified, &error); + g_assert_no_error (error); + g_assert (success); + g_assert (was_modified); + + /* again! */ + nmtst_assert_connection_verifies_without_normalization (con); +} + +inline static void +nmtst_assert_connection_unnormalizable (NMConnection *con, + GQuark expect_error_domain, + gint expect_error_code) +{ + /* assert that the connection does not verify, and it cannot be fixed by normalization */ + + GError *error = NULL; + gboolean success; + gboolean was_modified = FALSE; + gs_unref_object NMConnection *clone = NULL; + + g_assert (NM_IS_CONNECTION (con)); + + clone = nm_simple_connection_new_clone (con); + + success = nm_connection_verify (con, &error); + nmtst_assert_error (error, expect_error_domain, expect_error_code, NULL); + g_assert (!success); + g_clear_error (&error); + + success = nm_connection_normalize (con, NULL, &was_modified, &error); + nmtst_assert_error (error, expect_error_domain, expect_error_code, NULL); + g_assert (!success); + g_assert (!was_modified); + g_clear_error (&error); + + nmtst_assert_connection_equals (con, FALSE, clone, FALSE); +} + #endif From 5f26a9c11ff44474bbaff97a811f1eb636d888b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Jul 2014 13:18:17 +0200 Subject: [PATCH 09/30] keyfile: add NMSettingConnection in reader if missing The recent change c88b832ce9510bdcb2df2bf7e02b1ded88154581 allows for missing 'id' and 'uuid' entries. Further make the keyfile reader more accepting, by creating a missing NMSettingConnection. Signed-off-by: Thomas Haller --- src/settings/plugins/keyfile/reader.c | 68 ++++++++++++++------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c index aba232c0d5..f2a2d473a9 100644 --- a/src/settings/plugins/keyfile/reader.c +++ b/src/settings/plugins/keyfile/reader.c @@ -1281,44 +1281,46 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) * bridge port with all default settings). */ s_con = nm_connection_get_setting_connection (connection); - if (s_con) { - ctype = nm_setting_connection_get_connection_type (s_con); - if (ctype) { - setting = nm_connection_get_setting_by_name (connection, ctype); - if (!setting) { - NMSetting *base_setting; - GType base_setting_type; + if (!s_con) { + s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ()); + nm_connection_add_setting (connection, NM_SETTING (s_con)); + } + ctype = nm_setting_connection_get_connection_type (s_con); + if (ctype) { + setting = nm_connection_get_setting_by_name (connection, ctype); + if (!setting) { + NMSetting *base_setting; + GType base_setting_type; - base_setting_type = nm_setting_lookup_type (ctype); - if (base_setting_type != G_TYPE_INVALID) { - base_setting = (NMSetting *) g_object_new (base_setting_type, NULL); - g_assert (base_setting); - nm_connection_add_setting (connection, base_setting); - } + base_setting_type = nm_setting_lookup_type (ctype); + if (base_setting_type != G_TYPE_INVALID) { + base_setting = (NMSetting *) g_object_new (base_setting_type, NULL); + g_assert (base_setting); + nm_connection_add_setting (connection, base_setting); } } - - /* Make sure that we have 'id' even if not explictly specified in the keyfile */ - if (!nm_setting_connection_get_id (s_con)) { - char *base_name; - - base_name = g_path_get_basename (filename); - g_object_set (s_con, NM_SETTING_CONNECTION_ID, base_name, NULL); - g_free (base_name); - } - - /* Make sure that we have 'uuid' even if not explictly specified in the keyfile */ - if (!nm_setting_connection_get_uuid (s_con)) { - char *hashed_uuid; - - hashed_uuid = nm_utils_uuid_generate_from_string (filename); - g_object_set (s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); - g_free (hashed_uuid); - } - - ensure_slave_setting (connection); } + /* Make sure that we have 'id' even if not explictly specified in the keyfile */ + if (!nm_setting_connection_get_id (s_con)) { + char *base_name; + + base_name = g_path_get_basename (filename); + g_object_set (s_con, NM_SETTING_CONNECTION_ID, base_name, NULL); + g_free (base_name); + } + + /* Make sure that we have 'uuid' even if not explictly specified in the keyfile */ + if (!nm_setting_connection_get_uuid (s_con)) { + char *hashed_uuid; + + hashed_uuid = nm_utils_uuid_generate_from_string (filename); + g_object_set (s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); + g_free (hashed_uuid); + } + + ensure_slave_setting (connection); + /* Handle vpn secrets after the 'vpn' setting was read */ if (vpn_secrets) { NMSettingVpn *s_vpn; From 7d924d725a70622dffe023fa8dcb96d16fcdafd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 13:38:23 +0200 Subject: [PATCH 10/30] keyfile/tests: refactor tests to use nmtst_assert_connection_equals() function Needed in the next commit when we have to normalize the connection before comparing. Signed-off-by: Thomas Haller --- .../plugins/keyfile/tests/test-keyfile.c | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index acbf86dd9d..ba763175ad 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -1376,8 +1376,7 @@ test_write_string_ssid (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE, - "connection-write", "written and re-read connection weren't the same"); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1501,8 +1500,7 @@ test_write_intlist_ssid (void) g_assert_no_error (error); g_assert (reread); - success = nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT); - g_assert (success); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1653,8 +1651,7 @@ test_write_intlike_ssid (void) g_assert_no_error (error); g_assert (reread); - success = nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT); - g_assert (success); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1741,8 +1738,7 @@ test_write_intlike_ssid_2 (void) g_assert_no_error (error); g_assert (reread); - success = nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT); - g_assert (success); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1996,8 +1992,7 @@ test_write_bt_dun_connection (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE, - "connection-write", "written and re-read connection weren't the same"); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -2230,8 +2225,7 @@ test_write_gsm_connection (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE, - "connection-write", "written and re-read connection weren't the same"); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -3224,7 +3218,7 @@ test_write_new_wired_group_name (void) reread = nm_keyfile_plugin_connection_from_file (testfile, &error); g_assert_no_error (error); g_assert (reread); - g_assert (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); /* Look at the keyfile itself to ensure we wrote out the new group names and type */ kf = g_key_file_new (); @@ -3354,7 +3348,7 @@ test_write_new_wireless_group_names (void) reread = nm_keyfile_plugin_connection_from_file (testfile, &error); g_assert_no_error (error); g_assert (reread); - g_assert (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); + nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); /* Look at the keyfile itself to ensure we wrote out the new group names and type */ kf = g_key_file_new (); From 92d82866602067ece1f55e245c0f7061375f9888 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Jul 2014 14:12:58 +0200 Subject: [PATCH 11/30] keyfile: let reader normalize() the connection, not only verify() The new nm_connection_normalize() function allows to fixup an incomplete connection. The keyfile reader should call normalize on a connection, so that we can implement common normalizations there instead of inside the settings plugin. Signed-off-by: Thomas Haller --- src/settings/plugins/keyfile/reader.c | 4 ++-- .../plugins/keyfile/tests/test-keyfile.c | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c index f2a2d473a9..565ce3474e 100644 --- a/src/settings/plugins/keyfile/reader.c +++ b/src/settings/plugins/keyfile/reader.c @@ -1332,8 +1332,8 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) g_strfreev (groups); - /* Verify the connection */ - if (!nm_connection_verify (connection, &verify_error)) { + /* Normalize and verify the connection */ + if (!nm_connection_normalize (connection, NULL, NULL, &verify_error)) { g_set_error (error, KEYFILE_PLUGIN_ERROR, 0, "invalid or missing connection property '%s/%s'", verify_error ? g_type_name (nm_setting_lookup_type_by_quark (verify_error->domain)) : "(unknown)", diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index ba763175ad..369085dd9a 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -1376,7 +1376,7 @@ test_write_string_ssid (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1500,7 +1500,7 @@ test_write_intlist_ssid (void) g_assert_no_error (error); g_assert (reread); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1651,7 +1651,7 @@ test_write_intlike_ssid (void) g_assert_no_error (error); g_assert (reread); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1738,7 +1738,7 @@ test_write_intlike_ssid_2 (void) g_assert_no_error (error); g_assert (reread); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -1992,7 +1992,7 @@ test_write_bt_dun_connection (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -2225,7 +2225,7 @@ test_write_gsm_connection (void) reread = nm_keyfile_plugin_connection_from_file (testfile, NULL); ASSERT (reread != NULL, "connection-write", "failed to re-read test connection"); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); g_clear_error (&error); unlink (testfile); @@ -3218,7 +3218,7 @@ test_write_new_wired_group_name (void) reread = nm_keyfile_plugin_connection_from_file (testfile, &error); g_assert_no_error (error); g_assert (reread); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); /* Look at the keyfile itself to ensure we wrote out the new group names and type */ kf = g_key_file_new (); @@ -3348,7 +3348,7 @@ test_write_new_wireless_group_names (void) reread = nm_keyfile_plugin_connection_from_file (testfile, &error); g_assert_no_error (error); g_assert (reread); - nmtst_assert_connection_equals (connection, FALSE, reread, FALSE); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); /* Look at the keyfile itself to ensure we wrote out the new group names and type */ kf = g_key_file_new (); From 686e912b82405151566cd4deae9926196d09c79e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 14:20:40 +0200 Subject: [PATCH 12/30] ifcfg-rh: remove verify() connection during reading At the end of reading the connection, reader calls nm_connection_normalize() to normalize the connection. Normalization inplicitly verifies the connection. Doing a verify along the way is not needed and even harmful. Soon further checks will be added that make verify() fail, but normalize() can fix the connection. So, while reading, we might actually have an invalid connection, that will be normalized as last step. Signed-off-by: Thomas Haller --- src/settings/plugins/ifcfg-rh/reader.c | 34 -------------------------- 1 file changed, 34 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index c49d052d7a..e95bcb4226 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -3802,11 +3802,6 @@ wireless_connection_from_ifcfg (const char *file, } nm_connection_add_setting (connection, con_setting); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4020,11 +4015,6 @@ wired_connection_from_ifcfg (const char *file, if (s_8021x) nm_connection_add_setting (connection, NM_SETTING (s_8021x)); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4183,11 +4173,6 @@ infiniband_connection_from_ifcfg (const char *file, } nm_connection_add_setting (connection, infiniband_setting); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4308,11 +4293,6 @@ bond_connection_from_ifcfg (const char *file, if (s_8021x) nm_connection_add_setting (connection, NM_SETTING (s_8021x)); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4419,11 +4399,6 @@ team_connection_from_ifcfg (const char *file, if (s_8021x) nm_connection_add_setting (connection, NM_SETTING (s_8021x)); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4604,11 +4579,6 @@ bridge_connection_from_ifcfg (const char *file, } nm_connection_add_setting (connection, bridge_setting); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } - return connection; } @@ -4921,10 +4891,6 @@ vlan_connection_from_ifcfg (const char *file, if (s_8021x) nm_connection_add_setting (connection, NM_SETTING (s_8021x)); - if (!nm_connection_verify (connection, error)) { - g_object_unref (connection); - return NULL; - } return connection; } From 1effc604cbcdc06d40f505e2858845fc04788f1e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 15:01:54 +0200 Subject: [PATCH 13/30] libnm-core: add _nm_setting_find_in_list_required() function This is an utility function that can be called during verify() to find an NMSetting from @all_settings. This is especially useful for looking up the NMSettingConnection which usually is present. So just get it quickly. In the unexpected case that it is missing, it sets @error and we can return. Signed-off-by: Thomas Haller --- libnm-core/nm-setting-private.h | 6 ++++++ libnm-core/nm-setting.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 913b8a739e..e68d5a4278 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -90,6 +90,12 @@ static void __attribute__((constructor)) register_setting (void) \ NMSetting *nm_setting_find_in_list (GSList *settings_list, const char *setting_name); +NMSetting * _nm_setting_find_in_list_required (GSList *all_settings, + const char *setting_name, + GError **error, + const char *error_prefix_setting_name, + const char *error_prefix_property_name); + NMSettingVerifyResult _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, gboolean allow_missing, const char *setting_name, diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 6acb75ef27..830484b569 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1288,6 +1288,34 @@ nm_setting_get_virtual_iface_name (NMSetting *setting) return NULL; } +NMSetting * +_nm_setting_find_in_list_required (GSList *all_settings, + const char *setting_name, + GError **error, + const char *error_prefix_setting_name, + const char *error_prefix_property_name) +{ + NMSetting *setting; + + g_return_val_if_fail (!error || !*error, NULL); + g_return_val_if_fail (all_settings, NULL); + g_return_val_if_fail (setting_name, NULL); + g_return_val_if_fail (!error_prefix_setting_name == !error_prefix_property_name, NULL); + + setting = nm_setting_find_in_list (all_settings, setting_name); + if (!setting) { + g_set_error (error, + NM_CONNECTION_ERROR, + !strcmp (setting_name, NM_SETTING_CONNECTION_SETTING_NAME) + ? NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND + : NM_CONNECTION_ERROR_SETTING_NOT_FOUND, + _("Missing '%s' setting"), + setting_name); + if (error_prefix_setting_name) + g_prefix_error (error, "%s.%s: ", error_prefix_setting_name, error_prefix_property_name); + } + return setting; +} NMSettingVerifyResult _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, From b87ed28f0185cba3e6a4e3fae84dc5011c92c6ef Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Jul 2014 18:54:43 +0200 Subject: [PATCH 14/30] libnm-core: don't set GError on invalid @connection argument in _nm_connection_verify() In general, we don't set errors if passing a completely invalid @self pointer to a method. We usually also don't set the error argument when asserting. So, just drop it. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 6c6cf9fe36..9feb6ae9bf 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -671,16 +671,8 @@ _nm_connection_verify (NMConnection *connection, GError **error) GError *normalizable_error = NULL; NMSettingVerifyResult normalizable_error_type = NM_SETTING_VERIFY_SUCCESS; - if (error) - g_return_val_if_fail (*error == NULL, NM_SETTING_VERIFY_ERROR); - - if (!NM_IS_CONNECTION (connection)) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_UNKNOWN, - "invalid connection; failed verification"); - g_return_val_if_fail (NM_IS_CONNECTION (connection), NM_SETTING_VERIFY_ERROR); - } + g_return_val_if_fail (NM_IS_CONNECTION (connection), NM_SETTING_VERIFY_ERROR); + g_return_val_if_fail (!error || !*error, NM_SETTING_VERIFY_ERROR); priv = NM_CONNECTION_GET_PRIVATE (connection); From dfba4ce1e1c5bd0c1ae798bc538b5fab3e3faded Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 7 Jul 2014 17:36:12 +0200 Subject: [PATCH 15/30] libnm-core: properly disconnect "notify" signal for settings in NMConnection When removing/replacing a NMSetting in an NMConnection, we have to disconnect setting_changed_cb() from the "notify" signal. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 9feb6ae9bf..ea90329971 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -128,12 +128,23 @@ setting_changed_cb (NMSetting *setting, g_signal_emit (self, signals[CHANGED], 0); } +static gboolean +_setting_release (gpointer key, gpointer value, gpointer user_data) +{ + g_signal_handlers_disconnect_by_func (user_data, setting_changed_cb, value); + return TRUE; +} + static void _nm_connection_add_setting (NMConnection *connection, NMSetting *setting) { - g_hash_table_insert (NM_CONNECTION_GET_PRIVATE (connection)->settings, - (gpointer) G_OBJECT_TYPE_NAME (setting), - setting); + NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection); + const char *name = G_OBJECT_TYPE_NAME (setting); + NMSetting *s_old; + + if ((s_old = g_hash_table_lookup (priv->settings, (gpointer) name))) + g_signal_handlers_disconnect_by_func (s_old, setting_changed_cb, connection); + g_hash_table_insert (priv->settings, (gpointer) name, setting); /* Listen for property changes so we can emit the 'changed' signal */ g_signal_connect (setting, "notify", (GCallback) setting_changed_cb, connection); } @@ -290,7 +301,7 @@ hash_to_connection (NMConnection *connection, GHashTable *new, GError **error) NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection); if ((changed = g_hash_table_size (priv->settings) > 0)) - g_hash_table_remove_all (priv->settings); + g_hash_table_foreach_remove (priv->settings, _setting_release, connection); g_hash_table_iter_init (&iter, new); while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) { @@ -376,7 +387,7 @@ nm_connection_replace_settings_from_connection (NMConnection *connection, priv = NM_CONNECTION_GET_PRIVATE (connection); if ((changed = g_hash_table_size (priv->settings) > 0)) - g_hash_table_remove_all (priv->settings); + g_hash_table_foreach_remove (priv->settings, _setting_release, connection); if (g_hash_table_size (NM_CONNECTION_GET_PRIVATE (new_connection)->settings)) { g_hash_table_iter_init (&iter, NM_CONNECTION_GET_PRIVATE (new_connection)->settings); @@ -1884,12 +1895,8 @@ static void nm_connection_private_free (NMConnectionPrivate *priv) { NMConnection *self = priv->self; - GHashTableIter iter; - NMSetting *setting; - g_hash_table_iter_init (&iter, priv->settings); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &setting)) - g_signal_handlers_disconnect_by_func (setting, setting_changed_cb, self); + g_hash_table_foreach_remove (priv->settings, _setting_release, self); g_hash_table_destroy (priv->settings); g_slice_free (NMConnectionPrivate, priv); From 651daee516443b2523be63046fe0b6f439770659 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Jul 2014 19:49:52 +0200 Subject: [PATCH 16/30] libnm-core: move validation of NMSettingConnection:type to NMSettingConnection:verify() Partly it was already there. This makes NMSettingConnection:verify() stricter then before, but validates the same as of NMConnection:_nm_connection_verify(). Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 32 ------------------------------ libnm-core/nm-setting-connection.c | 31 +++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index ea90329971..549dc6527d 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -677,8 +677,6 @@ _nm_connection_verify (NMConnection *connection, GError **error) gpointer value; GSList *all_settings = NULL, *setting_i; NMSettingVerifyResult success = NM_SETTING_VERIFY_ERROR; - NMSetting *base; - const char *ctype; GError *normalizable_error = NULL; NMSettingVerifyResult normalizable_error_type = NM_SETTING_VERIFY_SUCCESS; @@ -750,36 +748,6 @@ _nm_connection_verify (NMConnection *connection, GError **error) } g_slist_free (all_settings); - /* Now make sure the given 'type' setting can actually be the base setting - * of the connection. Can't have type=ppp for example. - */ - ctype = nm_setting_connection_get_connection_type (s_con); - if (!ctype) { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, - "connection type missing"); - goto EXIT; - } - - base = nm_connection_get_setting_by_name (connection, ctype); - if (!base) { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, - "base setting GType not found"); - goto EXIT; - } - - if (!_nm_setting_is_base_type (base)) { - g_set_error (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, - "connection type '%s' is not a base type", - ctype); - goto EXIT; - } - s_ip4 = nm_connection_get_setting_ip4_config (connection); s_ip6 = nm_connection_get_setting_ip6_config (connection); diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 2205bff199..c359298250 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -837,14 +837,29 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) } /* Make sure the corresponding 'type' item is present */ - if (all_settings && !nm_setting_find_in_list (all_settings, priv->type)) { - g_set_error (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, - _("requires presence of '%s' setting in the connection"), - priv->type); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); - return FALSE; + if (all_settings) { + NMSetting *s_base; + + s_base = nm_setting_find_in_list (all_settings, priv->type); + if (!s_base) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, + _("requires presence of '%s' setting in the connection"), + priv->type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return FALSE; + } + + if (!_nm_setting_is_base_type (s_base)) { + g_set_error (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, + _("connection type '%s' is not a base type"), + priv->type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return FALSE; + } } is_slave = ( priv->slave_type From 7279e7e150180a1196913a0fb141f67d637f7adc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 7 Jul 2014 17:05:10 +0200 Subject: [PATCH 17/30] libnm-core: normalize slave-type and slave-settings of connections Some NMSettingConnection:slave-type types require a matching slave #NMSetting. Add normalization of either the 'slave-type' property or the slave-setting. Also be more strict in NMSettingConnection:verify() to enforce an existing slave-setting depending on the slave-type. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 43 ++++++++++++- libnm-core/nm-setting-bridge-port.c | 26 ++++++++ libnm-core/nm-setting-connection.c | 62 +++++++++++++++---- libnm-core/nm-setting-connection.h | 3 + libnm-core/nm-setting-private.h | 3 + libnm-core/nm-setting-team-port.c | 24 +++++++ libnm-core/nm-setting.c | 59 ++++++++++++++++++ libnm-core/tests/test-general.c | 60 ++++++++++++++++++ po/POTFILES.in | 1 + .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 7 ++- 10 files changed, 273 insertions(+), 15 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 549dc6527d..709fc20a64 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -25,7 +25,7 @@ #include #include #include "nm-connection.h" -#include "nm-utils.h" +#include "nm-utils-internal.h" #include "nm-dbus-glib-types.h" #include "nm-setting-private.h" @@ -580,6 +580,46 @@ _normalize_virtual_iface_name (NMConnection *self) return was_modified; } +static gboolean +_normalize_connection_slave_type (NMConnection *self) +{ + NMSettingConnection *s_con = nm_connection_get_setting_connection (self); + const char *slave_type, *port_type; + GSList *all_settings; + + if (!s_con) + return FALSE; + if (!nm_setting_connection_get_master (s_con)) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if (slave_type) { + if ( _nm_setting_slave_type_is_valid (slave_type, &port_type) + && port_type) { + NMSetting *s_port; + + s_port = nm_connection_get_setting_by_name (self, port_type); + if (!s_port) { + GType p_type = nm_setting_lookup_type (port_type); + + g_return_val_if_fail (p_type, FALSE); + nm_connection_add_setting (self, g_object_new (p_type, NULL)); + return TRUE; + } + } + } else { + all_settings = _nm_utils_hash_values_to_slist (NM_CONNECTION_GET_PRIVATE (self)->settings); + + if ((slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, NULL))) { + g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, NULL); + g_slist_free (all_settings); + return TRUE; + } + g_slist_free (all_settings); + } + return FALSE; +} + static gboolean _normalize_ip_config (NMConnection *self, GHashTable *parameters) { @@ -837,6 +877,7 @@ nm_connection_normalize (NMConnection *connection, * errors, because in that case we rather fail without touching the settings. */ was_modified |= _normalize_virtual_iface_name (connection); + was_modified |= _normalize_connection_slave_type (connection); was_modified |= _normalize_ip_config (connection, parameters); /* Verify anew. */ diff --git a/libnm-core/nm-setting-bridge-port.c b/libnm-core/nm-setting-bridge-port.c index 243f4668dd..0bc1a1c828 100644 --- a/libnm-core/nm-setting-bridge-port.c +++ b/libnm-core/nm-setting-bridge-port.c @@ -159,6 +159,32 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return FALSE; } + + if (all_settings) { + NMSettingConnection *s_con; + const char *slave_type; + + s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings, + NM_SETTING_CONNECTION_SETTING_NAME, + error, NULL, NULL)); + if (!s_con) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if ( slave_type + && strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"), + NM_SETTING_BRIDGE_PORT_SETTING_NAME, + NM_SETTING_BRIDGE_SETTING_NAME, + slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } + return TRUE; } diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index c359298250..fdfe5f7214 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -741,7 +741,11 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) { NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting); gboolean is_slave; + const char *slave_setting_type = NULL; GSList *iter; + const char *normerr_slave_setting_type = NULL; + const char *normerr_missing_slave_type = NULL; + const char *normerr_missing_slave_type_port = NULL; if (!priv->id) { g_set_error_literal (error, @@ -862,10 +866,9 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) } } - is_slave = ( priv->slave_type - && ( !strcmp (priv->slave_type, NM_SETTING_BOND_SETTING_NAME) - || !strcmp (priv->slave_type, NM_SETTING_BRIDGE_SETTING_NAME) - || !strcmp (priv->slave_type, NM_SETTING_TEAM_SETTING_NAME))); + is_slave = FALSE; + if (priv->slave_type) + is_slave = _nm_setting_slave_type_is_valid (priv->slave_type, &slave_setting_type); if (priv->slave_type && !is_slave) { g_set_error (error, @@ -873,7 +876,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, _("Unknown slave type '%s'"), priv->slave_type); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); - return NM_SETTING_VERIFY_ERROR; + return FALSE; } if (is_slave) { @@ -883,19 +886,54 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, _("Slave connections need a valid '" NM_SETTING_CONNECTION_MASTER "' property")); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER); - return NM_SETTING_VERIFY_ERROR; + return FALSE; } + if ( slave_setting_type + && all_settings /* only check for an existing slave-setting when having @all_settings */ + && !nm_setting_find_in_list (all_settings, slave_setting_type)) + normerr_slave_setting_type = slave_setting_type; } else { if (priv->master) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, - _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); - return NM_SETTING_VERIFY_ERROR; + const char *slave_type; + NMSetting *s_port; + + if ( all_settings + && (slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, &s_port))) { + normerr_missing_slave_type = slave_type; + normerr_missing_slave_type_port = nm_setting_get_name (s_port); + } else { + g_set_error_literal (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'")); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } } } + /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */ + + if (normerr_slave_setting_type) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND, + _("slave-type '%s' requires a '%s' setting in the connection"), + priv->slave_type, normerr_slave_setting_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + + if (normerr_missing_slave_type) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("Detect a slave connection with '" NM_SETTING_CONNECTION_MASTER "' set and a port type '%s'. '" NM_SETTING_CONNECTION_SLAVE_TYPE "' should be set to '%s'"), + normerr_missing_slave_type_port, normerr_missing_slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + return TRUE; } diff --git a/libnm-core/nm-setting-connection.h b/libnm-core/nm-setting-connection.h index 7782567e39..169e2c18ee 100644 --- a/libnm-core/nm-setting-connection.h +++ b/libnm-core/nm-setting-connection.h @@ -52,6 +52,8 @@ G_BEGIN_DECLS * #NMSettingConnection:type property was not present in the #NMConnection * @NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED: ip configuration is not * allowed to be present. + * @NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND: the mandatory #NMSetting + * object for the slave is missing. The slave type depends on #NMSettingConnection:slave-type * * Describes errors that may result from operations involving a * #NMSettingConnection. @@ -64,6 +66,7 @@ typedef enum NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, /*< nick=MissingProperty >*/ NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, /*< nick=TypeSettingNotFound >*/ NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED, /*< nick=IpConfigNotAllowed >*/ + NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND, /*< nick=SlaveSettingNotFound >*/ } NMSettingConnectionError; #define NM_SETTING_CONNECTION_ERROR nm_setting_connection_error_quark () diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index e68d5a4278..316838a32b 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -110,4 +110,7 @@ NMSettingVerifyResult _nm_setting_verify (NMSetting *setting, GSList *all_settings, GError **error); +gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); +const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); + #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index cb8d00c38f..e5cac6d61a 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -103,6 +103,30 @@ nm_setting_team_port_get_config (NMSettingTeamPort *setting) static gboolean verify (NMSetting *setting, GSList *all_settings, GError **error) { + if (all_settings) { + NMSettingConnection *s_con; + const char *slave_type; + + s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings, + NM_SETTING_CONNECTION_SETTING_NAME, + error, NULL, NULL)); + if (!s_con) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if ( slave_type + && strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME)) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"), + NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_SETTING_NAME, + slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } return TRUE; } diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 830484b569..5fc0b64867 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -288,6 +288,65 @@ _nm_setting_compare_priority (gconstpointer a, gconstpointer b) /*************************************************************/ +gboolean +_nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type) +{ + const char *port_type = NULL; + gboolean found = TRUE; + + if (!slave_type) + found = FALSE; + else if (!strcmp (slave_type, NM_SETTING_BOND_SETTING_NAME)) + ; + else if (!strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)) + port_type = NM_SETTING_BRIDGE_PORT_SETTING_NAME; + else if (!strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME)) + port_type = NM_SETTING_TEAM_PORT_SETTING_NAME; + else + found = FALSE; + + if (out_port_type) + *out_port_type = port_type; + return found; +} + + +const char * +_nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port) +{ + GSList *iter; + const char *slave_type = NULL; + NMSetting *s_port = NULL; + + for (iter = all_settings; iter; iter = iter->next) { + NMSetting *s_iter = NM_SETTING (iter->data); + const char *name = nm_setting_get_name (s_iter); + const char *i_slave_type = NULL; + + if (!strcmp (name, NM_SETTING_BRIDGE_PORT_SETTING_NAME)) + i_slave_type = NM_SETTING_BRIDGE_SETTING_NAME; + else if (!strcmp (name, NM_SETTING_TEAM_PORT_SETTING_NAME)) + i_slave_type = NM_SETTING_TEAM_SETTING_NAME; + else + continue; + + if (slave_type) { + /* there are more then one matching port types, cannot detect the slave type. */ + slave_type = NULL; + s_port = NULL; + break; + } + slave_type = i_slave_type; + s_port = s_iter; + } + + if (out_s_port) + *out_s_port = s_port; + return slave_type; +} + +/*************************************************************/ + static void destroy_gvalue (gpointer data) { diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 0c660d0384..f9a7714b8c 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2658,6 +2658,64 @@ test_connection_normalize_virtual_iface_name (void) g_object_unref (con); } +static void +test_connection_normalize_slave_type_1 (void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingConnection *s_con; + + con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_1", + "cc4cd5df-45dc-483e-b291-6b76c2338ecb", + NM_SETTING_WIRED_SETTING_NAME, &s_con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_MASTER, "master0", + NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type", + NULL); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY); + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + + g_object_set (s_con, + NM_SETTING_CONNECTION_SLAVE_TYPE, "bridge", + NULL); + + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND); + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME); +} + +static void +test_connection_normalize_slave_type_2 (void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingConnection *s_con; + + con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_2", + "40bea008-ca72-439a-946b-e65f827656f9", + NM_SETTING_WIRED_SETTING_NAME, &s_con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_MASTER, "master0", + NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type", + NULL); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY); + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + + g_object_set (s_con, + NM_SETTING_CONNECTION_SLAVE_TYPE, NULL, + NULL); + nm_connection_add_setting (con, nm_setting_bridge_port_new ()); + + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NULL); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME); +} + NMTST_DEFINE (); int main (int argc, char **argv) @@ -2697,6 +2755,8 @@ int main (int argc, char **argv) test_connection_new_from_hash (); test_connection_verify_sets_interface_name (); test_connection_normalize_virtual_iface_name (); + test_connection_normalize_slave_type_1 (); + test_connection_normalize_slave_type_2 (); test_setting_connection_permissions_helpers (); test_setting_connection_permissions_property (); diff --git a/po/POTFILES.in b/po/POTFILES.in index 37fd6199a6..86a0d1834b 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ libnm-core/nm-setting-ip6-config.c libnm-core/nm-setting-olpc-mesh.c libnm-core/nm-setting-ppp.c libnm-core/nm-setting-pppoe.c +libnm-core/nm-setting-team-port.c libnm-core/nm-setting-vlan.c libnm-core/nm-setting-vpn.c libnm-core/nm-setting-wimax.c diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 4b9c7aa675..d917159efe 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -14409,8 +14409,11 @@ test_read_team_port_empty_config (void) g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_WIRED_SETTING_NAME); g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, "team0"); - /* Empty TEAM_PORT_CONFIG means no team-port setting */ - g_assert (nm_connection_get_setting_team_port (connection) == NULL); + /* Normalization adds a team-port setting */ + g_assert (nm_connection_get_setting_team_port (connection)); + + /* empty/missing config */ + g_assert (!nm_setting_team_port_get_config (nm_connection_get_setting_team_port (connection))); g_object_unref (connection); } From cf44a15874f09c3d88a60dd4fa0b985bcfbaf8e2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 7 Jul 2014 19:40:39 +0200 Subject: [PATCH 18/30] keyfile: remove ensure_slave_setting() when reading connection nm_connection_normalize() can now add the slave setting as needed. Remove the duplicate functionality. This undoes commit 664d64e0c04bd4b83137a682ff9a9881966f6f94 but the same functionality is now provided via normalize(). Signed-off-by: Thomas Haller --- src/settings/plugins/keyfile/reader.c | 35 +++------------------------ 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c index 565ce3474e..2e356b92e2 100644 --- a/src/settings/plugins/keyfile/reader.c +++ b/src/settings/plugins/keyfile/reader.c @@ -1200,30 +1200,6 @@ read_vpn_secrets (GKeyFile *file, NMSettingVpn *s_vpn) g_strfreev (keys); } -static void -ensure_slave_setting (NMConnection *connection) -{ - NMSettingConnection *s_con = nm_connection_get_setting_connection (connection); - const char *slave_type; - GType slave_gtype = G_TYPE_INVALID; - NMSetting *setting; - - slave_type = nm_setting_connection_get_slave_type (s_con); - if (!slave_type) - return; - - if (g_strcmp0 (slave_type, NM_SETTING_BRIDGE_SETTING_NAME) == 0) - slave_gtype = NM_TYPE_SETTING_BRIDGE_PORT; - else if (g_strcmp0 (slave_type, NM_SETTING_TEAM_SETTING_NAME) == 0) - slave_gtype = NM_TYPE_SETTING_TEAM_PORT; - - if (slave_gtype != G_TYPE_INVALID && !nm_connection_get_setting (connection, slave_gtype)) { - setting = (NMSetting *) g_object_new (slave_gtype, NULL); - g_assert (setting); - nm_connection_add_setting (connection, setting); - } -} - NMConnection * nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) { @@ -1274,11 +1250,10 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) nm_connection_add_setting (connection, setting); } - /* Make sure that we have the base device type and slave type settings - * even if the keyfile didn't include it, which can happen when the - * setting in question is all default values (like ethernet where - * the MAC address isn't given, or VLAN when the VLAN ID is zero, or - * bridge port with all default settings). + /* Make sure that we have the base device type setting even if + * the keyfile didn't include it, which can happen when the base + * device type setting is all default values (like ethernet where + * the MAC address isn't given, or VLAN when the VLAN ID is zero). */ s_con = nm_connection_get_setting_connection (connection); if (!s_con) { @@ -1319,8 +1294,6 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) g_free (hashed_uuid); } - ensure_slave_setting (connection); - /* Handle vpn secrets after the 'vpn' setting was read */ if (vpn_secrets) { NMSettingVpn *s_vpn; From f60256e035ff650a86e4a8073d91c512d02bcb4d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 17:22:43 +0200 Subject: [PATCH 19/30] libnm-core: stricter verification of NMSettingConnection:type property Now also verify the 'type' property regardless of existing @all_settings. Signed-off-by: Thomas Haller --- libnm-core/nm-setting-connection.c | 47 ++++++++++++++++++------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index fdfe5f7214..baf94c7250 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -754,7 +754,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) _("property is missing")); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_ID); return FALSE; - } else if (!strlen (priv->id)) { + } else if (!priv->id[0]) { g_set_error_literal (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, @@ -831,35 +831,46 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) _("property is missing")); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); return FALSE; - } else if (!strlen (priv->type)) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, - _("property is empty")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); - return FALSE; - } + } else { + GType base_type; - /* Make sure the corresponding 'type' item is present */ - if (all_settings) { - NMSetting *s_base; + if (!priv->type[0]) { + g_set_error_literal (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("property is empty")); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return FALSE; + } - s_base = nm_setting_find_in_list (all_settings, priv->type); - if (!s_base) { + base_type = nm_setting_lookup_type (priv->type); + if (base_type == G_TYPE_INVALID) { g_set_error (error, NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, - _("requires presence of '%s' setting in the connection"), + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("connection type '%s' is not valid"), priv->type); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); return FALSE; } - if (!_nm_setting_is_base_type (s_base)) { + if (!_nm_setting_type_is_base_type (base_type)) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, - _("connection type '%s' is not a base type"), + _("connection type '%s' is not a valid base type"), + priv->type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return FALSE; + } + + /* Make sure the corresponding 'type' item is present */ + if ( all_settings + && !nm_setting_find_in_list (all_settings, priv->type)) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, + _("requires presence of '%s' setting in the connection"), priv->type); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); return FALSE; From a9e0796433cd89c7a2280777ac35e659c95b52d8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 18:36:02 +0200 Subject: [PATCH 20/30] libnm-core: normalize NMSettingConnection:type property nm_connection_normalize() can now detect the 'type' property based on existing base settings. It can also create a (default) base setting. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 36 ++++ libnm-core/nm-setting-connection.c | 63 ++++-- libnm-core/nm-setting-private.h | 1 + libnm-core/nm-setting.c | 23 ++ libnm-core/tests/test-general.c | 327 +++++++++++++++++++++++++++++ 5 files changed, 437 insertions(+), 13 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 709fc20a64..1f112603a1 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -580,6 +580,41 @@ _normalize_virtual_iface_name (NMConnection *self) return was_modified; } +static gboolean +_normalize_connection_type (NMConnection *self) +{ + NMSettingConnection *s_con = nm_connection_get_setting_connection (self); + NMSetting *s_base = NULL; + const char *type; + GSList *all_settings; + + type = nm_setting_connection_get_connection_type (s_con); + + if (type) { + s_base = nm_connection_get_setting_by_name (self, type); + + if (!s_base) { + GType base_type = nm_setting_lookup_type (type); + + g_return_val_if_fail (base_type, FALSE); + nm_connection_add_setting (self, g_object_new (base_type, NULL)); + return TRUE; + } + } else { + all_settings = _nm_utils_hash_values_to_slist (NM_CONNECTION_GET_PRIVATE (self)->settings); + + s_base = _nm_setting_find_in_list_base_type (all_settings); + g_return_val_if_fail (s_base, FALSE); + + type = nm_setting_get_name (s_base); + g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, type, NULL); + g_slist_free (all_settings); + return TRUE; + } + + return FALSE; +} + static gboolean _normalize_connection_slave_type (NMConnection *self) { @@ -876,6 +911,7 @@ nm_connection_normalize (NMConnection *connection, * We only do this, after verifying that the connection contains no un-normalizable * errors, because in that case we rather fail without touching the settings. */ + was_modified |= _normalize_connection_type (connection); was_modified |= _normalize_virtual_iface_name (connection); was_modified |= _normalize_connection_slave_type (connection); was_modified |= _normalize_ip_config (connection, parameters); diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index baf94c7250..28e5adf7de 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -735,6 +735,16 @@ nm_setting_connection_get_gateway_ping_timeout (NMSettingConnection *setting) return NM_SETTING_CONNECTION_GET_PRIVATE (setting)->gateway_ping_timeout; } +static void +_set_error_missing_base_setting (GError **error, const char *type) +{ + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, + _("requires presence of '%s' setting in the connection"), + type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); +} static gboolean verify (NMSetting *setting, GSList *all_settings, GError **error) @@ -743,9 +753,11 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) gboolean is_slave; const char *slave_setting_type = NULL; GSList *iter; + NMSetting *normerr_base_type = NULL; const char *normerr_slave_setting_type = NULL; const char *normerr_missing_slave_type = NULL; const char *normerr_missing_slave_type_port = NULL; + gboolean normerr_base_setting = FALSE; if (!priv->id) { g_set_error_literal (error, @@ -825,12 +837,14 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) } if (!priv->type) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, - _("property is missing")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); - return FALSE; + if (!(normerr_base_type = _nm_setting_find_in_list_base_type (all_settings))) { + g_set_error_literal (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("property is missing")); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return FALSE; + } } else { GType base_type; @@ -867,13 +881,21 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) /* Make sure the corresponding 'type' item is present */ if ( all_settings && !nm_setting_find_in_list (all_settings, priv->type)) { - g_set_error (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, - _("requires presence of '%s' setting in the connection"), - priv->type); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); - return FALSE; + NMSetting *s_base; + GSList *all_settings2; + + s_base = g_object_new (base_type, NULL); + all_settings2 = g_slist_prepend (all_settings, s_base); + + normerr_base_setting = nm_setting_verify (s_base, all_settings2, NULL); + + (void) g_slist_delete_link (all_settings2, all_settings2); + g_object_unref (s_base); + + if (!normerr_base_setting) { + _set_error_missing_base_setting (error, priv->type); + return FALSE; + } } } @@ -925,6 +947,21 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */ + if (normerr_base_type) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("property type should be set to '%s'"), + nm_setting_get_name (normerr_base_type)); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + + if (normerr_base_setting) { + _set_error_missing_base_setting (error, priv->type); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + if (normerr_slave_setting_type) { g_set_error (error, NM_SETTING_CONNECTION_ERROR, diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 316838a32b..4c316864b1 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -110,6 +110,7 @@ NMSettingVerifyResult _nm_setting_verify (NMSetting *setting, GSList *all_settings, GError **error); +NMSetting *_nm_setting_find_in_list_base_type (GSList *all_settings); gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 5fc0b64867..61cbdc3ede 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -311,6 +311,29 @@ _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_t } +NMSetting * +_nm_setting_find_in_list_base_type (GSList *all_settings) +{ + GSList *iter; + NMSetting *setting = NULL; + + for (iter = all_settings; iter; iter = iter->next) { + NMSetting *s_iter = NM_SETTING (iter->data); + + if (!_nm_setting_is_base_type (s_iter)) + continue; + + if (setting) { + /* FIXME: currently, if there is more than one matching base type, + * we cannot detect the base setting. + * See: https://bugzilla.gnome.org/show_bug.cgi?id=696936#c8 */ + return NULL; + } + setting = s_iter; + } + return setting; +} + const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port) { diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index f9a7714b8c..a77930f7aa 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2658,6 +2658,332 @@ test_connection_normalize_virtual_iface_name (void) g_object_unref (con); } +static void +_test_connection_normalize_type_normalizable_setting (const char *type, + void (*prepare_normalizable_fcn) (NMConnection *con)) +{ + NMSettingConnection *s_con; + NMSetting *s_base; + GType base_type; + gs_unref_object NMConnection *con = NULL; + gs_free char *id = g_strdup_printf ("%s[%s]", G_STRFUNC, type); + + base_type = nm_setting_lookup_type (type); + g_assert (base_type != G_TYPE_INVALID); + g_assert (_nm_setting_type_is_base_type (base_type)); + + con = nmtst_create_minimal_connection (id, NULL, NULL, &s_con); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + + g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, type, NULL); + + if (prepare_normalizable_fcn) + prepare_normalizable_fcn (con); + + g_assert (!nm_connection_get_setting_by_name (con, type)); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND); + + s_base = nm_connection_get_setting_by_name (con, type); + g_assert (s_base); + g_assert (G_OBJECT_TYPE (s_base) == base_type); +} + +static void +_test_connection_normalize_type_unnormalizable_setting (const char *type) +{ + NMSettingConnection *s_con; + GType base_type; + gs_unref_object NMConnection *con = NULL; + gs_free char *id = g_strdup_printf ("%s[%s]", G_STRFUNC, type); + + base_type = nm_setting_lookup_type (type); + g_assert (base_type != G_TYPE_INVALID); + g_assert (_nm_setting_type_is_base_type (base_type)); + + con = nmtst_create_minimal_connection (id, NULL, NULL, &s_con); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + + g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, type, NULL); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND); +} + +static void +_test_connection_normalize_type_normalizable_type (const char *type, + NMSetting *(*add_setting_fcn) (NMConnection *con)) +{ + NMSettingConnection *s_con; + NMSetting *s_base; + GType base_type; + gs_unref_object NMConnection *con = NULL; + gs_free char *id = g_strdup_printf ("%s[%s]", G_STRFUNC, type); + + base_type = nm_setting_lookup_type (type); + g_assert (base_type != G_TYPE_INVALID); + g_assert (_nm_setting_type_is_base_type (base_type)); + + con = nmtst_create_minimal_connection (id, NULL, NULL, &s_con); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + + if (add_setting_fcn) + s_base = add_setting_fcn (con); + else { + s_base = NM_SETTING (g_object_new (base_type, NULL)); + nm_connection_add_setting (con, s_base); + } + + g_assert (!nm_connection_get_connection_type (con)); + g_assert (nm_connection_get_setting_by_name (con, type) == s_base); + + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + + g_assert_cmpstr (nm_connection_get_connection_type (con), ==, type); + g_assert (nm_connection_get_setting_by_name (con, type) == s_base); +} + +static NMSetting * +_add_setting_fcn_adsl (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_ADSL, + NM_SETTING_ADSL_USERNAME, "test-user", + NM_SETTING_ADSL_PROTOCOL, NM_SETTING_ADSL_PROTOCOL_PPPOA, + NM_SETTING_ADSL_ENCAPSULATION, NM_SETTING_ADSL_ENCAPSULATION_VCMUX, + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_bluetooth (NMConnection *con) +{ + NMSetting *setting; + GByteArray *bdaddr = nm_utils_hwaddr_atoba ("11:22:33:44:55:66", ETH_ALEN); + + setting = g_object_new (NM_TYPE_SETTING_BLUETOOTH, + NM_SETTING_BLUETOOTH_BDADDR, bdaddr, + NM_SETTING_BLUETOOTH_TYPE, NM_SETTING_BLUETOOTH_TYPE_PANU, + NULL); + g_byte_array_free (bdaddr, TRUE); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_bond (NMConnection *con) +{ + NMSetting *setting; + NMSettingConnection *s_con; + + setting = g_object_new (NM_TYPE_SETTING_BOND, NULL); + + nm_connection_add_setting (con, setting); + + s_con = nm_connection_get_setting_connection (con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_INTERFACE_NAME, "test-bond", + NULL); + + return setting; +} + +static NMSetting * +_add_setting_fcn_bridge (NMConnection *con) +{ + NMSetting *setting; + NMSettingConnection *s_con; + + setting = g_object_new (NM_TYPE_SETTING_BRIDGE, NULL); + + nm_connection_add_setting (con, setting); + + s_con = nm_connection_get_setting_connection (con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_INTERFACE_NAME, "test-bridge", + NULL); + + return setting; +} + +static NMSetting * +_add_setting_fcn_cdma (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_CDMA, + NM_SETTING_CDMA_NUMBER, "test-number", + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_infiniband (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_INFINIBAND, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected", + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_olpc_mesh (NMConnection *con) +{ + NMSetting *setting; + const char *ssid_data = "ssid-test"; + GByteArray *ssid = g_byte_array_new (); + + g_byte_array_append (ssid, (const guint8 *) ssid_data, strlen (ssid_data)); + setting = g_object_new (NM_TYPE_SETTING_OLPC_MESH, + NM_SETTING_OLPC_MESH_SSID, ssid, + NM_SETTING_OLPC_MESH_CHANNEL, 1, + NULL); + g_byte_array_free (ssid, TRUE); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_team (NMConnection *con) +{ + NMSetting *setting; + NMSettingConnection *s_con; + + setting = g_object_new (NM_TYPE_SETTING_TEAM, NULL); + + nm_connection_add_setting (con, setting); + + s_con = nm_connection_get_setting_connection (con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_INTERFACE_NAME, "test-team", + NULL); + + return setting; +} + +static NMSetting * +_add_setting_fcn_vlan (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_VLAN, + NM_SETTING_VLAN_PARENT, "test-parent", + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_vpn (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_VPN, + NM_SETTING_VPN_SERVICE_TYPE, "test-vpn-service-type", + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_wimax (NMConnection *con) +{ + NMSetting *setting; + + setting = g_object_new (NM_TYPE_SETTING_WIMAX, + NM_SETTING_WIMAX_NETWORK_NAME, "test-network", + NULL); + + nm_connection_add_setting (con, setting); + return setting; +} + +static NMSetting * +_add_setting_fcn_wireless (NMConnection *con) +{ + NMSetting *setting; + const char *ssid_data = "ssid-test"; + GByteArray *ssid = g_byte_array_new (); + + g_byte_array_append (ssid, (const guint8 *) ssid_data, strlen (ssid_data)); + setting = g_object_new (NM_TYPE_SETTING_WIRELESS, + NM_SETTING_WIRELESS_SSID, ssid, + NULL); + g_byte_array_free (ssid, TRUE); + + nm_connection_add_setting (con, setting); + return setting; +} + +static void +_prepare_normalizable_fcn_vlan (NMConnection *con) +{ + GByteArray *mac_addr = nm_utils_hwaddr_atoba ("11:22:33:44:55:66", ETH_ALEN); + + nm_connection_add_setting (con, g_object_new (NM_TYPE_SETTING_WIRED, + NM_SETTING_WIRED_MAC_ADDRESS, mac_addr, + NULL)); + g_byte_array_free (mac_addr, TRUE); +} + +static void +test_connection_normalize_type (void) +{ + guint i; + struct { + const char *type; + gboolean normalizable; + NMSetting *(*add_setting_fcn) (NMConnection *con); + void (*prepare_normalizable_fcn) (NMConnection *con); + } types[] = { + { NM_SETTING_GENERIC_SETTING_NAME, TRUE }, + { NM_SETTING_GSM_SETTING_NAME, TRUE }, + { NM_SETTING_WIRED_SETTING_NAME, TRUE }, + { NM_SETTING_VLAN_SETTING_NAME, TRUE, _add_setting_fcn_vlan, _prepare_normalizable_fcn_vlan }, + + { NM_SETTING_ADSL_SETTING_NAME, FALSE, _add_setting_fcn_adsl }, + { NM_SETTING_BLUETOOTH_SETTING_NAME, FALSE, _add_setting_fcn_bluetooth }, + { NM_SETTING_BOND_SETTING_NAME, FALSE, _add_setting_fcn_bond }, + { NM_SETTING_BRIDGE_SETTING_NAME, FALSE, _add_setting_fcn_bridge }, + { NM_SETTING_CDMA_SETTING_NAME, FALSE, _add_setting_fcn_cdma }, + { NM_SETTING_INFINIBAND_SETTING_NAME, FALSE, _add_setting_fcn_infiniband }, + { NM_SETTING_OLPC_MESH_SETTING_NAME, FALSE, _add_setting_fcn_olpc_mesh }, + { NM_SETTING_TEAM_SETTING_NAME, FALSE, _add_setting_fcn_team }, + { NM_SETTING_VLAN_SETTING_NAME, FALSE, _add_setting_fcn_vlan }, + { NM_SETTING_VPN_SETTING_NAME, FALSE, _add_setting_fcn_vpn }, + { NM_SETTING_WIMAX_SETTING_NAME, FALSE, _add_setting_fcn_wimax }, + { NM_SETTING_WIRELESS_SETTING_NAME, FALSE, _add_setting_fcn_wireless }, + { 0 }, + }; + + for (i = 0; types[i].type; i++) { + const char *type = types[i].type; + + if (types[i].normalizable) + _test_connection_normalize_type_normalizable_setting (type, types[i].prepare_normalizable_fcn); + else + _test_connection_normalize_type_unnormalizable_setting (type); + _test_connection_normalize_type_normalizable_type (type, types[i].add_setting_fcn); + } +} + static void test_connection_normalize_slave_type_1 (void) { @@ -2755,6 +3081,7 @@ int main (int argc, char **argv) test_connection_new_from_hash (); test_connection_verify_sets_interface_name (); test_connection_normalize_virtual_iface_name (); + test_connection_normalize_type (); test_connection_normalize_slave_type_1 (); test_connection_normalize_slave_type_2 (); From 54ff670423abf86b12833f980f8cff9c39f8c1b9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jul 2014 11:25:13 +0200 Subject: [PATCH 21/30] keyfile: don't add [connection].type base setting which is done by nm_connection_normalize() This undoes commit 9f8b7ff51dfd7cdb828fe5e7b50016c5587e8657 but the same functionality is now provided via normalize(). Signed-off-by: Thomas Haller --- src/settings/plugins/keyfile/reader.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c index 2e356b92e2..e2d1f823b0 100644 --- a/src/settings/plugins/keyfile/reader.c +++ b/src/settings/plugins/keyfile/reader.c @@ -1213,7 +1213,6 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) gsize length; int i; gboolean vpn_secrets = FALSE; - const char *ctype; GError *verify_error = NULL; if (stat (filename, &statbuf) != 0 || !S_ISREG (statbuf.st_mode)) { @@ -1250,31 +1249,11 @@ nm_keyfile_plugin_connection_from_file (const char *filename, GError **error) nm_connection_add_setting (connection, setting); } - /* Make sure that we have the base device type setting even if - * the keyfile didn't include it, which can happen when the base - * device type setting is all default values (like ethernet where - * the MAC address isn't given, or VLAN when the VLAN ID is zero). - */ s_con = nm_connection_get_setting_connection (connection); if (!s_con) { s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ()); nm_connection_add_setting (connection, NM_SETTING (s_con)); } - ctype = nm_setting_connection_get_connection_type (s_con); - if (ctype) { - setting = nm_connection_get_setting_by_name (connection, ctype); - if (!setting) { - NMSetting *base_setting; - GType base_setting_type; - - base_setting_type = nm_setting_lookup_type (ctype); - if (base_setting_type != G_TYPE_INVALID) { - base_setting = (NMSetting *) g_object_new (base_setting_type, NULL); - g_assert (base_setting); - nm_connection_add_setting (connection, base_setting); - } - } - } /* Make sure that we have 'id' even if not explictly specified in the keyfile */ if (!nm_setting_connection_get_id (s_con)) { From 78edf6f5816c4d39537c96c5653f8da6e8c7ec0c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jul 2014 11:01:52 +0200 Subject: [PATCH 22/30] keyfile/tests: add keyfile_read_connection_from_file() utility function Signed-off-by: Thomas Haller --- .../plugins/keyfile/tests/test-keyfile.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 369085dd9a..fb58e22454 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -114,6 +114,22 @@ check_ip6_route (NMSettingIP6Config *config, int idx, const char *destination_st g_assert (nm_ip6_route_get_metric (route) == metric); } +static NMConnection * +keyfile_read_connection_from_file (const char *filename) +{ + GError *error = NULL; + NMConnection *connection; + + g_assert (filename); + + connection = nm_keyfile_plugin_connection_from_file (filename, &error); + g_assert_no_error (error); + + nmtst_assert_connection_verifies_without_normalization (connection); + + return connection; +} + static void test_read_valid_wired_connection (void) { From 9ea3653f1f0c6d1a9f4456ea0f717fb62e80dd9e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Aug 2014 01:24:49 +0200 Subject: [PATCH 23/30] core: add compatibility wrapper for g_test_add_data_func_full() to nm-glib-compat.h Signed-off-by: Thomas Haller --- include/nm-glib-compat.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/include/nm-glib-compat.h b/include/nm-glib-compat.h index 0489f3d8af..c8f1b5f11c 100644 --- a/include/nm-glib-compat.h +++ b/include/nm-glib-compat.h @@ -124,4 +124,29 @@ __nmtst_g_test_skip (const gchar *msg) } #define g_test_skip __nmtst_g_test_skip + +/* g_test_add_data_func_full() is only available since glib 2.34. Add a compatibility wrapper. */ +inline static void +__g_test_add_data_func_full (const char *testpath, + gpointer test_data, + GTestDataFunc test_func, + GDestroyNotify data_free_func) +{ +#if GLIB_CHECK_VERSION (2, 34, 0) + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + g_test_add_data_func_full (testpath, test_data, test_func, data_free_func); + G_GNUC_END_IGNORE_DEPRECATIONS +#else + g_return_if_fail (testpath != NULL); + g_return_if_fail (testpath[0] == '/'); + g_return_if_fail (test_func != NULL); + + g_test_add_vtable (testpath, 0, test_data, NULL, + (GTestFixtureFunc) test_func, + (GTestFixtureFunc) data_free_func); +#endif +} +#define g_test_add_data_func_full __g_test_add_data_func_full + + #endif /* __NM_GLIB_COMPAT_H__ */ From b8a475ba3f54b2354dceea16038ac435f3324546 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Aug 2014 00:49:43 +0200 Subject: [PATCH 24/30] tests: refactor tests to use g_test framework (g_test_add_func) Signed-off-by: Thomas Haller --- libnm-core/tests/test-general.c | 182 ++++++++++-------- .../plugins/keyfile/tests/test-keyfile.c | 85 ++++---- 2 files changed, 139 insertions(+), 128 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index a77930f7aa..e620e1de75 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -1837,11 +1837,29 @@ test_setting_compare_id (void) g_assert (success); } -static void -test_setting_compare_secrets (NMSettingSecretFlags secret_flags, - NMSettingCompareFlags comp_flags, - gboolean remove_secret) +typedef struct { + NMSettingSecretFlags secret_flags; + NMSettingCompareFlags comp_flags; + gboolean remove_secret; +} TestDataCompareSecrets; + +static TestDataCompareSecrets * +test_data_compare_secrets_new (NMSettingSecretFlags secret_flags, + NMSettingCompareFlags comp_flags, + gboolean remove_secret) { + TestDataCompareSecrets *data = g_new0 (TestDataCompareSecrets, 1); + + data->secret_flags = secret_flags; + data->comp_flags = comp_flags; + data->remove_secret = remove_secret; + return data; +} + +static void +test_setting_compare_secrets (gconstpointer test_data) +{ + const TestDataCompareSecrets *data = test_data; NMSetting *old, *new; gboolean success; @@ -1854,26 +1872,25 @@ test_setting_compare_secrets (NMSettingSecretFlags secret_flags, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-psk", NM_SETTING_WIRELESS_SECURITY_PSK, "really cool psk", NULL); - nm_setting_set_secret_flags (old, NM_SETTING_WIRELESS_SECURITY_PSK, secret_flags, NULL); + nm_setting_set_secret_flags (old, NM_SETTING_WIRELESS_SECURITY_PSK, data->secret_flags, NULL); /* Clear the PSK from the duplicated setting */ new = nm_setting_duplicate (old); - if (remove_secret) { + if (data->remove_secret) { g_object_set (new, NM_SETTING_WIRELESS_SECURITY_PSK, NULL, NULL); success = nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_EXACT); g_assert (success == FALSE); } - success = nm_setting_compare (old, new, comp_flags); + success = nm_setting_compare (old, new, data->comp_flags); g_assert (success); } static void -test_setting_compare_vpn_secrets (NMSettingSecretFlags secret_flags, - NMSettingCompareFlags comp_flags, - gboolean remove_secret) +test_setting_compare_vpn_secrets (gconstpointer test_data) { + const TestDataCompareSecrets *data = test_data; NMSetting *old, *new; gboolean success; @@ -1886,11 +1903,11 @@ test_setting_compare_vpn_secrets (NMSettingSecretFlags secret_flags, nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "asdfasdfasdf", "really adfasdfasdfasdf"); nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "0123456778", "abcdefghijklmnpqrstuvqxyz"); nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "borkbork", "yet another really secret password"); - nm_setting_set_secret_flags (old, "borkbork", secret_flags, NULL); + nm_setting_set_secret_flags (old, "borkbork", data->secret_flags, NULL); /* Clear "borkbork" from the duplicated setting */ new = nm_setting_duplicate (old); - if (remove_secret) { + if (data->remove_secret) { nm_setting_vpn_remove_secret (NM_SETTING_VPN (new), "borkbork"); /* First make sure they are different */ @@ -1898,7 +1915,7 @@ test_setting_compare_vpn_secrets (NMSettingSecretFlags secret_flags, g_assert (success == FALSE); } - success = nm_setting_compare (old, new, comp_flags); + success = nm_setting_compare (old, new, data->comp_flags); g_assert (success); } @@ -3046,86 +3063,85 @@ NMTST_DEFINE (); int main (int argc, char **argv) { - char *base; - nmtst_init (&argc, &argv, TRUE); /* The tests */ - test_setting_vpn_items (); - test_setting_vpn_update_secrets (); - test_setting_vpn_modify_during_foreach (); - test_setting_ip4_config_labels (); - test_setting_ip6_config_old_address_array (); - test_setting_gsm_apn_spaces (); - test_setting_gsm_apn_bad_chars (); - test_setting_gsm_apn_underscore (); - test_setting_gsm_without_number (); - test_setting_to_hash_all (); - test_setting_to_hash_no_secrets (); - test_setting_to_hash_only_secrets (); - test_setting_compare_id (); - test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE); - test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE); - test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE); - test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE); - test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE); - test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE); - test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE); - test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE); - test_setting_old_uuid (); + g_test_add_func ("/core/general/test_setting_vpn_items", test_setting_vpn_items); + g_test_add_func ("/core/general/test_setting_vpn_update_secrets", test_setting_vpn_update_secrets); + g_test_add_func ("/core/general/test_setting_vpn_modify_during_foreach", test_setting_vpn_modify_during_foreach); + g_test_add_func ("/core/general/test_setting_ip4_config_labels", test_setting_ip4_config_labels); + g_test_add_func ("/core/general/test_setting_ip6_config_old_address_array", test_setting_ip6_config_old_address_array); + g_test_add_func ("/core/general/test_setting_gsm_apn_spaces", test_setting_gsm_apn_spaces); + g_test_add_func ("/core/general/test_setting_gsm_apn_bad_chars", test_setting_gsm_apn_bad_chars); + g_test_add_func ("/core/general/test_setting_gsm_apn_underscore", test_setting_gsm_apn_underscore); + g_test_add_func ("/core/general/test_setting_gsm_without_number", test_setting_gsm_without_number); + g_test_add_func ("/core/general/test_setting_to_hash_all", test_setting_to_hash_all); + g_test_add_func ("/core/general/test_setting_to_hash_no_secrets", test_setting_to_hash_no_secrets); + g_test_add_func ("/core/general/test_setting_to_hash_only_secrets", test_setting_to_hash_only_secrets); + g_test_add_func ("/core/general/test_setting_compare_id", test_setting_compare_id); +#define ADD_FUNC(func, secret_flags, comp_flags, remove_secret) \ + g_test_add_data_func_full ("/core/general/" G_STRINGIFY (func), \ + test_data_compare_secrets_new (secret_flags, comp_flags, remove_secret), \ + func, g_free) + ADD_FUNC (test_setting_compare_secrets, NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_secrets, NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_secrets, NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_secrets, NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE); + ADD_FUNC (test_setting_compare_vpn_secrets, NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_vpn_secrets, NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_vpn_secrets, NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE); + ADD_FUNC (test_setting_compare_vpn_secrets, NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE); + g_test_add_func ("/core/general/test_setting_old_uuid", test_setting_old_uuid); - test_connection_to_hash_setting_name (); - test_setting_new_from_hash (); - test_connection_replace_settings (); - test_connection_replace_settings_from_connection (); - test_connection_new_from_hash (); - test_connection_verify_sets_interface_name (); - test_connection_normalize_virtual_iface_name (); - test_connection_normalize_type (); - test_connection_normalize_slave_type_1 (); - test_connection_normalize_slave_type_2 (); + g_test_add_func ("/core/general/test_connection_to_hash_setting_name", test_connection_to_hash_setting_name); + g_test_add_func ("/core/general/test_setting_new_from_hash", test_setting_new_from_hash); + g_test_add_func ("/core/general/test_connection_replace_settings", test_connection_replace_settings); + g_test_add_func ("/core/general/test_connection_replace_settings_from_connection", test_connection_replace_settings_from_connection); + g_test_add_func ("/core/general/test_connection_new_from_hash", test_connection_new_from_hash); + g_test_add_func ("/core/general/test_connection_verify_sets_interface_name", test_connection_verify_sets_interface_name); + g_test_add_func ("/core/general/test_connection_normalize_virtual_iface_name", test_connection_normalize_virtual_iface_name); + g_test_add_func ("/core/general/test_connection_normalize_type", test_connection_normalize_type); + g_test_add_func ("/core/general/test_connection_normalize_slave_type_1", test_connection_normalize_slave_type_1); + g_test_add_func ("/core/general/test_connection_normalize_slave_type_2", test_connection_normalize_slave_type_2); - test_setting_connection_permissions_helpers (); - test_setting_connection_permissions_property (); + g_test_add_func ("/core/general/test_setting_connection_permissions_helpers", test_setting_connection_permissions_helpers); + g_test_add_func ("/core/general/test_setting_connection_permissions_property", test_setting_connection_permissions_property); - test_connection_compare_same (); - test_connection_compare_key_only_in_a (); - test_connection_compare_setting_only_in_a (); - test_connection_compare_key_only_in_b (); - test_connection_compare_setting_only_in_b (); + g_test_add_func ("/core/general/test_connection_compare_same", test_connection_compare_same); + g_test_add_func ("/core/general/test_connection_compare_key_only_in_a", test_connection_compare_key_only_in_a); + g_test_add_func ("/core/general/test_connection_compare_setting_only_in_a", test_connection_compare_setting_only_in_a); + g_test_add_func ("/core/general/test_connection_compare_key_only_in_b", test_connection_compare_key_only_in_b); + g_test_add_func ("/core/general/test_connection_compare_setting_only_in_b", test_connection_compare_setting_only_in_b); - test_connection_diff_a_only (); - test_connection_diff_same (); - test_connection_diff_different (); - test_connection_diff_no_secrets (); - test_connection_diff_inferrable (); - test_connection_good_base_types (); - test_connection_bad_base_types (); + g_test_add_func ("/core/general/test_connection_diff_a_only", test_connection_diff_a_only); + g_test_add_func ("/core/general/test_connection_diff_same", test_connection_diff_same); + g_test_add_func ("/core/general/test_connection_diff_different", test_connection_diff_different); + g_test_add_func ("/core/general/test_connection_diff_no_secrets", test_connection_diff_no_secrets); + g_test_add_func ("/core/general/test_connection_diff_inferrable", test_connection_diff_inferrable); + g_test_add_func ("/core/general/test_connection_good_base_types", test_connection_good_base_types); + g_test_add_func ("/core/general/test_connection_bad_base_types", test_connection_bad_base_types); - test_hwaddr_aton_ether_normal (); - test_hwaddr_aton_ib_normal (); - test_hwaddr_aton_no_leading_zeros (); - test_hwaddr_aton_malformed (); - test_hwaddr_equal (); + g_test_add_func ("/core/general/test_hwaddr_aton_ether_normal", test_hwaddr_aton_ether_normal); + g_test_add_func ("/core/general/test_hwaddr_aton_ib_normal", test_hwaddr_aton_ib_normal); + g_test_add_func ("/core/general/test_hwaddr_aton_no_leading_zeros", test_hwaddr_aton_no_leading_zeros); + g_test_add_func ("/core/general/test_hwaddr_aton_malformed", test_hwaddr_aton_malformed); + g_test_add_func ("/core/general/test_hwaddr_equal", test_hwaddr_equal); - test_ip4_prefix_to_netmask (); - test_ip4_netmask_to_prefix (); + g_test_add_func ("/core/general/test_ip4_prefix_to_netmask", test_ip4_prefix_to_netmask); + g_test_add_func ("/core/general/test_ip4_netmask_to_prefix", test_ip4_netmask_to_prefix); - test_connection_changed_signal (); - test_setting_connection_changed_signal (); - test_setting_bond_changed_signal (); - test_setting_ip4_changed_signal (); - test_setting_ip6_changed_signal (); - test_setting_vlan_changed_signal (); - test_setting_vpn_changed_signal (); - test_setting_wired_changed_signal (); - test_setting_wireless_changed_signal (); - test_setting_wireless_security_changed_signal (); - test_setting_802_1x_changed_signal (); + g_test_add_func ("/core/general/test_connection_changed_signal", test_connection_changed_signal); + g_test_add_func ("/core/general/test_setting_connection_changed_signal", test_setting_connection_changed_signal); + g_test_add_func ("/core/general/test_setting_bond_changed_signal", test_setting_bond_changed_signal); + g_test_add_func ("/core/general/test_setting_ip4_changed_signal", test_setting_ip4_changed_signal); + g_test_add_func ("/core/general/test_setting_ip6_changed_signal", test_setting_ip6_changed_signal); + g_test_add_func ("/core/general/test_setting_vlan_changed_signal", test_setting_vlan_changed_signal); + g_test_add_func ("/core/general/test_setting_vpn_changed_signal", test_setting_vpn_changed_signal); + g_test_add_func ("/core/general/test_setting_wired_changed_signal", test_setting_wired_changed_signal); + g_test_add_func ("/core/general/test_setting_wireless_changed_signal", test_setting_wireless_changed_signal); + g_test_add_func ("/core/general/test_setting_wireless_security_changed_signal", test_setting_wireless_security_changed_signal); + g_test_add_func ("/core/general/test_setting_802_1x_changed_signal", test_setting_802_1x_changed_signal); - base = g_path_get_basename (argv[0]); - fprintf (stdout, "%s: SUCCESS\n", base); - g_free (base); - return 0; + return g_test_run (); } diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index fb58e22454..086fd72244 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -3442,69 +3442,64 @@ NMTST_DEFINE (); int main (int argc, char **argv) { - char *base; - nmtst_init_assert_logging (&argc, &argv); /* The tests */ - test_read_valid_wired_connection (); - test_write_wired_connection (); + g_test_add_func ("/keyfile/test_read_valid_wired_connection ", test_read_valid_wired_connection); + g_test_add_func ("/keyfile/test_write_wired_connection ", test_write_wired_connection); - test_read_ip6_wired_connection (); - test_write_ip6_wired_connection (); + g_test_add_func ("/keyfile/test_read_ip6_wired_connection ", test_read_ip6_wired_connection); + g_test_add_func ("/keyfile/test_write_ip6_wired_connection ", test_write_ip6_wired_connection); - test_read_wired_mac_case (); - test_read_mac_old_format (); - test_read_mac_ib_old_format (); + g_test_add_func ("/keyfile/test_read_wired_mac_case ", test_read_wired_mac_case); + g_test_add_func ("/keyfile/test_read_mac_old_format ", test_read_mac_old_format); + g_test_add_func ("/keyfile/test_read_mac_ib_old_format ", test_read_mac_ib_old_format); - test_read_valid_wireless_connection (); - test_write_wireless_connection (); + g_test_add_func ("/keyfile/test_read_valid_wireless_connection ", test_read_valid_wireless_connection); + g_test_add_func ("/keyfile/test_write_wireless_connection ", test_write_wireless_connection); - test_read_string_ssid (); - test_write_string_ssid (); + g_test_add_func ("/keyfile/test_read_string_ssid ", test_read_string_ssid); + g_test_add_func ("/keyfile/test_write_string_ssid ", test_write_string_ssid); - test_read_intlist_ssid (); - test_write_intlist_ssid (); + g_test_add_func ("/keyfile/test_read_intlist_ssid ", test_read_intlist_ssid); + g_test_add_func ("/keyfile/test_write_intlist_ssid ", test_write_intlist_ssid); - test_read_intlike_ssid (); - test_write_intlike_ssid (); + g_test_add_func ("/keyfile/test_read_intlike_ssid ", test_read_intlike_ssid); + g_test_add_func ("/keyfile/test_write_intlike_ssid ", test_write_intlike_ssid); - test_read_intlike_ssid_2 (); - test_write_intlike_ssid_2 (); + g_test_add_func ("/keyfile/test_read_intlike_ssid_2 ", test_read_intlike_ssid_2); + g_test_add_func ("/keyfile/test_write_intlike_ssid_2 ", test_write_intlike_ssid_2); - test_read_bt_dun_connection (); - test_write_bt_dun_connection (); + g_test_add_func ("/keyfile/test_read_bt_dun_connection ", test_read_bt_dun_connection); + g_test_add_func ("/keyfile/test_write_bt_dun_connection ", test_write_bt_dun_connection); - test_read_gsm_connection (); - test_write_gsm_connection (); + g_test_add_func ("/keyfile/test_read_gsm_connection ", test_read_gsm_connection); + g_test_add_func ("/keyfile/test_write_gsm_connection ", test_write_gsm_connection); - test_read_wired_8021x_tls_blob_connection (); - test_read_wired_8021x_tls_bad_path_connection (); + g_test_add_func ("/keyfile/test_read_wired_8021x_tls_blob_connection ", test_read_wired_8021x_tls_blob_connection); + g_test_add_func ("/keyfile/test_read_wired_8021x_tls_bad_path_connection ", test_read_wired_8021x_tls_bad_path_connection); - test_read_wired_8021x_tls_old_connection (); - test_read_wired_8021x_tls_new_connection (); - test_write_wired_8021x_tls_connection_path (); - test_write_wired_8021x_tls_connection_blob (); + g_test_add_func ("/keyfile/test_read_wired_8021x_tls_old_connection ", test_read_wired_8021x_tls_old_connection); + g_test_add_func ("/keyfile/test_read_wired_8021x_tls_new_connection ", test_read_wired_8021x_tls_new_connection); + g_test_add_func ("/keyfile/test_write_wired_8021x_tls_connection_path ", test_write_wired_8021x_tls_connection_path); + g_test_add_func ("/keyfile/test_write_wired_8021x_tls_connection_blob ", test_write_wired_8021x_tls_connection_blob); - test_read_infiniband_connection (); - test_write_infiniband_connection (); + g_test_add_func ("/keyfile/test_read_infiniband_connection ", test_read_infiniband_connection); + g_test_add_func ("/keyfile/test_write_infiniband_connection ", test_write_infiniband_connection); - test_read_bridge_main (); - test_write_bridge_main (); - test_read_bridge_component (); - test_write_bridge_component (); + g_test_add_func ("/keyfile/test_read_bridge_main ", test_read_bridge_main); + g_test_add_func ("/keyfile/test_write_bridge_main ", test_write_bridge_main); + g_test_add_func ("/keyfile/test_read_bridge_component ", test_read_bridge_component); + g_test_add_func ("/keyfile/test_write_bridge_component ", test_write_bridge_component); - test_read_new_wired_group_name (); - test_write_new_wired_group_name (); - test_read_new_wireless_group_names (); - test_write_new_wireless_group_names (); + g_test_add_func ("/keyfile/test_read_new_wired_group_name ", test_read_new_wired_group_name); + g_test_add_func ("/keyfile/test_write_new_wired_group_name ", test_write_new_wired_group_name); + g_test_add_func ("/keyfile/test_read_new_wireless_group_names ", test_read_new_wireless_group_names); + g_test_add_func ("/keyfile/test_write_new_wireless_group_names ", test_write_new_wireless_group_names); - test_read_missing_vlan_setting (); - test_read_missing_id_uuid (); + g_test_add_func ("/keyfile/test_read_missing_vlan_setting ", test_read_missing_vlan_setting); + g_test_add_func ("/keyfile/test_read_missing_id_uuid ", test_read_missing_id_uuid); - base = g_path_get_basename (argv[0]); - fprintf (stdout, "%s: SUCCESS\n", base); - g_free (base); - return 0; + return g_test_run (); } From c9be5a32daae818a3d18f62d3d8ba835dfa87354 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jul 2014 20:25:41 +0200 Subject: [PATCH 25/30] keyfile/tests: test reading minimal keyfiles that needs normalization of type and slave-type Signed-off-by: Thomas Haller --- .../keyfile/tests/keyfiles/Makefile.am | 6 ++ .../keyfile/tests/keyfiles/Test_minimal_1 | 2 + .../keyfile/tests/keyfiles/Test_minimal_2 | 1 + .../tests/keyfiles/Test_minimal_slave_1 | 4 + .../tests/keyfiles/Test_minimal_slave_2 | 7 ++ .../tests/keyfiles/Test_minimal_slave_3 | 4 + .../tests/keyfiles/Test_minimal_slave_4 | 4 + .../plugins/keyfile/tests/test-keyfile.c | 87 +++++++++++++++++++ 8 files changed, 115 insertions(+) create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_1 create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_2 create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_1 create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_2 create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_3 create mode 100644 src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_4 diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Makefile.am b/src/settings/plugins/keyfile/tests/keyfiles/Makefile.am index 217ac4acd6..c6ed0dbb15 100644 --- a/src/settings/plugins/keyfile/tests/keyfiles/Makefile.am +++ b/src/settings/plugins/keyfile/tests/keyfiles/Makefile.am @@ -21,6 +21,12 @@ KEYFILES = \ Test_Bridge_Component \ Test_New_Wired_Group_Name \ Test_New_Wireless_Group_Names \ + Test_minimal_1 \ + Test_minimal_2 \ + Test_minimal_slave_1 \ + Test_minimal_slave_2 \ + Test_minimal_slave_3 \ + Test_minimal_slave_4 \ Test_Missing_Vlan_Setting \ Test_Missing_ID_UUID diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_1 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_1 new file mode 100644 index 0000000000..cac135ad1d --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_1 @@ -0,0 +1,2 @@ +[connection] +type=802-3-ethernet diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_2 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_2 new file mode 100644 index 0000000000..bbf2d8d631 --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_2 @@ -0,0 +1 @@ +[802-3-ethernet] diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_1 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_1 new file mode 100644 index 0000000000..d3122d5395 --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_1 @@ -0,0 +1,4 @@ +[connection] +type=802-3-ethernet +master=br0 +slave-type=bridge diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_2 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_2 new file mode 100644 index 0000000000..eb1cdacebd --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_2 @@ -0,0 +1,7 @@ +[connection] +master=br0 + +[802-3-ethernet] + +[bridge-port] + diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_3 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_3 new file mode 100644 index 0000000000..7419e97dd2 --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_3 @@ -0,0 +1,4 @@ +[connection] +master=br0 +slave-type=bridge +[802-3-ethernet] diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_4 b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_4 new file mode 100644 index 0000000000..626b6f6840 --- /dev/null +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_minimal_slave_4 @@ -0,0 +1,4 @@ +[connection] +type=802-3-ethernet +master=br0 +[bridge-port] diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 086fd72244..97ff781c04 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -3438,6 +3438,90 @@ test_read_missing_id_uuid (void) g_object_unref (connection); } +static void +test_read_minimal () +{ + NMConnection *connection = NULL; + gs_unref_object NMConnection *con_archetype = NULL; + NMSettingConnection *s_con; + + con_archetype = nmtst_create_minimal_connection ("Test_minimal_x", + "a15bd68f-c32b-40b8-8d27-49e472a85919", + NM_SETTING_WIRED_SETTING_NAME, + &s_con); + nmtst_connection_normalize (con_archetype); + + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_1"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); + + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_2"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); +} + +static void +test_read_minimal_slave () +{ + NMConnection *connection = NULL; + gs_unref_object NMConnection *con_archetype = NULL; + NMSettingConnection *s_con; + + con_archetype = nmtst_create_minimal_connection ("Test_minimal_slave_x", + "a56b4ca5-7075-43d4-82c7-5d0cb15f7654", + NM_SETTING_WIRED_SETTING_NAME, + &s_con); + g_object_set (s_con, + NM_SETTING_CONNECTION_MASTER, "br0", + NM_SETTING_CONNECTION_SLAVE_TYPE, "bridge", + NULL); + nmtst_connection_normalize (con_archetype); + + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_slave_1"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); + + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_slave_2"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_slave_3"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); + + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_minimal_slave_4"); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, nm_connection_get_id (connection), + NM_SETTING_CONNECTION_UUID, nm_connection_get_uuid (connection), + NULL); + nmtst_assert_connection_equals (con_archetype, FALSE, connection, FALSE); + g_clear_object (&connection); +} + NMTST_DEFINE (); int main (int argc, char **argv) @@ -3500,6 +3584,9 @@ int main (int argc, char **argv) g_test_add_func ("/keyfile/test_read_missing_vlan_setting ", test_read_missing_vlan_setting); g_test_add_func ("/keyfile/test_read_missing_id_uuid ", test_read_missing_id_uuid); + g_test_add_func ("/keyfile/test_read_minimal", test_read_minimal); + g_test_add_func ("/keyfile/test_read_minimal_slave", test_read_minimal_slave); + return g_test_run (); } From 6163263b42ce0eaa493f1e02e540cfcb14da2e3e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jul 2014 12:00:28 +0200 Subject: [PATCH 26/30] libnm-core: make failure cases for nm_connection_normalize() more robust against bugs We would expect that attempts to normalize a connection are successful as verify() indicates. This way, a connection is not modified if it cannot be fixed, and a connection will be valid and modified after attempts to normalization. However, there might be subtle, unexpected ways how this can fail. For example, if NMSettingConnection:verify() detects a missing base type setting, it returns NORMALIZABLE_ERROR if it finds a valid NMSettingConnection:type. Normalization then adds an empty, default setting. However, a new verify() might fail due to other reasons. This would be a bug in NMSettingConnection:verify() which must not indicate that it is able to normalize the connection, when it actually is unable to do so. Such bugs need fixing, but the code should be more robust to this case because there might be complex, unanticipated situations. Especially since NM relies on having a valid connection after normalize(), so a strict error-out behavior is important. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 1f112603a1..24b1cd47f6 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -902,7 +902,16 @@ nm_connection_normalize (NMConnection *connection, success == NM_SETTING_VERIFY_SUCCESS) { if (normalizable_error) g_propagate_error (error, normalizable_error); - goto EXIT; + if (modified) + *modified = FALSE; + if (success == NM_SETTING_VERIFY_ERROR && error && !*error) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_UNKNOWN, + _("Unexpected failure to verify the connection")); + g_return_val_if_reached (FALSE); + } + return success == NM_SETTING_VERIFY_SUCCESS; } g_assert (success == NM_SETTING_VERIFY_NORMALIZABLE || success == NM_SETTING_VERIFY_NORMALIZABLE_ERROR); g_clear_error (&normalizable_error); @@ -919,17 +928,26 @@ nm_connection_normalize (NMConnection *connection, /* Verify anew. */ success = _nm_connection_verify (connection, error); - /* we would expect, that after normalization, the connection can be verified. */ - g_return_val_if_fail (success == NM_SETTING_VERIFY_SUCCESS, success); - - /* we would expect, that the connection was modified during normalization. */ - g_return_val_if_fail (was_modified, success); - -EXIT: if (modified) *modified = was_modified; - return success == NM_SETTING_VERIFY_SUCCESS; + if (success != NM_SETTING_VERIFY_SUCCESS) { + /* we would expect, that after normalization, the connection can be verified. + * Also treat NM_SETTING_VERIFY_NORMALIZABLE as failure, because there is something + * odd going on. */ + if (error && !*error) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_UNKNOWN, + _("Unexpected failure to normalize the connection")); + } + g_return_val_if_reached (FALSE); + } + + /* we would expect, that the connection was modified during normalization. */ + g_return_val_if_fail (was_modified, TRUE); + + return TRUE; } /** From 66d88dc00fe710790e2e41dc201f97ca542a1244 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Aug 2014 02:35:26 +0200 Subject: [PATCH 27/30] libnm-core: add normalize of MTU for NMSettingInfiniband Previously, NMSettingInfiniband:verify() silently modifies the setting for invalid MTU. verify() should not do that. For libnm-core we can change behavior and implement normalization of MTU. This changes behavior for NMSettingInfiniband:verify() so that MTU gets no longer fixed by verify() alone. Instead verify() fails with a verification error. Due the possibility to normalize the MTU, NM still can receive invalid settings and fix it. For libnm-core we don't change behavior, merely add a code comment. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 25 +++++++++++++++++ libnm-core/nm-setting-infiniband.c | 17 +++++++++-- libnm-core/tests/test-general.c | 45 ++++++++++++++++++++++++++++++ libnm-util/nm-setting-infiniband.c | 1 + 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 24b1cd47f6..8ff956e298 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -710,6 +710,30 @@ _normalize_ip_config (NMConnection *self, GHashTable *parameters) } } +static gboolean +_normalize_infiniband_mtu (NMConnection *self, GHashTable *parameters) +{ + NMSettingInfiniband *s_infini = nm_connection_get_setting_infiniband (self); + + if (s_infini) { + const char *transport_mode = nm_setting_infiniband_get_transport_mode (s_infini); + guint32 max_mtu = 0; + + if (transport_mode) { + if (!strcmp (transport_mode, "datagram")) + max_mtu = 2044; + else if (!strcmp (transport_mode, "connected")) + max_mtu = 65520; + + if (max_mtu && nm_setting_infiniband_get_mtu (s_infini) > max_mtu) { + g_object_set (s_infini, NM_SETTING_INFINIBAND_MTU, max_mtu, NULL); + return TRUE; + } + } + } + return FALSE; +} + /** * nm_connection_verify: * @connection: the #NMConnection to verify @@ -924,6 +948,7 @@ nm_connection_normalize (NMConnection *connection, was_modified |= _normalize_virtual_iface_name (connection); was_modified |= _normalize_connection_slave_type (connection); was_modified |= _normalize_ip_config (connection, parameters); + was_modified |= _normalize_infiniband_mtu (connection, parameters); /* Verify anew. */ success = _nm_connection_verify (connection, error); diff --git a/libnm-core/nm-setting-infiniband.c b/libnm-core/nm-setting-infiniband.c index 57708460ba..d60164ad85 100644 --- a/libnm-core/nm-setting-infiniband.c +++ b/libnm-core/nm-setting-infiniband.c @@ -194,6 +194,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) { NMSettingConnection *s_con; NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE (setting); + guint32 normerr_max_mtu = 0; if (priv->mac_address && priv->mac_address->len != INFINIBAND_ALEN) { g_set_error_literal (error, @@ -206,10 +207,10 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) if (!g_strcmp0 (priv->transport_mode, "datagram")) { if (priv->mtu > 2044) - priv->mtu = 2044; + normerr_max_mtu = 2044; } else if (!g_strcmp0 (priv->transport_mode, "connected")) { if (priv->mtu > 65520) - priv->mtu = 65520; + normerr_max_mtu = 65520; } else { g_set_error_literal (error, NM_SETTING_INFINIBAND_ERROR, @@ -287,6 +288,18 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) } } + /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */ + + if (normerr_max_mtu > 0) { + g_set_error (error, + NM_SETTING_INFINIBAND_ERROR, + NM_SETTING_INFINIBAND_ERROR_INVALID_PROPERTY, + _("mtu for transport mode '%s' can be at most %d but it is %d"), + priv->transport_mode, normerr_max_mtu, priv->mtu); + g_prefix_error (error, "%s.%s: ", NM_SETTING_INFINIBAND_SETTING_NAME, NM_SETTING_INFINIBAND_MTU); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + return TRUE; } diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index e620e1de75..8e01677294 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -3059,6 +3059,50 @@ test_connection_normalize_slave_type_2 (void) g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME); } +static void +test_connection_normalize_infiniband_mtu (void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingInfiniband *s_infini; + + con = nmtst_create_minimal_connection ("test_connection_normalize_infiniband_mtu", NULL, + NM_SETTING_INFINIBAND_SETTING_NAME, NULL); + + s_infini = nm_connection_get_setting_infiniband (con); + g_object_set (s_infini, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected", + NULL); + nmtst_assert_connection_verifies_and_normalizable (con); + + g_object_set (s_infini, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram", + NM_SETTING_INFINIBAND_MTU, (guint) 2044, + NULL); + nmtst_assert_connection_verifies_without_normalization (con); + g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini)); + + g_object_set (s_infini, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram", + NM_SETTING_INFINIBAND_MTU, (guint) 2045, + NULL); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_INFINIBAND_ERROR, NM_SETTING_INFINIBAND_ERROR_INVALID_PROPERTY); + g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini)); + + g_object_set (s_infini, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected", + NM_SETTING_INFINIBAND_MTU, (guint) 65520, + NULL); + nmtst_assert_connection_verifies_without_normalization (con); + g_assert_cmpint (65520, ==, nm_setting_infiniband_get_mtu (s_infini)); + + g_object_set (s_infini, + NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected", + NM_SETTING_INFINIBAND_MTU, (guint) 65521, + NULL); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_INFINIBAND_ERROR, NM_SETTING_INFINIBAND_ERROR_INVALID_PROPERTY); + g_assert_cmpint (65520, ==, nm_setting_infiniband_get_mtu (s_infini)); +} + NMTST_DEFINE (); int main (int argc, char **argv) @@ -3103,6 +3147,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_connection_normalize_type", test_connection_normalize_type); g_test_add_func ("/core/general/test_connection_normalize_slave_type_1", test_connection_normalize_slave_type_1); g_test_add_func ("/core/general/test_connection_normalize_slave_type_2", test_connection_normalize_slave_type_2); + g_test_add_func ("/core/general/test_connection_normalize_infiniband_mtu", test_connection_normalize_infiniband_mtu); g_test_add_func ("/core/general/test_setting_connection_permissions_helpers", test_setting_connection_permissions_helpers); g_test_add_func ("/core/general/test_setting_connection_permissions_property", test_setting_connection_permissions_property); diff --git a/libnm-util/nm-setting-infiniband.c b/libnm-util/nm-setting-infiniband.c index 4e470e561b..a9088d20cb 100644 --- a/libnm-util/nm-setting-infiniband.c +++ b/libnm-util/nm-setting-infiniband.c @@ -206,6 +206,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return FALSE; } + /* FIXME: verify() should not modify the setting, but return NORMALIZABLE success. */ if (!g_strcmp0 (priv->transport_mode, "datagram")) { if (priv->mtu > 2044) priv->mtu = 2044; From 4f8b45e802e49f5fe88ca99e2f5fe72e85287893 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Aug 2014 02:38:09 +0200 Subject: [PATCH 28/30] libnm-core: allow nm_setting_verify() to succeed individually without @all_settings When calling nm_setting_verify() without providing any settings in @all_settings, we assume that the setting on its own might be valid. Only when checked together with all settings, we want to consider the setting in the full context. nm_connection_verify() ensures to pass on a list of settings. Signed-off-by: Thomas Haller --- libnm-core/nm-setting-vlan.c | 3 ++- libnm-core/nm-setting.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 0cb1374719..22fb2d7403 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -566,7 +566,8 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) /* If parent is NULL, the parent must be specified via * NMSettingWired:mac-address. */ - if (!s_wired || !nm_setting_wired_get_mac_address (s_wired)) { + if ( all_settings + && (!s_wired || !nm_setting_wired_get_mac_address (s_wired))) { g_set_error (error, NM_SETTING_VLAN_ERROR, NM_SETTING_VLAN_ERROR_MISSING_PROPERTY, diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 61cbdc3ede..e6305f6ecb 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1413,6 +1413,22 @@ _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, NMSettingConnection *s_con; const char *con_name; + if (!all_settings) { + /* nm_setting_verify() was called without passing on any other settings. + * Perform a relaxed verification, the setting might be valid when checked + * together with a NMSettingConnection as part of a NMConnection. */ + if (interface_name && !nm_utils_iface_valid_name (interface_name)) { + /* Only if the interace name is invalid, there is an normalizable warning */ + g_set_error_literal (error, + error_quark, + e_invalid_property, + _("property is invalid")); + g_prefix_error (error, "%s.%s: ", setting_name, setting_property); + return NM_SETTING_VERIFY_NORMALIZABLE; + } + return NM_SETTING_VERIFY_SUCCESS; + } + s_con = NM_SETTING_CONNECTION (nm_setting_find_in_list (all_settings, NM_SETTING_CONNECTION_SETTING_NAME)); con_name = s_con ? nm_setting_connection_get_interface_name (s_con) : NULL; if (!interface_name && !con_name) { From 8bb4c540901abe20d5fafc5ade90158cd5c965e1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Aug 2014 15:02:30 +0200 Subject: [PATCH 29/30] libnm-core: fix NMSettingConnection:verify() not to modify interface-name verify() used to modify interface-name of the base settings. This is discouraged, because verify() should not touch the connection. For libnm-core we can change behavior and only modify the connection in normalize(). Also, be more strict not to verify() sucessfully on invalid interface-name. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 5 +- libnm-core/nm-setting-connection.c | 33 ----- libnm-core/nm-setting.c | 12 +- libnm-core/tests/test-general.c | 113 +++++++----------- libnm-util/nm-setting.c | 2 +- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 11 +- 6 files changed, 55 insertions(+), 121 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 8ff956e298..8a7aa10cb8 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -798,10 +798,7 @@ _nm_connection_verify (NMConnection *connection, GError **error) g_hash_table_iter_init (&iter, priv->settings); while (g_hash_table_iter_next (&iter, NULL, &value)) { /* Order NMSettingConnection so that it will be verified first. - * The reason is, that NMSettingConnection:verify() modifies the connection - * by setting NMSettingConnection:interface_name. So we want to call that - * verify() first, because the order can affect the outcome. - * Another reason is, that errors in this setting might be more fundamental + * The reason is, that errors in this setting might be more fundamental * and should be checked and reported with higher priority. * Another reason is, that some settings look especially at the * NMSettingConnection, so they find it first in the all_settings list. */ diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 28e5adf7de..b605d8289c 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -752,7 +752,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting); gboolean is_slave; const char *slave_setting_type = NULL; - GSList *iter; NMSetting *normerr_base_type = NULL; const char *normerr_slave_setting_type = NULL; const char *normerr_missing_slave_type = NULL; @@ -792,38 +791,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return FALSE; } - /* FIXME: previously, verify() set the NMSettingConnection:interface_name property, - * thus modifying the setting. verify() should not do this, but keep this not to change - * behaviour. - */ - if (!priv->interface_name) { - for (iter = all_settings; iter; iter = iter->next) { - NMSetting *s_current = iter->data; - char *virtual_iface_name = NULL; - - if (NM_IS_SETTING_BOND (s_current)) - g_object_get (s_current, NM_SETTING_BOND_INTERFACE_NAME, &virtual_iface_name, NULL); - else if (NM_IS_SETTING_BRIDGE (s_current)) - g_object_get (s_current, NM_SETTING_BRIDGE_INTERFACE_NAME, &virtual_iface_name, NULL); - else if (NM_IS_SETTING_TEAM (s_current)) - g_object_get (s_current, NM_SETTING_TEAM_INTERFACE_NAME, &virtual_iface_name, NULL); - else if (NM_IS_SETTING_VLAN (s_current)) - g_object_get (s_current, NM_SETTING_VLAN_INTERFACE_NAME, &virtual_iface_name, NULL); - /* For NMSettingInfiniband, virtual_iface_name has no backing field. - * No need to set the (unset) interface_name to the default value. - **/ - - if (virtual_iface_name) { - if (nm_utils_iface_valid_name (virtual_iface_name)) { - /* found a new interface name. */ - priv->interface_name = virtual_iface_name; - break; - } - g_free (virtual_iface_name); - } - } - } - if (priv->interface_name) { if (!nm_utils_iface_valid_name (priv->interface_name)) { g_set_error (error, diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index e6305f6ecb..a29b6678ea 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1424,7 +1424,7 @@ _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, e_invalid_property, _("property is invalid")); g_prefix_error (error, "%s.%s: ", setting_name, setting_property); - return NM_SETTING_VERIFY_NORMALIZABLE; + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; } return NM_SETTING_VERIFY_SUCCESS; } @@ -1459,7 +1459,7 @@ _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, _("property is missing")); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME); - return NM_SETTING_VERIFY_NORMALIZABLE; + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; } if (!nm_utils_iface_valid_name (con_name)) { /* NMSettingConnection:interface_name is invalid, we cannot normalize it. */ @@ -1477,19 +1477,17 @@ _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, e_missing_property, _("property is missing")); g_prefix_error (error, "%s.%s: ", setting_name, setting_property); - return NM_SETTING_VERIFY_NORMALIZABLE; + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; } if (strcmp (con_name, interface_name) != 0) { /* con_name and interface_name are different. It can be normalized by setting interface_name * to con_name. */ g_set_error_literal (error, error_quark, - e_missing_property, + e_invalid_property, _("property is invalid")); g_prefix_error (error, "%s.%s: ", setting_name, setting_property); - /* we would like to make this a NORMALIZEABLE_ERROR, but that might - * break older connections. */ - return NM_SETTING_VERIFY_NORMALIZABLE; + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; } return NM_SETTING_VERIFY_SUCCESS; diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 8e01677294..2d26440b91 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2562,43 +2562,27 @@ test_setting_old_uuid (void) g_assert (success == TRUE); } -/* - * nm_connection_verify() modifies the connection by setting - * the interface-name property to the virtual_iface_name of - * the type specific settings. - * - * It would be preferable of verify() not to touch the connection, - * but as it is now, stick with it and test it. - **/ static void -test_connection_verify_sets_interface_name (void) +test_connection_normalize_connection_interface_name (void) { NMConnection *con; NMSettingConnection *s_con; NMSettingBond *s_bond; - GError *error = NULL; - gboolean success; - s_con = (NMSettingConnection *) nm_setting_connection_new (); - g_object_set (G_OBJECT (s_con), - NM_SETTING_CONNECTION_ID, "test1", - NM_SETTING_CONNECTION_UUID, "22001632-bbb4-4616-b277-363dce3dfb5b", - NM_SETTING_CONNECTION_TYPE, NM_SETTING_BOND_SETTING_NAME, - NULL); - s_bond = (NMSettingBond *) nm_setting_bond_new (); + con = nmtst_create_minimal_connection ("test1", + "22001632-bbb4-4616-b277-363dce3dfb5b", + NM_SETTING_BOND_SETTING_NAME, + &s_con); + + s_bond = nm_connection_get_setting_bond (con); g_object_set (G_OBJECT (s_bond), NM_SETTING_BOND_INTERFACE_NAME, "bond-x", NULL); - con = nm_simple_connection_new (); - nm_connection_add_setting (con, NM_SETTING (s_con)); - nm_connection_add_setting (con, NM_SETTING (s_bond)); - g_assert_cmpstr (nm_connection_get_interface_name (con), ==, NULL); /* for backward compatiblity, normalizes the interface name */ - success = nm_connection_verify (con, &error); - g_assert (success && !error); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); g_assert_cmpstr (nm_connection_get_interface_name (con), ==, "bond-x"); @@ -2611,68 +2595,61 @@ test_connection_verify_sets_interface_name (void) static void test_connection_normalize_virtual_iface_name (void) { - NMConnection *con; + gs_unref_object NMConnection *con = NULL; NMSettingConnection *s_con; NMSettingVlan *s_vlan; - NMSetting *setting; - GError *error = NULL; - gboolean success; const char *IFACE_NAME = "iface"; const char *IFACE_VIRT = "iface-X"; - gboolean modified = FALSE; - con = nm_simple_connection_new (); + con = nmtst_create_minimal_connection ("test1", + "22001632-bbb4-4616-b277-363dce3dfb5b", + NM_SETTING_VLAN_SETTING_NAME, + &s_con); - setting = nm_setting_ip4_config_new (); - g_object_set (setting, - NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, - NULL); - nm_connection_add_setting (con, setting); + nm_connection_add_setting (con, + g_object_new (NM_TYPE_SETTING_IP4_CONFIG, + NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, + NULL)); - setting = nm_setting_ip6_config_new (); - g_object_set (setting, - NM_SETTING_IP6_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, - NM_SETTING_IP6_CONFIG_MAY_FAIL, TRUE, - NULL); - nm_connection_add_setting (con, setting); + nm_connection_add_setting (con, + g_object_new (NM_TYPE_SETTING_IP6_CONFIG, + NM_SETTING_IP6_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, + NULL)); + + s_vlan = nm_connection_get_setting_vlan (con); - s_con = (NMSettingConnection *) nm_setting_connection_new (); - g_object_set (G_OBJECT (s_con), - NM_SETTING_CONNECTION_ID, "test1", - NM_SETTING_CONNECTION_UUID, "22001632-bbb4-4616-b277-363dce3dfb5b", - NM_SETTING_CONNECTION_TYPE, NM_SETTING_VLAN_SETTING_NAME, - NM_SETTING_CONNECTION_INTERFACE_NAME, IFACE_NAME, - NULL); - s_vlan = (NMSettingVlan *) nm_setting_vlan_new (); g_object_set (G_OBJECT (s_vlan), - NM_SETTING_VLAN_INTERFACE_NAME, IFACE_VIRT, NM_SETTING_VLAN_PARENT, "eth0", NULL); - nm_connection_add_setting (con, NM_SETTING (s_con)); - nm_connection_add_setting (con, NM_SETTING (s_vlan)); + g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_INTERFACE_NAME, IFACE_NAME, NULL); + g_object_set (G_OBJECT (s_vlan), NM_SETTING_VLAN_INTERFACE_NAME, IFACE_VIRT, NULL); g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_VIRT); - - /* for backward compatiblity, normalizes the interface name */ - success = nm_connection_verify (con, &error); - g_assert (success && !error); - - g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); - g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_VIRT); - - success = nm_connection_normalize (con, NULL, &modified, &error); - g_assert (success && !error); - g_assert (modified); - + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_VLAN_ERROR, NM_SETTING_VLAN_ERROR_INVALID_PROPERTY); g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_NAME); - success = nm_connection_verify (con, &error); - g_assert (success && !error); - g_object_unref (con); + g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_INTERFACE_NAME, IFACE_NAME, NULL); + g_object_set (G_OBJECT (s_vlan), NM_SETTING_VLAN_INTERFACE_NAME, NULL, NULL); + + g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); + g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, NULL); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_VLAN_ERROR, NM_SETTING_VLAN_ERROR_MISSING_PROPERTY); + g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); + g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_NAME); + + + g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_INTERFACE_NAME, NULL, NULL); + g_object_set (G_OBJECT (s_vlan), NM_SETTING_VLAN_INTERFACE_NAME, IFACE_NAME, NULL); + + g_assert_cmpstr (nm_connection_get_interface_name (con), ==, NULL); + g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_NAME); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME); + g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_NAME); } static void @@ -3142,7 +3119,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_connection_replace_settings", test_connection_replace_settings); g_test_add_func ("/core/general/test_connection_replace_settings_from_connection", test_connection_replace_settings_from_connection); g_test_add_func ("/core/general/test_connection_new_from_hash", test_connection_new_from_hash); - g_test_add_func ("/core/general/test_connection_verify_sets_interface_name", test_connection_verify_sets_interface_name); + g_test_add_func ("/core/general/test_connection_normalize_connection_interface_name", test_connection_normalize_connection_interface_name); g_test_add_func ("/core/general/test_connection_normalize_virtual_iface_name", test_connection_normalize_virtual_iface_name); g_test_add_func ("/core/general/test_connection_normalize_type", test_connection_normalize_type); g_test_add_func ("/core/general/test_connection_normalize_slave_type_1", test_connection_normalize_slave_type_1); diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c index b479bf4aa0..b943369e2e 100644 --- a/libnm-util/nm-setting.c +++ b/libnm-util/nm-setting.c @@ -1340,7 +1340,7 @@ _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name, * to con_name. */ g_set_error_literal (error, error_quark, - e_missing_property, + e_invalid_property, _("property is invalid")); g_prefix_error (error, "%s.%s: ", setting_name, setting_property); /* we would like to make this a NORMALIZEABLE_ERROR, but that might diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index d917159efe..f835064ce2 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -12346,8 +12346,7 @@ test_write_bridge_main (void) NM_SETTING_IP6_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE, NULL); - g_assert (nm_connection_verify (connection, &error)); - g_assert_no_error (error); + nmtst_assert_connection_verifies_after_normalization (connection, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); /* Save the ifcfg */ success = writer_new_connection (connection, @@ -13131,9 +13130,7 @@ test_write_bond_main (void) NM_SETTING_IP6_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE, NULL); - ASSERT (nm_connection_verify (connection, &error) == TRUE, - "bond-main-write", "failed to verify connection: %s", - (error && error->message) ? error->message : "(unknown)"); + nmtst_assert_connection_verifies_after_normalization (connection, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); /* Save the ifcfg */ success = writer_new_connection (connection, @@ -14216,9 +14213,7 @@ test_write_team_master (void) s_wired = (NMSettingWired *) nm_setting_wired_new (); nm_connection_add_setting (connection, NM_SETTING (s_wired)); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + nmtst_assert_connection_verifies_after_normalization (connection, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); /* Save the ifcfg */ success = writer_new_connection (connection, From 0fe0e62d686774438fc83ad66f7bf40a1412a359 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 7 Jul 2014 17:36:12 +0200 Subject: [PATCH 30/30] libnm-util: properly disconnect "notify" signal for settings in NMConnection When removing/replacing a NMSetting in an NMConnection, we have to disconnect setting_changed_cb() from the "notify" signal. Backport commit dfba4ce1e1c5bd0c1ae798bc538b5fab3e3faded from libnm-core. Signed-off-by: Thomas Haller --- libnm-util/nm-connection.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c index 7e26309c57..e59f8f50a6 100644 --- a/libnm-util/nm-connection.c +++ b/libnm-util/nm-connection.c @@ -185,12 +185,23 @@ setting_changed_cb (NMSetting *setting, g_signal_emit (self, signals[CHANGED], 0); } +static gboolean +_setting_release (gpointer key, gpointer value, gpointer user_data) +{ + g_signal_handlers_disconnect_by_func (user_data, setting_changed_cb, value); + return TRUE; +} + static void _nm_connection_add_setting (NMConnection *connection, NMSetting *setting) { - g_hash_table_insert (NM_CONNECTION_GET_PRIVATE (connection)->settings, - (gpointer) G_OBJECT_TYPE_NAME (setting), - setting); + NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection); + const char *name = G_OBJECT_TYPE_NAME (setting); + NMSetting *s_old; + + if ((s_old = g_hash_table_lookup (priv->settings, (gpointer) name))) + g_signal_handlers_disconnect_by_func (s_old, setting_changed_cb, connection); + g_hash_table_insert (priv->settings, (gpointer) name, setting); /* Listen for property changes so we can emit the 'changed' signal */ g_signal_connect (setting, "notify", (GCallback) setting_changed_cb, connection); } @@ -347,7 +358,7 @@ hash_to_connection (NMConnection *connection, GHashTable *new, GError **error) NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection); if ((changed = g_hash_table_size (priv->settings) > 0)) - g_hash_table_remove_all (priv->settings); + g_hash_table_foreach_remove (priv->settings, _setting_release, connection); g_hash_table_iter_init (&iter, new); while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) { @@ -435,7 +446,7 @@ nm_connection_replace_settings_from_connection (NMConnection *connection, priv = NM_CONNECTION_GET_PRIVATE (connection); if ((changed = g_hash_table_size (priv->settings) > 0)) - g_hash_table_remove_all (priv->settings); + g_hash_table_foreach_remove (priv->settings, _setting_release, connection); if (g_hash_table_size (NM_CONNECTION_GET_PRIVATE (new_connection)->settings)) { g_hash_table_iter_init (&iter, NM_CONNECTION_GET_PRIVATE (new_connection)->settings); @@ -1494,7 +1505,7 @@ nm_connection_duplicate (NMConnection *connection) g_hash_table_iter_init (&iter, NM_CONNECTION_GET_PRIVATE (connection)->settings); while (g_hash_table_iter_next (&iter, NULL, (gpointer) &setting)) - nm_connection_add_setting (dup, nm_setting_duplicate (setting)); + _nm_connection_add_setting (dup, nm_setting_duplicate (setting)); return dup; } @@ -2048,14 +2059,8 @@ dispose (GObject *object) { NMConnection *self = NM_CONNECTION (object); NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (self); - GHashTableIter iter; - NMSetting *setting; - g_hash_table_iter_init (&iter, priv->settings); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &setting)) { - g_signal_handlers_disconnect_by_func (setting, setting_changed_cb, self); - g_hash_table_iter_remove (&iter); - } + g_hash_table_foreach_remove (priv->settings, _setting_release, self); G_OBJECT_CLASS (nm_connection_parent_class)->dispose (object); } @@ -2066,6 +2071,7 @@ finalize (GObject *object) NMConnection *connection = NM_CONNECTION (object); NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection); + g_assert (g_hash_table_size (priv->settings) == 0); g_hash_table_destroy (priv->settings); g_free (priv->path);