From 18a232be89c05d8eadd99c637418a05b016f777d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 May 2019 12:53:56 +0100 Subject: [PATCH 1/6] glib: Various minor scan-build fixes These squash various warnings from `scan-build`. None of them are legitimate bugs, but some of them do improve code readability a bit. Signed-off-by: Philip Withnall Helps: #1767 --- gio/gcredentials.c | 3 +-- glib/gchecksum.c | 1 + glib/gkeyfile.c | 1 + glib/gmain.c | 1 + glib/gmarkup.c | 6 ++++-- glib/gmessages.c | 2 +- glib/grand.c | 3 +-- glib/gspawn.c | 2 -- glib/gtestutils.c | 3 +++ glib/gunidecomp.c | 1 + glib/gvariant-parser.c | 2 ++ glib/tests/bookmarkfile.c | 1 + 12 files changed, 17 insertions(+), 9 deletions(-) diff --git a/gio/gcredentials.c b/gio/gcredentials.c index 57a39f2a2..c350e3c88 100644 --- a/gio/gcredentials.c +++ b/gio/gcredentials.c @@ -542,13 +542,12 @@ g_credentials_set_unix_user (GCredentials *credentials, uid_t uid, GError **error) { - gboolean ret; + gboolean ret = FALSE; g_return_val_if_fail (G_IS_CREDENTIALS (credentials), FALSE); g_return_val_if_fail (uid != -1, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - ret = FALSE; #if G_CREDENTIALS_USE_LINUX_UCRED credentials->native.uid = uid; ret = TRUE; diff --git a/glib/gchecksum.c b/glib/gchecksum.c index 1ad21fff8..f8a3f9ab8 100644 --- a/glib/gchecksum.c +++ b/glib/gchecksum.c @@ -1344,6 +1344,7 @@ sha512_sum_close (Sha512sum *sha512) memset (pad + pad_len, 0x00, zeros / 8); pad_len += zeros / 8; zeros = zeros % 8; + (void) zeros; /* don’t care about the dead store */ /* put message bit length at the end of padding */ PUT_UINT64 (sha512->data_len[1], pad, pad_len); diff --git a/glib/gkeyfile.c b/glib/gkeyfile.c index e486f6928..dc80ce5d4 100644 --- a/glib/gkeyfile.c +++ b/glib/gkeyfile.c @@ -3317,6 +3317,7 @@ g_key_file_set_key_comment (GKeyFile *key_file, pair->value = g_key_file_parse_comment_as_value (key_file, comment); key_node = g_list_insert (key_node, pair, 1); + (void) key_node; return TRUE; } diff --git a/glib/gmain.c b/glib/gmain.c index af979c8b8..ae86d5a2c 100644 --- a/glib/gmain.c +++ b/glib/gmain.c @@ -1040,6 +1040,7 @@ find_source_list_for_priority (GMainContext *context, * context->source_lists without having to walk the list again. */ last = g_list_append (last, source_list); + (void) last; } return source_list; } diff --git a/glib/gmarkup.c b/glib/gmarkup.c index 5b70cef8d..945f40695 100644 --- a/glib/gmarkup.c +++ b/glib/gmarkup.c @@ -2917,7 +2917,6 @@ g_markup_collect_attributes (const gchar *element_name, failure: /* replay the above to free allocations */ type = first_type; - attr = first_attr; va_start (ap, first_attr); while (type != G_MARKUP_COLLECT_INVALID) @@ -2952,7 +2951,10 @@ failure: type = va_arg (ap, GMarkupCollectType); if (type != G_MARKUP_COLLECT_INVALID) - attr = va_arg (ap, const char *); + { + attr = va_arg (ap, const char *); + (void) attr; + } } va_end (ap); diff --git a/glib/gmessages.c b/glib/gmessages.c index bb1ab8f84..ea3f6c1e9 100644 --- a/glib/gmessages.c +++ b/glib/gmessages.c @@ -1313,7 +1313,7 @@ g_logv (const gchar *log_domain, { GLogLevelFlags test_level; - test_level = 1 << i; + test_level = 1L << i; if (log_level & test_level) { GLogDomain *domain; diff --git a/glib/grand.c b/glib/grand.c index b5b1b71f6..cf935fc18 100644 --- a/glib/grand.c +++ b/glib/grand.c @@ -502,7 +502,7 @@ g_rand_int_range (GRand *rand, gint32 end) { guint32 dist = end - begin; - guint32 random; + guint32 random = 0; g_return_val_if_fail (rand != NULL, begin); g_return_val_if_fail (end > begin, begin); @@ -563,7 +563,6 @@ g_rand_int_range (GRand *rand, } break; default: - random = 0; /* Quiet GCC */ g_assert_not_reached (); } diff --git a/glib/gspawn.c b/glib/gspawn.c index 3f6d700c1..7dd4bdcbb 100644 --- a/glib/gspawn.c +++ b/glib/gspawn.c @@ -435,8 +435,6 @@ g_spawn_sync (const gchar *working_directory, (outpipe >= 0 || errpipe >= 0)) { - ret = 0; - FD_ZERO (&fds); if (outpipe >= 0) FD_SET (outpipe, &fds); diff --git a/glib/gtestutils.c b/glib/gtestutils.c index dd789482f..5c2a55896 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -3694,6 +3694,9 @@ g_test_trap_assertions (const char *domain, g_assertion_message (domain, file, line, func, msg); g_free (msg); } + + (void) logged_child_output; /* shut up scan-build about the final unread assignment */ + g_free (process_id); } diff --git a/glib/gunidecomp.c b/glib/gunidecomp.c index 19ecdae36..32addc985 100644 --- a/glib/gunidecomp.c +++ b/glib/gunidecomp.c @@ -443,6 +443,7 @@ _g_utf8_normalize_wc (const gchar *str, { g_unicode_canonical_ordering (wc_buffer + last_start, n_wc - last_start); last_start = n_wc; + (void) last_start; } wc_buffer[n_wc] = 0; diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c index 5e96c1321..bee1f47ff 100644 --- a/glib/gvariant-parser.c +++ b/glib/gvariant-parser.c @@ -2527,6 +2527,8 @@ g_variant_new_parsed_va (const gchar *format, if (*stream.stream) g_error ("g_variant_new_parsed: trailing text after value"); + g_clear_error (&error); + return result; } diff --git a/glib/tests/bookmarkfile.c b/glib/tests/bookmarkfile.c index cc922c0fc..d2ece45a4 100644 --- a/glib/tests/bookmarkfile.c +++ b/glib/tests/bookmarkfile.c @@ -175,6 +175,7 @@ test_misc (void) &error); g_assert_error (error, G_BOOKMARK_FILE_ERROR, G_BOOKMARK_FILE_ERROR_INVALID_VALUE); g_clear_error (&error); + g_assert_false (res); g_bookmark_file_set_mime_type (bookmark, "file:///tmp/schedule1.ps", From 0b4162e71403863cff688128f28082df297f58d8 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 5 Sep 2019 13:53:13 +0100 Subject: [PATCH 2/6] build: Disable dtrace probes under static analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macros for the probes confuse the static analyser, and are often called with arguments which the analyser things shouldn’t be used any more (for example, the address of a block of memory which has just been freed). Signed-off-by: Philip Withnall Helps: #1767 --- gio/gio_trace.h | 4 +++- glib/glib_trace.h | 4 +++- gobject/gobject_trace.h | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gio/gio_trace.h b/gio/gio_trace.h index 47d4eacb2..addb70ae4 100644 --- a/gio/gio_trace.h +++ b/gio/gio_trace.h @@ -25,7 +25,9 @@ #error "config.h must be included prior to gio_trace.h" #endif -#ifdef HAVE_DTRACE +/* Ignore probes when doing static analysis, as they do weird things which + * confuses the analyser. */ +#if defined(HAVE_DTRACE) && !defined(__clang_analyzer__) /* include the generated probes header and put markers in code */ #include "gio_probes.h" diff --git a/glib/glib_trace.h b/glib/glib_trace.h index 1876738cf..fdb8f8c46 100644 --- a/glib/glib_trace.h +++ b/glib/glib_trace.h @@ -25,7 +25,9 @@ #error "config.h must be included prior to glib_trace.h" #endif -#ifdef HAVE_DTRACE +/* Ignore probes when doing static analysis, as they do weird things which + * confuses the analyser. */ +#if defined(HAVE_DTRACE) && !defined(__clang_analyzer__) /* include the generated probes header and put markers in code */ #include "glib_probes.h" diff --git a/gobject/gobject_trace.h b/gobject/gobject_trace.h index 30ef2fb5b..261fdac07 100644 --- a/gobject/gobject_trace.h +++ b/gobject/gobject_trace.h @@ -25,7 +25,9 @@ #error "config.h must be included prior to gobject_trace.h" #endif -#ifdef HAVE_DTRACE +/* Ignore probes when doing static analysis, as they do weird things which + * confuses the analyser. */ +#if defined(HAVE_DTRACE) && !defined(__clang_analyzer__) /* include the generated probes header and put markers in code */ #include "gobject_probes.h" From df647a583d2008f6d0514bcfe505fb0ad8a25cb8 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 5 Sep 2019 14:13:51 +0100 Subject: [PATCH 3/6] tests: Fix a couple of static analysis warnings in autoptr tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The static analyser can’t yet work out how `g_autofree` works, so disable those tests. Signed-off-by: Philip Withnall Helps: #1767 --- glib/tests/autoptr.c | 7 +++++++ gobject/tests/autoptr.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/glib/tests/autoptr.c b/glib/tests/autoptr.c index fccdfe55e..4eed862af 100644 --- a/glib/tests/autoptr.c +++ b/glib/tests/autoptr.c @@ -10,6 +10,9 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(HasNonVoidCleanup, non_void_cleanup) static void test_autofree (void) { +#ifdef __clang_analyzer__ + g_test_skip ("autofree tests aren’t understood by the clang analyser"); +#else g_autofree gchar *p = NULL; g_autofree gchar *p2 = NULL; g_autofree gchar *alwaysnull = NULL; @@ -35,6 +38,7 @@ test_autofree (void) } g_assert_null (alwaysnull); +#endif /* __clang_analyzer__ */ } static void @@ -592,6 +596,9 @@ test_autolist (void) l = g_list_prepend (l, b1); l = g_list_prepend (l, b3); + + /* Squash warnings about dead stores */ + (void) l; } /* Only assert if autoptr works */ diff --git a/gobject/tests/autoptr.c b/gobject/tests/autoptr.c index cf7687d84..1f68cb3f9 100644 --- a/gobject/tests/autoptr.c +++ b/gobject/tests/autoptr.c @@ -113,6 +113,9 @@ test_autolist (void) l = g_list_prepend (l, tac1); l = g_list_prepend (l, tac2); + + /* Squash warnings about dead stores */ + (void) l; } /* Only assert if autoptr works */ From d99653f6fec1bede269887fd67ca445f50cc2847 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 5 Sep 2019 14:15:19 +0100 Subject: [PATCH 4/6] gdbusdaemon: Add sanity checks on name refcounting This should make the code a bit easier to reason about, and squash some static analysis warnings. Signed-off-by: Philip Withnall Helps: #1767 --- gio/gdbusdaemon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gio/gdbusdaemon.c b/gio/gdbusdaemon.c index 0d5058f70..e03d7aabd 100644 --- a/gio/gdbusdaemon.c +++ b/gio/gdbusdaemon.c @@ -170,6 +170,7 @@ name_new (GDBusDaemon *daemon, const char *str) static Name * name_ref (Name *name) { + g_assert (name->refcount > 0); name->refcount++; return name; } @@ -177,6 +178,7 @@ name_ref (Name *name) static void name_unref (Name *name) { + g_assert (name->refcount > 0); if (--name->refcount == 0) { g_hash_table_remove (name->daemon->names, name->name); From 8fe58ffe12fe675e9f3a065efa33fd50d273adff Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 5 Sep 2019 14:15:49 +0100 Subject: [PATCH 5/6] gdbusdaemon: Fix unused variable warning Signed-off-by: Philip Withnall Helps: #1767 --- gio/gdbusdaemon.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gio/gdbusdaemon.c b/gio/gdbusdaemon.c index e03d7aabd..7ae55ece8 100644 --- a/gio/gdbusdaemon.c +++ b/gio/gdbusdaemon.c @@ -1465,10 +1465,11 @@ filter_function (GDBusConnection *connection, gpointer user_data) { Client *client = user_data; - const char *types[] = {"invalid", "method_call", "method_return", "error", "signal" }; if (0) - g_printerr ("%s%s %s %d(%d) sender: %s destination: %s %s %s.%s\n", + { + const char *types[] = {"invalid", "method_call", "method_return", "error", "signal" }; + g_printerr ("%s%s %s %d(%d) sender: %s destination: %s %s %s.%s\n", client->id, incoming? "->" : "<-", types[g_dbus_message_get_message_type (message)], @@ -1479,6 +1480,7 @@ filter_function (GDBusConnection *connection, g_dbus_message_get_path (message), g_dbus_message_get_interface (message), g_dbus_message_get_member (message)); + } if (incoming) { From 8f52d2cb022a2bce6ade375db45f162123552ace Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 5 Sep 2019 14:16:11 +0100 Subject: [PATCH 6/6] gboxed: Fix two potential NULL pointer dereferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I don’t think these could be hit in practice due to the guarantees of the type system, but the static analyser doesn’t know that — so make the assertions clearer to shut it up. Signed-off-by: Philip Withnall Helps: #1767 --- gobject/gboxed.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gobject/gboxed.c b/gobject/gboxed.c index 4404de3d6..87cc5d2c2 100644 --- a/gobject/gboxed.c +++ b/gobject/gboxed.c @@ -346,8 +346,7 @@ g_boxed_copy (GType boxed_type, g_return_val_if_fail (src_boxed != NULL, NULL); value_table = g_type_value_table_peek (boxed_type); - if (!value_table) - g_return_val_if_fail (G_TYPE_IS_VALUE_TYPE (boxed_type), NULL); + g_assert (value_table != NULL); /* check if our proxying implementation is used, we can short-cut here */ if (value_table->value_copy == boxed_proxy_value_copy) @@ -404,8 +403,7 @@ g_boxed_free (GType boxed_type, g_return_if_fail (boxed != NULL); value_table = g_type_value_table_peek (boxed_type); - if (!value_table) - g_return_if_fail (G_TYPE_IS_VALUE_TYPE (boxed_type)); + g_assert (value_table != NULL); /* check if our proxying implementation is used, we can short-cut here */ if (value_table->value_free == boxed_proxy_value_free)