Re: commit c70fe01eeffe530040004eae26c655dd1682f1bf

[Ben Pfaff]

John Darrington <address@hidden> writes:

> commit c70fe01eeffe530040004eae26c655dd1682f1bf
> Author: Ben Pfaff <address@hidden>
> Date:   Sun Jul 8 23:27:52 2012 -0700
>
>     psppire-data-editor: Unref old data_window upon change.
>     
>     Otherwise there's a reference leak.
>     
>     Found by inspection; I haven't looked for clients that actually
>     change the data window after setting it.
>
> diff --git a/src/ui/gui/psppire-data-editor.c 
> b/src/ui/gui/psppire-data-editor.c
> index 16b1393..aae4369 100644
> --- a/src/ui/gui/psppire-data-editor.c
> +++ b/src/ui/gui/psppire-data-editor.c
> @@ -385,6 +385,8 @@ psppire_data_editor_set_property (GObject         *object,
>        psppire_data_editor_split_window (de, g_value_get_boolean (value));
>        break;
>      case PROP_DATA_WINDOW:
> +      if (de->data_window)
> +        g_object_unref (de->data_window);
>        de->data_window = g_value_get_pointer (value);
>        g_object_ref (de->data_window);
>        break;
>
>
> There doesn't seem to be anything initialising de->data_window
> to NULL so there is still a potential leak here.

A leak, or passing a bad pointer to g_object_unref()?

> However I don't see a need to keep a reference here at
> all. ->data_window is used only by the do_sort function, and
> that cannot be called unles the window is realized.

That sounds like a better approach, then, as long as we add a
comment somewhere that says why we don't need a reference.  Do
you want to make that change?