Re: [PATCH v2 6/8] makedumpfile: Read and process 'for' command from config file.

[Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>]

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@linux.vnet.ibm.com> wrote:
> > 
> > [PATCH] makedumpfile: Fix array traversal for array of structure and char type.
> > 
> > From: Mahesh Salgaonkar <mahesh@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.


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec