libnm: remove unused "nm-property-compare.c"

"nm-property-compare.c" only contains nm_property_compare(), which is
broken.

It tries to compare string dictionaries as equal regardless of the
order of elements. It gets it wrong, for dictionaries with duplicate
keys. Which means, it can only be used with trusted variants that are
known to not contain duplicates. Which is quite a non-starter.

Also, the idea of a compare function for GVariant that ignores the order
of dictionary elements seems wrong. Even if for a certain application
the order does not matter, it still depends what the upper layer makes
of duplicate keys (will they bail out, or take the first/last occurrence
of a duplicate key?). nm_property_compare() doesn't have the knowledge
how upper layer handles it, and it's not obvious what's the right
choice. For example, if you use g_variant_lookup(), the first occurrence
is preferred. If you iterate over the children, possibly later
occurrences overwrite earlier ones.

It's ill defined, and maybe shouldn't be done. What should instead
happen, is that upper layers normalize (sort, uniquify) the keys, so
that we can do a full comparison. For that we have nm_g_variant_cmp().

Drop the now unused code. The core of the function still exists as
nm_g_variant_cmp().

9 files changed, 1 insertions, 395 deletions

diff --git a/.gitignore b/.gitignore
index e2a2f5607a..42e24fe416 100644
--- a/.gitignore
+++ b/.gitignore
@@ -141,7 +141,6 @@ test-*.trs
 /src/libnm-core-public/nm-version-macros.h
 /src/libnm-core-public/nm-dbus-types.xml
 /src/libnm-core-public/nm-vpn-dbus-types.xml
-/src/libnm-core-impl/tests/test-compare
 /src/libnm-core-impl/tests/test-crypto
 /src/libnm-core-impl/tests/test-settings-defaults
 /src/libnm-core-impl/tests/test-general
@@ -439,6 +438,7 @@ test-*.trs
 /src/initrd/tests/test-dt-reader
 /src/initrd/tests/test-ibft-reader
 /src/libnm-client-impl/nm-settings-docs-gir.xml
+/src/libnm-core-impl/tests/test-compare
 /src/ndisc/tests/test-ndisc-fake
 /src/ndisc/tests/test-ndisc-linux
 /src/nm-daemon-helper/nm-daemon-helper
diff --git a/Makefile.am b/Makefile.am
index f83cf25bd7..4a4ea61628 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1320,7 +1320,6 @@ src_libnm_core_public_mkenums_h = \
 src_libnm_core_impl_lib_h_priv = \
 	src/libnm-core-impl/nm-connection-private.h \
 	src/libnm-core-impl/nm-default-libnm-core.h \
-	src/libnm-core-impl/nm-property-compare.h \
 	src/libnm-core-impl/nm-setting-private.h \
 	src/libnm-core-impl/nm-team-utils.h \
 	src/libnm-core-impl/nm-utils-private.h \
@@ -1396,7 +1395,6 @@ src_libnm_core_impl_lib_c_real = \
 	src/libnm-core-impl/nm-keyfile-utils.c \
 	src/libnm-core-impl/nm-keyfile.c \
 	src/libnm-core-impl/nm-meta-setting-base-impl.c \
-	src/libnm-core-impl/nm-property-compare.c \
 	src/libnm-core-impl/nm-setting.c \
 	src/libnm-core-impl/nm-simple-connection.c \
 	src/libnm-core-impl/nm-team-utils.c \
@@ -1602,7 +1600,6 @@ EXTRA_DIST += \
 ###############################################################################
 
 check_programs += \
-	src/libnm-core-impl/tests/test-compare \
 	src/libnm-core-impl/tests/test-crypto \
 	src/libnm-core-impl/tests/test-general \
 	src/libnm-core-impl/tests/test-keyfile \
@@ -1631,7 +1628,6 @@ src_libnm_core_impl_tests_cppflags = \
 	$(SANITIZER_EXEC_CFLAGS) \
 	$(NULL)
 
-src_libnm_core_impl_tests_test_compare_CPPFLAGS = $(src_libnm_core_impl_tests_cppflags)
 src_libnm_core_impl_tests_test_crypto_CPPFLAGS = $(src_libnm_core_impl_tests_cppflags)
 src_libnm_core_impl_tests_test_general_CPPFLAGS = $(src_libnm_core_impl_tests_cppflags)
 src_libnm_core_impl_tests_test_keyfile_CPPFLAGS = $(src_libnm_core_impl_tests_cppflags)
@@ -1668,7 +1664,6 @@ src_libnm_core_impl_tests_ldflags = \
 	$(SANITIZER_EXEC_LDFLAGS) \
 	$(NULL)
 
-src_libnm_core_impl_tests_test_compare_LDADD = $(src_libnm_core_impl_tests_ldadd)
 src_libnm_core_impl_tests_test_crypto_LDADD = $(src_libnm_core_impl_tests_ldadd)
 src_libnm_core_impl_tests_test_general_LDADD = $(src_libnm_core_impl_tests_ldadd)
 src_libnm_core_impl_tests_test_keyfile_LDADD = $(src_libnm_core_impl_tests_ldadd)
@@ -1676,7 +1671,6 @@ src_libnm_core_impl_tests_test_secrets_LDADD = $(src_libnm_core_impl_tests_ldadd
 src_libnm_core_impl_tests_test_setting_LDADD = $(src_libnm_core_impl_tests_ldadd)
 src_libnm_core_impl_tests_test_settings_defaults_LDADD = $(src_libnm_core_impl_tests_ldadd)
 
-src_libnm_core_impl_tests_test_compare_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
 src_libnm_core_impl_tests_test_crypto_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
 src_libnm_core_impl_tests_test_general_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
 src_libnm_core_impl_tests_test_keyfile_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
@@ -1684,7 +1678,6 @@ src_libnm_core_impl_tests_test_secrets_LDFLAGS = $(src_libnm_core_impl_tests_ldf
 src_libnm_core_impl_tests_test_setting_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
 src_libnm_core_impl_tests_test_settings_defaults_LDFLAGS = $(src_libnm_core_impl_tests_ldflags)
 
-$(src_libnm_core_impl_tests_test_compare_OBJECTS): $(src_libnm_core_public_mkenums_h)
 $(src_libnm_core_impl_tests_test_crypto_OBJECTS): $(src_libnm_core_public_mkenums_h)
 $(src_libnm_core_impl_tests_test_general_OBJECTS): $(src_libnm_core_public_mkenums_h)
 $(src_libnm_core_impl_tests_test_keyfile_OBJECTS): $(src_libnm_core_public_mkenums_h)
diff --git a/docs/libnm/Makefile.am b/docs/libnm/Makefile.am
index 7a10848f0a..87ab81cc9c 100644
--- a/docs/libnm/Makefile.am
+++ b/docs/libnm/Makefile.am
@@ -52,7 +52,6 @@ IGNORE_HFILES= \
 	\
 	nm-connection-private.h \
 	nm-default-libnm-core.h \
-	nm-property-compare.h \
 	nm-setting-private.h \
 	nm-team-utils.h \
 	nm-utils-private.h \
diff --git a/docs/libnm/meson.build b/docs/libnm/meson.build
index fcc4611dcc..faac586d3c 100644
--- a/docs/libnm/meson.build
+++ b/docs/libnm/meson.build
@@ -15,7 +15,6 @@ private_headers = [
 
   'nm-connection-private.h',
   'nm-default-libnm-core.h',
-  'nm-property-compare.h',
   'nm-setting-private.h',
   'nm-team-utils.h',
   'nm-utils-private.h',
diff --git a/src/libnm-core-impl/meson.build b/src/libnm-core-impl/meson.build
index d6cf950557..e1f11f323f 100644
--- a/src/libnm-core-impl/meson.build
+++ b/src/libnm-core-impl/meson.build
@@ -68,7 +68,6 @@ libnm_core_impl_sources = files(
   'nm-keyfile-utils.c',
   'nm-keyfile.c',
   'nm-meta-setting-base-impl.c',
-  'nm-property-compare.c',
   'nm-setting.c',
   'nm-simple-connection.c',
   'nm-team-utils.c',
diff --git a/src/libnm-core-impl/nm-property-compare.c b/src/libnm-core-impl/nm-property-compare.c
deleted file mode 100644
index 6273e90bc3..0000000000
--- a/src/libnm-core-impl/nm-property-compare.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2007 - 2014 Red Hat, Inc.
- * Copyright (C) 2007 - 2008 Novell, Inc.
- */
-
-#include "libnm-core-impl/nm-default-libnm-core.h"
-
-#include "nm-property-compare.h"
-
-#include <netinet/in.h>
-
-static int
-_nm_property_compare_collection(GVariant *value1, GVariant *value2)
-{
-    GVariant *child1, *child2;
-    int       i, len1, len2;
-    int       ret;
-
-    len1 = g_variant_n_children(value1);
-    len2 = g_variant_n_children(value2);
-
-    if (len1 != len2)
-        return len1 < len2 ? -1 : len1 > len2;
-
-    for (i = 0; i < len1; i++) {
-        child1 = g_variant_get_child_value(value1, i);
-        child2 = g_variant_get_child_value(value2, i);
-
-        ret = nm_property_compare(child1, child2);
-        g_variant_unref(child1);
-        g_variant_unref(child2);
-
-        if (ret)
-            return ret;
-    }
-
-    return 0;
-}
-
-static int
-_nm_property_compare_vardict(GVariant *value1, GVariant *value2)
-{
-    GVariantIter iter;
-    int          len1, len2;
-    const char  *key;
-    GVariant    *val1, *val2;
-
-    len1 = g_variant_n_children(value1);
-    len2 = g_variant_n_children(value2);
-
-    if (len1 != len2)
-        return len1 < len2 ? -1 : 1;
-
-    g_variant_iter_init(&iter, value1);
-    while (g_variant_iter_next(&iter, "{&sv}", &key, &val1)) {
-        if (!g_variant_lookup(value2, key, "v", &val2)) {
-            g_variant_unref(val1);
-            return -1;
-        }
-        if (!g_variant_equal(val1, val2)) {
-            g_variant_unref(val1);
-            g_variant_unref(val2);
-            return -1;
-        }
-        g_variant_unref(val1);
-        g_variant_unref(val2);
-    }
-
-    return 0;
-}
-
-static int
-_nm_property_compare_strdict(GVariant *value1, GVariant *value2)
-{
-    GVariantIter iter;
-    int          len1, len2;
-    const char  *key, *val1, *val2;
-    int          ret;
-
-    len1 = g_variant_n_children(value1);
-    len2 = g_variant_n_children(value2);
-
-    if (len1 != len2)
-        return len1 < len2 ? -1 : len1 > len2;
-
-    g_variant_iter_init(&iter, value1);
-    while (g_variant_iter_next(&iter, "{&s&s}", &key, &val1)) {
-        if (!g_variant_lookup(value2, key, "&s", &val2))
-            return -1;
-
-        ret = strcmp(val1, val2);
-        if (ret)
-            return ret;
-    }
-
-    return 0;
-}
-
-int
-nm_property_compare(GVariant *value1, GVariant *value2)
-{
-    const GVariantType *type1;
-    const GVariantType *type2;
-    int                 ret;
-
-    if (value1 == value2)
-        return 0;
-    if (!value1)
-        return 1;
-    if (!value2)
-        return -1;
-
-    type1 = g_variant_get_type(value1);
-    type2 = g_variant_get_type(value2);
-
-    if (!g_variant_type_equal(type1, type2))
-        return type1 < type2 ? -1 : type1 > type2;
-
-    if (g_variant_type_is_basic(type1))
-        ret = g_variant_compare(value1, value2);
-    else if (g_variant_is_of_type(value1, G_VARIANT_TYPE("a{ss}")))
-        ret = _nm_property_compare_strdict(value1, value2);
-    else if (g_variant_is_of_type(value1, G_VARIANT_TYPE("a{sv}")))
-        ret = _nm_property_compare_vardict(value1, value2);
-    else if (g_variant_type_is_array(type1))
-        ret = _nm_property_compare_collection(value1, value2);
-    else if (g_variant_type_is_tuple(type1))
-        ret = _nm_property_compare_collection(value1, value2);
-    else {
-        g_warning("Don't know how to compare variant type '%s'", (const char *) type1);
-        ret = value1 == value2;
-    }
-
-    return ret;
-}
diff --git a/src/libnm-core-impl/nm-property-compare.h b/src/libnm-core-impl/nm-property-compare.h
deleted file mode 100644
index 7444400d7c..0000000000
--- a/src/libnm-core-impl/nm-property-compare.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2007 - 2014 Red Hat, Inc.
- * Copyright (C) 2007 - 2008 Novell, Inc.
- */
-
-#ifndef __NM_PROPERTY_COMPARE_H__
-#define __NM_PROPERTY_COMPARE_H__
-
-#if !((NETWORKMANAGER_COMPILATION) & NM_NETWORKMANAGER_COMPILATION_WITH_LIBNM_CORE_PRIVATE)
-#error Cannot use this header.
-#endif
-
-int nm_property_compare(GVariant *value1, GVariant *value2);
-
-#endif /* __NM_PROPERTY_COMPARE_H__ */
diff --git a/src/libnm-core-impl/tests/meson.build b/src/libnm-core-impl/tests/meson.build
index 988c60db86..80b588274e 100644
--- a/src/libnm-core-impl/tests/meson.build
+++ b/src/libnm-core-impl/tests/meson.build
@@ -8,7 +8,6 @@ enum_sources = gnome.mkenums_simple(
 )
 
 test_units = [
-  'test-compare',
   'test-crypto',
   'test-general',
   'test-keyfile',
diff --git a/src/libnm-core-impl/tests/test-compare.c b/src/libnm-core-impl/tests/test-compare.c
deleted file mode 100644
index 77d2e17b68..0000000000
--- a/src/libnm-core-impl/tests/test-compare.c
+++ /dev/null
@@ -1,231 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2007 - 2014 Red Hat, Inc.
- * Copyright (C) 2007 - 2008 Novell, Inc.
- */
-
-#include "libnm-core-impl/nm-default-libnm-core.h"
-
-#include <arpa/inet.h>
-#include <netinet/in.h>
-
-#include "nm-property-compare.h"
-
-#include "libnm-glib-aux/nm-test-utils.h"
-
-static void
-compare_ints(void)
-{
-    GVariant *value1, *value2;
-
-    value1 = g_variant_new_int32(5);
-    value2 = g_variant_new_int32(5);
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_int32(10);
-    g_assert(nm_property_compare(value1, value2) < 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_int32(-1);
-    g_assert(nm_property_compare(value1, value2) > 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-static void
-compare_strings(void)
-{
-    GVariant   *value1, *value2;
-    const char *str1 = "hello";
-    const char *str2 = "world";
-
-    value1 = g_variant_new_string(str1);
-    value2 = g_variant_new_string(str1);
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_string(str2);
-    g_assert(nm_property_compare(value1, value2) < 0);
-
-    g_assert(nm_property_compare(value2, value1) > 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-static void
-compare_strv(void)
-{
-    GVariant         *value1, *value2;
-    const char *const strv1[] = {"foo", "bar", "baz", NULL};
-    const char *const strv2[] = {"foo", "bar", "bar", NULL};
-    const char *const strv3[] = {"foo", "bar", NULL};
-    const char *const strv4[] = {"foo", "bar", "baz", "bam", NULL};
-
-    value1 = g_variant_new_strv(strv1, -1);
-    value2 = g_variant_new_strv(strv1, -1);
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_strv(strv2, -1);
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_strv(strv3, -1);
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_strv(strv4, -1);
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-static void
-compare_arrays(void)
-{
-    GVariant *value1, *value2;
-    guint32   array[] = {0, 1, 2, 3, 4};
-
-    value1 = g_variant_new_fixed_array(G_VARIANT_TYPE_UINT32,
-                                       array,
-                                       G_N_ELEMENTS(array),
-                                       sizeof(guint32));
-    value2 = g_variant_new_fixed_array(G_VARIANT_TYPE_UINT32,
-                                       array,
-                                       G_N_ELEMENTS(array),
-                                       sizeof(guint32));
-
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new_fixed_array(G_VARIANT_TYPE_UINT32,
-                                       array + 1,
-                                       G_N_ELEMENTS(array) - 1,
-                                       sizeof(guint32));
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    array[0] = 7;
-    g_variant_unref(value2);
-    value2 = g_variant_new_fixed_array(G_VARIANT_TYPE_UINT32,
-                                       array,
-                                       G_N_ELEMENTS(array),
-                                       sizeof(guint32));
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-static void
-compare_str_hash(void)
-{
-    GVariant       *value1, *value2;
-    GVariantBuilder builder;
-
-    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{ss}"));
-    g_variant_builder_add(&builder, "{ss}", "key1", "hello");
-    g_variant_builder_add(&builder, "{ss}", "key2", "world");
-    g_variant_builder_add(&builder, "{ss}", "key3", "!");
-    value1 = g_variant_builder_end(&builder);
-
-    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{ss}"));
-    g_variant_builder_add(&builder, "{ss}", "key3", "!");
-    g_variant_builder_add(&builder, "{ss}", "key2", "world");
-    g_variant_builder_add(&builder, "{ss}", "key1", "hello");
-    value2 = g_variant_builder_end(&builder);
-
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{ss}"));
-    g_variant_builder_add(&builder, "{ss}", "key1", "hello");
-    g_variant_builder_add(&builder, "{ss}", "key3", "!");
-    value2 = g_variant_builder_end(&builder);
-
-    g_assert(nm_property_compare(value1, value2) != 0);
-    g_assert(nm_property_compare(value2, value1) != 0);
-
-    g_variant_unref(value2);
-    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{ss}"));
-    g_variant_builder_add(&builder, "{ss}", "key1", "hello");
-    g_variant_builder_add(&builder, "{ss}", "key2", "moon");
-    g_variant_builder_add(&builder, "{ss}", "key3", "!");
-    value2 = g_variant_builder_end(&builder);
-
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-static void
-compare_ip6_addresses(void)
-{
-    GVariant       *value1, *value2;
-    struct in6_addr addr1;
-    struct in6_addr addr2;
-    struct in6_addr addr3;
-    guint32         prefix1 = 64;
-    guint32         prefix2 = 64;
-    guint32         prefix3 = 0;
-
-    inet_pton(AF_INET6, "1:2:3:4:5:6:7:8", &addr1);
-    inet_pton(AF_INET6, "ffff:2:3:4:5:6:7:8", &addr2);
-    inet_pton(AF_INET6, "::", &addr3);
-
-    value1 = g_variant_new(
-        "(@ayu@ay)",
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr1.s6_addr, 16, 1),
-        prefix1,
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr3.s6_addr, 16, 1));
-
-    value2 = g_variant_new(
-        "(@ayu@ay)",
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr1.s6_addr, 16, 1),
-        prefix1,
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr3.s6_addr, 16, 1));
-
-    g_assert(nm_property_compare(value1, value2) == 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new(
-        "(@ayu@ay)",
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr2.s6_addr, 16, 1),
-        prefix2,
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr3.s6_addr, 16, 1));
-
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value2);
-    value2 = g_variant_new(
-        "(@ayu@ay)",
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr3.s6_addr, 16, 1),
-        prefix3,
-        g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (guint8 *) addr3.s6_addr, 16, 1));
-
-    g_assert(nm_property_compare(value1, value2) != 0);
-
-    g_variant_unref(value1);
-    g_variant_unref(value2);
-}
-
-NMTST_DEFINE();
-
-int
-main(int argc, char *argv[])
-{
-    nmtst_init(&argc, &argv, TRUE);
-
-    g_test_add_func("/libnm/compare/ints", compare_ints);
-    g_test_add_func("/libnm/compare/strings", compare_strings);
-    g_test_add_func("/libnm/compare/strv", compare_strv);
-    g_test_add_func("/libnm/compare/arrays", compare_arrays);
-    g_test_add_func("/libnm/compare/str_hash", compare_str_hash);
-    g_test_add_func("/libnm/compare/ip6_addresses", compare_ip6_addresses);
-
-    return g_test_run();
-}