[Glade-devel] small patch

[pborelli katamail com (Paolo Borelli)]

On Wed, 2004-01-14 at 07:35, Sridhar R wrote:
> --- Paolo Borelli <pborelli katamail com> wrote:
> > On Tue, 2004-01-13 at 14:10, Sridhar R wrote:
> > > Hi,
> >
> > Hi Sridhar,
> >     I've not tried the patch (yet), but if I understand
> > it correctly the
> > result is that, e.g. for a GtkButton, the GtkButton
> > signals list node in
> > the tree is expanded.
> > Did I get it right? If yes, I like it :)
>
>   Yes.

Great, anyhow I'll wait with this patch since, as you read in this
thread, Joaquin is reworking the whole signal editor ui.

> > 1) it seems to me that your mailer ate the patch a
> > bit (it breaks some
> > lines). If you use Evolution you can send the patch
> > as an attachment and
> > it will be sent in plain text, don't know if other
> > mail apps do the
> > same...
>
>    Anyhow, I also (and will) attached the files
> inlined.  Since I am using Yahoo web client, i doubt
> whether this is possible.

Oh sorry, I received two copy of your original message and the first one
had no patch attached. In the second one the patch can be read just
fine.


> > > *** ../../glade3.orig/src/glade-signal-editor.c
> > Mon
> > > Jan 12 21:38:09 2004
> > > --- glade-signal-editor.c       Mon Jan 12
> > 22:04:06
> > > 2004
> > > ***************
> > > *** 158,161 ****
> > > --- 158,162 ----
> > >         GtkTreeIter *parent = NULL;
> > >         GList *list = NULL;
> > > +       GtkTreePath *path_first = NULL;
> > >         GladeWidgetClassSignal *signal;
> > >     
> >
> > No need to initialize local vars to NULL as far as I
> > can see, while you
> > are at it remove it also from the other vars;
> > beside, not related to
> > your code, but usually a TreeIter is allocated on
> > the stack: i.e.
> > GtkTreeIter iter;
> > gtk_tree_functio (..., &iter, ...);
>
>    I don't understand.
About the variables declaration you can omit the "= NULL;".

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.

> > > ***************
> > > *** 183,186 ****
> > > --- 188,193 ----
> > >         gint response;
> > >   
> > > +       g_assert(editor);
> > > +       g_assert(editor->class);
> > >         g_return_if_fail (editor->class->signals
> > !=
> > > NULL);
> > >   
> >
> > Use g_return_if_fail instead of assert to check
> > function args.
>
>   No.  If you have seen my bug report on glade3, the
> program actually segfaults.  But with these assert
> statement, it shows the correct message as 'assert
> failure', saving minutes of fiddling up with gdb.
>   Any thoughts?

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.


ciao
        paolo