Re: [PATCH v3 1/2] tools: port perf ui from GTK 2 to GTK 4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Non-atomic GSList updates race with traversal in signal handler, risking list corruption and crashes.
- [Low] The GTK 4 port incorrectly uses `gtk_widget_show` and `gtk_widget_hide`, which were completely removed in GTK 4, leading to build and linkage failures.
--

commit 42bf24edb84bb238d5385cd7669415baed4ad796
Author: Matt Turner <mattst88@gmail.com>

tools: port perf ui from GTK 2 to GTK 4

This patch ports the perf GTK UI directly to GTK 4, migrating widget APIs,
monitor geometry access, and event loops. It also removes GTK 2 compatibility
and converts manual loops to signal-driven GMainLoops.

> diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
> index d2dadf3873fbd..e1d2869458329 100644
> --- a/tools/perf/ui/gtk/browser.c
> +++ b/tools/perf/ui/gtk/browser.c
[ ... ]
>  GtkWidget *perf_gtk__setup_info_bar(void)
>  {
>  	GtkWidget *info_bar;
>  	GtkWidget *label;
>  
>  	info_bar = gtk_info_bar_new();
> -	gtk_widget_set_no_show_all(info_bar, TRUE);
> +	gtk_widget_set_visible(info_bar, FALSE);
>  
>  	label = gtk_label_new("");
>  	gtk_widget_show(label);

[Severity: Low]
Does this successfully compile against GTK 4? It appears gtk_widget_show()
was removed in GTK 4, but this line remains unchanged. Should this be
migrated to gtk_widget_set_visible() instead?

>  
> -	content_area = gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
> -	gtk_container_add(GTK_CONTAINER(content_area), label);
> +	gtk_info_bar_add_child(GTK_INFO_BAR(info_bar), label);
>  
> -	gtk_info_bar_add_button(GTK_INFO_BAR(info_bar), GTK_STOCK_OK,
> +	gtk_info_bar_add_button(GTK_INFO_BAR(info_bar), "_OK",
>  				GTK_RESPONSE_OK);
>  	g_signal_connect(info_bar, "response",
>  			 G_CALLBACK(gtk_widget_hide), NULL);

[Severity: Low]
Similarly, gtk_widget_hide() is passed as a signal callback here, but it
was also removed in GTK 4. Could this cause linkage or build failures?

[ ... ]
> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> index c47f5c387838c..8a922851257e7 100644
> --- a/tools/perf/ui/gtk/util.c
> +++ b/tools/perf/ui/gtk/util.c
[ ... ]
> +static GSList *perf_gtk__error_loops;
> +
> +static void perf_gtk__quit_loop(gpointer data, gpointer user_data __maybe_unused)
> +{
> +	g_main_loop_quit(data);
> +}
> +
> +void perf_gtk__quit_error_dialog(void)
> +{
> +	g_slist_foreach(perf_gtk__error_loops, perf_gtk__quit_loop, NULL);
> +}
[ ... ]
>  static int perf_gtk__error(const char *format, va_list args)
>  {
[ ... ]
> +	loop = g_main_loop_new(NULL, FALSE);
> +	perf_gtk__error_loops = g_slist_prepend(perf_gtk__error_loops, loop);
> +	g_signal_connect(dialog, "response",
> +			 G_CALLBACK(perf_gtk__dialog_response), NULL);
> +	g_signal_connect_swapped(dialog, "destroy",
> +				 G_CALLBACK(g_main_loop_quit), loop);
> +
> +	gtk_widget_set_visible(dialog, TRUE);
> +	g_main_loop_run(loop);
> +	perf_gtk__error_loops = g_slist_remove(perf_gtk__error_loops, loop);
> +	g_main_loop_unref(loop);
> +
>  	free(msg);
>  	return 0;
>  }

[Severity: Medium]
Can these non-atomic list updates to perf_gtk__error_loops race with the
signal handler?

If a user presses Ctrl+C (sending SIGINT) at the exact moment the
application is inside perf_gtk__error() modifying the list via
g_slist_prepend() or g_slist_remove(), the async signal handler path
is triggered:

perf_gtk__signal() ->
  perf_gtk__exit() ->
    perf_gtk__quit_error_dialog() ->
      g_slist_foreach(perf_gtk__error_loops, ...)
      
Could this traversal crash or enter an infinite loop due to the interrupted
list modifications?

[ ... ]
>  static int perf_gtk__warning_info_bar(const char *format, va_list args)
>  {
[ ... ]
>  	gtk_label_set_text(GTK_LABEL(pgctx->message_label), msg);
>  	gtk_info_bar_set_message_type(GTK_INFO_BAR(pgctx->info_bar),
>  				      GTK_MESSAGE_WARNING);
>  	gtk_widget_show(pgctx->info_bar);
>  
>  	free(msg);
>  	return 0;
>  }

[Severity: Low]
Does this call to gtk_widget_show() also need to be replaced with
gtk_widget_set_visible() for the GTK 4 build to succeed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-perf-gtk2-v3-0-e1f2086214a0@gmail.com?part=1