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

[Peter Chen]

On Wed, Oct 28, 2015 at 10:17:59AM -0400, Jason Baron wrote:
> 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

Oh, Jason. skip this patch pleaes, the u-boot needs to use "\\" below
double quoting to setenv. eg: dyndbg="\\"file drivers/usb/chipidea/*+p"\\",
then, the cmdline will like: dyndbg="file drivers/usb/chipidea/* +p",
otherwise, there is no double quoting around QUERY.

Peter
> 
> 
> > 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@xxxxxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > ---
> >  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 */
> > 
> 

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/