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 <hbathini@xxxxxxxxxxxxxxxxxx> wrote:

> From: Michal Suchanek <msuchanek@xxxxxxx>
> 
> 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 <msuchanek@xxxxxxx>
> Signed-off-by: Hari Bathini <hbathini@xxxxxxxxxxxxxxxxxx>

> @@ -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