From 24b652d3ca9818ab97907e9f2f68513632cd4693 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 11:22:17 +0100 Subject: [PATCH 1/6] test_string_replace: Make types agree g_string_replace() returns guint, not gint. Signed-off-by: Simon McVittie --- glib/tests/string.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/glib/tests/string.c b/glib/tests/string.c index 6e22cd287..b222d6194 100644 --- a/glib/tests/string.c +++ b/glib/tests/string.c @@ -499,27 +499,27 @@ static void test_string_replace (void) { GString *s; - gint n; + guint n; s = g_string_new ("foo bar foo baz foo bar foobarbaz"); n = g_string_replace (s, "bar", "baz", 0); g_assert_cmpstr ("foo baz foo baz foo baz foobazbaz", ==, s->str); - g_assert_cmpint (n, ==, 3); + g_assert_cmpuint (n, ==, 3); n = g_string_replace (s, "baz", "bar", 3); g_assert_cmpstr ("foo bar foo bar foo bar foobazbaz", ==, s->str); - g_assert_cmpint (n, ==, 3); + g_assert_cmpuint (n, ==, 3); n = g_string_replace (s, "foobar", "bar", 1); g_assert_cmpstr ("foo bar foo bar foo bar foobazbaz", ==, s->str); - g_assert_cmpint (n, ==, 0); + g_assert_cmpuint (n, ==, 0); s = g_string_assign (s, "aaaaaaaa"); n = g_string_replace (s, "a", "abcdefghijkl", 0); g_assert_cmpstr ("abcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijkl", ==, s->str); - g_assert_cmpint (n, ==, 8); + g_assert_cmpuint (n, ==, 8); g_string_free (s, TRUE); } From c64e6cfc795c19b5fe3aad85f3bd18d95f5ef06f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 11:25:56 +0100 Subject: [PATCH 2/6] test_string_replace: Make the test table-driven This makes it straightforward to add more test-cases. Signed-off-by: Simon McVittie --- glib/tests/string.c | 62 +++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/glib/tests/string.c b/glib/tests/string.c index b222d6194..72c9016dd 100644 --- a/glib/tests/string.c +++ b/glib/tests/string.c @@ -498,30 +498,48 @@ test_string_to_bytes (void) static void test_string_replace (void) { - GString *s; - guint n; + static const struct + { + const char *string; + const char *original; + const char *replacement; + guint limit; + const char *expected; + guint expected_n; + } + tests[] = + { + { "foo bar foo baz foo bar foobarbaz", "bar", "baz", 0, + "foo baz foo baz foo baz foobazbaz", 3 }, + { "foo baz foo baz foo baz foobazbaz", "baz", "bar", 3, + "foo bar foo bar foo bar foobazbaz", 3 }, + { "foo bar foo bar foo bar foobazbaz", "foobar", "bar", 1, + "foo bar foo bar foo bar foobazbaz", 0 }, + { "aaaaaaaa", "a", "abcdefghijkl", 0, + "abcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijkl", + 8 }, + }; + gsize i; - s = g_string_new ("foo bar foo baz foo bar foobarbaz"); + for (i = 0; i < G_N_ELEMENTS (tests); i++) + { + GString *s; + guint n; - n = g_string_replace (s, "bar", "baz", 0); - g_assert_cmpstr ("foo baz foo baz foo baz foobazbaz", ==, s->str); - g_assert_cmpuint (n, ==, 3); - - n = g_string_replace (s, "baz", "bar", 3); - g_assert_cmpstr ("foo bar foo bar foo bar foobazbaz", ==, s->str); - g_assert_cmpuint (n, ==, 3); - - n = g_string_replace (s, "foobar", "bar", 1); - g_assert_cmpstr ("foo bar foo bar foo bar foobazbaz", ==, s->str); - g_assert_cmpuint (n, ==, 0); - - s = g_string_assign (s, "aaaaaaaa"); - n = g_string_replace (s, "a", "abcdefghijkl", 0); - g_assert_cmpstr ("abcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijkl", - ==, s->str); - g_assert_cmpuint (n, ==, 8); - - g_string_free (s, TRUE); + s = g_string_new (tests[i].string); + g_test_message ("%" G_GSIZE_FORMAT ": Replacing \"%s\" with \"%s\" (limit %u) in \"%s\"", + i, tests[i].original, tests[i].replacement, + tests[i].limit, tests[i].string); + n = g_string_replace (s, tests[i].original, tests[i].replacement, + tests[i].limit); + g_test_message ("-> %u replacements, \"%s\"", + n, s->str); + g_assert_cmpstr (tests[i].expected, ==, s->str); + g_assert_cmpuint (strlen (tests[i].expected), ==, s->len); + g_assert_cmpuint (strlen (tests[i].expected) + 1, <=, s->allocated_len); + g_assert_cmpuint (tests[i].expected_n, ==, n); + g_string_free (s, TRUE); + } } int From 7d35e49c4245d232b8f764b8cbf5e17f52a6baa4 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 11:27:24 +0100 Subject: [PATCH 3/6] test_string_replace: Expand test coverage These are taken from another project (steam-runtime-tools) where I implemented a similar replace method before realising that more recent GLib versions had g_string_replace(). Signed-off-by: Simon McVittie --- glib/tests/string.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/glib/tests/string.c b/glib/tests/string.c index 72c9016dd..333e0254e 100644 --- a/glib/tests/string.c +++ b/glib/tests/string.c @@ -518,6 +518,16 @@ test_string_replace (void) { "aaaaaaaa", "a", "abcdefghijkl", 0, "abcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijklabcdefghijkl", 8 }, + { "/usr/$LIB/libMangoHud.so", "$LIB", "lib32", 0, + "/usr/lib32/libMangoHud.so", 1 }, + { "food for foals", "o", "", 0, + "fd fr fals", 4 }, + { "aaa", "a", "aaa", 0, + "aaaaaaaaa", 3 }, + { "aaa", "a", "", 0, + "", 3 }, + { "aaa", "aa", "bb", 0, + "bba", 1 }, }; gsize i; From 0a8c7e57ab3737be4fff795deab8ab5e7e975377 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 11:51:56 +0100 Subject: [PATCH 4/6] g_string_replace: Don't replace empty string more than once per location This matches the behaviour of Python `str.replace()`, and avoids carrying out 2**32 replacements before n wraps around, which is almost certainly not what we want. Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2452 Signed-off-by: Simon McVittie --- glib/gstring.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/glib/gstring.c b/glib/gstring.c index ad2ec1771..e3ec98b0b 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -995,6 +995,15 @@ g_string_replace (GString *string, g_string_insert (string, pos, replace); cur = string->str + pos + r_len; n++; + /* Only match the empty string once at any given position, to + * avoid infinite loops */ + if (f_len == 0) + { + if (cur[0] == '\0') + break; + else + cur++; + } if (n == limit) break; } From bf70d58d55d8a77de72c86e68aa0960d71b22eed Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 11:52:58 +0100 Subject: [PATCH 5/6] test_string_replace: Exercise zero-length replacements Previously, these would have done 2**32 replacements, and the first one would have consumed 6GB of memory in the process. They now match what Python `str.replace()` does. Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/2452 Signed-off-by: Simon McVittie --- glib/tests/string.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/glib/tests/string.c b/glib/tests/string.c index 333e0254e..24098d1be 100644 --- a/glib/tests/string.c +++ b/glib/tests/string.c @@ -528,6 +528,12 @@ test_string_replace (void) "", 3 }, { "aaa", "aa", "bb", 0, "bba", 1 }, + { "foo", "", "bar", 0, + "barfbarobarobar", 4 }, + { "", "", "x", 0, + "x", 1 }, + { "", "", "", 0, + "", 1 }, }; gsize i; From b13777841ffe6e0a3217b7620b8dcb8634fb7ed3 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 2 Aug 2021 12:36:26 +0100 Subject: [PATCH 6/6] g_string_replace: Document behaviour of zero-length match pattern Signed-off-by: Simon McVittie --- glib/gstring.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/glib/gstring.c b/glib/gstring.c index e3ec98b0b..da9c5f7ab 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -966,6 +966,11 @@ g_string_erase (GString *string, * less than @limit, all instances are replaced. If @limit is `0`, * all instances of @find are replaced. * + * If @find is the empty string, since versions 2.69.1 and 2.68.4 the + * replacement will be inserted no more than once per possible position + * (beginning of string, end of string and between characters). This did + * not work correctly in earlier versions. + * * Returns: the number of find and replace operations performed. * * Since: 2.68