[XForms] Re: Xforms : popup menus

[laurent FOURNIER]

Jean-Marc Lasgouttes wrote:

>Sorry for not answering earlier to your message. I wanted to take a
>look at the code first. I'd like to settle this popup issue before
>1.1.0, but I am somhow not satisfied with the solution of using memset
>(why? I do not know precisely :)
>  
>
I agree with your point of view. The use of memset() is only a way to 
ensure data initialization. As a rule of thumb we develop our code 
assuming that a NULL pointer is free, then we use to call calloc(), or 
realloc() with a memset() on any additional data. The best way would be 
to use the exact amount of memory we need for the menu item array.

>What particular data should have been set to zero and caused problem? 
>At least the pointers are correctly reset. Was the problem related to
>callbacks? 
>  
>
The data what causes problems is the menu item array, when setting 
fl_maxpup to a higher value than FL_MAXPUP with fl_setpup_maxpup(), i.e. 
the pointer to menu items and the menu items themselves are not NULL. 
When calling fl_setpup_maxpup(),  the default array of popup menus is 
resized and then reallocated. But at the initialization time (the very 
first call to fl_init_pup), the first allocation is made using a 
calloc() which sets all data to zero, and in particular the pointer 
"menu_rec->item". When not set to zero, we get garbage into the pointer 
array which leads to invalid data segments on the array (and thus menu 
items), and LynxOS gives a SIGSEGV when freeing any segment beginning 
with '9' (these are reserved adresses for the kernel but this is not the 
only case with this very sensitive OS).

Thus, the problem is not directly related to callbacks, even if 
fl_freepup() is called on FL_FREEMEM events.

>I agree that memset is not expensive. I would just like to understand
>where the real problem was, in order to be sure that we do not miss
>something. 
>  
>
If you prefer not to call memset(), one solution to the real problem 
could be :
1 - to ensure proper initialization of unused pointers (especially 
menu_rec->item of course) when calling fl_setpup_maxpup(),
2 - to limit the loop counter in the function fl_freepup() to p->nitem 
instead of  FL_MAXPUPI (line 1157 in v0.9999/lib/xpopup.c) because 
p->nitem handles the number of actually allocated menu item structures 
(i.e. valid pointers).

Anyhow, fl_safe_free() checks only NULL pointers and is not enough for 
invalid pointers unless you ensure that a NULL pointer is free. This is 
the main reason for crashes under LynxOS with the so-called '9' segment. 
Moreover, memset() gives a slightly shorter code because we do not have 
to set many fields to 0 (we have limited space on Lynx CPUs).

>I am not sure whether automatic reallocation is really a good thing.
>It may bite you if you have a popup leak (which may happen easily).
>  
>
This is quite true. My application works fine when allocating a (big) 
default number of popups (and a call to memset() at this time). The only 
thing what I have to assume is to ensure enough (initialized) memory 
space. Moreover, reallocating memory for popups during the running phase 
seems not to be a problem. But in that case I need to gain access to the 
static variable fl_max_pup to count the number of popups and get more 
space when needed.

I thank you for your kind attention on this problem...  I thoroughly 
look for version 1.1 !
Amicalement,
Laurent.