Hi Ken'ichi, On 09/07/2011 12:11 PM, Ken'ichi Ohmichi wrote: > > Hi Mahesh, > > On Mon, 5 Sep 2011 20:10:33 +0530 > Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote: >>>> >>>> This patch fixes following BUGs: >>> >>> [..] >>> >>>> - The loop construct used for array of char* (pointer) silently fails and >>>> does not filter the strings. >>> >>> Did the silent failure happen at the following code of list_entry_empty() ? >>> >>> 7373 addr = get_config_symbol_addr(le, 0, NULL); >>> 7374 if (!addr) >>> 7375 return TRUE; >>> >> >> Nope. It use to fail in resolve_list_entry()->resolve_config_entry() >> and following hunk from the patch fixes it: >>We dont allow no-array >> @@ -6866,7 +6882,7 @@ resolve_config_entry(struct config_entry *ce, unsigned long long base_addr, >> * If this is a struct or list_head data type then >> * create a leaf node entry with 'next' member. >> */ >> - if ((ce->type_flag & TYPE_BASE) >> + if (((ce->type_flag & (TYPE_BASE | TYPE_ARRAY)) == TYPE_BASE) >> && (strcmp(ce->type_name, "void"))) >> return FALSE; >> >> The old code use to check only TYPE_BASE flag ignoring TYPE_ARRAY flag. > > Thank you for the explanation. > I feel I see it. > Is the below understanding right ? > > At the part of earlier resolve_config_entry(), necessary information > (sym_addr, type_name, and array_length) can be gotten in the case > of pointer array. Yes, including the size information for type of element (type_name) in the LIST entry. > However, the old resolve_config_entry() returned FALSE because of > the check lack you said. Yes. The old resolve_config_entry() use to return FALSE only for array of base type elements (e.g. array of char, int, long etc.). However, it was working well for array of structures (pointer/non-pointer). The LIST entry can be of one of the following kind: 1. Array of base types (pointer/non-pointer). 2. Array of structures (pointer/non-pointer). 3. Link list. 4. list_head link list. The old code was working for all of the above except (1). Thanks, -Mahesh.