Re: [Evolution-hackers] [RFC PATCH] Uninitialized item.type

[Johnny Jacob <jjohnny novell com>]

On Sun, 2006-08-13 at 03:51 -0400, Pavel Roskin wrote: 
> Hello!
> 
> Current Evolution from CVS have become very unstable, and I'm trying to
> fix at least some breakage using Valgrind.
> 
> The first error it has found is jump depending on uninitialized variable
> in e-search-bar.c on line 637:
> 
> switch (items[i].type) {
> 
> Items are created in e-filter-bar.c in function build_items().  I see
> that item.type is initialized to 0 if the "type" argument is 0.
> Otherwise, it's simply not initialized.
> 
> set_option() in the same e-filter-bar.c calls build_items() with type=1.
> 
> I also noticed that ESearchBarItemType is not used in the sources.  I
> think it exists to improve readability of the sources.
> 
> Please be careful with this patch.  I have no idea what "filter-bar" is
> and how to invoke it.
> 
> The patch initializes item.type with type not only when type is 0.  It
> also uses ESearchBarItemType consistently.
> 
> --- widgets/misc/e-filter-bar.c
> +++ widgets/misc/e-filter-bar.c
> @@ -337,7 +337,7 @@ dup_item_no_subitems (ESearchBarItem *de
>  }
>  
>  static GArray *
> -build_items (ESearchBar *esb, ESearchBarItem *items, int type, int *start, GPtrArray *rules)
> +build_items (ESearchBar *esb, ESearchBarItem *items, ESearchBarItemType type, int *start, GPtrArray *rules)
>  {
>  	FilterRule *rule = NULL;
>  	EFilterBar *efb = (EFilterBar *)esb;
> @@ -368,14 +368,14 @@ build_items (ESearchBar *esb, ESearchBar
>  	
>  	*start = id;
>  
> -	if (type == 0) {Thanks for pointing the issue of "uninitialized item.type". 

'type' here denotes the type of the _menu_ to build and not the type of
the _menu item_ :
type == 0 : Search menu with saved searches. (in menu bar)
type == 1 : Search Options menu in search bar.
So "ESearchBarItemType " cannot be used here.

> +	item.type = type;
> +	if (type == ESB_ITEMTYPE_NORMAL) {
>  		source = FILTER_SOURCE_INCOMING;
>  
>  		/* Add a separator if there is at least one custom rule.  */
>  		if (rule_context_next_rule (efb->context, rule, source) != NULL) {
>  			item.id = 0;
>  			item.text = NULL;
> -			item.type = 0;
>  			g_array_append_vals (menu, &item, 1);
>  		}
>  	} else {
> @@ -386,7 +386,7 @@ build_items (ESearchBar *esb, ESearchBar
>  	while ((rule = rule_context_next_rule (efb->context, rule, source))) {
>  		item.id = id++;
>  
> -		if (type == 0 && num <= 10) {
> +		if (type == ESB_ITEMTYPE_NORMAL && num <= 10) {
>  			item.text = g_strdup_printf ("_%d. %s", num % 10, rule->name);
>  			num ++;
>  		} else {
> @@ -418,7 +418,7 @@ build_items (ESearchBar *esb, ESearchBar
>  	}
>  	
>  	/* always add on the advanced menu */
> -	if (type == 1) {
> +	if (type == ESB_ITEMTYPE_CHECK) {
>  		ESearchBarItem sb_items[2] = { E_FILTERBAR_SEPARATOR, E_FILTERBAR_ADVANCED, 
>  					       /* E_FILTERBAR_SEPARATOR, E_FILTERBAR_SAVE */ };
>  		ESearchBarItem dup_items[2];
> @@ -458,7 +458,7 @@ generate_menu (ESearchBar *esb, ESearchB
>  	EFilterBar *efb = (EFilterBar *)esb;
>  	GArray *menu;
>  
> -	menu = build_items (esb, items, 0, &efb->menu_base, efb->menu_rules);
> +	menu = build_items (esb, items, ESB_ITEMTYPE_NORMAL, &efb->menu_base, efb->menu_rules);
>  	((ESearchBarClass *)parent_class)->set_menu (esb, (ESearchBarItem *)menu->data);
>  	free_built_items (menu);
>  }
> @@ -507,7 +507,7 @@ set_option (ESearchBar *esb, ESearchBarI
>  	GArray *menu;
>  	EFilterBar *efb = (EFilterBar *)esb;
>  	
> -	menu = build_items (esb, items, 1, &efb->option_base, efb->option_rules);
> +	menu = build_items (esb, items, ESB_ITEMTYPE_CHECK, &efb->option_base, efb->option_rules);
>  	((ESearchBarClass *)parent_class)->set_option (esb, (ESearchBarItem *)menu->data);
>  	free_built_items (menu);
>  	
> 


-- Johnny