Hi Ken'ichi,
On 2011-09-01 17:06:58 Thu, Ken'ichi Ohmichi wrote:
>
> Hi Mahesh,
>
> Thank you for the patch.
> I have one question.
>
> On Tue, 30 Aug 2011 23:46:42 +0530
> Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> >
> > [PATCH] makedumpfile: Fix array traversal for array of structure and char type.
> >
> > From: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> >
> > Introduce new function get_next_list_entry() that returns config entry
> > for each element in the LIST entry. This approach improves the handling
> > of LIST entry. With this change the function get_config_symbol_addr()
> > no longer required to handle LIST entry separately.
> >
> > 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:
@@ -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.
> If list_entry_empty() returns TRUE, a "while" loop of process_config() breaks
> and update_filter_info() is not called.
> Is that the problem you said ?
>
> If yes, I feel the behavior related with this problem has not changed.
> because the method for getting a pointer array has not changed like the
> following:
The while loop below still holds good as it just ignores the pointer array
elements which are NULL pointers and we dont have anything to be erased for
NULL pointers.
>
> o OLD
> 111 - while (ce->index < ce->array_length) {
> 112 - addr = read_pointer_value(ce->addr +
> 113 - (ce->index * get_pointer_size()));
> 114 - ce->index++;
> 115 - if (addr)
> 116 - break;
> 117 - }
> 118 - return addr;
>
> o NEW
> 197 + while (ce->index < ce->array_length) {
> 198 + addr = read_pointer_value(ce->addr +
> 199 + (ce->index * get_pointer_size()));
> 200 + if (addr)
> 201 + break;
> 202 + ce->index++;
> 203 + }
> 204 + if (ce->index == ce->array_length)
> 205 + return FALSE;
>
>
> I think the other bugfixes are right, thanks.
>
>
> Thanks
> Ken'ichi Ohmichi
>
Thanks,
-Mahesh.
