From bccff754b65628bd08e124566eb0ea4aa6603a08 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Tue, 24 Jan 2023 19:07:46 +0300 Subject: [PATCH 1/4] Use g_string_free_and_steal () more Now that there is g_string_free_and_steal (), we can use it instead of the older g_string_free (_, FALSE). Make sure to use its return value while doing so, as opposed to manually accessing string->str, to avoid compiler warnings and make the intent more explicit. This is all done in preparation for making g_string_free (_, FALSE) warn on unused return value much like g_string_free_and_steal (), which will happen in the next commit. Signed-off-by: Sergey Bugaev --- gio/glib-compile-schemas.c | 5 +---- glib/gkeyfile.c | 5 +---- glib/tests/markup-subparser.c | 8 ++------ 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/gio/glib-compile-schemas.c b/gio/glib-compile-schemas.c index acabfb6b4..efe6f8e77 100644 --- a/gio/glib-compile-schemas.c +++ b/gio/glib-compile-schemas.c @@ -709,8 +709,8 @@ key_state_serialise (KeyState *state) gsize size; gsize i; - data = state->strinfo->str; size = state->strinfo->len; + data = g_string_free_and_steal (g_steal_pointer (&state->strinfo)); words = data; for (i = 0; i < size / sizeof (guint32); i++) @@ -720,9 +720,6 @@ key_state_serialise (KeyState *state) data, size, TRUE, g_free, data); - g_string_free (state->strinfo, FALSE); - state->strinfo = NULL; - g_variant_builder_add (&builder, "(y@au)", state->is_flags ? 'f' : state->is_enum ? 'e' : 'c', diff --git a/glib/gkeyfile.c b/glib/gkeyfile.c index 6ae738ca1..fb65d55c5 100644 --- a/glib/gkeyfile.c +++ b/glib/gkeyfile.c @@ -3546,10 +3546,7 @@ g_key_file_get_key_comment (GKeyFile *key_file, } if (string != NULL) - { - comment = string->str; - g_string_free (string, FALSE); - } + comment = g_string_free_and_steal (g_steal_pointer (&string)); else comment = NULL; diff --git a/glib/tests/markup-subparser.c b/glib/tests/markup-subparser.c index e2e94ea90..626b4274d 100644 --- a/glib/tests/markup-subparser.c +++ b/glib/tests/markup-subparser.c @@ -83,9 +83,7 @@ subparser_end (GMarkupParseContext *ctx, char *result; string = g_markup_parse_context_pop (ctx); - result = string->str; - - g_string_free (string, FALSE); + result = g_string_free_and_steal (g_steal_pointer (&string)); strings_allocated--; if (result == NULL || result[0] == '\0') @@ -156,9 +154,7 @@ replay_parser_end (GMarkupParseContext *ctx, return NULL; } - result = string->str; - - g_string_free (string, FALSE); + result = g_string_free_and_steal (g_steal_pointer (&string)); strings_allocated--; if (result == NULL || result[0] == '\0') From c3d07a625a407b56432f4b8c60295fd32ee1ee2b Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Tue, 24 Jan 2023 19:15:00 +0300 Subject: [PATCH 2/4] string: Make g_string_free (_, FALSE) warn on unused result ...much like g_string_free_and_steal () does; by redirecting g_string_free (_, FALSE) calls (when we can detect them) to g_string_free_and_steal (). This relies on some unpretty macros, but should be entirely transparent to any users of g_string_free (). In particular, the macro only evaluates its arguments once, no matter which branch ends up being taken. The ternary operator the macro expands to always gets optimized out, even at -O0: there is only one call to either g_string_free () or g_string_free_and_steal () in the compiled code, with no run-time branching. Add a test for ensuring this works as expected in C++ too. Signed-off-by: Sergey Bugaev --- glib/gstring.c | 6 +++--- glib/gstring.h | 15 ++++++++++++++- glib/tests/cxx.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/glib/gstring.c b/glib/gstring.c index 5fb7c3d1e..ad6b92231 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -206,8 +206,8 @@ g_string_new_len (const gchar *init, * (i.e. %NULL if @free_segment is %TRUE) */ gchar * -g_string_free (GString *string, - gboolean free_segment) +(g_string_free) (GString *string, + gboolean free_segment) { gchar *segment; @@ -242,7 +242,7 @@ g_string_free (GString *string, gchar * g_string_free_and_steal (GString *string) { - return g_string_free (string, FALSE); + return (g_string_free) (string, FALSE); } /** diff --git a/glib/gstring.h b/glib/gstring.h index b82123198..fde18db88 100644 --- a/glib/gstring.h +++ b/glib/gstring.h @@ -57,10 +57,23 @@ GString* g_string_new_len (const gchar *init, GLIB_AVAILABLE_IN_ALL GString* g_string_sized_new (gsize dfl_size); GLIB_AVAILABLE_IN_ALL -gchar* g_string_free (GString *string, +gchar* (g_string_free) (GString *string, gboolean free_segment); GLIB_AVAILABLE_IN_2_76 gchar* g_string_free_and_steal (GString *string) G_GNUC_WARN_UNUSED_RESULT; + +#if G_GNUC_CHECK_VERSION (2, 0) && (GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_76) + +#define g_string_free(str, free_segment) \ + (__builtin_constant_p (free_segment) ? \ + ((free_segment) ? \ + (g_string_free) ((str), (free_segment)) : \ + g_string_free_and_steal (str)) \ + : \ + (g_string_free) ((str), (free_segment))) + +#endif /* G_GNUC_CHECK_VERSION (2, 0) && (GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_76) */ + GLIB_AVAILABLE_IN_2_34 GBytes* g_string_free_to_bytes (GString *string); GLIB_AVAILABLE_IN_ALL diff --git a/glib/tests/cxx.cpp b/glib/tests/cxx.cpp index abd8a8d67..8c31f7380 100644 --- a/glib/tests/cxx.cpp +++ b/glib/tests/cxx.cpp @@ -472,6 +472,35 @@ test_string_append (void) g_string_free (string, TRUE); } +static void +test_string_free (void) +{ + GString *str; + gchar *data; + + g_test_message ("Test that g_string_free() macro compiles and doesn’t " + "cause any compiler warnings in C++ mode"); + + /* Test that g_string_free (_, TRUE) does not cause a warning if + * its return value is unused. */ + str = g_string_new ("test"); + g_string_free (str, TRUE); + + /* Test that g_string_free (_, FALSE) does not emit a warning if + * its return value is used. */ + str = g_string_new ("test"); + data = g_string_free (str, FALSE); + g_free (data); + + /* Test that g_string_free () with an expression that is always FALSE + * at runtime, but the compiler can't know it, does not cause any + * warnings if its return value is unused. */ + str = g_string_new ("test"); + data = str->str; + g_string_free (str, g_test_get_path ()[0] == 0); + g_free (data); +} + int main (int argc, char *argv[]) { @@ -502,6 +531,7 @@ main (int argc, char *argv[]) g_test_add_func ("/C++/str-has-suffix", test_str_has_suffix); g_test_add_func ("/C++/str-has-suffix/macro", test_str_has_suffix_macro); g_test_add_func ("/C++/string-append", test_string_append); + g_test_add_func ("/C++/string-free", test_string_free); return g_test_run (); } From 718f05d090babba3bebe1f7ead08e5c55d4a9a8e Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Tue, 24 Jan 2023 19:27:06 +0300 Subject: [PATCH 3/4] gwakeup: #include Since commit 94b658ab4cc86df1352a34f63c9a672abf39f8aa, gwakeup.c has started using C99 integer types, but has not included . This broke building on GNU/Hurd. Fix this by adding the missing include. Signed-off-by: Sergey Bugaev --- glib/gwakeup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/glib/gwakeup.c b/glib/gwakeup.c index 582b2aa34..24d85b669 100644 --- a/glib/gwakeup.c +++ b/glib/gwakeup.c @@ -21,6 +21,7 @@ #include "config.h" +#include /* gwakeup.c is special -- GIO and some test cases include it. As such, * it cannot include other glib headers without triggering the single From c121118bc4b5cd7b8f154b14be7e2e0398054818 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Tue, 24 Jan 2023 19:37:58 +0300 Subject: [PATCH 4/4] tests: Silence a warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In case the OS does not support epoll and kqueue, we get the warning: gio/tests/pollable.c: In function ‘test_pollable_unix_nulldev’: gio/tests/pollable.c:266:7: warning: unused variable ‘fd’ [-Wunused-variable] 266 | int fd; Get rid of it. Signed-off-by: Sergey Bugaev --- gio/tests/pollable.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gio/tests/pollable.c b/gio/tests/pollable.c index 56dfa3a31..540f200c6 100644 --- a/gio/tests/pollable.c +++ b/gio/tests/pollable.c @@ -263,14 +263,12 @@ test_pollable_unix_file (void) static void test_pollable_unix_nulldev (void) { - int fd; - g_test_summary ("Test that /dev/null is not considered pollable, but only if " "on a system where we are able to tell it apart from devices " "that actually implement poll"); #if defined (HAVE_EPOLL_CREATE) || defined (HAVE_KQUEUE) - fd = g_open ("/dev/null", O_RDWR, 0); + int fd = g_open ("/dev/null", O_RDWR, 0); g_assert_cmpint (fd, !=, -1); g_assert_not_pollable (fd);