Re: src/toolbar-factory.c:320

["M . Thielker" <balsa t-data com>]

Hi,

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.
If none is found, a new one is created.
This is then populated with the current set of buttons for that toolbar
type.

> Unfortunately, there is an assumption that a window is uniquely defined
> by its pointer forever. IT is possible however that the window is
> created, destroyed, and then another window is allocated exactly in the

The idea is that every window using toolbars _must_ call release_toolbars()
in it's destroy handler. That will eliminate the table entry, so if another
window is later created with that same address, a new toolbar will be
allocated.

> 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. When B is created, the same will
happen. It's a completely distinct toolbar, not in any way connected to the
toolbar of window A.
When window A is destroyed, it's toolbar and table slot is freed, without
influencing toolbar B.
Each toolbar is a completely separate entity, no toolbar is ever shared
between windows.

> My opinon is that this code should be rewriten. My first idea would to
> be use lists instead of static arrays but I guess there is more than one
> way to do it.
> 
Lists are OK with me. I didn't stumble upon g_list until after I had written
the table code...

Melanie