After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 673555 - gnome-settings-daemon unrefs member objects twice when dispose
gnome-settings-daemon unrefs member objects twice when dispose
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: general
3.4.x
Other Linux
: Normal trivial
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-04-05 07:58 UTC by Daiki Ueno
Modified: 2012-05-23 14:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.24 KB, patch)
2012-04-05 07:58 UTC, Daiki Ueno
none Details | Review
patch (1.01 KB, patch)
2012-05-17 05:12 UTC, Daiki Ueno
committed Details | Review

Description Daiki Ueno 2012-04-05 07:58:54 UTC
Created attachment 211356 [details] [review]
patch

gnome_settings_manager_stop() is called from gnome_settings_manager_dispose(), which may be called multiple times.  However, there lacks non-NULL checks before g_object_unref().
Comment 1 Matthias Clasen 2012-05-17 03:29:49 UTC
Could just use g_clear_object here
Comment 2 Daiki Ueno 2012-05-17 05:12:38 UTC
Created attachment 214234 [details] [review]
patch
Comment 3 Bastien Nocera 2012-05-17 16:02:47 UTC
Review of attachment 214234 [details] [review]:

In the docs for g_clear_object: "object_ptr must not be NULL."
Comment 4 Bastien Nocera 2012-05-17 16:05:27 UTC
Fixed the problem mentioned and committed to master and gnome-3-4
Comment 5 Daiki Ueno 2012-05-17 23:23:48 UTC
(In reply to comment #3)
> Review of attachment 214234 [details] [review]:
> 
> In the docs for g_clear_object: "object_ptr must not be NULL."

I wrote:

g_clear_object (&manager->priv->settings);

Note that it is a pointer to a struct member and the pointer will never be NULL.

You changed it to:

if (manager->priv->settings)
  g_clear_object (&manager->priv->settings);

The NULL check is redundant.
Comment 6 Daiki Ueno 2012-05-18 02:13:08 UTC
I meant, the doc says "object_ptr must not be NULL" not "*object_ptr must not be NULL", like the way passing around GError **.
Comment 7 Daiki Ueno 2012-05-23 02:11:26 UTC
Sorry for reopening such a minor issue, but I think the wrong commit should be fixed, as it will perhaps make the g_clear_object usage misleading.
Comment 8 Bastien Nocera 2012-05-23 14:16:22 UTC
commit ea8f5d1e00fad8e526af243a89de1d5aff88e4e8
Author: Daiki Ueno <ueno@unixuser.org>
Date:   Wed May 23 15:15:16 2012 +0100

    daemon: Fix g_clear_object() usage
    
    https://bugzilla.gnome.org/show_bug.cgi?id=673555