From ea014f24ebe5b8d3c5311a07dc971a8afaed76b7 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 14 May 2024 14:05:14 +0100 Subject: [PATCH] general: Fix various strict-aliasing warnings with g_clear_pointer() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is just unfortunate. It’s an aliasing violation to cast a pointer to a pointer (and there’s no way round that), although in practice it will not cause a problem. People do quite often compile with `-Werror=strict-aliasing`, though, so fixing the warnings is helpful. Warnings are of the form: ``` ../../source/gnome-control-center/panels/keyboard/cc-keyboard-shortcut-dialog.c: In function ‘cc_keyboard_shortcut_dialog_finalize’: ../../source/gnome-control-center/panels/keyboard/cc-keyboard-shortcut-dialog.c:518:20: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] 518 | g_clear_pointer ((GtkWindow**)&self->shortcut_editor, gtk_window_destroy); /opt/gnome/install/include/glib-2.0/glib/gmacros.h:871:47: note: in definition of macro ‘G_STATIC_ASSERT’ 871 | #define G_STATIC_ASSERT(expr) _Static_assert (expr, "Expression evaluates to false") | ^~~~ ../../source/gnome-control-center/panels/keyboard/cc-keyboard-shortcut-dialog.c:518:3: note: in expansion of macro ‘g_clear_pointer’ 518 | g_clear_pointer ((GtkWindow**)&self->shortcut_editor, gtk_window_destroy); | ^~~~~~~~~~~~~~~ In file included from /opt/gnome/install/include/glib-2.0/glib/gatomic.h:30, from /opt/gnome/install/include/glib-2.0/glib/gthread.h:34, from /opt/gnome/install/include/glib-2.0/glib/gasyncqueue.h:34, from /opt/gnome/install/include/glib-2.0/glib.h:34: ../../source/gnome-control-center/panels/keyboard/cc-keyboard-shortcut-dialog.c:518:20: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] 518 | g_clear_pointer ((GtkWindow**)&self->shortcut_editor, gtk_window_destroy); /opt/gnome/install/include/glib-2.0/glib/glib-typeof.h:39:36: note: in definition of macro ‘glib_typeof’ 39 | #define glib_typeof(t) __typeof__ (t) | ^ ../../source/gnome-control-center/panels/keyboard/cc-keyboard-shortcut-dialog.c:518:3: note: in expansion of macro ‘g_clear_pointer’ 518 | g_clear_pointer ((GtkWindow**)&self->shortcut_editor, gtk_window_destroy); | ^~~~~~~~~~~~~~~ cc1: all warnings being treated as errors ``` I believe it’s better to fix these by expanding out the `g_clear_pointer()` call, than by changing the types of variables — the latter approach means everything becomes a `GtkWidget` or a `GtkWindow`, which loses type specificity. So this approach is in contrast to that taken in commit 1bafd46ea32db901e970662fe2c83d6f26d6ee28, for example. Alternative approaches would be: 1. Add internal `cc_clear_window()` and `cc_clear_widget()` helpers which do this in a single line without aliasing violations. 2. Enforce compiling with `-Wno-strict-aliasing` if strict aliasing is not something that g-c-c maintainers want to care about (which would be fine, aliasing checks probably won’t catch any bugs in this kind of code). Signed-off-by: Philip Withnall Fixes: #2563 --- panels/color/cc-color-panel.c | 6 +++++- panels/common/cc-vertical-row.c | 5 ++++- panels/keyboard/cc-keyboard-shortcut-dialog.c | 6 +++++- .../network/connection-editor/ce-page-8021x-security.c | 6 +++++- panels/privacy/bolt/cc-bolt-page.c | 6 +++++- panels/sound/cc-volume-slider.c | 10 ++++++++-- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/panels/color/cc-color-panel.c b/panels/color/cc-color-panel.c index 127fdb1a4..791abc02d 100644 --- a/panels/color/cc-color-panel.c +++ b/panels/color/cc-color-panel.c @@ -1848,7 +1848,11 @@ cc_color_panel_dispose (GObject *object) g_clear_object (&self->list_box_size); g_clear_pointer (&self->sensors, g_ptr_array_unref); g_clear_pointer (&self->list_box_filter, g_free); - g_clear_pointer ((GtkWindow **)&self->dialog_assign, gtk_window_destroy); + + if (self->dialog_assign != NULL) { + gtk_window_destroy (GTK_WINDOW (self->dialog_assign)); + self->dialog_assign = NULL; + } G_OBJECT_CLASS (cc_color_panel_parent_class)->dispose (object); } diff --git a/panels/common/cc-vertical-row.c b/panels/common/cc-vertical-row.c index 13fa599c4..50f88bfa3 100644 --- a/panels/common/cc-vertical-row.c +++ b/panels/common/cc-vertical-row.c @@ -189,7 +189,10 @@ cc_vertical_row_dispose (GObject *object) } cc_vertical_row_set_activatable_widget (self, NULL); - g_clear_pointer ((GtkWidget**)&priv->header, gtk_widget_unparent); + if (priv->header != NULL) { + gtk_widget_unparent (GTK_WIDGET (priv->header)); + priv->header = NULL; + } G_OBJECT_CLASS (cc_vertical_row_parent_class)->dispose (object); } diff --git a/panels/keyboard/cc-keyboard-shortcut-dialog.c b/panels/keyboard/cc-keyboard-shortcut-dialog.c index 815128781..a950f5d8f 100644 --- a/panels/keyboard/cc-keyboard-shortcut-dialog.c +++ b/panels/keyboard/cc-keyboard-shortcut-dialog.c @@ -515,7 +515,11 @@ cc_keyboard_shortcut_dialog_finalize (GObject *object) g_clear_pointer (&self->search_terms, g_strfreev); g_clear_object (&self->sections); g_clear_object (&self->filtered_shortcuts); - g_clear_pointer ((GtkWindow**)&self->shortcut_editor, gtk_window_destroy); + + if (self->shortcut_editor != NULL) { + gtk_window_destroy (GTK_WINDOW (self->shortcut_editor)); + self->shortcut_editor = NULL; + } G_OBJECT_CLASS (cc_keyboard_shortcut_dialog_parent_class)->finalize (object); } diff --git a/panels/network/connection-editor/ce-page-8021x-security.c b/panels/network/connection-editor/ce-page-8021x-security.c index 3826946af..402d4d9b8 100644 --- a/panels/network/connection-editor/ce-page-8021x-security.c +++ b/panels/network/connection-editor/ce-page-8021x-security.c @@ -154,9 +154,13 @@ ce_page_8021x_security_dispose (GObject *object) CEPage8021xSecurity *self = CE_PAGE_8021X_SECURITY (object); g_clear_object (&self->connection); - g_clear_pointer ((GtkWidget **) &self->security, gtk_widget_unparent); g_clear_object (&self->group); + if (self->security != NULL) { + gtk_widget_unparent (GTK_WIDGET (self->security)); + self->security = NULL; + } + G_OBJECT_CLASS (ce_page_8021x_security_parent_class)->dispose (object); } diff --git a/panels/privacy/bolt/cc-bolt-page.c b/panels/privacy/bolt/cc-bolt-page.c index f374861d9..26fb31799 100644 --- a/panels/privacy/bolt/cc-bolt-page.c +++ b/panels/privacy/bolt/cc-bolt-page.c @@ -936,7 +936,11 @@ cc_bolt_page_dispose (GObject *object) /* Must be destroyed in dispose, not finalize. */ cc_bolt_device_dialog_set_device (self->device_dialog, NULL, NULL); - g_clear_pointer ((GtkWindow **) &self->device_dialog, gtk_window_destroy); + + if (self->device_dialog != NULL) { + gtk_window_destroy (GTK_WINDOW (self->device_dialog)); + self->device_dialog = NULL; + } G_OBJECT_CLASS (cc_bolt_page_parent_class)->dispose (object); } diff --git a/panels/sound/cc-volume-slider.c b/panels/sound/cc-volume-slider.c index 977b7c030..87e3c2b52 100644 --- a/panels/sound/cc-volume-slider.c +++ b/panels/sound/cc-volume-slider.c @@ -204,8 +204,14 @@ cc_volume_slider_dispose (GObject *object) { CcVolumeSlider *self = CC_VOLUME_SLIDER (object); - g_clear_pointer ((GtkWidget **) &self->volume_scale, gtk_widget_unparent); - g_clear_pointer ((GtkWidget **) &self->mute_button, gtk_widget_unparent); + if (self->volume_scale != NULL) { + gtk_widget_unparent (GTK_WIDGET (self->volume_scale)); + self->volume_scale = NULL; + } + if (self->mute_button != NULL) { + gtk_widget_unparent (GTK_WIDGET (self->mute_button)); + self->mute_button = NULL; + } g_clear_object (&self->mixer_control); g_clear_object (&self->stream);