From f38cbce65319b7d5fa3c004d840dc36fead40f75 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 19:53:53 +0200 Subject: [PATCH 1/4] shared: add nm_utils_gbytes_equal_mem() util --- shared/nm-utils/nm-shared-utils.c | 30 ++++++++++++++++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 4 ++++ 2 files changed, 34 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 25661eaa7c..b8f95f3d62 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -117,6 +117,36 @@ nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) /*****************************************************************************/ +/** + * nm_utils_gbytes_equals: + * @bytes: (allow-none): a #GBytes array to compare. Note that + * %NULL is treated like an #GBytes array of length zero. + * @mem_data: the data pointer with @mem_len bytes + * @mem_len: the length of the data pointer + * + * Returns: %TRUE if @bytes contains the same data as @mem_data. As a + * special case, a %NULL @bytes is treated like an empty array. + */ +gboolean +nm_utils_gbytes_equal_mem (GBytes *bytes, + gconstpointer mem_data, + gsize mem_len) +{ + gconstpointer p; + gsize l; + + if (!bytes) { + /* as a special case, let %NULL GBytes compare idential + * to an empty array. */ + return (mem_len == 0); + } + + p = g_bytes_get_data (bytes, &l); + return l == mem_len + && ( mem_len == 0 /* allow @mem_data to be %NULL */ + || memcmp (p, mem_data, mem_len) == 0); +} + GVariant * nm_utils_gbytes_to_variant_ay (GBytes *bytes) { diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index a85497e854..26875534d9 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -202,6 +202,10 @@ nm_utils_is_separator (const char c) /*****************************************************************************/ +gboolean nm_utils_gbytes_equal_mem (GBytes *bytes, + gconstpointer mem_data, + gsize mem_len); + GVariant *nm_utils_gbytes_to_variant_ay (GBytes *bytes); /*****************************************************************************/ From 1b448aeb307b6557bc8949d7847af3ec40da5682 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Aug 2018 20:49:43 +0200 Subject: [PATCH 2/4] all: use nm_utils_gbytes_equal_mem() --- libnm-core/tests/test-general.c | 11 ++++---- libnm-core/tests/test-keyfile.c | 9 ++---- libnm/nm-object.c | 10 ++----- src/devices/wifi/nm-wifi-ap.c | 12 +------- src/dhcp/tests/test-dhcp-dhclient.c | 13 ++------- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 3 +- .../plugins/keyfile/tests/test-keyfile.c | 28 ++++++------------- 7 files changed, 23 insertions(+), 63 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 31dd704189..0b2545a9d3 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -5345,13 +5345,12 @@ test_hexstr2bin (void) for (i = 0; i < G_N_ELEMENTS (items); i++) { b = nm_utils_hexstr2bin (items[i].str); - if (items[i].expected_len) { + if (items[i].expected_len) g_assert (b); - g_assert_cmpint (g_bytes_get_size (b), ==, items[i].expected_len); - g_assert (memcmp (g_bytes_get_data (b, NULL), items[i].expected, g_bytes_get_size (b)) == 0); - g_bytes_unref (b); - } else - g_assert (b == NULL); + else + g_assert (!b); + g_assert (nm_utils_gbytes_equal_mem (b, items[i].expected, items[i].expected_len)); + g_bytes_unref (b); } } diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 672c72b63d..5855aeacf9 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -127,10 +127,8 @@ _assert_gbytes (GBytes *bytes, gconstpointer data, gssize len) if (!len) g_assert (!bytes); - else { - g_assert_cmpint (g_bytes_get_size (bytes), ==, len); - g_assert (memcmp (g_bytes_get_data (bytes, NULL), data, len) == 0); - } + + g_assert (nm_utils_gbytes_equal_mem (bytes, data, len)); } static GKeyFile * @@ -344,8 +342,7 @@ _test_8021x_cert_check (NMConnection *con, } g_assert (blob); - g_assert_cmpint (g_bytes_get_size (blob), ==, val_len); - g_assert (!memcmp (g_bytes_get_data (blob, NULL), value, val_len)); + g_assert (nm_utils_gbytes_equal_mem (blob, value, val_len)); kval = g_key_file_get_string (keyfile, "802-1x", "ca-cert", NULL); g_assert (kval); diff --git a/libnm/nm-object.c b/libnm/nm-object.c index dbcdb86837..43bbba7f4c 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -844,16 +844,12 @@ demarshal_generic (NMObject *object, g_free (newval); } else if (pspec->value_type == G_TYPE_BYTES) { GBytes **param = (GBytes **)field; - gconstpointer val, old_val = NULL; - gsize length, old_length = 0; + gconstpointer val; + gsize length = 0; val = g_variant_get_fixed_array (value, &length, 1); - if (*param) - old_val = g_bytes_get_data (*param, &old_length); - different = old_length != length - || ( length > 0 - && memcmp (old_val, val, length) != 0); + different = !nm_utils_gbytes_equal_mem (*param, val, length); if (different) { if (*param) g_bytes_unref (*param); diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 1dbc054af4..e5573383cb 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -109,8 +109,6 @@ nm_wifi_ap_set_ssid_arr (NMWifiAP *ap, gsize ssid_len) { NMWifiAPPrivate *priv; - const guint8 *my_data; - gsize my_len; g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); @@ -119,15 +117,7 @@ nm_wifi_ap_set_ssid_arr (NMWifiAP *ap, priv = NM_WIFI_AP_GET_PRIVATE (ap); - if (priv->ssid) - my_data = g_bytes_get_data (priv->ssid, &my_len); - else { - my_data = NULL; - my_len = 0; - } - - if ( my_len == ssid_len - && memcmp (ssid, my_data, ssid_len) == 0) + if (nm_utils_gbytes_equal_mem (priv->ssid, ssid, ssid_len)) return FALSE; nm_clear_pointer (&priv->ssid, g_bytes_unref); diff --git a/src/dhcp/tests/test-dhcp-dhclient.c b/src/dhcp/tests/test-dhcp-dhclient.c index 031a92da99..6ea8a43469 100644 --- a/src/dhcp/tests/test-dhcp-dhclient.c +++ b/src/dhcp/tests/test-dhcp-dhclient.c @@ -677,15 +677,10 @@ test_one_duid (const char *escaped, const guint8 *unescaped, guint len) { GBytes *t; char *w; - gsize t_len; - gconstpointer t_arr; t = nm_dhcp_dhclient_unescape_duid (escaped); g_assert (t); - t_arr = g_bytes_get_data (t, &t_len); - g_assert (t_arr); - g_assert_cmpint (t_len, ==, len); - g_assert_cmpint (memcmp (t_arr, unescaped, len), ==, 0); + g_assert (nm_utils_gbytes_equal_mem (t, unescaped, len)); g_bytes_unref (t); t = g_bytes_new_static (unescaped, len); @@ -735,15 +730,11 @@ test_read_duid_from_leasefile (void) 0x13, 0x60, 0x67, 0x20, 0xec, 0x4c, 0x70 }; gs_unref_bytes GBytes *duid = NULL; GError *error = NULL; - gconstpointer duid_arr; - gsize duid_len; duid = nm_dhcp_dhclient_read_duid (TEST_DIR"/test-dhclient-duid.leases", &error); g_assert_no_error (error); g_assert (duid); - duid_arr = g_bytes_get_data (duid, &duid_len); - g_assert_cmpint (duid_len, ==, sizeof (expected)); - g_assert_cmpint (memcmp (duid_arr, expected, duid_len), ==, 0); + g_assert (nm_utils_gbytes_equal_mem (duid, expected, G_N_ELEMENTS (expected))); } static void diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 99e2beb748..ab31fbe099 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -3120,8 +3120,7 @@ test_read_wifi_wpa_psk_hex (void) ssid = nm_setting_wireless_get_ssid (s_wireless); g_assert (ssid); - g_assert_cmpint (g_bytes_get_size (ssid), ==, strlen (expected_ssid)); - g_assert (memcmp (g_bytes_get_data (ssid, NULL), expected_ssid, strlen (expected_ssid)) == 0); + g_assert (nm_utils_gbytes_equal_mem (ssid, expected_ssid, strlen (expected_ssid))); /* ===== WIRELESS SECURITY SETTING ===== */ diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 20e9b0237d..2ce53697b2 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -1018,8 +1018,6 @@ test_read_intlike_ssid (void) gs_free_error GError *error = NULL; gboolean success; GBytes *ssid; - const guint8 *ssid_data; - gsize ssid_len; const char *expected_ssid = "101"; connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID", &error); @@ -1035,10 +1033,8 @@ test_read_intlike_ssid (void) g_assert (s_wifi); ssid = nm_setting_wireless_get_ssid (s_wifi); - g_assert (ssid != NULL); - ssid_data = g_bytes_get_data (ssid, &ssid_len); - g_assert_cmpint (ssid_len, ==, strlen (expected_ssid)); - g_assert_cmpint (memcmp (ssid_data, expected_ssid, strlen (expected_ssid)), ==, 0); + g_assert (ssid); + g_assert (nm_utils_gbytes_equal_mem (ssid, expected_ssid, strlen (expected_ssid))); } static void @@ -1049,8 +1045,6 @@ test_read_intlike_ssid_2 (void) gs_free_error GError *error = NULL; gboolean success; GBytes *ssid; - const guint8 *ssid_data; - gsize ssid_len; const char *expected_ssid = "11;12;13;"; connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID_2", &error); @@ -1066,10 +1060,8 @@ test_read_intlike_ssid_2 (void) g_assert (s_wifi); ssid = nm_setting_wireless_get_ssid (s_wifi); - g_assert (ssid != NULL); - ssid_data = g_bytes_get_data (ssid, &ssid_len); - g_assert_cmpint (ssid_len, ==, strlen (expected_ssid)); - g_assert_cmpint (memcmp (ssid_data, expected_ssid, strlen (expected_ssid)), ==, 0); + g_assert (ssid); + g_assert (nm_utils_gbytes_equal_mem (ssid, expected_ssid, strlen (expected_ssid))); } static void @@ -1787,7 +1779,8 @@ test_write_wired_8021x_tls_connection_blob (void) const char *uuid; gboolean reread_same = FALSE; gs_free_error GError *error = NULL; - GBytes *password_raw = NULL; + GBytes *password_raw; + #define PASSWORD_RAW "password-raw\0test" connection = create_wired_tls_connection (NM_SETTING_802_1X_CK_SCHEME_BLOB); @@ -1846,8 +1839,7 @@ test_write_wired_8021x_tls_connection_blob (void) password_raw = nm_setting_802_1x_get_password_raw (s_8021x); g_assert (password_raw); - g_assert (g_bytes_get_size (password_raw) == NM_STRLEN (PASSWORD_RAW)); - g_assert (!memcmp (g_bytes_get_data (password_raw, NULL), PASSWORD_RAW, NM_STRLEN (PASSWORD_RAW))); + g_assert (nm_utils_gbytes_equal_mem (password_raw, PASSWORD_RAW, NM_STRLEN (PASSWORD_RAW))); unlink (testfile); @@ -2234,8 +2226,6 @@ test_read_new_wireless_group_names (void) NMSettingWireless *s_wifi; NMSettingWirelessSecurity *s_wsec; GBytes *ssid; - const guint8 *ssid_data; - gsize ssid_len; const char *expected_ssid = "foobar"; gs_free_error GError *error = NULL; gboolean success; @@ -2253,9 +2243,7 @@ test_read_new_wireless_group_names (void) ssid = nm_setting_wireless_get_ssid (s_wifi); g_assert (ssid); - ssid_data = g_bytes_get_data (ssid, &ssid_len); - g_assert_cmpint (ssid_len, ==, strlen (expected_ssid)); - g_assert_cmpint (memcmp (ssid_data, expected_ssid, ssid_len), ==, 0); + g_assert (nm_utils_gbytes_equal_mem (ssid, expected_ssid, strlen (expected_ssid))); g_assert_cmpstr (nm_setting_wireless_get_mode (s_wifi), ==, NM_SETTING_WIRELESS_MODE_INFRA); From dd4a6f307c5faf7b98171379ec9eb5f590d9bb2b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Aug 2018 21:35:33 +0200 Subject: [PATCH 3/4] tests: minor code cleanup in tests Use nmtst_assert_success(), nm_auto() macros, and minor cleanups. --- libnm-core/tests/test-general.c | 4 +-- libnm-core/tests/test-keyfile.c | 5 ++-- libnm-core/tests/test-setting.c | 4 +-- src/dhcp/tests/test-dhcp-dhclient.c | 24 ++++++++---------- .../plugins/keyfile/tests/test-keyfile.c | 25 ++++++------------- 5 files changed, 24 insertions(+), 38 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 0b2545a9d3..324c9d110f 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -5340,17 +5340,17 @@ test_hexstr2bin (void) { "aab:ccc:ddd" }, { "aab::ccc:ddd" }, }; - GBytes *b; guint i; for (i = 0; i < G_N_ELEMENTS (items); i++) { + gs_unref_bytes GBytes *b = NULL; + b = nm_utils_hexstr2bin (items[i].str); if (items[i].expected_len) g_assert (b); else g_assert (!b); g_assert (nm_utils_gbytes_equal_mem (b, items[i].expected, items[i].expected_len)); - g_bytes_unref (b); } } diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 5855aeacf9..d941fa22c4 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -275,8 +275,9 @@ _keyfile_convert (NMConnection **con, b1 = nm_setting_802_1x_get_ca_cert_blob (s1); b2 = nm_setting_802_1x_get_ca_cert_blob (s2); - g_assert_cmpint (g_bytes_get_size (b1), ==, g_bytes_get_size (b2)); - g_assert (memcmp (g_bytes_get_data (b1, NULL), g_bytes_get_data (b2, NULL), g_bytes_get_size (b1)) == 0); + g_assert (b1); + g_assert (b2); + g_assert (g_bytes_equal (b1, b2)); break; } default: diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index 18dadd886e..fb431ed702 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -50,7 +50,7 @@ compare_blob_data (const char *test, const char *key_path, GBytes *key) { - char *contents = NULL; + gs_free char *contents = NULL; gsize len = 0; GError *error = NULL; gboolean success; @@ -61,8 +61,6 @@ compare_blob_data (const char *test, nmtst_assert_success (success, error); g_assert_cmpmem (contents, len, g_bytes_get_data (key, NULL), g_bytes_get_size (key)); - - g_free (contents); } static void diff --git a/src/dhcp/tests/test-dhcp-dhclient.c b/src/dhcp/tests/test-dhcp-dhclient.c index 6ea8a43469..14a1c78634 100644 --- a/src/dhcp/tests/test-dhcp-dhclient.c +++ b/src/dhcp/tests/test-dhcp-dhclient.c @@ -675,22 +675,18 @@ test_existing_multiline_alsoreq (void) static void test_one_duid (const char *escaped, const guint8 *unescaped, guint len) { - GBytes *t; - char *w; + gs_unref_bytes GBytes *t1 = NULL; + gs_unref_bytes GBytes *t2 = NULL; + gs_free char *w = NULL; - t = nm_dhcp_dhclient_unescape_duid (escaped); - g_assert (t); - g_assert (nm_utils_gbytes_equal_mem (t, unescaped, len)); - g_bytes_unref (t); + t1 = nm_dhcp_dhclient_unescape_duid (escaped); + g_assert (t1); + g_assert (nm_utils_gbytes_equal_mem (t1, unescaped, len)); - t = g_bytes_new_static (unescaped, len); - w = nm_dhcp_dhclient_escape_duid (t); + t2 = g_bytes_new (unescaped, len); + w = nm_dhcp_dhclient_escape_duid (t2); g_assert (w); - g_assert_cmpint (strlen (escaped), ==, strlen (w)); g_assert_cmpstr (escaped, ==, w); - - g_bytes_unref (t); - g_free (w); } static void @@ -732,8 +728,8 @@ test_read_duid_from_leasefile (void) GError *error = NULL; duid = nm_dhcp_dhclient_read_duid (TEST_DIR"/test-dhclient-duid.leases", &error); - g_assert_no_error (error); - g_assert (duid); + nmtst_assert_success (duid, error); + g_assert (nm_utils_gbytes_equal_mem (duid, expected, G_N_ELEMENTS (expected))); } diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 2ce53697b2..b4c6b1e2ce 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -1021,14 +1021,11 @@ test_read_intlike_ssid (void) const char *expected_ssid = "101"; connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID", &error); - g_assert_no_error (error); - g_assert (connection); + nmtst_assert_success (connection, error); success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + nmtst_assert_success (success, error); - /* SSID */ s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -1048,14 +1045,11 @@ test_read_intlike_ssid_2 (void) const char *expected_ssid = "11;12;13;"; connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID_2", &error); - g_assert_no_error (error); - g_assert (connection); + nmtst_assert_success (connection, error); success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + nmtst_assert_success (success, error); - /* SSID */ s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -2231,13 +2225,11 @@ test_read_new_wireless_group_names (void) gboolean success; connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_New_Wireless_Group_Names", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + nmtst_assert_success (connection, error); + + success = nm_connection_verify (connection, &error); + nmtst_assert_success (success, error); - /* Wifi setting */ s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -2247,7 +2239,6 @@ test_read_new_wireless_group_names (void) g_assert_cmpstr (nm_setting_wireless_get_mode (s_wifi), ==, NM_SETTING_WIRELESS_MODE_INFRA); - /* Wifi security setting */ s_wsec = nm_connection_get_setting_wireless_security (connection); g_assert (s_wsec); g_assert_cmpstr (nm_setting_wireless_security_get_key_mgmt (s_wsec), ==, "wpa-psk"); From b8a57fb2725961759bdf45f1156fa7ef2c4d0498 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 Aug 2018 13:11:16 +0200 Subject: [PATCH 4/4] libnm-core: remove unused utlity functions for GSList I think GSList is not a great data type. Most of the time when we used it, we better had choosen another data type. These utility functions were unused, and I think we should use GSList less. Drop them. --- libnm-core/nm-core-internal.h | 6 ------ libnm-core/nm-utils.c | 33 --------------------------------- 2 files changed, 39 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 23e5dd71d8..3dbbb7aa64 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -238,12 +238,6 @@ _nm_auto_ip_route_unref (NMIPRoute **v) } #define nm_auto_ip_route_unref nm_auto (_nm_auto_ip_route_unref) -GPtrArray *_nm_utils_copy_slist_to_array (const GSList *list, - NMUtilsCopyFunc copy_func, - GDestroyNotify unref_func); -GSList *_nm_utils_copy_array_to_slist (const GPtrArray *array, - NMUtilsCopyFunc copy_func); - GPtrArray *_nm_utils_copy_array (const GPtrArray *array, NMUtilsCopyFunc copy_func, GDestroyNotify free_func); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 9565c0af39..cb71d6e77d 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -622,39 +622,6 @@ _nm_utils_copy_strdict (GHashTable *strdict) return copy; } -GPtrArray * -_nm_utils_copy_slist_to_array (const GSList *list, - NMUtilsCopyFunc copy_func, - GDestroyNotify unref_func) -{ - const GSList *iter; - GPtrArray *array; - - array = g_ptr_array_new_with_free_func (unref_func); - for (iter = list; iter; iter = iter->next) - g_ptr_array_add (array, copy_func ? copy_func (iter->data) : iter->data); - return array; -} - -GSList * -_nm_utils_copy_array_to_slist (const GPtrArray *array, - NMUtilsCopyFunc copy_func) -{ - GSList *slist = NULL; - gpointer item; - int i; - - if (!array) - return NULL; - - for (i = 0; i < array->len; i++) { - item = array->pdata[i]; - slist = g_slist_prepend (slist, copy_func (item)); - } - - return g_slist_reverse (slist); -} - GPtrArray * _nm_utils_copy_array (const GPtrArray *array, NMUtilsCopyFunc copy_func,