Re: Minor(ish) changes to PsppireValueEntry

[Ben Pfaff]

John Darrington <address@hidden> writes:

> On Mon, Apr 23, 2012 at 09:14:36PM -0700, Ben Pfaff wrote:
>      John Darrington <address@hidden> writes:
>      
>      > From 4c2e195ca63240c1f6ad9b6b75d761f54ee94f77 Mon Sep 17 00:00:00 2001
>      > From: John Darrington <address@hidden>
>      > Date: Mon, 23 Apr 2012 21:12:04 +0200
>      > Subject: [PATCH 3/5] PsppireValueEntry: unref old model before setting 
> the new one
>      >
>      > diff --git a/src/ui/gui/psppire-value-entry.c 
> b/src/ui/gui/psppire-value-entry.c
>      > index 85dbaa0..44ad2d0 100644
>      > --- a/src/ui/gui/psppire-value-entry.c
>      > +++ b/src/ui/gui/psppire-value-entry.c
>      > @@ -278,6 +278,8 @@ psppire_value_entry_refresh_model 
> (PsppireValueEntry *obj)
>      >        GtkEntry *entry = GTK_ENTRY (gtk_bin_get_child (GTK_BIN (obj)));
>      >        gtk_entry_set_text (entry, "");
>      >      }
>      > +  else if (old_model)
>      > +    g_object_unref (old_model);
>      >  
>      >    gtk_combo_box_set_model (GTK_COMBO_BOX (obj), model);
>      >    gtk_combo_box_entry_set_text_column (GTK_COMBO_BOX_ENTRY (obj), 
> COL_LABEL);
>      
>      I think that we should unref the new model, e.g. instead add:
>              if (model != NULL)
>                g_object_unref (model);
>      after the gtk_combo_box_set_model() call.  
>
> Unless gtk_combo_box_set_model takes a reference to the model, which I don't 
> think it does,
> then this will cause the model to be immediately destroyed.

My reading of the gtk_combo_box_set_model() code (see below) is
that it takes a reference to the model.

/**
 * gtk_combo_box_set_model:
 * @combo_box: A #GtkComboBox
 * @model: (allow-none): A #GtkTreeModel
 *
 * Sets the model used by @combo_box to be @model. Will unset a previously set
 * model (if applicable). If model is %NULL, then it will unset the model.
 *
 * Note that this function does not clear the cell renderers, you have to 
 * call gtk_cell_layout_clear() yourself if you need to set up different 
 * cell renderers for the new model.
 *
 * Since: 2.4
 */
void
gtk_combo_box_set_model (GtkComboBox  *combo_box,
                         GtkTreeModel *model)
{
  g_return_if_fail (GTK_IS_COMBO_BOX (combo_box));
  g_return_if_fail (model == NULL || GTK_IS_TREE_MODEL (model));

  if (model == combo_box->priv->model)
    return;
  
  gtk_combo_box_unset_model (combo_box);

  if (model == NULL)
    goto out;

  combo_box->priv->model = model;
  g_object_ref (combo_box->priv->model);

  combo_box->priv->inserted_id =
    g_signal_connect (combo_box->priv->model, "row-inserted",
                      G_CALLBACK (gtk_combo_box_model_row_inserted),
                      combo_box);
  combo_box->priv->deleted_id =
    g_signal_connect (combo_box->priv->model, "row-deleted",
                      G_CALLBACK (gtk_combo_box_model_row_deleted),
                      combo_box);
  combo_box->priv->reordered_id =
    g_signal_connect (combo_box->priv->model, "rows-reordered",
                      G_CALLBACK (gtk_combo_box_model_rows_reordered),
                      combo_box);
  combo_box->priv->changed_id =
    g_signal_connect (combo_box->priv->model, "row-changed",
                      G_CALLBACK (gtk_combo_box_model_row_changed),
                      combo_box);
      
  if (combo_box->priv->tree_view)
    {
      /* list mode */
      gtk_tree_view_set_model (GTK_TREE_VIEW (combo_box->priv->tree_view),
                               combo_box->priv->model);
      gtk_combo_box_list_popup_resize (combo_box);
    }
  else
    {
      /* menu mode */
      if (combo_box->priv->popup_widget)
        gtk_combo_box_menu_fill (combo_box);

    }

  if (combo_box->priv->cell_view)
    gtk_cell_view_set_model (GTK_CELL_VIEW (combo_box->priv->cell_view),
                             combo_box->priv->model);

  if (combo_box->priv->active != -1)
    {
      /* If an index was set in advance, apply it now */
      gtk_combo_box_set_active (combo_box, combo_box->priv->active);
      combo_box->priv->active = -1;
    }

out:
  gtk_combo_box_update_sensitivity (combo_box);

  g_object_notify (G_OBJECT (combo_box), "model");
}