[Glade-devel] small patch

[pborelli katamail com (Paolo Borelli)]

On Wed, 2004-01-14 at 10:56, Sridhar R wrote:
> --
> > About the treeiter: I took a closer look at the code
> > and in this case I
> > have to admit the using a pointer to the iter is
> > correct since
> > glade_signal_editor_dialog_append_signal returns a
> > iter allocated in the
> > heap memory with g_new0. Not your fault, but as far
> > as I can see this
> > memory is leaked since there isn't a g_free (iter).
> >
> > In general, if you look at the docs in gtk you'll
> > see that most of the
> > times, even if the iter is an object, it is used as
> > a local variable,
> > not as a pointer: i.e. GtkTreeIter iter; this means
> > that the memory for
> > the object is automatically allocated on the stack
> > when the function is
> > called and automatically freed when the function
> > ends. Of course since
> > some of the API need to get a pointer to iter, you
> > pass them the var
> > address, i.e. &iter.
> > In our case using this approach would mean modify
> > the
> > glade_signal_editor_dialog_append_signal function to
> > take another iter
> > pointer as parameter instead of returning a newly
> > allocated iter, and
> > pass &parent to it.
> >
> > Hope this clear things a bit.
>
>   Oh! I got the problem. I should have used 
> gtk_tree_model_get_iter_first, instead of the one I
> used before.

Maybe, but it isn't what I was talking about. As a matter of fact the
problem I was pointing out is not related to your patch at all, it is
just in the same function you are modifying.

What I was talking about is a memory management problem: in that
function (even before your patch) the GtkTreeIter structure is allocated
but never freed, so the memory it takes is leaked.
The short way is add a g_free (parent) before the function end.
The long way is rework the api so that the iter is a simple local
variable instead of pointer allocated with g_new0, since this is similar
to how GtkTreeIter are used usually in gtk.

> > g_return_if_fail *are* asserts made exactly for this
> > use; when they fail
> > they print a critical message on the console and
> > then return so that the
> > function is not executed.
>
>    But without the above two g_assert statements, the
> statement
>    g_return_if_fail (editor->class->signals);
>   _will_ cause segfault, if either `editor` or `class`
> is NULL!

I meant you should add
 g_return_if_fail (editor);
 g_return_if_fail (editor->class);
instead of the asserts.

Anyway both the asserts and the g_return_if_fails are just sanity checks
that get triggered when something is wrong. If the program segfaults
because of a pointer gone wild, we have to track down *why* it happens
and fix it, not just fix the symptoms.

ciao
        paolo