Re: src/toolbar-factory.c:320

[Pawel Salek <pawsa TheoChem kth se>]

On 2001-07-20 19:18 M . Thielker wrote:
> On 2001.07.20 17:32 Pawel Salek wrote:
> > I would like to get your comment on
> src/toolbar-factory.c(get_factory)
> > function. I have impression it is incorrectly implemented. In
> > particular, the following part makes me suspicious:
> > (around line 320).
> > 
> >     for(i=0; i<toolbar_map_entries; i++) {
> > 	if(toolbar_map[i].window == window && toolbar_map[i].type ==
> > toolbar)
> > 	    break;
> >     }
> > 
> > This code tries to find if there is an existing toolbar for given
> window
> > (identified by the pointer) of given type toolbar. IF the entry is
> > found, the toolbar is destroyed and later recreated.
> > 
> No, it shouldn't get destroyed. It looks for a toolbar already
> existing for the given window. If one is found, It's _emptied_, but
not destroyed.

Well, the bug report I pointed to clearly shows that it *does* get
destroyed because the gtk_type_check_object_cast() fails.

http://bugzilla.gnome.org/show_bug.cgi?id=57708

Also, it is a bad implementation to destroy GtkToolbarChildren by hand.
Destroying GtkToolbar object would have the same effect and would not
rely on the GtkToolbar implementation details. The GtkTolbar should be
treated as opaque object.

> > Also, release_toolbars() is broken (imagine situation when two
> compose
> > windows are created, first A than B; subsequently, A is destroyed
> and
> > then B).
> > 
> Well, at least not conceptually. When window A is created, a table
> slot and toolbar is allocated for the window. 

I apologise, I haven't paid attention to the fact that the upper limit
of the loop is changed within it.

/Pawel
-- 
Pawel Salek (pawsa@theochem.kth.se) http://www.theochem.kth.se/~pawsa/
Theoretical Chemistry Division, KTH voice: +46 8 790-8202