From b44a9ef7f705949544ff20189406aa1856c5434e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 08:36:48 +0100 Subject: [PATCH 01/11] libnm: fix nm_setting_compare() for fuzzy comparison Fixes: b1ebbf4c809c859d1ea94032ce7eb8fbf5e45d40 --- libnm-core/nm-setting.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 4662912a89..aec01a1750 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1343,7 +1343,7 @@ nm_setting_compare (NMSetting *a, /* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */ if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_FUZZY) - && !NM_FLAGS_ANY (prop_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) + && NM_FLAGS_ANY (prop_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) continue; if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) From 157644f20063d940eed9775cceb2b82302be92d7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 08:41:33 +0100 Subject: [PATCH 02/11] libnm: fix nm_setting_diff() for ignore-reapply-immediately comparison This bug had no effect, because NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY has only one user, and it's used there in combination with nm_setting_compare(). No caller passed this flag to nm_setting_diff(). Fixes: c9b3617c35b3380428a1800e8aa6692ee68b4b74 --- libnm-core/nm-setting.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index aec01a1750..a972a1f6de 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1379,7 +1379,7 @@ should_compare_prop (NMSetting *setting, if ((comp_flags & NM_SETTING_COMPARE_FLAG_INFERRABLE) && !(prop_flags & NM_SETTING_PARAM_INFERRABLE)) return FALSE; - if ((comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) && !(prop_flags & NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) + if ((comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) && (prop_flags & NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) return FALSE; if (prop_flags & NM_SETTING_PARAM_SECRET) { From 20a738c92f8b9cf9a590237e1201867628215d46 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 16:06:40 +0100 Subject: [PATCH 03/11] libnm: fix NM_SETTING_DIFF_RESULT_IN_B_DEFAULT flag for nm_setting_diff() This is public API, but if it was used, it was very wrong. Cannot help but fix it. Fixes: 68bc95c12f64df1973e7c3f63131cedc6fae280d --- libnm-core/nm-setting.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index e3c128ffb2..3576bdc663 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -270,7 +270,7 @@ typedef enum { NM_SETTING_DIFF_RESULT_IN_A = 0x00000001, NM_SETTING_DIFF_RESULT_IN_B = 0x00000002, NM_SETTING_DIFF_RESULT_IN_A_DEFAULT = 0x00000004, - NM_SETTING_DIFF_RESULT_IN_B_DEFAULT = 0x00000004, + NM_SETTING_DIFF_RESULT_IN_B_DEFAULT = 0x00000008, } NMSettingDiffResult; gboolean nm_setting_diff (NMSetting *a, From a75372cc597b0a60ae54928469f8f4a6217be60e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 12:44:35 +0100 Subject: [PATCH 04/11] libnm: mark wifi.cloned-mac-address GObject property inferrable ethernet.cloned-mac-address is also marked as inferrable. I think the concept of NM_SETTING_PARAM_INFERRABLE is fundamentally wrong just like the entire assume approach. Anyway, if ethernet's property is inferrable, so should be Wi-Fi's. --- libnm-core/nm-setting-wireless.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c index 02ff534b2f..cfa79b54f6 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -1452,6 +1452,7 @@ nm_setting_wireless_class_init (NMSettingWirelessClass *klass) g_param_spec_string (NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS, "", "", NULL, G_PARAM_READWRITE | + NM_SETTING_PARAM_INFERRABLE | G_PARAM_STATIC_STRINGS)); _properties_override_add_override (properties_override, From 290dbf11703cdc90bc7cecb9fcd52a795f812ab1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 14:41:55 +0100 Subject: [PATCH 05/11] libnm/tests: add tests for comparing settings with different secret-flags The flags NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS and NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS act on the secret flags to decide whether to ignore a secret. But there was not test how this behaved, if the two settings had differing flags. --- libnm-core/tests/test-general.c | 143 ++++++++++++++++++++++++++++++-- 1 file changed, 135 insertions(+), 8 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index b6dd532616..0806e7b97b 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -3306,12 +3306,111 @@ test_data_compare_secrets_new (NMSettingSecretFlags secret_flags, return data; } +static void +_test_compare_secrets_check_diff (NMSetting *a, + NMSetting *b, + NMSettingCompareFlags flags, + gboolean exp_same_psk, + gboolean exp_same_psk_flags) +{ + gs_unref_hashtable GHashTable *h = NULL; + NMSettingDiffResult _RESULT_IN_A = NM_SETTING_DIFF_RESULT_IN_A; + NMSettingDiffResult _RESULT_IN_B = NM_SETTING_DIFF_RESULT_IN_B; + gboolean invert_results; + gboolean diff_result; + NMSettingSecretFlags a_psk_flags = nm_setting_wireless_security_get_psk_flags (NM_SETTING_WIRELESS_SECURITY (a)); + NMSettingSecretFlags b_psk_flags = nm_setting_wireless_security_get_psk_flags (NM_SETTING_WIRELESS_SECURITY (b)); + const char *a_psk = nm_setting_wireless_security_get_psk (NM_SETTING_WIRELESS_SECURITY (a)); + const char *b_psk = nm_setting_wireless_security_get_psk (NM_SETTING_WIRELESS_SECURITY (b)); + + g_assert (NM_IS_SETTING_WIRELESS_SECURITY (a)); + g_assert (NM_IS_SETTING_WIRELESS_SECURITY (b)); + + invert_results = nmtst_get_rand_bool (); + if (invert_results) { + _RESULT_IN_A = NM_SETTING_DIFF_RESULT_IN_B; + _RESULT_IN_B = NM_SETTING_DIFF_RESULT_IN_A; + } + + diff_result = nm_setting_diff (a, b, flags, invert_results, &h); + + g_assert (exp_same_psk_flags == (a_psk_flags == b_psk_flags)); + + if (nm_streq0 (a_psk, b_psk)) + g_assert (exp_same_psk); + else { + if (flags == NM_SETTING_COMPARE_FLAG_EXACT) + g_assert (!exp_same_psk); + else if (flags == NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) { + if ( !NM_FLAGS_HAS (a_psk_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED) + && !NM_FLAGS_HAS (b_psk_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) + g_assert (!exp_same_psk); + else if ( !NM_FLAGS_HAS (a_psk_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED) + && NM_FLAGS_HAS (b_psk_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) + g_assert (!exp_same_psk); + else + g_assert (exp_same_psk); + } else if (flags == NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) { + if ( !NM_FLAGS_HAS (a_psk_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED) + && !NM_FLAGS_HAS (b_psk_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED)) + g_assert (!exp_same_psk); + else if ( !NM_FLAGS_HAS (a_psk_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED) + && NM_FLAGS_HAS (b_psk_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED)) + g_assert (!exp_same_psk); + else + g_assert (exp_same_psk); + } else if (flags == NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) + g_assert (exp_same_psk); + else + g_assert_not_reached (); + } + + g_assert (diff_result == (exp_same_psk && exp_same_psk_flags)); + g_assert (diff_result == (!h)); + + if (!diff_result) { + if (flags == NM_SETTING_COMPARE_FLAG_EXACT) + g_assert (!exp_same_psk); + else if ( NM_IN_SET (flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, + NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) + && (a_psk_flags != b_psk_flags) + && nm_setting_wireless_security_get_psk_flags (NM_SETTING_WIRELESS_SECURITY (a)) == NM_SETTING_SECRET_FLAG_NONE) + g_assert (!exp_same_psk); + else + g_assert (exp_same_psk); + + g_assert ((!exp_same_psk) == g_hash_table_contains (h, NM_SETTING_WIRELESS_SECURITY_PSK)); + if (!exp_same_psk) { + if (nm_setting_wireless_security_get_psk (NM_SETTING_WIRELESS_SECURITY (a))) + g_assert_cmpint (GPOINTER_TO_UINT (g_hash_table_lookup (h, NM_SETTING_WIRELESS_SECURITY_PSK)), ==, _RESULT_IN_A); + else + g_assert_cmpint (GPOINTER_TO_UINT (g_hash_table_lookup (h, NM_SETTING_WIRELESS_SECURITY_PSK)), ==, _RESULT_IN_B); + } + + g_assert ((!exp_same_psk_flags) == g_hash_table_contains (h, NM_SETTING_WIRELESS_SECURITY_PSK_FLAGS)); + if (!exp_same_psk_flags) { + if (nm_setting_wireless_security_get_psk_flags (NM_SETTING_WIRELESS_SECURITY (a)) != NM_SETTING_SECRET_FLAG_NONE) + g_assert_cmpint (GPOINTER_TO_UINT (g_hash_table_lookup (h, NM_SETTING_WIRELESS_SECURITY_PSK_FLAGS)), ==, _RESULT_IN_A); + else + g_assert_cmpint (GPOINTER_TO_UINT (g_hash_table_lookup (h, NM_SETTING_WIRELESS_SECURITY_PSK_FLAGS)), ==, _RESULT_IN_B); + } + + g_assert_cmpint (g_hash_table_size (h), ==, (!exp_same_psk) + (!exp_same_psk_flags)); + } + + g_assert (diff_result == nm_setting_compare (a, b, flags)); + g_assert (diff_result == nm_setting_compare (b, a, flags)); + +} + static void test_setting_compare_secrets (gconstpointer test_data) { const TestDataCompareSecrets *data = test_data; - gs_unref_object NMSetting *old = NULL, *new = NULL; - gboolean success; + gs_unref_object NMConnection *conn_old = NULL; + gs_unref_object NMConnection *conn_new = NULL; + gs_unref_object NMSetting *old = NULL; + gs_unref_object NMSetting *new = NULL; /* Make sure that a connection with transient/unsaved secrets compares * successfully to the same connection without those secrets. @@ -3324,17 +3423,45 @@ test_setting_compare_secrets (gconstpointer test_data) 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 (data->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); + g_assert ((!data->remove_secret) == nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_EXACT)); + g_assert ((!data->remove_secret) == nm_setting_compare (new, old, NM_SETTING_COMPARE_FLAG_EXACT)); + + _test_compare_secrets_check_diff (old, new, NM_SETTING_COMPARE_FLAG_EXACT, !data->remove_secret, TRUE); + _test_compare_secrets_check_diff (new, old, NM_SETTING_COMPARE_FLAG_EXACT, !data->remove_secret, TRUE); + + g_assert (nm_setting_compare (old, new, data->comp_flags)); + g_assert (nm_setting_compare (new, old, data->comp_flags)); + + _test_compare_secrets_check_diff (old, new, data->comp_flags, TRUE, TRUE); + _test_compare_secrets_check_diff (new, old, data->comp_flags, TRUE, TRUE); + + /* OK. Try again, but this time not only change the secret, also let the secret flags differ... */ + if (data->secret_flags != NM_SETTING_SECRET_FLAG_NONE) { + nm_setting_set_secret_flags (new, NM_SETTING_WIRELESS_SECURITY_PSK, NM_SETTING_SECRET_FLAG_NONE, NULL); + + _test_compare_secrets_check_diff (old, new, NM_SETTING_COMPARE_FLAG_EXACT, FALSE, FALSE); + _test_compare_secrets_check_diff (new, old, NM_SETTING_COMPARE_FLAG_EXACT, FALSE, FALSE); + + _test_compare_secrets_check_diff (old, new, data->comp_flags, TRUE, FALSE); + _test_compare_secrets_check_diff (new, old, data->comp_flags, FALSE, FALSE); + + nm_setting_set_secret_flags (new, NM_SETTING_WIRELESS_SECURITY_PSK, data->secret_flags, NULL); } - success = nm_setting_compare (old, new, data->comp_flags); - g_assert (success); + conn_old = nmtst_create_minimal_connection ("test-compare-secrets", NULL, NM_SETTING_WIRELESS_SETTING_NAME, NULL); + nm_connection_add_setting (conn_old, nm_setting_duplicate (old)); + conn_new = nm_simple_connection_new_clone (conn_old); + nm_connection_add_setting (conn_new, nm_setting_duplicate (new)); + + g_assert ((!data->remove_secret) == nm_connection_compare (conn_old, conn_new, NM_SETTING_COMPARE_FLAG_EXACT)); + g_assert ((!data->remove_secret) == nm_connection_compare (conn_new, conn_old, NM_SETTING_COMPARE_FLAG_EXACT)); + + g_assert (nm_connection_compare (conn_old, conn_new, data->comp_flags)); + g_assert (nm_connection_compare (conn_new, conn_old, data->comp_flags)); } static void From 5aace3dfea61b2409d6b62127853bd5ed4721bf2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Jan 2019 21:29:14 +0100 Subject: [PATCH 06/11] libnm: unify handling whether to compare properties in NMSetting nm_setting_compare() and nm_setting_diff() both call the virtual function compare_property(). But their check for determining whether to call the virtual function differs. In a first step, merge the implementations so that the check is clearly similar in both cases. --- libnm-core/nm-setting.c | 103 ++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 52 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index a972a1f6de..ab4567ca5c 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -86,6 +86,11 @@ G_DEFINE_ABSTRACT_TYPE (NMSetting, nm_setting, G_TYPE_OBJECT) static GenData *_gendata_hash (NMSetting *setting, gboolean create_if_necessary); +static gboolean should_compare_prop (gboolean for_diff, + NMSetting *setting, + const NMSettInfoProperty *property_info, + NMSettingCompareFlags comp_flags); + /*****************************************************************************/ static NMSettingPriority @@ -1334,88 +1339,85 @@ nm_setting_compare (NMSetting *a, g_variant_equal); } - /* And now all properties */ for (i = 0; i < sett_info->property_infos_len; i++) { - GParamSpec *prop_spec = sett_info->property_infos[i].param_spec; + const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; - if (!prop_spec) + if (!should_compare_prop (FALSE, a, property_info, flags)) continue; - /* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */ - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_FUZZY) - && NM_FLAGS_ANY (prop_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) - continue; - - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) - && !NM_FLAGS_HAS (prop_spec->flags, NM_SETTING_PARAM_INFERRABLE)) - continue; - - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) - && NM_FLAGS_HAS (prop_spec->flags, NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) - continue; - - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) - && NM_FLAGS_HAS (prop_spec->flags, NM_SETTING_PARAM_SECRET)) - continue; - - if (!NM_SETTING_GET_CLASS (a)->compare_property (a, b, prop_spec, flags)) + if (!NM_SETTING_GET_CLASS (a)->compare_property (a, b, property_info->param_spec, flags)) return FALSE; } return TRUE; } -static inline gboolean -should_compare_prop (NMSetting *setting, - const char *prop_name, - NMSettingCompareFlags comp_flags, - GParamFlags prop_flags) +static gboolean +should_compare_prop (gboolean for_diff, + NMSetting *setting, + const NMSettInfoProperty *property_info, + NMSettingCompareFlags comp_flags) { - /* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */ - if ( (comp_flags & NM_SETTING_COMPARE_FLAG_FUZZY) - && (prop_flags & (NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET))) + const GParamSpec *param_spec = property_info->param_spec; + + if (!param_spec) return FALSE; - if ((comp_flags & NM_SETTING_COMPARE_FLAG_INFERRABLE) && !(prop_flags & NM_SETTING_PARAM_INFERRABLE)) + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_FUZZY) + && NM_FLAGS_ANY (param_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) return FALSE; - if ((comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) && (prop_flags & NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) + && !NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_INFERRABLE)) return FALSE; - if (prop_flags & NM_SETTING_PARAM_SECRET) { + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) + && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) + return FALSE; + + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) + && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET)) + return FALSE; + + if (nm_streq (param_spec->name, NM_SETTING_NAME)) + return FALSE; + + if ( for_diff + && NM_FLAGS_ANY (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS + | NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) + && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET)) { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - if (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) - return FALSE; - if ( NM_IS_SETTING_VPN (setting) - && g_strcmp0 (prop_name, NM_SETTING_VPN_SECRETS) == 0) { + && nm_streq (param_spec->name, NM_SETTING_VPN_SECRETS)) { /* FIXME: NMSettingVPN:NM_SETTING_VPN_SECRETS has NM_SETTING_PARAM_SECRET. * nm_setting_get_secret_flags() quite possibly fails, but it might succeed if the * setting accidentally uses a key "secrets". */ return TRUE; } - if (!nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL)) + if (!nm_setting_get_secret_flags (setting, param_spec->name, &secret_flags, NULL)) g_return_val_if_reached (FALSE); - if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) - && (secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)) + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) + && NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) return FALSE; - if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) - && (secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED)) + if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) + && NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED)) return FALSE; } - if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_ID) + if ( for_diff + && NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_ID) && NM_IS_SETTING_CONNECTION (setting) - && !strcmp (prop_name, NM_SETTING_CONNECTION_ID)) + && nm_streq (param_spec->name, NM_SETTING_CONNECTION_ID)) return FALSE; - if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP) + if ( for_diff + && NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP) && NM_IS_SETTING_CONNECTION (setting) - && !strcmp (prop_name, NM_SETTING_CONNECTION_TIMESTAMP)) + && nm_streq (param_spec->name, NM_SETTING_CONNECTION_TIMESTAMP)) return FALSE; return TRUE; @@ -1552,17 +1554,14 @@ nm_setting_diff (NMSetting *a, } } else { for (i = 0; i < sett_info->property_infos_len; i++) { - GParamSpec *prop_spec = sett_info->property_infos[i].param_spec; + const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; + GParamSpec *prop_spec; NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN; - if (!prop_spec) + if (!should_compare_prop (TRUE, a, property_info, flags)) continue; - /* Handle compare flags */ - if (!should_compare_prop (a, prop_spec->name, flags, prop_spec->flags)) - continue; - if (strcmp (prop_spec->name, NM_SETTING_NAME) == 0) - continue; + prop_spec = property_info->param_spec; compared_any = TRUE; From 885c872d5a542f820afdf4e8ca59c55f39467c7a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 09:08:39 +0100 Subject: [PATCH 07/11] libnm: rework compare_property() implementation for NMSetting NMSetting's compare_property() has and had two callers: nm_setting_compare() and nm_setting_diff(). compare_property() accepts a NMSettingCompareFlags argument, but at the same time, both callers have another complex (and inconsistent!) set of pre-checks for shortcuting the call of compare_property(): should_compare_prop(). Merge should_compare_prop() into compare_property(). This way, nm_setting_compare() and nm_setting_diff() has less additional code, and are simpler to follow. Especially nm_setting_compare() is now trivial. And nm_setting_diff() is still complicated, but not related to the question how the property compares or whether it should be compared at all. If you want to know whether it should be compared, all you need to do now is follow NMSettingClass.compare_property(). This changes function pointer NMSettingClass.compare_property(), which is public API. However, no user can actually use this (and shall not!), because _nm_setting_class_commit_full() etc. is private API. A user outside of libnm-core cannot create his/her own subclasses of NMSetting, and never could in the past. So, this API/ABI change doesn't matter. --- libnm-core/nm-setting-bond.c | 54 ++--- libnm-core/nm-setting-connection.c | 27 ++- libnm-core/nm-setting-ip-config.c | 50 ++-- libnm-core/nm-setting-private.h | 5 + libnm-core/nm-setting-sriov.c | 34 +-- libnm-core/nm-setting-tc-config.c | 39 ++-- libnm-core/nm-setting-team-port.c | 82 ++++--- libnm-core/nm-setting-team.c | 73 +++--- libnm-core/nm-setting-user.c | 37 +-- libnm-core/nm-setting-vpn.c | 94 ++++---- libnm-core/nm-setting-wired.c | 22 +- libnm-core/nm-setting-wireless.c | 23 +- libnm-core/nm-setting.c | 356 ++++++++++++++++------------- libnm-core/nm-setting.h | 18 +- 14 files changed, 509 insertions(+), 405 deletions(-) diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c index 0c84b3a01c..7bf7612aee 100644 --- a/libnm-core/nm-setting-bond.c +++ b/libnm-core/nm-setting-bond.c @@ -811,15 +811,15 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) /*****************************************************************************/ static gboolean -options_hash_match (NMSettingBond *s_bond, - GHashTable *options1, - GHashTable *options2, +options_equal_asym (NMSettingBond *s_bond, + NMSettingBond *s_bond2, NMSettingCompareFlags flags) { + GHashTable *options2 = NM_SETTING_BOND_GET_PRIVATE (s_bond2)->options; GHashTableIter iter; const char *key, *value, *value2; - g_hash_table_iter_init (&iter, options1); + g_hash_table_iter_init (&iter, NM_SETTING_BOND_GET_PRIVATE (s_bond)->options); while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) { if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) { @@ -841,15 +841,10 @@ options_hash_match (NMSettingBond *s_bond, value2 = g_hash_table_lookup (options2, "num_grat_arp"); } - if (value2) { - if (nm_streq (value, value2)) - continue; - } else { - if (nm_streq (value, nm_setting_bond_get_option_default (s_bond, key))) - continue; - } - - return FALSE; + if (!value2) + value2 = nm_setting_bond_get_option_default (s_bond2, key); + if (!nm_streq (value, value2)) + return FALSE; } return TRUE; @@ -857,31 +852,32 @@ options_hash_match (NMSettingBond *s_bond, static gboolean options_equal (NMSettingBond *s_bond, - GHashTable *options1, - GHashTable *options2, + NMSettingBond *s_bond2, NMSettingCompareFlags flags) { - return options_hash_match (s_bond, options1, options2, flags) - && options_hash_match (s_bond, options2, options1, flags); + return options_equal_asym (s_bond, s_bond2, flags) + && options_equal_asym (s_bond2, s_bond, flags); } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingClass *setting_class; - - if (nm_streq0 (prop_spec->name, NM_SETTING_BOND_OPTIONS)) { - return options_equal (NM_SETTING_BOND (setting), - NM_SETTING_BOND_GET_PRIVATE (setting)->options, - NM_SETTING_BOND_GET_PRIVATE (other)->options, - flags); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_BOND_OPTIONS)) { + return ( !other + || options_equal (NM_SETTING_BOND (setting), + NM_SETTING_BOND (other), + flags)); } - setting_class = NM_SETTING_CLASS (nm_setting_bond_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_bond_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } /*****************************************************************************/ diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 3c3a4dcee5..12a2af5bec 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -1281,23 +1281,26 @@ nm_setting_connection_no_interface_name (NMSetting *setting, return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - /* Handle ignore ID */ - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_ID) - && g_strcmp0 (prop_spec->name, NM_SETTING_CONNECTION_ID) == 0) - return TRUE; + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_ID) + && nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_CONNECTION_ID)) + return NM_TERNARY_DEFAULT; - /* Handle ignore timestamp */ - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP) - && g_strcmp0 (prop_spec->name, NM_SETTING_CONNECTION_TIMESTAMP) == 0) - return TRUE; + if ( NM_FLAGS_HAS(flags, NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP) + && nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_CONNECTION_TIMESTAMP)) + return NM_TERNARY_DEFAULT; - return NM_SETTING_CLASS (nm_setting_connection_parent_class)->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_connection_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static void diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 23e211f2e8..3e20263247 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -2612,44 +2612,52 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingIPConfigPrivate *a_priv, *b_priv; - NMSettingClass *setting_class; + NMSettingIPConfigPrivate *a_priv; + NMSettingIPConfigPrivate *b_priv; guint i; - if (nm_streq (prop_spec->name, NM_SETTING_IP_CONFIG_ADDRESSES)) { - a_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting); - b_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (other); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_IP_CONFIG_ADDRESSES)) { + if (other) { + a_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting); + b_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (other); - if (a_priv->addresses->len != b_priv->addresses->len) - return FALSE; - for (i = 0; i < a_priv->addresses->len; i++) { - if (!_nm_ip_address_equal (a_priv->addresses->pdata[i], b_priv->addresses->pdata[i], TRUE)) + if (a_priv->addresses->len != b_priv->addresses->len) return FALSE; + for (i = 0; i < a_priv->addresses->len; i++) { + if (!_nm_ip_address_equal (a_priv->addresses->pdata[i], b_priv->addresses->pdata[i], TRUE)) + return FALSE; + } } return TRUE; } - if (nm_streq (prop_spec->name, NM_SETTING_IP_CONFIG_ROUTES)) { - a_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting); - b_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (other); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_IP_CONFIG_ROUTES)) { + if (other) { + a_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (setting); + b_priv = NM_SETTING_IP_CONFIG_GET_PRIVATE (other); - if (a_priv->routes->len != b_priv->routes->len) - return FALSE; - for (i = 0; i < a_priv->routes->len; i++) { - if (!nm_ip_route_equal_full (a_priv->routes->pdata[i], b_priv->routes->pdata[i], NM_IP_ROUTE_EQUAL_CMP_FLAGS_WITH_ATTRS)) + if (a_priv->routes->len != b_priv->routes->len) return FALSE; + for (i = 0; i < a_priv->routes->len; i++) { + if (!nm_ip_route_equal_full (a_priv->routes->pdata[i], b_priv->routes->pdata[i], NM_IP_ROUTE_EQUAL_CMP_FLAGS_WITH_ATTRS)) + return FALSE; + } } return TRUE; } - setting_class = NM_SETTING_CLASS (nm_setting_ip_config_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_ip_config_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } /*****************************************************************************/ diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 49d27336fa..2f46bc6770 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -201,6 +201,11 @@ gboolean _nm_setting_use_legacy_property (NMSetting *setting, GPtrArray *_nm_setting_need_secrets (NMSetting *setting); +gboolean _nm_setting_should_compare_secret_property (NMSetting *setting, + NMSetting *other, + const char *secret_name, + NMSettingCompareFlags flags); + /*****************************************************************************/ #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting-sriov.c b/libnm-core/nm-setting-sriov.c index b7756e3506..fa32f55cde 100644 --- a/libnm-core/nm-setting-sriov.c +++ b/libnm-core/nm-setting-sriov.c @@ -1188,29 +1188,37 @@ get_property (GObject *object, guint prop_id, } } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingSriov *a = NM_SETTING_SRIOV (setting); - NMSettingSriov *b = NM_SETTING_SRIOV (other); - NMSettingClass *setting_class; + NMSettingSriov *a; + NMSettingSriov *b; guint i; - if (nm_streq (prop_spec->name, NM_SETTING_SRIOV_VFS)) { - if (a->vfs->len != b->vfs->len) - return FALSE; - for (i = 0; i < a->vfs->len; i++) { - if (!nm_sriov_vf_equal (a->vfs->pdata[i], b->vfs->pdata[i])) + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_SRIOV_VFS)) { + if (other) { + a = NM_SETTING_SRIOV (setting); + b = NM_SETTING_SRIOV (other); + + if (a->vfs->len != b->vfs->len) return FALSE; + for (i = 0; i < a->vfs->len; i++) { + if (!nm_sriov_vf_equal (a->vfs->pdata[i], b->vfs->pdata[i])) + return FALSE; + } } return TRUE; } - setting_class = NM_SETTING_CLASS (nm_setting_sriov_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_sriov_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static void diff --git a/libnm-core/nm-setting-tc-config.c b/libnm-core/nm-setting-tc-config.c index 43015d3708..4fe3c0fd96 100644 --- a/libnm-core/nm-setting-tc-config.c +++ b/libnm-core/nm-setting-tc-config.c @@ -1233,39 +1233,46 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { NMSettingTCConfig *a_tc_config = NM_SETTING_TC_CONFIG (setting); NMSettingTCConfig *b_tc_config = NM_SETTING_TC_CONFIG (other); - NMSettingClass *setting_class; guint i; - if (nm_streq (prop_spec->name, NM_SETTING_TC_CONFIG_QDISCS)) { - if (a_tc_config->qdiscs->len != b_tc_config->qdiscs->len) - return FALSE; - for (i = 0; i < a_tc_config->qdiscs->len; i++) { - if (!nm_tc_qdisc_equal (a_tc_config->qdiscs->pdata[i], b_tc_config->qdiscs->pdata[i])) + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TC_CONFIG_QDISCS)) { + if (other) { + if (a_tc_config->qdiscs->len != b_tc_config->qdiscs->len) return FALSE; + for (i = 0; i < a_tc_config->qdiscs->len; i++) { + if (!nm_tc_qdisc_equal (a_tc_config->qdiscs->pdata[i], b_tc_config->qdiscs->pdata[i])) + return FALSE; + } } return TRUE; } - if (nm_streq (prop_spec->name, NM_SETTING_TC_CONFIG_TFILTERS)) { - if (a_tc_config->tfilters->len != b_tc_config->tfilters->len) - return FALSE; - for (i = 0; i < a_tc_config->tfilters->len; i++) { - if (!nm_tc_tfilter_equal (a_tc_config->tfilters->pdata[i], b_tc_config->tfilters->pdata[i])) + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TC_CONFIG_TFILTERS)) { + if (other) { + if (a_tc_config->tfilters->len != b_tc_config->tfilters->len) return FALSE; + for (i = 0; i < a_tc_config->tfilters->len; i++) { + if (!nm_tc_tfilter_equal (a_tc_config->tfilters->pdata[i], b_tc_config->tfilters->pdata[i])) + return FALSE; + } } return TRUE; } - setting_class = NM_SETTING_CLASS (nm_setting_tc_config_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_tc_config_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static void diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index 1901386c62..3ba2d44e08 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -401,48 +401,68 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingClass *setting_class; - NMSettingTeamPortPrivate *a_priv, *b_priv; + NMSettingTeamPortPrivate *a_priv; + NMSettingTeamPortPrivate *b_priv; guint i, j; - /* If we are trying to match a connection in order to assume it (and thus - * @flags contains INFERRABLE), use the "relaxed" matching for team - * configuration. Otherwise, for all other purposes (including connection - * comparison before an update), resort to the default string comparison. - */ - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) - && nm_streq0 (prop_spec->name, NM_SETTING_TEAM_PORT_CONFIG)) { - return _nm_utils_team_config_equal (NM_SETTING_TEAM_PORT_GET_PRIVATE (setting)->config, - NM_SETTING_TEAM_PORT_GET_PRIVATE (other)->config, - TRUE); - } - if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_PORT_LINK_WATCHERS)) { - a_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (setting); - b_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (other); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TEAM_PORT_LINK_WATCHERS)) { - if (a_priv->link_watchers->len != b_priv->link_watchers->len) - return FALSE; - for (i = 0; i < a_priv->link_watchers->len; i++) { - for (j = 0; j < b_priv->link_watchers->len; j++) { - if (nm_team_link_watcher_equal (a_priv->link_watchers->pdata[i], - b_priv->link_watchers->pdata[j])) { - break; - } - } - if (j == b_priv->link_watchers->len) + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) + return NM_TERNARY_DEFAULT; + + if (other) { + a_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (setting); + b_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (other); + + if (a_priv->link_watchers->len != b_priv->link_watchers->len) return FALSE; + for (i = 0; i < a_priv->link_watchers->len; i++) { + for (j = 0; j < b_priv->link_watchers->len; j++) { + if (nm_team_link_watcher_equal (a_priv->link_watchers->pdata[i], + b_priv->link_watchers->pdata[j])) { + break; + } + } + if (j == b_priv->link_watchers->len) + return FALSE; + } } return TRUE; } - setting_class = NM_SETTING_CLASS (nm_setting_team_port_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TEAM_PORT_CONFIG)) { + if (other) { + a_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (setting); + b_priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (other); + + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) { + /* If we are trying to match a connection in order to assume it (and thus + * @flags contains INFERRABLE), use the "relaxed" matching for team + * configuration. Otherwise, for all other purposes (including connection + * comparison before an update), resort to the default string comparison. */ + return _nm_utils_team_config_equal (a_priv->config, + b_priv->config, + TRUE); + } + + return nm_streq0 (a_priv->config, b_priv->config); + } + + return TRUE; + } + + return NM_SETTING_CLASS (nm_setting_team_port_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static void diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c index 0ca47e83fa..4021aa0cb8 100644 --- a/libnm-core/nm-setting-team.c +++ b/libnm-core/nm-setting-team.c @@ -1297,38 +1297,33 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { NMSettingTeamPrivate *a_priv, *b_priv; - NMSettingClass *setting_class; guint i, j; - /* If we are trying to match a connection in order to assume it (and thus - * @flags contains INFERRABLE), use the "relaxed" matching for team - * configuration. Otherwise, for all other purposes (including connection - * comparison before an update), resort to the default string comparison. - */ - if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) - && nm_streq0 (prop_spec->name, NM_SETTING_TEAM_CONFIG)) { - return _nm_utils_team_config_equal (NM_SETTING_TEAM_GET_PRIVATE (setting)->config, - NM_SETTING_TEAM_GET_PRIVATE (other)->config, - FALSE); - } - if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_LINK_WATCHERS)) { - a_priv = NM_SETTING_TEAM_GET_PRIVATE (setting); - b_priv = NM_SETTING_TEAM_GET_PRIVATE (other); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TEAM_LINK_WATCHERS)) { - if (a_priv->link_watchers->len != b_priv->link_watchers->len) - return FALSE; - for (i = 0; i < a_priv->link_watchers->len; i++) { - for (j = 0; j < b_priv->link_watchers->len; j++) { - if (nm_team_link_watcher_equal (a_priv->link_watchers->pdata[i], - b_priv->link_watchers->pdata[j])) { - break; + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) + return NM_TERNARY_DEFAULT; + + if (other) { + a_priv = NM_SETTING_TEAM_GET_PRIVATE (setting); + b_priv = NM_SETTING_TEAM_GET_PRIVATE (other); + + if (a_priv->link_watchers->len != b_priv->link_watchers->len) + return FALSE; + for (i = 0; i < a_priv->link_watchers->len; i++) { + for (j = 0; j < b_priv->link_watchers->len; j++) { + if (nm_team_link_watcher_equal (a_priv->link_watchers->pdata[i], + b_priv->link_watchers->pdata[j])) { + break; + } } if (j == b_priv->link_watchers->len) return FALSE; @@ -1337,8 +1332,32 @@ compare_property (NMSetting *setting, return TRUE; } - setting_class = NM_SETTING_CLASS (nm_setting_team_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_TEAM_CONFIG)) { + if (other) { + a_priv = NM_SETTING_TEAM_GET_PRIVATE (setting); + b_priv = NM_SETTING_TEAM_GET_PRIVATE (other); + + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) { + /* If we are trying to match a connection in order to assume it (and thus + * @flags contains INFERRABLE), use the "relaxed" matching for team + * configuration. Otherwise, for all other purposes (including connection + * comparison before an update), resort to the default string comparison. */ + return _nm_utils_team_config_equal (a_priv->config, + b_priv->config, + TRUE); + } + + return nm_streq0 (a_priv->config, b_priv->config); + } + + return TRUE; + } + + return NM_SETTING_CLASS (nm_setting_team_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static void diff --git a/libnm-core/nm-setting-user.c b/libnm-core/nm-setting-user.c index d72c909462..cef02092c8 100644 --- a/libnm-core/nm-setting-user.c +++ b/libnm-core/nm-setting-user.c @@ -395,33 +395,34 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { NMSettingUserPrivate *priv, *pri2; - g_return_val_if_fail (NM_IS_SETTING_USER (setting), FALSE); - g_return_val_if_fail (NM_IS_SETTING_USER (other), FALSE); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_USER_DATA)) { - if (!nm_streq0 (prop_spec->name, NM_SETTING_USER_DATA)) - goto call_parent; + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) + return NM_TERNARY_DEFAULT; - priv = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (setting)); - pri2 = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (other)); + if (!other) + return TRUE; - if (!nm_utils_hash_table_equal (priv->data, pri2->data, TRUE, g_str_equal)) - return FALSE; + priv = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (setting)); + pri2 = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (other)); + return nm_utils_hash_table_equal (priv->data, pri2->data, TRUE, g_str_equal) + && nm_utils_hash_table_equal (priv->data_invalid, pri2->data_invalid, TRUE, g_str_equal); + } - if (!nm_utils_hash_table_equal (priv->data_invalid, pri2->data_invalid, TRUE, g_str_equal)) - return FALSE; - - return TRUE; - -call_parent: - return NM_SETTING_CLASS (nm_setting_user_parent_class)->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_user_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } /*****************************************************************************/ diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index d485b2a802..b0c13b1573 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -765,77 +765,63 @@ need_secrets (NMSetting *setting) return g_ptr_array_sized_new (1); } -static gboolean -_compare_secrets (NMSettingVpn *a, - NMSettingVpn *b, - NMSettingCompareFlags flags) +static NMTernary +compare_property_secrets (NMSettingVpn *a, + NMSettingVpn *b, + NMSettingCompareFlags flags) { - GHashTable *a_secrets; GHashTableIter iter; const char *key, *val; + int run; - a_secrets = NM_SETTING_VPN_GET_PRIVATE (a)->secrets; - g_hash_table_iter_init (&iter, a_secrets); - while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &val)) { - NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE; - NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE; + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_FUZZY)) + return NM_TERNARY_DEFAULT; + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS)) + return NM_TERNARY_DEFAULT; - nm_setting_get_secret_flags (NM_SETTING (a), key, &a_secret_flags, NULL); - nm_setting_get_secret_flags (NM_SETTING (b), key, &b_secret_flags, NULL); + if (!b) + return TRUE; + + for (run = 0; run < 2; run++) { + NMSettingVpn *current_a = (run == 0) ? a : b; + NMSettingVpn *current_b = (run == 0) ? b : a; + + g_hash_table_iter_init (&iter, NM_SETTING_VPN_GET_PRIVATE (current_a)->secrets); + while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &val)) { + + if (nm_streq0 (val, nm_setting_vpn_get_secret (current_b, key))) + continue; + if (!_nm_setting_should_compare_secret_property (NM_SETTING (current_a), + NM_SETTING (current_b), + key, + flags)) + continue; - /* If the secret flags aren't the same, the settings aren't the same */ - if (a_secret_flags != b_secret_flags) - return FALSE; - - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) - && (a_secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - continue; - - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) - && (a_secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED)) - continue; - - /* Now compare the values themselves */ - if (g_strcmp0 (val, nm_setting_vpn_get_secret (b, key)) != 0) return FALSE; + } } return TRUE; } -static gboolean -compare_one_secret (NMSettingVpn *a, - NMSettingVpn *b, - NMSettingCompareFlags flags) -{ - if (!_compare_secrets (a, b, flags)) - return FALSE; - if (!_compare_secrets (b, a, flags)) - return FALSE; - - return TRUE; -} - -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - gboolean same; - - /* We only need to treat the 'secrets' property specially */ - if (g_strcmp0 (prop_spec->name, NM_SETTING_VPN_SECRETS) != 0) - return NM_SETTING_CLASS (nm_setting_vpn_parent_class)->compare_property (setting, other, prop_spec, flags); - - /* Compare A to B to ensure everything in A is found in B */ - same = compare_one_secret (NM_SETTING_VPN (setting), NM_SETTING_VPN (other), flags); - if (same) { - /* And then B to A to ensure everything in B is also found in A */ - same = compare_one_secret (NM_SETTING_VPN (other), NM_SETTING_VPN (setting), flags); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_VPN_SECRETS)) { + if (NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE)) + return NM_TERNARY_DEFAULT; + return compare_property_secrets (NM_SETTING_VPN (setting), NM_SETTING_VPN (other), flags); } - return same; + return NM_SETTING_CLASS (nm_setting_vpn_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static gboolean diff --git a/libnm-core/nm-setting-wired.c b/libnm-core/nm-setting-wired.c index 5a005b2f63..96467c02c3 100644 --- a/libnm-core/nm-setting-wired.c +++ b/libnm-core/nm-setting-wired.c @@ -775,21 +775,25 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingClass *setting_class; - if (nm_streq (prop_spec->name, NM_SETTING_WIRED_CLONED_MAC_ADDRESS)) { - return nm_streq0 (NM_SETTING_WIRED_GET_PRIVATE (setting)->cloned_mac_address, - NM_SETTING_WIRED_GET_PRIVATE (other)->cloned_mac_address); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_WIRED_CLONED_MAC_ADDRESS)) { + return !other + || nm_streq0 (NM_SETTING_WIRED_GET_PRIVATE (setting)->cloned_mac_address, + NM_SETTING_WIRED_GET_PRIVATE (other)->cloned_mac_address); } - setting_class = NM_SETTING_CLASS (nm_setting_wired_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_wired_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } static GVariant * diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c index cfa79b54f6..dcba8d81e3 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -929,21 +929,24 @@ mac_addr_rand_ok: return TRUE; } -static gboolean -compare_property (NMSetting *setting, +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, NMSetting *other, - const GParamSpec *prop_spec, NMSettingCompareFlags flags) { - NMSettingClass *setting_class; - - if (nm_streq (prop_spec->name, NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS)) { - return nm_streq0 (NM_SETTING_WIRELESS_GET_PRIVATE (setting)->cloned_mac_address, - NM_SETTING_WIRELESS_GET_PRIVATE (other)->cloned_mac_address); + if (nm_streq (sett_info->property_infos[property_idx].name, NM_SETTING_WIRELESS_CLONED_MAC_ADDRESS)) { + return !other + || nm_streq0 (NM_SETTING_WIRELESS_GET_PRIVATE (setting)->cloned_mac_address, + NM_SETTING_WIRELESS_GET_PRIVATE (other)->cloned_mac_address); } - setting_class = NM_SETTING_CLASS (nm_setting_wireless_parent_class); - return setting_class->compare_property (setting, other, prop_spec, flags); + return NM_SETTING_CLASS (nm_setting_wireless_parent_class)->compare_property (sett_info, + property_idx, + setting, + other, + flags); } /*****************************************************************************/ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index ab4567ca5c..bea47b3509 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -86,11 +86,6 @@ G_DEFINE_ABSTRACT_TYPE (NMSetting, nm_setting, G_TYPE_OBJECT) static GenData *_gendata_hash (NMSetting *setting, gboolean create_if_necessary); -static gboolean should_compare_prop (gboolean for_diff, - NMSetting *setting, - const NMSettInfoProperty *property_info, - NMSettingCompareFlags comp_flags); - /*****************************************************************************/ static NMSettingPriority @@ -1246,58 +1241,148 @@ _nm_setting_verify_secret_string (const char *str, return TRUE; } -static gboolean -compare_property (NMSetting *setting, - NMSetting *other, - const GParamSpec *prop_spec, - NMSettingCompareFlags flags) +gboolean +_nm_setting_should_compare_secret_property (NMSetting *setting, + NMSetting *other, + const char *secret_name, + NMSettingCompareFlags flags) { - const NMSettInfoProperty *property; - GVariant *value1, *value2; - int cmp; + NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE; + NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE; - /* Handle compare flags */ - if (prop_spec->flags & NM_SETTING_PARAM_SECRET) { - NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE; - NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE; + nm_assert (NM_IS_SETTING (setting)); + nm_assert (!other || G_OBJECT_TYPE (setting) == G_OBJECT_TYPE (other)); - g_return_val_if_fail (!NM_IS_SETTING_VPN (setting), FALSE); + /* secret_name must be a valid secret for @setting. */ + nm_assert (nm_setting_get_secret_flags (setting, secret_name, NULL, NULL)); - if (!nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL)) - g_return_val_if_reached (FALSE); - if (!nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL)) - g_return_val_if_reached (FALSE); + if (!NM_FLAGS_ANY (flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS + | NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) + return TRUE; - /* If the secret flags aren't the same the settings aren't the same */ - if (a_secret_flags != b_secret_flags) - return FALSE; - - /* Check for various secret flags that might cause us to ignore comparing - * this property. - */ - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) - && (a_secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - return TRUE; - - if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) - && (a_secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED)) - return TRUE; + nm_setting_get_secret_flags (setting, secret_name, &a_secret_flags, NULL); + if (other) { + if (!nm_setting_get_secret_flags (other, secret_name, &b_secret_flags, NULL)) { + /* secret-name may not be a valid secret for @other. That is fine, we ignore that + * and treat @b_secret_flags as NM_SETTING_SECRET_FLAG_NONE. + * + * This can happen with VPN secrets, where the caller knows that @secret_name + * is a secret for setting, but it may not be a secret for @other. Accept that. + * + * Mark @other as missing. */ + other = NULL; + } } - property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), prop_spec->name); - g_return_val_if_fail (property != NULL, FALSE); + /* when @setting has the secret-flags that should be ignored, + * we skip the comparisong if: + * + * - @other is not present, + * - @other does not have a secret named @secret_name + * - @other also has the secret flat to be ignored. + * + * This makes the check symmetric (aside the fact that @setting must + * have the secret while @other may not -- which is asymetric). */ + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) + && NM_FLAGS_HAS (a_secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED) + && ( !other + || NM_FLAGS_HAS (b_secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED))) + return FALSE; - value1 = get_property_for_dbus (setting, property, TRUE); - value2 = get_property_for_dbus (other, property, TRUE); + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) + && NM_FLAGS_HAS (a_secret_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED) + && ( !other + || NM_FLAGS_HAS (b_secret_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED))) + return FALSE; - cmp = nm_property_compare (value1, value2); + return TRUE; +} - if (value1) - g_variant_unref (value1); - if (value2) - g_variant_unref (value2); +static NMTernary +compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, + NMSetting *other, + NMSettingCompareFlags flags) +{ + const NMSettInfoProperty *property_info = &sett_info->property_infos[property_idx]; + const GParamSpec *param_spec = property_info->param_spec; - return cmp == 0; + if (!param_spec) + return NM_TERNARY_DEFAULT; + + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_FUZZY) + && NM_FLAGS_ANY (param_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) + return NM_TERNARY_DEFAULT; + + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) + && !NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_INFERRABLE)) + return NM_TERNARY_DEFAULT; + + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) + && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) + return NM_TERNARY_DEFAULT; + + if ( NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) + && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET)) + return NM_TERNARY_DEFAULT; + + if (nm_streq (param_spec->name, NM_SETTING_NAME)) + return NM_TERNARY_DEFAULT; + + if ( NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET) + && !_nm_setting_should_compare_secret_property (setting, + other, + param_spec->name, + flags)) + return NM_TERNARY_DEFAULT; + + if (other) { + gs_unref_variant GVariant *value1 = NULL; + gs_unref_variant GVariant *value2 = NULL; + + value1 = get_property_for_dbus (setting, property_info, TRUE); + value2 = get_property_for_dbus (other, property_info, TRUE); + + if (nm_property_compare (value1, value2) != 0) + return NM_TERNARY_FALSE; + } + + return NM_TERNARY_TRUE; +} + +static NMTernary +_compare_property (const NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, + NMSetting *other, + NMSettingCompareFlags flags) +{ + NMTernary compare_result; + + nm_assert (sett_info); + nm_assert (NM_IS_SETTING_CLASS (sett_info->setting_class)); + nm_assert (property_idx < sett_info->property_infos_len); + nm_assert (NM_SETTING_GET_CLASS (setting) == sett_info->setting_class); + nm_assert (!other || NM_SETTING_GET_CLASS (other) == sett_info->setting_class); + + compare_result = NM_SETTING_GET_CLASS (setting)->compare_property (sett_info, + property_idx, + setting, + other, + flags); + + nm_assert (NM_IN_SET (compare_result, NM_TERNARY_DEFAULT, + NM_TERNARY_FALSE, + NM_TERNARY_TRUE)); + + /* check that the inferable flag and the GObject property flag corresponds. */ + nm_assert ( !NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) + || !sett_info->property_infos[property_idx].param_spec + || NM_FLAGS_HAS (sett_info->property_infos[property_idx].param_spec->flags, NM_SETTING_PARAM_INFERRABLE) + || compare_result == NM_TERNARY_DEFAULT); + + return compare_result; } /** @@ -1340,89 +1425,13 @@ nm_setting_compare (NMSetting *a, } for (i = 0; i < sett_info->property_infos_len; i++) { - const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; - - if (!should_compare_prop (FALSE, a, property_info, flags)) - continue; - - if (!NM_SETTING_GET_CLASS (a)->compare_property (a, b, property_info->param_spec, flags)) + if (_compare_property (sett_info, i, a, b, flags) == NM_TERNARY_FALSE) return FALSE; } return TRUE; } -static gboolean -should_compare_prop (gboolean for_diff, - NMSetting *setting, - const NMSettInfoProperty *property_info, - NMSettingCompareFlags comp_flags) -{ - const GParamSpec *param_spec = property_info->param_spec; - - if (!param_spec) - return FALSE; - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_FUZZY) - && NM_FLAGS_ANY (param_spec->flags, NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)) - return FALSE; - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_INFERRABLE) - && !NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_INFERRABLE)) - return FALSE; - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY) - && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_REAPPLY_IMMEDIATELY)) - return FALSE; - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) - && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET)) - return FALSE; - - if (nm_streq (param_spec->name, NM_SETTING_NAME)) - return FALSE; - - if ( for_diff - && NM_FLAGS_ANY (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS - | NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) - && NM_FLAGS_HAS (param_spec->flags, NM_SETTING_PARAM_SECRET)) { - NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - - if ( NM_IS_SETTING_VPN (setting) - && nm_streq (param_spec->name, NM_SETTING_VPN_SECRETS)) { - /* FIXME: NMSettingVPN:NM_SETTING_VPN_SECRETS has NM_SETTING_PARAM_SECRET. - * nm_setting_get_secret_flags() quite possibly fails, but it might succeed if the - * setting accidentally uses a key "secrets". */ - return TRUE; - } - - if (!nm_setting_get_secret_flags (setting, param_spec->name, &secret_flags, NULL)) - g_return_val_if_reached (FALSE); - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS) - && NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - return FALSE; - - if ( NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) - && NM_FLAGS_HAS (secret_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED)) - return FALSE; - } - - if ( for_diff - && NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_ID) - && NM_IS_SETTING_CONNECTION (setting) - && nm_streq (param_spec->name, NM_SETTING_CONNECTION_ID)) - return FALSE; - - if ( for_diff - && NM_FLAGS_HAS (comp_flags, NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP) - && NM_IS_SETTING_CONNECTION (setting) - && nm_streq (param_spec->name, NM_SETTING_CONNECTION_TIMESTAMP)) - return FALSE; - - return TRUE; -} - static void _setting_diff_add_result (GHashTable *results, const char *prop_name, NMSettingDiffResult r) { @@ -1554,65 +1563,92 @@ nm_setting_diff (NMSetting *a, } } else { for (i = 0; i < sett_info->property_infos_len; i++) { - const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; - GParamSpec *prop_spec; NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN; + const NMSettInfoProperty *property_info; + NMTernary compare_result; + GParamSpec *prop_spec; - if (!should_compare_prop (TRUE, a, property_info, flags)) + compare_result = _compare_property (sett_info, i, a, b, flags); + if (compare_result == NM_TERNARY_DEFAULT) continue; - prop_spec = property_info->param_spec; + if ( NM_FLAGS_ANY (flags, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS + | NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS) + && b + && compare_result == NM_TERNARY_FALSE) { + /* we have setting @b and the property is not the same. But we also are instructed + * to ignore secrets based on the flags. + * + * Note that compare_property() called with two settings will ignore secrets + * based on the flags, but it will do so if *both* settings have the flag we + * look for. So that is symetric behavior and good. + * + * But for the purpose of diff(), we do a asymmetric comparison because and + * we want to skip testing the property if setting @a alone indicates to do + * so. + * + * We need to double-check whether the property should be ignored by + * looking at @a alone. */ + if (_compare_property (sett_info, i, a, NULL, flags) == NM_TERNARY_DEFAULT) + continue; + } compared_any = TRUE; + property_info = &sett_info->property_infos[i]; + prop_spec = property_info->param_spec; + if (b) { - gboolean different; + if (compare_result == NM_TERNARY_FALSE) { + if (prop_spec) { + gboolean a_is_default, b_is_default; + GValue value = G_VALUE_INIT; - different = !NM_SETTING_GET_CLASS (a)->compare_property (a, b, prop_spec, flags); - if (different) { - gboolean a_is_default, b_is_default; - GValue value = G_VALUE_INIT; + g_value_init (&value, prop_spec->value_type); + g_object_get_property (G_OBJECT (a), prop_spec->name, &value); + a_is_default = g_param_value_defaults (prop_spec, &value); - g_value_init (&value, prop_spec->value_type); - g_object_get_property (G_OBJECT (a), prop_spec->name, &value); - a_is_default = g_param_value_defaults (prop_spec, &value); + g_value_reset (&value); + g_object_get_property (G_OBJECT (b), prop_spec->name, &value); + b_is_default = g_param_value_defaults (prop_spec, &value); - g_value_reset (&value); - g_object_get_property (G_OBJECT (b), prop_spec->name, &value); - b_is_default = g_param_value_defaults (prop_spec, &value); - - g_value_unset (&value); - if ((flags & NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT) == 0) { - if (!a_is_default) - r |= a_result; - if (!b_is_default) - r |= b_result; - } else { + g_value_unset (&value); + if (!NM_FLAGS_HAS (flags, NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT)) { + if (!a_is_default) + r |= a_result; + if (!b_is_default) + r |= b_result; + } else { + r |= a_result | b_result; + if (a_is_default) + r |= a_result_default; + if (b_is_default) + r |= b_result_default; + } + } else r |= a_result | b_result; - if (a_is_default) - r |= a_result_default; - if (b_is_default) - r |= b_result_default; - } } } else if ((flags & (NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT)) == 0) r = a_result; /* only in A */ else { - GValue value = G_VALUE_INIT; + if (prop_spec) { + GValue value = G_VALUE_INIT; - g_value_init (&value, prop_spec->value_type); - g_object_get_property (G_OBJECT (a), prop_spec->name, &value); - if (!g_param_value_defaults (prop_spec, &value)) + g_value_init (&value, prop_spec->value_type); + g_object_get_property (G_OBJECT (a), prop_spec->name, &value); + if (!g_param_value_defaults (prop_spec, &value)) + r |= a_result; + else if (flags & NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT) + r |= a_result | a_result_default; + + g_value_unset (&value); + } else r |= a_result; - else if (flags & NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT) - r |= a_result | a_result_default; - - g_value_unset (&value); } if (r != NM_SETTING_DIFF_RESULT_UNKNOWN) { diff_found = TRUE; - _setting_diff_add_result (*results, prop_spec->name, r); + _setting_diff_add_result (*results, property_info->name, r); } } } diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 3576bdc663..326b533f12 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -169,6 +169,7 @@ typedef gboolean (*NMSettingClearSecretsWithFlagsFn) (NMSetting *setting, gpointer user_data); struct _NMMetaSettingInfo; +struct _NMSettInfoSetting; typedef struct { GObjectClass parent; @@ -204,11 +205,18 @@ typedef struct { NMSettingClearSecretsWithFlagsFn func, gpointer user_data); - /* Returns TRUE if the given property contains the same value in both settings */ - gboolean (*compare_property) (NMSetting *setting, - NMSetting *other, - const GParamSpec *prop_spec, - NMSettingCompareFlags flags); + /* compare_property() returns a ternary, where DEFAULT means that the property should not + * be compared due to the compare @flags. A TRUE/FALSE result means that the property is + * equal/not-equal. + * + * @other may be %NULL, in which case the function only determines whether + * the setting should be compared (TRUE) or not (DEFAULT). */ + /*< private >*/ + NMTernary (*compare_property) (const struct _NMSettInfoSetting *sett_info, + guint property_idx, + NMSetting *setting, + NMSetting *other, + NMSettingCompareFlags flags); /*< private >*/ const struct _NMMetaSettingInfo *setting_info; From d61cfc1774b72c7a33feb30322e477a031b9d4f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 17:03:34 +0100 Subject: [PATCH 08/11] libnm: assert in _nm_setting_aggregate() that we handle secret-flags Curreently all aggregate types only care about secrets. The check for secets is done by checking for NM_SETTING_PARAM_SECRET flag. Assert that this check is suitable to identify a secret. --- libnm-core/nm-setting.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index bea47b3509..6749eed12e 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1779,14 +1779,19 @@ _nm_setting_aggregate (NMSetting *setting, sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { - GParamSpec *prop_spec = sett_info->property_infos[i].param_spec; + const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; + GParamSpec *prop_spec = property_info->param_spec; nm_auto_unset_gvalue GValue value = G_VALUE_INIT; NMSettingSecretFlags secret_flags; - if (!prop_spec) - continue; - if (!NM_FLAGS_HAS (prop_spec->flags, NM_SETTING_PARAM_SECRET)) + if ( !prop_spec + || !NM_FLAGS_HAS (prop_spec->flags, NM_SETTING_PARAM_SECRET)) { + nm_assert (!nm_setting_get_secret_flags (setting, property_info->name, NULL, NULL)); continue; + } + + /* for the moment, all aggregate types only care about secrets. */ + nm_assert (nm_setting_get_secret_flags (setting, property_info->name, NULL, NULL)); switch (type) { From 2e7aba2b4629826beab787968fbe6f51157e9b06 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 19:44:30 +0100 Subject: [PATCH 09/11] libnm/trivial: rename sett-info getters --- libnm-core/nm-core-internal.h | 6 +++--- libnm-core/nm-keyfile.c | 4 ++-- libnm-core/nm-setting.c | 36 +++++++++++++++++------------------ 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index ae43d27545..d3e3ad39a4 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -707,10 +707,10 @@ _nm_sett_info_property_info_get_sorted (const NMSettInfoSetting *sett_info, : &sett_info->property_infos[idx]; } -const NMSettInfoSetting *_nm_sett_info_setting_get (NMSettingClass *setting_class); +const NMSettInfoSetting *_nm_setting_class_get_sett_info (NMSettingClass *setting_class); -const NMSettInfoProperty *_nm_sett_info_property_get (NMSettingClass *setting_class, - const char *property_name); +const NMSettInfoProperty *_nm_setting_class_get_property_info (NMSettingClass *setting_class, + const char *property_name); /*****************************************************************************/ diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 86a80e1dc7..e20681300b 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2815,7 +2815,7 @@ _read_setting (KeyfileReaderInfo *info) info->setting = setting; - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { gs_free char **keys = NULL; @@ -3232,7 +3232,7 @@ nm_keyfile_write (NMConnection *connection, const NMSettInfoSetting *sett_info; NMSetting *setting = settings[i]; - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { guint k, n_keys; diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 6749eed12e..8f2a789974 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -509,7 +509,7 @@ _nm_setting_class_commit_full (NMSettingClass *setting_class, } const NMSettInfoSetting * -_nm_sett_info_setting_get (NMSettingClass *setting_class) +_nm_setting_class_get_sett_info (NMSettingClass *setting_class) { if ( NM_IS_SETTING_CLASS (setting_class) && setting_class->setting_info) { @@ -520,10 +520,10 @@ _nm_sett_info_setting_get (NMSettingClass *setting_class) } const NMSettInfoProperty * -_nm_sett_info_property_get (NMSettingClass *setting_class, - const char *property_name) +_nm_setting_class_get_property_info (NMSettingClass *setting_class, + const char *property_name) { - const NMSettInfoSetting *sett_info = _nm_sett_info_setting_get (setting_class); + const NMSettInfoSetting *sett_info = _nm_setting_class_get_sett_info (setting_class); const NMSettInfoProperty *property; gssize idx; @@ -731,7 +731,7 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnection *connection, NMConnectionS g_hash_table_lookup (priv->gendata->hash, gendata_keys[i])); } - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { const NMSettInfoProperty *property = &sett_info->property_infos[i]; GParamSpec *prop_spec = property->param_spec; @@ -856,7 +856,7 @@ _nm_setting_new_from_dbus (GType setting_type, } } - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { GHashTable *hash; @@ -1002,7 +1002,7 @@ nm_setting_get_dbus_property_type (NMSetting *setting, g_return_val_if_fail (NM_IS_SETTING (setting), NULL); g_return_val_if_fail (property_name != NULL, NULL); - property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), property_name); + property = _nm_setting_class_get_property_info (NM_SETTING_GET_CLASS (setting), property_name); g_return_val_if_fail (property != NULL, NULL); if (property->dbus_type) @@ -1021,7 +1021,7 @@ _nm_setting_get_property (NMSetting *setting, const char *property_name, GValue g_return_val_if_fail (property_name, FALSE); g_return_val_if_fail (value, FALSE); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { GVariant *variant; @@ -1085,7 +1085,7 @@ nm_setting_duplicate (NMSetting *setting) dup = g_object_new (G_OBJECT_TYPE (setting), NULL); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { GenData *gendata = _gendata_hash (setting, FALSE); @@ -1412,7 +1412,7 @@ nm_setting_compare (NMSetting *a, if (G_OBJECT_TYPE (a) != G_OBJECT_TYPE (b)) return FALSE; - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (a)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (a)); if (sett_info->detail.gendata_info) { GenData *a_gendata = _gendata_hash (a, FALSE); @@ -1519,7 +1519,7 @@ nm_setting_diff (NMSetting *a, results_created = TRUE; } - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (a)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (a)); if (sett_info->detail.gendata_info) { const char *key; @@ -1697,7 +1697,7 @@ nm_setting_enumerate_values (NMSetting *setting, g_return_if_fail (NM_IS_SETTING (setting)); g_return_if_fail (func != NULL); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); if (sett_info->detail.gendata_info) { const char *const*names; @@ -1777,7 +1777,7 @@ _nm_setting_aggregate (NMSetting *setting, if (NM_IS_SETTING_VPN (setting)) return _nm_setting_vpn_aggregate (NM_SETTING_VPN (setting), type, arg); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; GParamSpec *prop_spec = property_info->param_spec; @@ -1838,7 +1838,7 @@ _nm_setting_clear_secrets (NMSetting *setting) g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { GParamSpec *prop_spec = sett_info->property_infos[i].param_spec; @@ -1915,7 +1915,7 @@ _nm_setting_clear_secrets_with_flags (NMSetting *setting, g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); g_return_val_if_fail (func != NULL, FALSE); - sett_info = _nm_sett_info_setting_get (NM_SETTING_GET_CLASS (setting)); + sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { GParamSpec *prop_spec = sett_info->property_infos[i].param_spec; @@ -1967,7 +1967,7 @@ update_one_secret (NMSetting *setting, const char *key, GVariant *value, GError GParamSpec *prop_spec; GValue prop_value = { 0, }; - property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), key); + property = _nm_setting_class_get_property_info (NM_SETTING_GET_CLASS (setting), key); if (!property) { g_set_error_literal (error, NM_CONNECTION_ERROR, @@ -2074,7 +2074,7 @@ _nm_setting_property_is_regular_secret (NMSetting *setting, nm_assert (NM_IS_SETTING (setting)); nm_assert (secret_name); - property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), secret_name); + property = _nm_setting_class_get_property_info (NM_SETTING_GET_CLASS (setting), secret_name); return property && property->param_spec && NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET); @@ -2089,7 +2089,7 @@ _nm_setting_property_is_regular_secret_flags (NMSetting *setting, nm_assert (NM_IS_SETTING (setting)); nm_assert (secret_flags_name); - property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), secret_flags_name); + property = _nm_setting_class_get_property_info (NM_SETTING_GET_CLASS (setting), secret_flags_name); return property && property->param_spec && !NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET) From d2f0e16ccd9462dc2de67994557900ad58315755 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 19:50:14 +0100 Subject: [PATCH 10/11] libnm: add _nm_sett_info_setting_get_property_info() helper --- libnm-core/nm-core-internal.h | 12 ++++++++++-- libnm-core/nm-setting.c | 29 +++++++++++++++-------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index d3e3ad39a4..6af293a013 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -707,10 +707,18 @@ _nm_sett_info_property_info_get_sorted (const NMSettInfoSetting *sett_info, : &sett_info->property_infos[idx]; } +const NMSettInfoProperty *_nm_sett_info_setting_get_property_info (const NMSettInfoSetting *sett_info, + const char *property_name); + const NMSettInfoSetting *_nm_setting_class_get_sett_info (NMSettingClass *setting_class); -const NMSettInfoProperty *_nm_setting_class_get_property_info (NMSettingClass *setting_class, - const char *property_name); +static inline const NMSettInfoProperty * +_nm_setting_class_get_property_info (NMSettingClass *setting_class, + const char *property_name) +{ + return _nm_sett_info_setting_get_property_info (_nm_setting_class_get_sett_info (setting_class), + property_name); +} /*****************************************************************************/ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 8f2a789974..cfd95a2d3e 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -508,25 +508,15 @@ _nm_setting_class_commit_full (NMSettingClass *setting_class, setting_class); } -const NMSettInfoSetting * -_nm_setting_class_get_sett_info (NMSettingClass *setting_class) -{ - if ( NM_IS_SETTING_CLASS (setting_class) - && setting_class->setting_info) { - nm_assert (setting_class->setting_info->meta_type < G_N_ELEMENTS (_sett_info_settings)); - return &_sett_info_settings[setting_class->setting_info->meta_type]; - } - return NULL; -} - const NMSettInfoProperty * -_nm_setting_class_get_property_info (NMSettingClass *setting_class, - const char *property_name) +_nm_sett_info_setting_get_property_info (const NMSettInfoSetting *sett_info, + const char *property_name) { - const NMSettInfoSetting *sett_info = _nm_setting_class_get_sett_info (setting_class); const NMSettInfoProperty *property; gssize idx; + nm_assert (property_name); + if (!sett_info) return NULL; @@ -549,6 +539,17 @@ _nm_setting_class_get_property_info (NMSettingClass *setting_class, return property; } +const NMSettInfoSetting * +_nm_setting_class_get_sett_info (NMSettingClass *setting_class) +{ + if ( NM_IS_SETTING_CLASS (setting_class) + && setting_class->setting_info) { + nm_assert (setting_class->setting_info->meta_type < G_N_ELEMENTS (_sett_info_settings)); + return &_sett_info_settings[setting_class->setting_info->meta_type]; + } + return NULL; +} + /*****************************************************************************/ gboolean From 3846976eb68e107aa325c978ca1dd3ef5570a503 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 19:38:36 +0100 Subject: [PATCH 11/11] libnm,core: accept failure to _nm_setting_get_property() in _log_connection_get_property() _log_connection_get_property() is a hack, as it cannot meaningfully print complex properties. Also, it uses _nm_setting_get_property() which can only work with GObject base properties. Don't assert against _nm_setting_get_property() returning success. Eventually we should replace _nm_setting_get_property() by something better. But for the moment, it's fine to being unable to print a property value. --- libnm-core/nm-setting.c | 9 +++++---- src/nm-core-utils.c | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index cfd95a2d3e..0feaa87fb7 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1016,7 +1016,7 @@ gboolean _nm_setting_get_property (NMSetting *setting, const char *property_name, GValue *value) { const NMSettInfoSetting *sett_info; - GParamSpec *prop_spec; + const NMSettInfoProperty *property_info; g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); g_return_val_if_fail (property_name, FALSE); @@ -1040,13 +1040,14 @@ _nm_setting_get_property (NMSetting *setting, const char *property_name, GValue return TRUE; } - prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), property_name); - if (!prop_spec) { + property_info = _nm_sett_info_setting_get_property_info (sett_info, property_name); + if ( !property_info + || !property_info->param_spec) { g_value_unset (value); return FALSE; } - g_value_init (value, prop_spec->value_type); + g_value_init (value, property_info->param_spec->value_type); g_object_get_property (G_OBJECT (setting), property_name, value); return TRUE; } diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 89f8f329f8..3047ebc846 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2033,7 +2033,7 @@ _log_connection_get_property (NMSetting *setting, const char *name) return g_strdup ("****"); if (!_nm_setting_get_property (setting, name, &val)) - g_return_val_if_reached (FALSE); + return g_strdup (""); if (G_VALUE_HOLDS_STRING (&val)) { const char *val_s;