From be30f78e62ade655b03903e1c6075f033d74cf39 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 04:24:30 +0100 Subject: [PATCH 01/12] treeview: Don't initialize variable twice --- gtk/gtktreeview.c | 1 - 1 file changed, 1 deletion(-) diff --git a/gtk/gtktreeview.c b/gtk/gtktreeview.c index a966c8ef0e..24cb0d8726 100644 --- a/gtk/gtktreeview.c +++ b/gtk/gtktreeview.c @@ -2823,7 +2823,6 @@ gtk_tree_view_click_gesture_pressed (GtkGestureClick *gesture, GList *list; gboolean rtl; - rtl = (_gtk_widget_get_direction (widget) == GTK_TEXT_DIR_RTL); gtk_tree_view_stop_editing (tree_view, FALSE); button = gtk_gesture_single_get_current_button (GTK_GESTURE_SINGLE (gesture)); From b231a401063ce016fef4cb5f9eea906d8f3a9e9c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 04:31:08 +0100 Subject: [PATCH 02/12] testsuite: Avoid passing NULL to strcmp() Use g_strcmp0() instead. --- testsuite/gtk/action.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/testsuite/gtk/action.c b/testsuite/gtk/action.c index 056e60fc80..23a78fa23e 100644 --- a/testsuite/gtk/action.c +++ b/testsuite/gtk/action.c @@ -379,10 +379,8 @@ test_introspection (void) { g_assert (expected[i].owner == owner); g_assert (strcmp (expected[i].name, name) == 0); - g_assert ((expected[i].params == NULL && params == NULL) || - strcmp (expected[i].params, g_variant_type_peek_string (params)) == 0); - g_assert ((expected[i].property == NULL && property == NULL) || - strcmp (expected[i].property, property) == 0); + g_assert (g_strcmp0 (expected[i].params, params ? g_variant_type_peek_string (params) : NULL) == 0); + g_assert (g_strcmp0 (expected[i].property, property) == 0); i++; } g_assert (i == G_N_ELEMENTS (expected)); From 03e7c7fab18c6be39b6089c0db9ca35ef39fd142 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 04:31:30 +0100 Subject: [PATCH 03/12] treepath: Use g_renew() --- gtk/gtktreemodel.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gtk/gtktreemodel.c b/gtk/gtktreemodel.c index 226791ab5a..6a34674749 100644 --- a/gtk/gtktreemodel.c +++ b/gtk/gtktreemodel.c @@ -802,12 +802,8 @@ gtk_tree_path_append_index (GtkTreePath *path, if (path->depth == path->alloc) { - gint *indices; path->alloc = MAX (path->alloc * 2, 1); - indices = g_new (gint, path->alloc); - memcpy (indices, path->indices, path->depth * sizeof (gint)); - g_free (path->indices); - path->indices = indices; + path->indices = g_renew (gint, path->indices, path->alloc); } path->depth += 1; From 3b8d9dbd281a175dca74e9713d78dfd1742f2378 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:01:01 +0100 Subject: [PATCH 04/12] testtreeview: Fix 19 year old use-after-free --- tests/testtreeview.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testtreeview.c b/tests/testtreeview.c index 8df5ca4925..af670e313d 100644 --- a/tests/testtreeview.c +++ b/tests/testtreeview.c @@ -1019,8 +1019,8 @@ gtk_real_model_types_iter_next (GtkTreeModel *tree_model, if (children[i] != G_TYPE_INVALID) { - g_free (children); iter->user_data = GINT_TO_POINTER (children[i]); + g_free (children); return TRUE; } else From e37729756de14b4681a18f219fc8f383bfbd8a05 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:03:45 +0100 Subject: [PATCH 05/12] liststore: Fix gtk_list_store_iter_is_valid() The iter may be invalid, so we may not read from it. testsuite/gtk/treemodel tests this and valgrind is shouting about it, but it never crashed until I just ran it... This bug is from 2004 and the test is from 2007. I guess invalid memory accesses don't get caught by CI much. --- gtk/gtkliststore.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/gtk/gtkliststore.c b/gtk/gtkliststore.c index 8b582fd526..8c488fabed 100644 --- a/gtk/gtkliststore.c +++ b/gtk/gtkliststore.c @@ -1448,10 +1448,34 @@ gboolean gtk_list_store_iter_is_valid (GtkListStore *list_store, GtkTreeIter *iter) { + GtkListStorePrivate *priv; + GSequenceIter *seq_iter; + g_return_val_if_fail (GTK_IS_LIST_STORE (list_store), FALSE); g_return_val_if_fail (iter != NULL, FALSE); - return iter_is_valid (iter, list_store); + /* can't use iter_is_valid() here, because iter might point + * to random memory. + * + * We MUST NOT dereference it. + */ + + priv = list_store->priv; + + if (iter == NULL || + iter->user_data == NULL || + priv->stamp != iter->stamp) + return FALSE; + + for (seq_iter = g_sequence_get_begin_iter (priv->seq); + !g_sequence_iter_is_end (seq_iter); + seq_iter = g_sequence_iter_next (seq_iter)) + { + if (seq_iter == iter->user_data) + return TRUE; + } + + return FALSE; } static gboolean real_gtk_list_store_row_draggable (GtkTreeDragSource *drag_source, From df282a13bbd920c3aedf73bd0e6a93bb69841721 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:06:25 +0100 Subject: [PATCH 06/12] applicationaccels: Use g_renew() --- gtk/gtkapplicationaccels.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gtk/gtkapplicationaccels.c b/gtk/gtkapplicationaccels.c index b4a14b9daa..a27501facf 100644 --- a/gtk/gtkapplicationaccels.c +++ b/gtk/gtkapplicationaccels.c @@ -96,8 +96,7 @@ add_entry (GtkApplicationAccels *accels, else n = 0; - new = g_new (const gchar *, n + 1 + 1); - memcpy (new, old, n * sizeof (const gchar *)); + new = g_renew (const gchar *, old, n + 1 + 1); new[n] = action_and_target; new[n + 1] = NULL; From 317dcddddb76d2f87eee08e2b929b1b901c23aaa Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:29:46 +0100 Subject: [PATCH 07/12] builder-tool: Don't allow property to be both resize and shrink Otherwise builder-tool crashes when you do 1 Thanks to the static analyzer for figuring that one out. --- gtk/tools/gtk-builder-tool-simplify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtk/tools/gtk-builder-tool-simplify.c b/gtk/tools/gtk-builder-tool-simplify.c index a2b42d135b..32cda19afd 100644 --- a/gtk/tools/gtk-builder-tool-simplify.c +++ b/gtk/tools/gtk-builder-tool-simplify.c @@ -1215,7 +1215,7 @@ rewrite_paned_child (Element *element, if (g_str_equal (elt2->element_name, "property") && has_attribute (elt2, "name", "resize")) resize = elt2; - if (g_str_equal (elt2->element_name, "property") && + else if (g_str_equal (elt2->element_name, "property") && has_attribute (elt2, "name", "shrink")) shrink = elt2; } From 510e17d123391d8121ec5eb1ac0602843b7f224e Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:50:35 +0100 Subject: [PATCH 08/12] cellarea: Be very clear The static analyzer needs to know we absolutely DO NOT want to use this return value. --- gtk/gtkcellarea.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gtk/gtkcellarea.c b/gtk/gtkcellarea.c index e9b10bd355..157c3bc5ed 100644 --- a/gtk/gtkcellarea.c +++ b/gtk/gtkcellarea.c @@ -2950,7 +2950,9 @@ gtk_cell_area_add_focus_sibling (GtkCellArea *area, siblings = g_hash_table_lookup (priv->focus_siblings, renderer); if (siblings) - siblings = g_list_append (siblings, sibling); + { + G_GNUC_UNUSED GList *unused = g_list_append (siblings, sibling); + } else { siblings = g_list_append (siblings, sibling); From a106b54355f684a615758141e050ec4024c0eed2 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:51:37 +0100 Subject: [PATCH 09/12] treeview: Don't assign value twice. --- gtk/gtktreeview.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/gtk/gtktreeview.c b/gtk/gtktreeview.c index 24cb0d8726..a30d0c0973 100644 --- a/gtk/gtktreeview.c +++ b/gtk/gtktreeview.c @@ -7378,8 +7378,6 @@ gtk_tree_view_drag_data_received (GObject *source, gboolean drop_append_mode; const GValue *value; - suggested_action = 0; - value = gdk_drop_read_value_finish (drop, result, NULL); if (value == NULL) return; From b1a257c0c309a577f417e09ce922f88ad32d78a6 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:52:44 +0100 Subject: [PATCH 10/12] reftest: Plug memleak --- testsuite/reftests/gtk-reftest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/reftests/gtk-reftest.c b/testsuite/reftests/gtk-reftest.c index f54244db3e..9ddd0dab1e 100644 --- a/testsuite/reftests/gtk-reftest.c +++ b/testsuite/reftests/gtk-reftest.c @@ -172,7 +172,7 @@ get_test_file (const char *test_file, get_components_of_test_file (test_file, &dir, &base); - file = g_string_new (dir); + g_string_append (file, dir); g_string_append (file, G_DIR_SEPARATOR_S); g_string_append (file, base); g_string_append (file, extension); From 394955cceb6b1c9f2146a19d1d1da92eff3d8678 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:53:20 +0100 Subject: [PATCH 11/12] pathbar: Don't do the same thing twice. Don't do the same thing twice. --- gtk/gtkpathbar.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/gtk/gtkpathbar.c b/gtk/gtkpathbar.c index b257e1b4d1..6e1599b69e 100644 --- a/gtk/gtkpathbar.c +++ b/gtk/gtkpathbar.c @@ -201,8 +201,6 @@ gtk_path_bar_init (GtkPathBar *path_bar) GtkPathBarPrivate *priv = gtk_path_bar_get_instance_private (path_bar); GtkEventController *controller; - priv = gtk_path_bar_get_instance_private (path_bar); - priv->up_slider_button = gtk_button_new_from_icon_name ("pan-start-symbolic"); gtk_widget_set_parent (priv->up_slider_button, GTK_WIDGET (path_bar)); From cda9007f0f12b1b436380e93d62b137862bbf3de Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 6 Mar 2020 05:56:29 +0100 Subject: [PATCH 12/12] stack: Make static analyzer happy --- gtk/gtkstack.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gtk/gtkstack.c b/gtk/gtkstack.c index fc6a46347e..887c9cf6c9 100644 --- a/gtk/gtkstack.c +++ b/gtk/gtkstack.c @@ -1301,6 +1301,7 @@ stack_child_visibility_notify_cb (GObject *obj, GtkStackPage *child_info; child_info = find_child_info_for_widget (stack, child); + g_return_if_fail (child_info != NULL); if (priv->visible_child == NULL && gtk_widget_get_visible (child))