[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Minor(ish) changes to PsppireValueEntry
From: |
Ben Pfaff |
Subject: |
Re: Minor(ish) changes to PsppireValueEntry |
Date: |
Mon, 23 Apr 2012 21:07:53 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
John Darrington <address@hidden> writes:
> On Sun, Apr 22, 2012 at 04:37:11PM -0700, Ben Pfaff wrote:
> > + old_model = gtk_combo_box_get_model (GTK_COMBO_BOX (obj));
> > +
> > + if (old_model != model)
> > + {
> > + GtkEntry *entry = GTK_ENTRY (gtk_bin_get_child (GTK_BIN (obj)));
> > + gtk_entry_set_text (entry, "");
> > + }
> > +
> > + if (old_model)
> > + g_object_unref (old_model);
>
> gtk_combo_box_get_model() doesn't ref the returned pointer, so
> I'm pretty sure this "unref" call is wrong.
>
> But the model is created with gtk_list_store_new which returns
> the object with a ref count of 1. Something needs to unref it
> before the model is replaced with a new one.
I see from the implementation of gtk_combo_box_set_model() that
it refs the passed-in model. I didn't expect that; I thought
that it would take ownership from the caller.
I think that, instead of unrefing the old model, we should unref
the new model after setting it, like this:
gtk_combo_box_set_model (GTK_COMBO_BOX (obj), model);
if (model != NULL)
g_object_unref (model);
That way we transfer ownership to the combo box so that, when the
combo box is either destroyed or a new model is set, the old
model gets destroyed properly.
I suspect that most uses of *_set_model functions in the GUI are
wrong in the same way. Running a "grep" shows that the first one
in alphabetical order, in aggregate-dialog.c, needs an unref
call, for example. I'll try to send out a patch.
I'll look at the new patches too.
Thanks,
Ben.
- Re: Minor(ish) changes to PsppireValueEntry, (continued)
- Re: Minor(ish) changes to PsppireValueEntry, John Darrington, 2012/04/23
- Re: Minor(ish) changes to PsppireValueEntry, Ben Pfaff, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry, Ben Pfaff, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry, John Darrington, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry, Ben Pfaff, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry, Ben Pfaff, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry, Ben Pfaff, 2012/04/24
- Re: Minor(ish) changes to PsppireValueEntry,
Ben Pfaff <=
Re: Minor(ish) changes to PsppireValueEntry, John Darrington, 2012/04/24