Re: [PATCH 1/1] params: don't ignore the rest of cmdline if parse_one() fails

[Rusty Russell <rusty@rustcorp.com.au>]

Oleg Nesterov <oleg@redhat.com> writes:
> parse_args() just aborts after it hits an error, so other args
> at the same initcall level are simply ignored. This can lead to
> other hard-to-understand problems, for example my testing machine
> panics during the boot if I pass "locktorture.verbose=true".
>
> Change parse_args() to save the err code for return and continue.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Thanks!

Applied,
Rusty.

> ---
>  kernel/params.c |   17 +++++++++--------
>  1 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index a22d6a7..b21139f 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -223,7 +223,7 @@ char *parse_args(const char *doing,
>  		 int (*unknown)(char *param, char *val,
>  				const char *doing, void *arg))
>  {
> -	char *param, *val;
> +	char *param, *val, *err = NULL;
>  
>  	/* Chew leading spaces */
>  	args = skip_spaces(args);
> @@ -238,7 +238,7 @@ char *parse_args(const char *doing,
>  		args = next_arg(args, &param, &val);
>  		/* Stop at -- */
>  		if (!val && strcmp(param, "--") == 0)
> -			return args;
> +			return err ?: args;
>  		irq_was_disabled = irqs_disabled();
>  		ret = parse_one(param, val, doing, params, num,
>  				min_level, max_level, arg, unknown);
> @@ -247,24 +247,25 @@ char *parse_args(const char *doing,
>  				doing, param);
>  
>  		switch (ret) {
> +		case 0:
> +			continue;
>  		case -ENOENT:
>  			pr_err("%s: Unknown parameter `%s'\n", doing, param);
> -			return ERR_PTR(ret);
> +			break;
>  		case -ENOSPC:
>  			pr_err("%s: `%s' too large for parameter `%s'\n",
>  			       doing, val ?: "", param);
> -			return ERR_PTR(ret);
> -		case 0:
>  			break;
>  		default:
>  			pr_err("%s: `%s' invalid for parameter `%s'\n",
>  			       doing, val ?: "", param);
> -			return ERR_PTR(ret);
> +			break;
>  		}
> +
> +		err = ERR_PTR(ret);
>  	}
>  
> -	/* All parsed OK. */
> -	return NULL;
> +	return err;
>  }
>  
>  /* Lazy bastard, eh? */