[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#813023: flash-kernel: quoting error with bootargs in generic U-Boot boot script



On Thu, 2016-01-28 at 17:30 +0100, Karsten Merker wrote:
> On Thu, Jan 28, 2016 at 04:19:27PM +0100, Jérémy Bobbio wrote:
> > Package: flash-kernel
> > Version: 3.56
> > Tag: patch
> > 
> > Hi!
> > 
> > There's a small quoting mistake in the generic U-Boot script. It
> > probably hasn't been noticed so far because the default
> > LINUX_KERNEL_CMDLINE doesn't contain spaces. See the attached patch.
> [...]
> > -setenv bootargs ${bootargs} @@LINUX_KERNEL_CMDLINE@@
> > +setenv bootargs "${bootargs} @@LINUX_KERNEL_CMDLINE@@"
> 
> Hello,
> 
> having no quotes should not be a problem for the u-boot setenv command:
> 
> => setenv foo a b c d e
> => printenv foo
> foo=a b c d e
> => setenv foo ${foo} bar baz
> => printenv foo
> foo=a b c d e bar baz
> =>

IIRC some version (probably hacky old vendor branches) of u-boot behave in
weird and wonderful ways wrt quoting.

That said, including the quoting seems likely to work more broadly than not
doing so.

> > While, I am at it, I wonder if it would not make more sense to
> > reverse the order when setting that variable so it reads:
> > 
> >     setenv bootargs "@@LINUX_KERNEL_CMDLINE@@ ${bootargs}"
> > 
> > That way, it's possible to override the default command line by doing
> > something like:
> > 
> >     setenv bootargs console=ttyAMA0,115200
> >     boot
> 
> Yes, I guess that makes sense. Ian, do you see anything that would speak
> against this change?

It would prevent @@LINUX_KERNEL_CMDLINE@@ from overriding a bad (or just
inconvenient) ${bootargs} baked into a system's default firmware? In some
cases things are headless so you can't fix ${bootargs} yourself.

Perhaps we should switch things as suggested but also add
@@LINUX_KERNEL_CMDLINE_OVERRIDES@@ at the end to allow us to put things at
both the beginning and the end?

Ian.


Reply to: