Re: Minor(ish) changes to PsppireValueEntry

[Ben Pfaff]

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.