Re: [PATCH v9 2/8] boot/param: add pointer to current and next argument to unknown parameter callback

[Michal Suchánek]

Hello,

On Wed, 15 Nov 2017 20:47:14 +0530
Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:

> From: Michal Suchanek <msucha...@suse.de>
> 
> Add pointer to current and next argument to make parameter processing
> more robust. This can make parameter processing easier and less error
> prone in cases where the parameters need to be enforced/ignored based
> on firmware/system state.
> 
> Signed-off-by: Michal Suchanek <msucha...@suse.de>
> Signed-off-by: Hari Bathini <hbath...@linux.vnet.ibm.com>

> @@ -179,16 +183,18 @@ char *parse_args(const char *doing,
>       if (*args)
>               pr_debug("doing %s, parsing ARGS: '%s'\n", doing,
> args); 
> -     while (*args) {
> +     next = next_arg(args, &param, &val);
> +     while (*next) {
>               int ret;
>               int irq_was_disabled;
>  
> -             args = next_arg(args, &param, &val);
> +             args = next;
> +             next = next_arg(args, &param, &val);
>               /* Stop at -- */

The [PATCH v8 5/6] you refreshed here moves the while(*next) to the end
of the cycle for a reason. Checking *args at the start is mostly
equivalent checking *next at the end. Checking *next at the start on
the other hand skips the last argument.

The "mostly" part is that there is a bug here because *args is not
checked at the start of the cycle making it possible to crash if it is
0. To fix that the if(*args) above should be extended to wrap the cycle.

Thanks

Michal