public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot-Users] [PATCH] limit length of kernel command line in bootm
@ 2005-06-30  8:18 Murray.Jensen at csiro.au
  2005-06-30 10:51 ` Wolfgang Denk
  0 siblings, 1 reply; 5+ messages in thread
From: Murray.Jensen at csiro.au @ 2005-06-30  8:18 UTC (permalink / raw)
  To: u-boot

This patch limits the length of the command line that the bootm command will
pass to the kernel. Note that as it stands, it uses CFG_BARGSIZE which must
be <= the size of the command line array used in the kernel (most boards have
this set to CFG_CBSIZE, which in turn is usually 256 - which is usually ok).
I use 1024 for both, and have a kernel patch which increases the command line
aray to 1024. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

To the extent permitted by law, CSIRO does not represent, warrant and/or
guarantee that the integrity of this communication has been maintained or
that the communication is free of errors, virus, interception or interference.

The information contained in this e-mail may be confidential or privileged.
Any unauthorised use or disclosure is prohibited. If you have received this
e-mail in error, please delete it immediately and notify Murray Jensen on
+61 3 9662 7763. Thank you.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: text/x-patch
Size: 699 bytes
Desc: limit-bootargs-len.patch
Url : http://lists.denx.de/pipermail/u-boot/attachments/20050630/cdd9b7d6/attachment.bin 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot-Users] [PATCH] limit length of kernel command line in bootm
  2005-06-30  8:18 [U-Boot-Users] [PATCH] limit length of kernel command line in bootm Murray.Jensen at csiro.au
@ 2005-06-30 10:51 ` Wolfgang Denk
  2005-06-30 13:37   ` Murray.Jensen at csiro.au
  0 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Denk @ 2005-06-30 10:51 UTC (permalink / raw)
  To: u-boot

In message <29494.1120119515@gerd> you wrote:
>
> This patch limits the length of the command line that the bootm command will
> pass to the kernel. Note that as it stands, it uses CFG_BARGSIZE which must
> be <= the size of the command line array used in the kernel (most boards have
> this set to CFG_CBSIZE, which in turn is usually 256 - which is usually ok).
> I use 1024 for both, and have a kernel patch which increases the command line
> aray to 1024. Cheers!

I don't see any improvement by this patch,  as  you  cannot  know  in
U-Boot  which value is correct for the kernel image you are booting -
ther eis no way to "ask" the kernel for this. So your patch may cause
booting to fail in situtations where it should work,  or  not  detect
problems  when  they  are  there.  In  other  words,  I don;t see any
improvement  over  the  current  situation   which   has   the   same
expectation: that the user knows what he is doing. Also, a hard coded
limit is probably a bad approach anyhow.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Severe culture shock results when experts from another protocol suite
[...] try to read OSI documents. The term "osified" is used to  refer
to  such  documents. [...] Any relationship to the word "ossified" is
purely intentional.                                - Marshall T. Rose

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot-Users] [PATCH] limit length of kernel command line in bootm
  2005-06-30 10:51 ` Wolfgang Denk
@ 2005-06-30 13:37   ` Murray.Jensen at csiro.au
  2005-06-30 13:51     ` Wolfgang Denk
  0 siblings, 1 reply; 5+ messages in thread
From: Murray.Jensen at csiro.au @ 2005-06-30 13:37 UTC (permalink / raw)
  To: u-boot

On Thu, 30 Jun 2005 12:51:16 +0200, Wolfgang Denk writes:
>> This patch limits the length of the command line that the bootm command will
>> pass to the kernel. Note that as it stands, it uses CFG_BARGSIZE which must
>> be <= the size of the command line array used in the kernel (most boards have
>> this set to CFG_CBSIZE, which in turn is usually 256 - which is usually ok).
>> I use 1024 for both, and have a kernel patch which increases the command line
>> aray to 1024. Cheers!
>
>I don't see any improvement by this patch,  as  you  cannot  know  in
>U-Boot  which value is correct for the kernel image you are booting -

Like most other configuration variables, you have to set it correctly in
your board specific configs/*.h header file.

>ther eis no way to "ask" the kernel for this.

Obviously, since the kernel is not yet running :-)

>So your patch may cause
>booting to fail in situtations where it should work,  or  not  detect
>problems  when  they  are  there.

On the contrary, it detects a specific situation where it knows the kernel
will crash and burn without any obvious reason and refuses to boot, after
outputting a clear error message.

I was burnt by this exact thing happening, and the patch was the result,
to prevent it from ever happening again.

>In  other  words,  I don;t see any
>improvement  over  the  current  situation   which   has   the   same
>expectation: that the user knows what he is doing.

Yes, indeed - they know what they are doing, and they will ensure that the
correct number is put in their board config .h file.

>Also, a hard coded
>limit is probably a bad approach anyhow.

The size of the Linux kernel command line array is hardcoded in a number
of places in the kernel (I know, because I have a patch which increases its
size). Everyone knows this is a bad approach, and yet it has survived
unchanged in the Linux (ppc) kernel for years, up to and including v2.6 (where
there was some slight improvement when it was at least defined as a macro
in a <asm-ppc/*.h> file - previously it has been a number in numerous places
ie. unsigned char array[512]; !!!)

This patch at least catches one case where it knows the size has been
exceeded. Since the "bootargs" variable is a variable length string,
which is set by the user via the command prompt, I believe it should
be detected when that user exceeds the allowed space - the linux kernel
should not just start crashing. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen at csiro.au

To the extent permitted by law, CSIRO does not represent, warrant and/or
guarantee that the integrity of this communication has been maintained or
that the communication is free of errors, virus, interception or interference.

The information contained in this e-mail may be confidential or privileged.
Any unauthorised use or disclosure is prohibited. If you have received this
e-mail in error, please delete it immediately and notify Murray Jensen on
+61 3 9662 7763. Thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot-Users] [PATCH] limit length of kernel command line in bootm
  2005-06-30 13:37   ` Murray.Jensen at csiro.au
@ 2005-06-30 13:51     ` Wolfgang Denk
  2005-06-30 14:44       ` Murray.Jensen at csiro.au
  0 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Denk @ 2005-06-30 13:51 UTC (permalink / raw)
  To: u-boot

In message <7976.1120138625@huldra> you wrote:
>
> >So your patch may cause
> >booting to fail in situtations where it should work,  or  not  detect
> >problems  when  they  are  there.
> 
> On the contrary, it detects a specific situation where it knows the kernel
> will crash and burn without any obvious reason and refuses to boot, after
> outputting a clear error message.

How can it know that|  The  image  may  contain  a  kernel  that  was
adjusted appropriately?

IMHO it's not a good idea  to  enforce  a  boot  loader  update  just
because a number in the kernel configuration changes.

> Yes, indeed - they know what they are doing, and they will ensure that the
> correct number is put in their board config .h file.

This is  unacceptable  for  me.  Kernel  and  boot  loader  shall  be
independent  as  far  as  possible.  Even  on  the  risk  of  using a
combination that does not work.

> The size of the Linux kernel command line array is hardcoded in a number
> of places in the kernel (I know, because I have a patch which increases its

Then let's not make the same error again.

> This patch at least catches one case where it knows the size has been
> exceeded. Since the "bootargs" variable is a variable length string,

It does not know this. It CANNOT know this, as it has no  information
what  I'm  going to load. Actually it might be 2.8 kernel without any
such limitation at all.


Sorry, but I reject this patch as I consider it conceptually broken.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Never face facts; if you do, you'll never get up in the morning."
- Marlo Thomas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot-Users] [PATCH] limit length of kernel command line in bootm
  2005-06-30 13:51     ` Wolfgang Denk
@ 2005-06-30 14:44       ` Murray.Jensen at csiro.au
  0 siblings, 0 replies; 5+ messages in thread
From: Murray.Jensen at csiro.au @ 2005-06-30 14:44 UTC (permalink / raw)
  To: u-boot

On Thu, 30 Jun 2005 15:51:24 +0200, Wolfgang Denk writes:
>Sorry, but I reject this patch as I consider it conceptually broken.

OK, I see your point and agree with you now (although I am going to
leave the changes in my code - I don't want to be burnt again). Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen at csiro.au

To the extent permitted by law, CSIRO does not represent, warrant and/or
guarantee that the integrity of this communication has been maintained or
that the communication is free of errors, virus, interception or interference.

The information contained in this e-mail may be confidential or privileged.
Any unauthorised use or disclosure is prohibited. If you have received this
e-mail in error, please delete it immediately and notify Murray Jensen on
+61 3 9662 7763. Thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2005-06-30 14:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-06-30  8:18 [U-Boot-Users] [PATCH] limit length of kernel command line in bootm Murray.Jensen at csiro.au
2005-06-30 10:51 ` Wolfgang Denk
2005-06-30 13:37   ` Murray.Jensen at csiro.au
2005-06-30 13:51     ` Wolfgang Denk
2005-06-30 14:44       ` Murray.Jensen at csiro.au

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox