Re: [PATCH 1/2] dynamic_debug: fix boot parameters parse

[Jason Baron <jbaron@akamai.com>]

On 10/27/2015 03:40 AM, Peter Chen wrote:
> The parse_args will delete space between boot parameters, so
> if we add dyndbg="file drivers/usb/* +p" at bootargs, the parse_args
> will split it as three parameters, and only "file" is for dyndbg,
> then below error will occur at ddebug, it causes all non-module
> ddebug fail during the boot process.
> 
> ===============================================================
> dynamic_debug:ddebug_parse_flags: bad flag-op f, at start of file
> dynamic_debug:ddebug_exec_query: flags parse failed
> ===============================================================
> 

Hi,

So the spaces there should be preserved if they are in quotes. Looking at
kernel/params.c:next_arg(), the comment there explicitly says:

/* You can use " around spaces, but can't escape ". */

So this should still be working. And if its not, I suspect other
command-line parsing would be broken as well. So if it did change,
what changed in command line parsing?

What's the output of $ cat /proc/cmdline ? Perhaps, the quotes
were not passed properly from the bootloader to the kernel?

Thanks,

-Jason


> As a solution, we can use comma to split parameters for ddebug booting
> parameter, and replace comma with space at code, in that case, the ddebug
> core can handle it with the same way we do it for /sys.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> Cc: <stable@vger.kernel.org>
> ---
>  lib/dynamic_debug.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index e491e02..da4883b 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -880,6 +880,8 @@ static int ddebug_dyndbg_param_cb(char *param, char *val,
>  	if (strcmp(param, "dyndbg"))
>  		return on_err; /* determined by caller */
>  
> +	/* relace comma with space */
> +	strreplace(val, ',', ' ');
>  	ddebug_exec_queries((val ? val : "+p"), modname);
>  
>  	return 0; /* query failure shouldnt stop module load */
>