libnm-util: let nm_setting_diff() be symetric not to return properties that are set to default

Previously, nm_setting_diff() (and thus nm_connection_diff()), returned
only properties that are different AND not set to the default value.
However, if the opposite setting 'B' was missing, it would always
include all properties from 'A', even the default ones.

This behaviour was asymetric. Add two new compare flags
@NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT and
@NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT to control the
behaviour of whether to include default properties.

Signed-off-by: Thomas Haller <thaller@redhat.com>

4 files changed, 144 insertions, 22 deletions

diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index ede4635a38..f103b753fc 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -1173,6 +1173,8 @@ nm_setting_diff (NMSetting *a,
 	guint i;
 	NMSettingDiffResult a_result = NM_SETTING_DIFF_RESULT_IN_A;
 	NMSettingDiffResult b_result = NM_SETTING_DIFF_RESULT_IN_B;
+	NMSettingDiffResult a_result_default = NM_SETTING_DIFF_RESULT_IN_A_DEFAULT;
+	NMSettingDiffResult b_result_default = NM_SETTING_DIFF_RESULT_IN_B_DEFAULT;
 	gboolean results_created = FALSE;
 
 	g_return_val_if_fail (results != NULL, FALSE);
@@ -1182,6 +1184,12 @@ nm_setting_diff (NMSetting *a,
 		g_return_val_if_fail (G_OBJECT_TYPE (a) == G_OBJECT_TYPE (b), FALSE);
 	}
 
+	if ((flags & (NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT)) ==
+	             (NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT)) {
+		/* conflicting flags: default to WITH_DEFAULT (clearing NO_DEFAULT). */
+		flags &= ~NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT;
+	}
+
 	/* If the caller is calling this function in a pattern like this to get
 	 * complete diffs:
 	 *
@@ -1194,6 +1202,8 @@ nm_setting_diff (NMSetting *a,
 	if (invert_results) {
 		a_result = NM_SETTING_DIFF_RESULT_IN_B;
 		b_result = NM_SETTING_DIFF_RESULT_IN_A;
+		a_result_default = NM_SETTING_DIFF_RESULT_IN_B_DEFAULT;
+		b_result_default = NM_SETTING_DIFF_RESULT_IN_A_DEFAULT;
 	}
 
 	if (*results == NULL) {
@@ -1206,8 +1216,7 @@ nm_setting_diff (NMSetting *a,
 
 	for (i = 0; i < n_property_specs; i++) {
 		GParamSpec *prop_spec = property_specs[i];
-		NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN, tmp;
-		gboolean different = TRUE;
+		NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN;
 
 		/* Handle compare flags */
 		if (!should_compare_prop (a, prop_spec->name, flags, prop_spec->flags))
@@ -1216,28 +1225,58 @@ nm_setting_diff (NMSetting *a,
 			continue;
 
 		if (b) {
+			gboolean different;
+
 			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);
-				if (!g_param_value_defaults (prop_spec, &value))
-					r |= a_result;
+				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);
-				if (!g_param_value_defaults (prop_spec, &value))
-					r |= b_result;
+				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 {
+					r |= a_result | b_result;
+					if (a_is_default)
+						r |= a_result_default;
+					if (b_is_default)
+						r |= b_result_default;
+				}
 			}
-		} else
+		} 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;
+
+			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);
+		}
+
+		if (r != NM_SETTING_DIFF_RESULT_UNKNOWN) {
+			void *p;
 
-		if (different) {
-			tmp = GPOINTER_TO_UINT (g_hash_table_lookup (*results, prop_spec->name));
-			g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (tmp | r));
+			if (g_hash_table_lookup_extended (*results, prop_spec->name, NULL, &p)) {
+				if ((r & GPOINTER_TO_UINT (p)) != r)
+					g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (r | GPOINTER_TO_UINT (p)));
+			} else
+				g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (r));
 		}
 	}
 	g_free (property_specs);
diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h
index bf8dd23362..8e179f2a4f 100644
--- a/libnm-core/nm-setting.h
+++ b/libnm-core/nm-setting.h
@@ -125,6 +125,20 @@ typedef enum { /*< flags >*/
  * @NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS: ignore secrets for which
  * the secret's flags indicate the secret should not be saved to persistent
  * storage (ie, the secret's flag includes @NM_SETTING_SECRET_FLAG_NOT_SAVED)
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT: if this flag is set,
+ * nm_setting_diff() and nm_connection_diff() will also include properties that
+ * are set to their default value. See also @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT.
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT: if this flag is set,
+ * nm_setting_diff() and nm_connection_diff() will not include properties that
+ * are set to their default value. This is the opposite of
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT. If both flags are set together,
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT wins. If both flags are unset,
+ * this means to exclude default properties if there is a setting to compare,
+ * but include all properties, if the setting 'b' is missing. This is the legacy
+ * behaviour of libnm-util, where nm_setting_diff() behaved differently depending
+ * on whether the setting 'b' was available. If @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT
+ * is set, nm_setting_diff() will also set the flags @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT
+ * and @NM_SETTING_DIFF_RESULT_IN_B_DEFAULT, if the values are default values.
  *
  * These flags modify the comparison behavior when comparing two settings or
  * two connections.
@@ -136,7 +150,9 @@ typedef enum {
 	NM_SETTING_COMPARE_FLAG_IGNORE_ID = 0x00000002,
 	NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS = 0x00000004,
 	NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS = 0x00000008,
-	NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS = 0x00000010
+	NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS = 0x00000010,
+	NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT = 0x00000020,
+	NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT = 0x00000040,
 
 	/* 0x80000000 is used for a private flag */
 } NMSettingCompareFlags;
@@ -247,6 +263,10 @@ gboolean    nm_setting_compare       (NMSetting *a,
  * @NM_SETTING_DIFF_RESULT_UNKNOWN: unknown result
  * @NM_SETTING_DIFF_RESULT_IN_A: the property is present in setting A
  * @NM_SETTING_DIFF_RESULT_IN_B: the property is present in setting B
+ * @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT: the property is present in
+ * setting A but is set to the default value. This flag is only set,
+ * if you specify @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT.
+ * @NM_SETTING_DIFF_RESULT_IN_B_DEFAULT: analog to @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT.
  *
  * These values indicate the result of a setting difference operation.
  **/
@@ -254,6 +274,8 @@ typedef enum {
 	NM_SETTING_DIFF_RESULT_UNKNOWN = 0x00000000,
 	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,
 } NMSettingDiffResult;
 
 gboolean    nm_setting_diff          (NMSetting *a,
diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c
index 0ca28dba16..160858d933 100644
--- a/libnm-util/nm-setting.c
+++ b/libnm-util/nm-setting.c
@@ -713,6 +713,8 @@ nm_setting_diff (NMSetting *a,
 	guint i;
 	NMSettingDiffResult a_result = NM_SETTING_DIFF_RESULT_IN_A;
 	NMSettingDiffResult b_result = NM_SETTING_DIFF_RESULT_IN_B;
+	NMSettingDiffResult a_result_default = NM_SETTING_DIFF_RESULT_IN_A_DEFAULT;
+	NMSettingDiffResult b_result_default = NM_SETTING_DIFF_RESULT_IN_B_DEFAULT;
 	gboolean results_created = FALSE;
 
 	g_return_val_if_fail (results != NULL, FALSE);
@@ -722,6 +724,12 @@ nm_setting_diff (NMSetting *a,
 		g_return_val_if_fail (G_OBJECT_TYPE (a) == G_OBJECT_TYPE (b), FALSE);
 	}
 
+	if ((flags & (NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT)) ==
+	             (NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT)) {
+		/* conflicting flags: default to WITH_DEFAULT (clearing NO_DEFAULT). */
+		flags &= ~NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT;
+	}
+
 	/* If the caller is calling this function in a pattern like this to get
 	 * complete diffs:
 	 *
@@ -734,6 +742,8 @@ nm_setting_diff (NMSetting *a,
 	if (invert_results) {
 		a_result = NM_SETTING_DIFF_RESULT_IN_B;
 		b_result = NM_SETTING_DIFF_RESULT_IN_A;
+		a_result_default = NM_SETTING_DIFF_RESULT_IN_B_DEFAULT;
+		b_result_default = NM_SETTING_DIFF_RESULT_IN_A_DEFAULT;
 	}
 
 	if (*results == NULL) {
@@ -746,8 +756,7 @@ nm_setting_diff (NMSetting *a,
 
 	for (i = 0; i < n_property_specs; i++) {
 		GParamSpec *prop_spec = property_specs[i];
-		NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN, tmp;
-		gboolean different = TRUE;
+		NMSettingDiffResult r = NM_SETTING_DIFF_RESULT_UNKNOWN;
 
 		/* Handle compare flags */
 		if (!should_compare_prop (a, prop_spec->name, flags, prop_spec->flags))
@@ -756,28 +765,58 @@ nm_setting_diff (NMSetting *a,
 			continue;
 
 		if (b) {
+			gboolean different;
+
 			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);
-				if (!g_param_value_defaults (prop_spec, &value))
-					r |= a_result;
+				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);
-				if (!g_param_value_defaults (prop_spec, &value))
-					r |= b_result;
+				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 {
+					r |= a_result | b_result;
+					if (a_is_default)
+						r |= a_result_default;
+					if (b_is_default)
+						r |= b_result_default;
+				}
 			}
-		} else
+		} 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;
+
+			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);
+		}
+
+		if (r != NM_SETTING_DIFF_RESULT_UNKNOWN) {
+			void *p;
 
-		if (different) {
-			tmp = GPOINTER_TO_UINT (g_hash_table_lookup (*results, prop_spec->name));
-			g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (tmp | r));
+			if (g_hash_table_lookup_extended (*results, prop_spec->name, NULL, &p)) {
+				if ((r & GPOINTER_TO_UINT (p)) != r)
+					g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (r | GPOINTER_TO_UINT (p)));
+			} else
+				g_hash_table_insert (*results, g_strdup (prop_spec->name), GUINT_TO_POINTER (r));
 		}
 	}
 	g_free (property_specs);
diff --git a/libnm-util/nm-setting.h b/libnm-util/nm-setting.h
index b58137a6e7..28c6975a8d 100644
--- a/libnm-util/nm-setting.h
+++ b/libnm-util/nm-setting.h
@@ -124,6 +124,20 @@ typedef enum {
  * @NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS: ignore secrets for which
  * the secret's flags indicate the secret should not be saved to persistent
  * storage (ie, the secret's flag includes @NM_SETTING_SECRET_FLAG_NOT_SAVED)
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT: if this flag is set,
+ * nm_setting_diff() and nm_connection_diff() will also include properties that
+ * are set to their default value. See also @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT.
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT: if this flag is set,
+ * nm_setting_diff() and nm_connection_diff() will not include properties that
+ * are set to their default value. This is the opposite of
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT. If both flags are set together,
+ * @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT wins. If both flags are unset,
+ * this means to exclude default properties if there is a setting to compare,
+ * but include all properties, if the setting 'b' is missing. This is the legacy
+ * behaviour of libnm-util, where nm_setting_diff() behaved differently depending
+ * on whether the setting 'b' was available. If @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT
+ * is set, nm_setting_diff() will also set the flags @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT
+ * and @NM_SETTING_DIFF_RESULT_IN_B_DEFAULT, if the values are default values.
  *
  * These flags modify the comparison behavior when comparing two settings or
  * two connections.
@@ -135,7 +149,9 @@ typedef enum {
 	NM_SETTING_COMPARE_FLAG_IGNORE_ID = 0x00000002,
 	NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS = 0x00000004,
 	NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS = 0x00000008,
-	NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS = 0x00000010
+	NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS = 0x00000010,
+	NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT = 0x00000020,
+	NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT = 0x00000040,
 
 	/* 0x80000000 is used for a private flag */
 } NMSettingCompareFlags;
@@ -267,6 +283,10 @@ gboolean    nm_setting_compare       (NMSetting *a,
  * @NM_SETTING_DIFF_RESULT_UNKNOWN: unknown result
  * @NM_SETTING_DIFF_RESULT_IN_A: the property is present in setting A
  * @NM_SETTING_DIFF_RESULT_IN_B: the property is present in setting B
+ * @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT: the property is present in
+ * setting A but is set to the default value. This flag is only set,
+ * if you specify @NM_SETTING_COMPARE_FLAG_DIFF_RESULT_WITH_DEFAULT.
+ * @NM_SETTING_DIFF_RESULT_IN_B_DEFAULT: analog to @NM_SETTING_DIFF_RESULT_IN_A_DEFAULT.
  *
  * These values indicate the result of a setting difference operation.
  **/
@@ -274,6 +294,8 @@ typedef enum {
 	NM_SETTING_DIFF_RESULT_UNKNOWN = 0x00000000,
 	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,
 } NMSettingDiffResult;
 
 gboolean    nm_setting_diff          (NMSetting *a,