From 082c2117bb72909bf1728b24b523af06ab2dcdca Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Jun 2021 12:13:20 +0100 Subject: [PATCH 1/3] tests: Use g_assert_*() rather than g_assert() in bytes.c `g_assert()` is compiled out with `G_DISABLE_ASSERT`. Signed-off-by: Philip Withnall --- glib/tests/bytes.c | 74 +++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/glib/tests/bytes.c b/glib/tests/bytes.c index e4be70aad..6b86cc197 100644 --- a/glib/tests/bytes.c +++ b/glib/tests/bytes.c @@ -38,8 +38,8 @@ test_new (void) data = "test"; bytes = g_bytes_new (data, 4); - g_assert (bytes != NULL); - g_assert (g_bytes_get_data (bytes, &size) != data); + g_assert_nonnull (bytes); + g_assert_true (g_bytes_get_data (bytes, &size) != data); g_assert_cmpuint (size, ==, 4); g_assert_cmpuint (g_bytes_get_size (bytes), ==, 4); g_assert_cmpmem (data, 4, g_bytes_get_data (bytes, NULL), g_bytes_get_size (bytes)); @@ -56,8 +56,8 @@ test_new_take (void) data = g_strdup ("test"); bytes = g_bytes_new_take (data, 4); - g_assert (bytes != NULL); - g_assert (g_bytes_get_data (bytes, &size) == data); + g_assert_nonnull (bytes); + g_assert_true (g_bytes_get_data (bytes, &size) == data); g_assert_cmpuint (size, ==, 4); g_assert_cmpuint (g_bytes_get_size (bytes), ==, 4); @@ -73,8 +73,8 @@ test_new_static (void) data = "test"; bytes = g_bytes_new_static (data, 4); - g_assert (bytes != NULL); - g_assert (g_bytes_get_data (bytes, &size) == data); + g_assert_nonnull (bytes); + g_assert_true (g_bytes_get_data (bytes, &size) == data); g_assert_cmpuint (size, ==, 4); g_assert_cmpuint (g_bytes_get_size (bytes), ==, 4); @@ -91,8 +91,8 @@ test_new_from_bytes (void) bytes = g_bytes_new (data, 14); sub = g_bytes_new_from_bytes (bytes, 10, 4); - g_assert (sub != NULL); - g_assert (g_bytes_get_data (sub, NULL) == ((gchar *)g_bytes_get_data (bytes, NULL)) + 10); + g_assert_nonnull (sub); + g_assert_true (g_bytes_get_data (sub, NULL) == ((gchar *)g_bytes_get_data (bytes, NULL)) + 10); g_bytes_unref (bytes); g_assert_cmpmem (g_bytes_get_data (sub, NULL), g_bytes_get_size (sub), "wave", 4); @@ -116,9 +116,9 @@ test_new_from_bytes_slice (void) g_assert_cmpint (bytes3->ref_count, ==, 1); g_assert_null (bytes->user_data); - g_assert (bytes1->user_data == bytes); - g_assert (bytes2->user_data == bytes); - g_assert (bytes3->user_data == bytes); + g_assert_true (bytes1->user_data == bytes); + g_assert_true (bytes2->user_data == bytes); + g_assert_true (bytes3->user_data == bytes); g_assert_cmpint (17, ==, g_bytes_get_size (bytes)); g_assert_cmpint (13, ==, g_bytes_get_size (bytes1)); @@ -145,7 +145,7 @@ test_new_from_bytes_shared_ref (void) GBytes *bytes = g_bytes_new_static ("Some data", strlen ("Some data") + 1); GBytes *other = g_bytes_new_from_bytes (bytes, 0, g_bytes_get_size (bytes)); - g_assert (bytes == other); + g_assert_true (bytes == other); g_assert_cmpint (bytes->ref_count, ==, 2); g_bytes_unref (bytes); @@ -156,7 +156,7 @@ static void on_destroy_increment (gpointer data) { gint *count = data; - g_assert (count != NULL); + g_assert_nonnull (count); (*count)++; } @@ -170,9 +170,9 @@ test_new_with_free_func (void) data = "test"; bytes = g_bytes_new_with_free_func (data, 4, on_destroy_increment, &count); - g_assert (bytes != NULL); + g_assert_nonnull (bytes); g_assert_cmpint (count, ==, 0); - g_assert (g_bytes_get_data (bytes, &size) == data); + g_assert_true (g_bytes_get_data (bytes, &size) == data); g_assert_cmpuint (size, ==, 4); g_assert_cmpuint (g_bytes_get_size (bytes), ==, 4); @@ -193,7 +193,7 @@ test_hash (void) hash1 = g_bytes_hash (bytes1); hash2 = g_bytes_hash (bytes2); - g_assert (hash1 == hash2); + g_assert_cmpuint (hash1, ==, hash2); g_bytes_unref (bytes1); g_bytes_unref (bytes2); @@ -208,18 +208,18 @@ test_equal (void) bytes = g_bytes_new ("blah", 4); bytes2 = g_bytes_new ("blah", 4); - g_assert (g_bytes_equal (bytes, bytes2)); - g_assert (g_bytes_equal (bytes2, bytes)); + g_assert_true (g_bytes_equal (bytes, bytes2)); + g_assert_true (g_bytes_equal (bytes2, bytes)); g_bytes_unref (bytes2); bytes2 = g_bytes_new ("bla", 3); - g_assert (!g_bytes_equal (bytes, bytes2)); - g_assert (!g_bytes_equal (bytes2, bytes)); + g_assert_false (g_bytes_equal (bytes, bytes2)); + g_assert_false (g_bytes_equal (bytes2, bytes)); g_bytes_unref (bytes2); bytes2 = g_bytes_new ("true", 4); - g_assert (!g_bytes_equal (bytes, bytes2)); - g_assert (!g_bytes_equal (bytes2, bytes)); + g_assert_false (g_bytes_equal (bytes, bytes2)); + g_assert_false (g_bytes_equal (bytes2, bytes)); g_bytes_unref (bytes2); g_bytes_unref (bytes); @@ -272,7 +272,7 @@ test_to_data_transferred (void) bytes = g_bytes_new (NYAN, N_NYAN); memory = g_bytes_get_data (bytes, NULL); data = g_bytes_unref_to_data (bytes, &size); - g_assert (data == memory); + g_assert_true (data == memory); g_assert_cmpmem (data, size, NYAN, N_NYAN); g_free (data); } @@ -290,10 +290,10 @@ test_to_data_two_refs (void) bytes = g_bytes_ref (bytes); memory = g_bytes_get_data (bytes, NULL); data = g_bytes_unref_to_data (bytes, &size); - g_assert (data != memory); + g_assert_true (data != memory); g_assert_cmpmem (data, size, NYAN, N_NYAN); g_free (data); - g_assert (g_bytes_get_data (bytes, &size) == memory); + g_assert_true (g_bytes_get_data (bytes, &size) == memory); g_assert_cmpuint (size, ==, N_NYAN); g_assert_cmpuint (g_bytes_get_size (bytes), ==, N_NYAN); g_bytes_unref (bytes); @@ -308,9 +308,9 @@ test_to_data_non_malloc (void) /* Memory copied: non malloc memory */ bytes = g_bytes_new_static (NYAN, N_NYAN); - g_assert (g_bytes_get_data (bytes, NULL) == NYAN); + g_assert_true (g_bytes_get_data (bytes, NULL) == NYAN); data = g_bytes_unref_to_data (bytes, &size); - g_assert (data != (gpointer)NYAN); + g_assert_true (data != (gpointer)NYAN); g_assert_cmpmem (data, size, NYAN, N_NYAN); g_free (data); } @@ -326,8 +326,8 @@ test_to_array_transferred (void) bytes = g_bytes_new (NYAN, N_NYAN); memory = g_bytes_get_data (bytes, NULL); array = g_bytes_unref_to_array (bytes); - g_assert (array != NULL); - g_assert (array->data == memory); + g_assert_nonnull (array); + g_assert_true (array->data == memory); g_assert_cmpmem (array->data, array->len, NYAN, N_NYAN); g_byte_array_unref (array); } @@ -377,11 +377,11 @@ test_to_array_two_refs (void) bytes = g_bytes_ref (bytes); memory = g_bytes_get_data (bytes, NULL); array = g_bytes_unref_to_array (bytes); - g_assert (array != NULL); - g_assert (array->data != memory); + g_assert_nonnull (array); + g_assert_true (array->data != memory); g_assert_cmpmem (array->data, array->len, NYAN, N_NYAN); g_byte_array_unref (array); - g_assert (g_bytes_get_data (bytes, &size) == memory); + g_assert_true (g_bytes_get_data (bytes, &size) == memory); g_assert_cmpuint (size, ==, N_NYAN); g_assert_cmpuint (g_bytes_get_size (bytes), ==, N_NYAN); g_bytes_unref (bytes); @@ -395,10 +395,10 @@ test_to_array_non_malloc (void) /* Memory copied: non malloc memory */ bytes = g_bytes_new_static (NYAN, N_NYAN); - g_assert (g_bytes_get_data (bytes, NULL) == NYAN); + g_assert_true (g_bytes_get_data (bytes, NULL) == NYAN); array = g_bytes_unref_to_array (bytes); - g_assert (array != NULL); - g_assert (array->data != (gpointer)NYAN); + g_assert_nonnull (array); + g_assert_true (array->data != (gpointer)NYAN); g_assert_cmpmem (array->data, array->len, NYAN, N_NYAN); g_byte_array_unref (array); } @@ -414,8 +414,8 @@ test_null (void) data = g_bytes_unref_to_data (bytes, &size); - g_assert (data == NULL); - g_assert (size == 0); + g_assert_null (data); + g_assert_cmpuint (size, ==, 0); } static void From fc7b316bc7b236bb1d21b1337196515de4053c47 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Jun 2021 12:14:06 +0100 Subject: [PATCH 2/3] tests: Test that g_bytes_unref(NULL) is a no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s documented as such, and this marginally increases the code coverage on `gbytes.c`. Signed-off-by: Philip Withnall --- glib/tests/bytes.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/glib/tests/bytes.c b/glib/tests/bytes.c index 6b86cc197..534687fe6 100644 --- a/glib/tests/bytes.c +++ b/glib/tests/bytes.c @@ -450,6 +450,13 @@ test_get_region (void) g_bytes_unref (bytes); } +static void +test_unref_null (void) +{ + g_test_summary ("Test that calling g_bytes_unref() on NULL is a no-op"); + g_bytes_unref (NULL); +} + int main (int argc, char *argv[]) { @@ -474,6 +481,7 @@ main (int argc, char *argv[]) g_test_add_func ("/bytes/to-array/non-malloc", test_to_array_non_malloc); g_test_add_func ("/bytes/null", test_null); g_test_add_func ("/bytes/get-region", test_get_region); + g_test_add_func ("/bytes/unref-null", test_unref_null); return g_test_run (); } From b63a3189e7f3fdaf3ce8b3dc9d89c31df09765e5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Jun 2021 12:27:11 +0100 Subject: [PATCH 3/3] tests: Add a test for GBytes memory transfer with an odd free func This is basically a contrived test to trigger the `bytes->user_data != bytes->data` condition (and none of the earlier short-circuiting conditions in that statement) in `try_steal_and_unref()`. This gives 100% line and branch coverage for `gbytes.c`. Signed-off-by: Philip Withnall --- glib/tests/bytes.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/glib/tests/bytes.c b/glib/tests/bytes.c index 534687fe6..8178bc2a8 100644 --- a/glib/tests/bytes.c +++ b/glib/tests/bytes.c @@ -315,6 +315,30 @@ test_to_data_non_malloc (void) g_free (data); } +static void +test_to_data_different_free_func (void) +{ + gpointer data; + gsize size; + GBytes *bytes; + gchar *sentinel = g_strdup ("hello"); + + /* Memory copied: free func and user_data don’t point to the bytes data */ + bytes = g_bytes_new_with_free_func (NYAN, N_NYAN, g_free, sentinel); + g_assert_true (g_bytes_get_data (bytes, NULL) == NYAN); + + data = g_bytes_unref_to_data (bytes, &size); + g_assert_true (data != (gpointer)NYAN); + g_assert_cmpmem (data, size, NYAN, N_NYAN); + g_free (data); + + /* @sentinel should not be leaked; testing that requires this test to be run + * under valgrind. We can’t use a custom free func to check it isn’t leaked, + * as the point of this test is to hit a condition in `try_steal_and_unref()` + * which is short-circuited if the free func isn’t g_free(). + * See discussion in https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2152 */ +} + static void test_to_array_transferred (void) { @@ -475,6 +499,7 @@ main (int argc, char *argv[]) g_test_add_func ("/bytes/to-data/transferred", test_to_data_transferred); g_test_add_func ("/bytes/to-data/two-refs", test_to_data_two_refs); g_test_add_func ("/bytes/to-data/non-malloc", test_to_data_non_malloc); + g_test_add_func ("/bytes/to-data/different-free-func", test_to_data_different_free_func); g_test_add_func ("/bytes/to-array/transferred", test_to_array_transferred); g_test_add_func ("/bytes/to-array/transferred/oversize", test_to_array_transferred_oversize); g_test_add_func ("/bytes/to-array/two-refs", test_to_array_two_refs);