All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] U-boot env variables parsing
@ 2010-04-01  5:27 Nitin Mahajan
  2010-04-01 12:15 ` Detlev Zundel
  0 siblings, 1 reply; 15+ messages in thread
From: Nitin Mahajan @ 2010-04-01  5:27 UTC (permalink / raw)
  To: u-boot

Hi!

I am doing env settings some thing like this,

ROOT1=/dev/mmcblk0p1
ROOT2=/dev/mmcblk0p2
ROOT=${ROOT1}
bootargs1=console=ttyS0,115200n8 mem=256M noinitrd rw rootdelay=1 ${ROOT}

when I say 'setenv bootargs ${bootargs1}', ${ROOT} gets resolved to 'ROOT1', it does not get completely resolved to '/dev/mmcblk0p1'. 

Is there something fundamentally wrong in setting the env variables this way?

What would be the right way to achieve this, as I want ROOT to be ${ROOT1} sometimes and ${ROOT2} some times?

regards

-Nitin




      Get your preferred Email name!
Now you can @ymail.com and @rocketmail.com. 
http://mail.promotions.yahoo.com/newdomains/aa/

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

* [U-Boot] U-boot env variables parsing
  2010-04-01  5:27 [U-Boot] U-boot env variables parsing Nitin Mahajan
@ 2010-04-01 12:15 ` Detlev Zundel
  2010-04-01 12:31   ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Detlev Zundel @ 2010-04-01 12:15 UTC (permalink / raw)
  To: u-boot

Hi Nitin,

> Hi!
>
> I am doing env settings some thing like this,
>
> ROOT1=/dev/mmcblk0p1
> ROOT2=/dev/mmcblk0p2
> ROOT=${ROOT1}
> bootargs1=console=ttyS0,115200n8 mem=256M noinitrd rw rootdelay=1 ${ROOT}
>
> when I say 'setenv bootargs ${bootargs1}', ${ROOT} gets resolved to 'ROOT1', it does not get completely resolved to '/dev/mmcblk0p1'. 
>
> Is there something fundamentally wrong in setting the env variables
> this way?

There is nothing fundamentally wrong - it will simply not work ;)  No,
seriously, you want U-Boot to do two evaluation passes over bootargs,
which we cannot do.  In a regular shell this would be done by a separate
"eval" or backtick round, but we lack this.

What we usually do is something like this (example taken from an
arbitrary board):

nfsargs=setenv bootargs root=/dev/nfs rw nfsroot=${serverip}:${rootpath}
addip=setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}:${netdev}:off panic=1
addtty=setenv bootargs ${bootargs} console=tty0 console=${consdev},${baudrate}
addfb=setenv bootargs ${bootargs} fbcon=map:5 video=fslfb:800x480-32 at 60
flash_nfs=run nfsargs addip addtty addfb;bootm ${kernel_addr} - ${fdt_addr}


When "run flash_nfs" is executed, little "scripts" are called and they
assemble a full command line from different variables.  So in this
example, one could simply change ${kernel_addr} and "run flash_nfs"
would boot the new kernel.

If you don't like such constructs, you can also get more funky and start
using "if ... then .. else .." construct in the hush shell parser.

> What would be the right way to achieve this, as I want ROOT to be
> ${ROOT1} sometimes and ${ROOT2} some times?

There is more than one way to do it ;)

Cheers
  Detlev

-- 
Another helpful hint for successful MIME processing:

application/msword; rm -f %s; description="MS Word Text";
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 12:15 ` Detlev Zundel
@ 2010-04-01 12:31   ` Joakim Tjernlund
  2010-04-01 12:47     ` Wolfgang Denk
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-01 12:31 UTC (permalink / raw)
  To: u-boot

>
> Hi Nitin,
>
> > Hi!
> >
> > I am doing env settings some thing like this,
> >
> > ROOT1=/dev/mmcblk0p1
> > ROOT2=/dev/mmcblk0p2
> > ROOT=${ROOT1}
> > bootargs1=console=ttyS0,115200n8 mem=256M noinitrd rw rootdelay=1 ${ROOT}
> >
> > when I say 'setenv bootargs ${bootargs1}', ${ROOT} gets resolved to 'ROOT1',
> it does not get completely resolved to '/dev/mmcblk0p1'.
> >
> > Is there something fundamentally wrong in setting the env variables
> > this way?
>
> There is nothing fundamentally wrong - it will simply not work ;)  No,
> seriously, you want U-Boot to do two evaluation passes over bootargs,
> which we cannot do.  In a regular shell this would be done by a separate
> "eval" or backtick round, but we lack this.

I recall impl. something like this for the old shell. It enabled me
to do:

 linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
 tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000

it was fairly simple to do but I don't think WD applied it since the old shell was obsolete

I guess it would not be hard to do the same for hush. If I fund the correct commit
it looked like this for the old shell:

Index: main.c
===================================================================
--- main.c	(revision 15)
+++ main.c	(revision 16)
@@ -463,9 +463,9 @@

 /****************************************************************************/

-static void process_macros (const char *input, char *output)
+static int process_macros (const char *input, char *output)
 {
-	char c, prev;
+	char c, prev, macro_processed =0;
 	const char *varname_start;
 	int inputcnt  = strlen (input);
 	int outputcnt = CFG_CBSIZE;
@@ -540,6 +540,7 @@
 				}
 			/* Look for another '$' */
 			state = 0;
+			macro_processed = 1;
 		}
 		break;
 	    }
@@ -549,7 +550,7 @@

 	if (outputcnt)
 		*output = 0;
-
+	return macro_processed;
 #ifdef DEBUG_PARSER
 	printf ("[PROCESS_MACROS] OUTPUT len %d: \"%s\"\n",
 		strlen(output_start), output_start);
@@ -580,6 +581,7 @@
 	char *token;			/* start of token in cmdbuf	*/
 	char *sep;			/* end of token (separator) in cmdbuf */
 	char finaltoken[CFG_CBSIZE];
+	char tmptoken[CFG_CBSIZE];
 	char *str = cmdbuf;
 	char *argv[CFG_MAXARGS + 1];	/* NULL terminated	*/
 	int argc;
@@ -631,7 +633,11 @@
 #endif

 		/* find macros in this token and replace them */
-		process_macros (token, finaltoken);
+		if(process_macros (token, finaltoken)){
+		  strcpy(tmptoken,finaltoken);
+		  while(process_macros (tmptoken, finaltoken))
+			strcpy(tmptoken,finaltoken);
+		}

 		/* Extract arguments */
 		argc = parse_line (finaltoken, argv);


    Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 12:31   ` Joakim Tjernlund
@ 2010-04-01 12:47     ` Wolfgang Denk
  2010-04-01 12:56       ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfgang Denk @ 2010-04-01 12:47 UTC (permalink / raw)
  To: u-boot

Dear Joakim Tjernlund,

In message <OFEBF95CA1.44E68372-ONC12576F8.0043F4F7-C12576F8.0044D7F5@transmode.se> you wrote:
>
>  linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
>  tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000
> 
> it was fairly simple to do but I don't think WD applied it since the old shell was obsolete

I don't think you ever posted this before. I cannot find any trace of
such a patch - not in the public archives nor locally.

>  		/* find macros in this token and replace them */
> -		process_macros (token, finaltoken);
> +		if(process_macros (token, finaltoken)){
> +		  strcpy(tmptoken,finaltoken);
> +		  while(process_macros (tmptoken, finaltoken))
> +			strcpy(tmptoken,finaltoken);
> +		}

Hm... will this not make escaping impossible? Assume you want to pass

	arg=${name}

to Linux. How would you escape this so it does NOT get expanded if
you run process_macros() arbitrarily often?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Brontosaurus Principle: Organizations  can  grow  faster  than  their
brains  can manage them in relation to their environment and to their
own physiology: when this occurs, they are an endangered species.
                                                - Thomas K. Connellan

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 12:47     ` Wolfgang Denk
@ 2010-04-01 12:56       ` Joakim Tjernlund
  2010-04-01 13:05         ` Wolfgang Denk
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-01 12:56 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk <wd@denx.de> wrote on 2010-04-01 14:47:45:
>
> Dear Joakim Tjernlund,
>
> In message <OFEBF95CA1.44E68372-ONC12576F8.0043F4F7-C12576F8.
> 0044D7F5 at transmode.se> you wrote:
> >
> >  linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
> >  tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000
> >
> > it was fairly simple to do but I don't think WD applied it since the old
> shell was obsolete
>
> I don't think you ever posted this before. I cannot find any trace of
> such a patch - not in the public archives nor locally.

I think I did, but this would have been late 2001 or 2002 I think :)
Not 100% sure about the date since the old CVS log looks a bit fishy
after the repo got converted to Subversion.

>
> >        /* find macros in this token and replace them */
> > -      process_macros (token, finaltoken);
> > +      if(process_macros (token, finaltoken)){
> > +        strcpy(tmptoken,finaltoken);
> > +        while(process_macros (tmptoken, finaltoken))
> > +         strcpy(tmptoken,finaltoken);
> > +      }
>
> Hm... will this not make escaping impossible? Assume you want to pass
>
>    arg=${name}
>
> to Linux. How would you escape this so it does NOT get expanded if
> you run process_macros() arbitrarily often?

Possibly, I never needed that though.
One would probably need to add an escape char for that. Something
like arg=\${name}

 Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 12:56       ` Joakim Tjernlund
@ 2010-04-01 13:05         ` Wolfgang Denk
  2010-04-01 13:11           ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfgang Denk @ 2010-04-01 13:05 UTC (permalink / raw)
  To: u-boot

Dear Joakim Tjernlund,

In message <OF6D501B3A.40F7E6A2-ONC12576F8.00466BE2-C12576F8.00471566@transmode.se> you wrote:
>
> > I don't think you ever posted this before. I cannot find any trace of
> > such a patch - not in the public archives nor locally.
> 
> I think I did, but this would have been late 2001 or 2002 I think :)
> Not 100% sure about the date since the old CVS log looks a bit fishy
> after the repo got converted to Subversion.

Ah... that was at PPCBoot times, then. Found it:
Tue, 11 Sep 2001 15:29:26 +0200

AFAICT the patch was late for a release, but has not been formally
rejected. Probably just got forgotten.

> > Hm... will this not make escaping impossible? Assume you want to pass
> >
> >    arg=${name}
> >
> > to Linux. How would you escape this so it does NOT get expanded if
> > you run process_macros() arbitrarily often?
> 
> Possibly, I never needed that though.
> One would probably need to add an escape char for that. Something
> like arg=\${name}

Woudn't this then just cause another cycle through process_macros(),
where it then gets substitured anyway? [Guess I gotta try this out.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Es ist nicht genug zu wissen, man mu? auch anwenden; es ist nicht ge-
nug zu wollen, man mu? auch tun.   -- Goethe, Maximen und Reflexionen

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 13:05         ` Wolfgang Denk
@ 2010-04-01 13:11           ` Joakim Tjernlund
  2010-04-01 14:56             ` Detlev Zundel
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-01 13:11 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk <wd@denx.de> wrote on 2010-04-01 15:05:29:
>
> Dear Joakim Tjernlund,
>
> In message <OF6D501B3A.40F7E6A2-ONC12576F8.00466BE2-C12576F8.
> 00471566 at transmode.se> you wrote:
> >
> > > I don't think you ever posted this before. I cannot find any trace of
> > > such a patch - not in the public archives nor locally.
> >
> > I think I did, but this would have been late 2001 or 2002 I think :)
> > Not 100% sure about the date since the old CVS log looks a bit fishy
> > after the repo got converted to Subversion.
>
> Ah... that was at PPCBoot times, then. Found it:
> Tue, 11 Sep 2001 15:29:26 +0200

Wow, you got your archives in order :)

>
> AFAICT the patch was late for a release, but has not been formally
> rejected. Probably just got forgotten.

Ah, I must have misunderstood then.

>
> > > Hm... will this not make escaping impossible? Assume you want to pass
> > >
> > >    arg=${name}
> > >
> > > to Linux. How would you escape this so it does NOT get expanded if
> > > you run process_macros() arbitrarily often?
> >
> > Possibly, I never needed that though.
> > One would probably need to add an escape char for that. Something
> > like arg=\${name}
>
> Woudn't this then just cause another cycle through process_macros(),
> where it then gets substitured anyway? [Guess I gotta try this out.]

I am not sure what will happen, it was so man years ago.
But I would guess that if you really want to pass arg=$(name)
to the kernel you will probably need to do some adjustment.

  Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 13:11           ` Joakim Tjernlund
@ 2010-04-01 14:56             ` Detlev Zundel
  2010-04-01 17:13               ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Detlev Zundel @ 2010-04-01 14:56 UTC (permalink / raw)
  To: u-boot

Hi Jocke,

>> > > Hm... will this not make escaping impossible? Assume you want to pass
>> > >
>> > >    arg=${name}
>> > >
>> > > to Linux. How would you escape this so it does NOT get expanded if
>> > > you run process_macros() arbitrarily often?
>> >
>> > Possibly, I never needed that though.
>> > One would probably need to add an escape char for that. Something
>> > like arg=\${name}
>>
>> Woudn't this then just cause another cycle through process_macros(),
>> where it then gets substitured anyway? [Guess I gotta try this out.]
>
> I am not sure what will happen, it was so man years ago.
> But I would guess that if you really want to pass arg=$(name)
> to the kernel you will probably need to do some adjustment.

To me it looks like the new code would indeed do a "greedy" substitution
only stopping when no more substitutions can be done.  This is very
un-unixy and thus not something I'd like to see as a default behaviour.

When we want to move in this direction, then we should implement
something with finer control over the substitution rounds.

Cheers and happy easter all around
  Detlev

-- 
Q:  What does FAQ stand for?
A:  We are Frequently Asked this Question, and we have no idea.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 14:56             ` Detlev Zundel
@ 2010-04-01 17:13               ` Joakim Tjernlund
  2010-04-01 18:27                 ` Wolfgang Denk
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-01 17:13 UTC (permalink / raw)
  To: u-boot

Detlev Zundel <dzu@denx.de> wrote on 2010-04-01 16:56:40:
>
> Hi Jocke,
>
> >> > > Hm... will this not make escaping impossible? Assume you want to pass
> >> > >
> >> > >    arg=${name}
> >> > >
> >> > > to Linux. How would you escape this so it does NOT get expanded if
> >> > > you run process_macros() arbitrarily often?
> >> >
> >> > Possibly, I never needed that though.
> >> > One would probably need to add an escape char for that. Something
> >> > like arg=\${name}
> >>
> >> Woudn't this then just cause another cycle through process_macros(),
> >> where it then gets substitured anyway? [Guess I gotta try this out.]
> >
> > I am not sure what will happen, it was so man years ago.
> > But I would guess that if you really want to pass arg=$(name)
> > to the kernel you will probably need to do some adjustment.
>
> To me it looks like the new code would indeed do a "greedy" substitution
> only stopping when no more substitutions can be done.  This is very
> un-unixy and thus not something I'd like to see as a default behaviour.

Why not? What is gained by the current method?

 Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 17:13               ` Joakim Tjernlund
@ 2010-04-01 18:27                 ` Wolfgang Denk
  2010-04-01 20:08                   ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfgang Denk @ 2010-04-01 18:27 UTC (permalink / raw)
  To: u-boot

Dear Joakim Tjernlund,

In message <OF5552925C.8F998468-ONC12576F8.005E3247-C12576F8.005E9BF2@transmode.se> you wrote:
>
> > To me it looks like the new code would indeed do a "greedy" substitution
> > only stopping when no more substitutions can be done.  This is very
> > un-unixy and thus not something I'd like to see as a default behaviour.
> 
> Why not? What is gained by the current method?

We follow the principle of least surprise [1]; the command line
interpeter in U-Boot should behave as similar to a (say, POSIX
compatible) shell as possible. Restrictions and deviations are not
intentional, but caused by the attempt to do with a minimal memory
footprint.

Like Detlev I feel/fear that the suggested change will cause more
annoyance due to unexpected behaviour that it will do good.


I also see little actual need for such an extension, as in all cases
I've seen so far it has been possible to acchieve the goal without
code changes by just minimal adjustments of the definitions. For
example, your code:

linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000

could be rewritten from plain variable definitions into an equivalent
command sequence, like that:

setenv setip 'setenv bootargs ${bootargs} ip=${ipaddr}::${gatewayip}:${netmask}:${hostname}:${linuxif}:off'

Do the same for "linuxroot" (=> setroot) and "extra" (=> setextra),
and then use:

setenv tboot 'run setroot setip setextra;tftp 100000;bootm'

will do exactly what you want. Detlev quoted similar code earlier.
This is what many, many existing boards use, and what we explain in
great detail in the manual.


[1] http://en.wikipedia.org/wiki/Principle_of_least_surprise


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Old programmers never die, they just become managers.

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 18:27                 ` Wolfgang Denk
@ 2010-04-01 20:08                   ` Joakim Tjernlund
  2010-04-08 10:00                     ` Detlev Zundel
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-01 20:08 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk <wd@denx.de> wrote on 2010-04-01 20:27:47:
>
> Dear Joakim Tjernlund,
>
> In message <OF5552925C.8F998468-ONC12576F8.005E3247-C12576F8.
> 005E9BF2 at transmode.se> you wrote:
> >
> > > To me it looks like the new code would indeed do a "greedy" substitution
> > > only stopping when no more substitutions can be done.  This is very
> > > un-unixy and thus not something I'd like to see as a default behaviour.
> >
> > Why not? What is gained by the current method?
>
> We follow the principle of least surprise [1]; the command line
> interpeter in U-Boot should behave as similar to a (say, POSIX
> compatible) shell as possible. Restrictions and deviations are not
> intentional, but caused by the attempt to do with a minimal memory
> footprint.
>
> Like Detlev I feel/fear that the suggested change will cause more
> annoyance due to unexpected behaviour that it will do good.

what bad do you think it might do? You mentioned the possibility
to pass arg=$(name) literally(why would you do that?)
Have you ever done that? Then one should be able to
pass $(linuxip) too, which you can't.

>
>
> I also see little actual need for such an extension, as in all cases
> I've seen so far it has been possible to acchieve the goal without
> code changes by just minimal adjustments of the definitions. For
> example, your code:
>
> linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
> tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000
>
> could be rewritten from plain variable definitions into an equivalent
> command sequence, like that:
>
> setenv setip 'setenv bootargs ${bootargs} ip=${ipaddr}::${gatewayip}:$
> {netmask}:${hostname}:${linuxif}:off'
>
> Do the same for "linuxroot" (=> setroot) and "extra" (=> setextra),
> and then use:
>
> setenv tboot 'run setroot setip setextra;tftp 100000;bootm'
>
> will do exactly what you want. Detlev quoted similar code earlier.
> This is what many, many existing boards use, and what we explain in
> great detail in the manual.

While you can do that, it is ugly and clumsy(which was why I wrote the patch)

I want the shell to make things simpler/easier for me and the above
isn't either.

 Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-01 20:08                   ` Joakim Tjernlund
@ 2010-04-08 10:00                     ` Detlev Zundel
  2010-04-08 15:41                       ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Detlev Zundel @ 2010-04-08 10:00 UTC (permalink / raw)
  To: u-boot

Hi Jocke,

>> > > To me it looks like the new code would indeed do a "greedy" substitution
>> > > only stopping when no more substitutions can be done.  This is very
>> > > un-unixy and thus not something I'd like to see as a default behaviour.
>> >
>> > Why not? What is gained by the current method?
>>
>> We follow the principle of least surprise [1]; the command line
>> interpeter in U-Boot should behave as similar to a (say, POSIX
>> compatible) shell as possible. Restrictions and deviations are not
>> intentional, but caused by the attempt to do with a minimal memory
>> footprint.
>>
>> Like Detlev I feel/fear that the suggested change will cause more
>> annoyance due to unexpected behaviour that it will do good.
>
> what bad do you think it might do? You mentioned the possibility
> to pass arg=$(name) literally(why would you do that?)
> Have you ever done that?

Nope, I haven't, but if some linux commandline parameter needs such a
construct, I surely do not want to change the U-Boot shell to be able to
do it.  The change you propose would make such a thing impossible.

> Then one should be able to
> pass $(linuxip) too, which you can't.

Why do you think so?

=> run nfsargs addip addtty addfb
=> prin ipaddr
ipaddr=192.168.160.43
=> setenv bootargs ${bootargs} \${ipaddr}
=> bootm ${kernel_addr}

[....]

-bash-3.2# cat /proc/cmdline
root=/dev/nfs rw nfsroot=192.168.1.1:/opt/eldk/ppc_6xx
ip=192.168.160.43:192.168.1.1:192.168.83.201:255.255.0.0:pdm360ng:eth0:off panic=1 console=tty0 console=ttyPSC5,115200 fbcon=map:5 video=fslfb:800x480-32 at 60 ${ipaddr}

>> I also see little actual need for such an extension, as in all cases
>> I've seen so far it has been possible to acchieve the goal without
>> code changes by just minimal adjustments of the definitions. For
>> example, your code:
>>
>> linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
>> tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000
>>
>> could be rewritten from plain variable definitions into an equivalent
>> command sequence, like that:
>>
>> setenv setip 'setenv bootargs ${bootargs} ip=${ipaddr}::${gatewayip}:$
>> {netmask}:${hostname}:${linuxif}:off'
>>
>> Do the same for "linuxroot" (=> setroot) and "extra" (=> setextra),
>> and then use:
>>
>> setenv tboot 'run setroot setip setextra;tftp 100000;bootm'
>>
>> will do exactly what you want. Detlev quoted similar code earlier.
>> This is what many, many existing boards use, and what we explain in
>> great detail in the manual.
>
> While you can do that, it is ugly and clumsy(which was why I wrote the patch)
>
> I want the shell to make things simpler/easier for me and the above
> isn't either.

This is a personal preference.  Personally I value the quoted principle
of least surprise more.  Moreover I much rather make all use cases
possible and maybe use some extra characters for exotic ones rather than
precluding a specific use case completely in order to save a few
characters.

Cheers
  Detlev

-- 
SWYM XYZ (Sympathize with your machinery).  [..] In fact it is often
called a no-op, because it performs no operation.  It does, however,
keep the machine running smoothly, just as real-world swimming helps
to keep programmers healthy.             -- Donald Knuth, Fascicle 1
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

* [U-Boot] U-boot env variables parsing
  2010-04-08 10:00                     ` Detlev Zundel
@ 2010-04-08 15:41                       ` Joakim Tjernlund
  2010-04-08 16:06                         ` Detlev Zundel
  0 siblings, 1 reply; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-08 15:41 UTC (permalink / raw)
  To: u-boot

Detlev Zundel <dzu@denx.de> wrote on 2010-04-08 12:00:05:
>
> Hi Jocke,

Hi Detlev :)

>
> >> > > To me it looks like the new code would indeed do a "greedy" substitution
> >> > > only stopping when no more substitutions can be done.  This is very
> >> > > un-unixy and thus not something I'd like to see as a default behaviour.
> >> >
> >> > Why not? What is gained by the current method?
> >>
> >> We follow the principle of least surprise [1]; the command line
> >> interpeter in U-Boot should behave as similar to a (say, POSIX
> >> compatible) shell as possible. Restrictions and deviations are not
> >> intentional, but caused by the attempt to do with a minimal memory
> >> footprint.
> >>
> >> Like Detlev I feel/fear that the suggested change will cause more
> >> annoyance due to unexpected behaviour that it will do good.
> >
> > what bad do you think it might do? You mentioned the possibility
> > to pass arg=$(name) literally(why would you do that?)
> > Have you ever done that?
>
> Nope, I haven't, but if some linux commandline parameter needs such a
> construct, I surely do not want to change the U-Boot shell to be able to
> do it.  The change you propose would make such a thing impossible.
>
> > Then one should be able to
> > pass $(linuxip) too, which you can't.
>
> Why do you think so?
>
> => run nfsargs addip addtty addfb
> => prin ipaddr
> ipaddr=192.168.160.43
> => setenv bootargs ${bootargs} \${ipaddr}

Oh, so there is an escape char(\) after all. I got the impression that
there wasn't one.

> => bootm ${kernel_addr}
>
> [....]
>
> -bash-3.2# cat /proc/cmdline
> root=/dev/nfs rw nfsroot=192.168.1.1:/opt/eldk/ppc_6xx
> ip=192.168.160.43:192.168.1.1:192.168.83.201:255.255.0.0:pdm360ng:eth0:off
> panic=1 console=tty0 console=ttyPSC5,115200 fbcon=map:5 video=fslfb:
> 800x480-32 at 60 ${ipaddr}
>
> >> I also see little actual need for such an extension, as in all cases
> >> I've seen so far it has been possible to acchieve the goal without
> >> code changes by just minimal adjustments of the definitions. For
> >> example, your code:
> >>
> >> linuxip=ip=$(ipaddr)::$(gatewayip):$(netmask):$(hostname):$(linuxif):off
> >> tboot=setenv bootargs $(linuxroot) $(linuxip) $(extra);tftp 100000; bootm 100000
> >>
> >> could be rewritten from plain variable definitions into an equivalent
> >> command sequence, like that:
> >>
> >> setenv setip 'setenv bootargs ${bootargs} ip=${ipaddr}::${gatewayip}:$
> >> {netmask}:${hostname}:${linuxif}:off'
> >>
> >> Do the same for "linuxroot" (=> setroot) and "extra" (=> setextra),
> >> and then use:
> >>
> >> setenv tboot 'run setroot setip setextra;tftp 100000;bootm'
> >>
> >> will do exactly what you want. Detlev quoted similar code earlier.
> >> This is what many, many existing boards use, and what we explain in
> >> great detail in the manual.
> >
> > While you can do that, it is ugly and clumsy(which was why I wrote the patch)
> >
> > I want the shell to make things simpler/easier for me and the above
> > isn't either.
>
> This is a personal preference.  Personally I value the quoted principle
> of least surprise more.  Moreover I much rather make all use cases
> possible and maybe use some extra characters for exotic ones rather than
> precluding a specific use case completely in order to save a few
> characters.

Since an escape char appear to exist, one should be able to use it much like you
did above so I don't think that any use case disappears. Instead the common usage
becomes simpler and the so far artificial use case needs an extra escape char.

 Jocke

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

* [U-Boot] U-boot env variables parsing
  2010-04-08 15:41                       ` Joakim Tjernlund
@ 2010-04-08 16:06                         ` Detlev Zundel
  2010-04-08 17:49                           ` Joakim Tjernlund
  0 siblings, 1 reply; 15+ messages in thread
From: Detlev Zundel @ 2010-04-08 16:06 UTC (permalink / raw)
  To: u-boot

Hi Jocke,

[...]

> Since an escape char appear to exist, one should be able to use it much like you
> did above so I don't think that any use case disappears. Instead the common usage
> becomes simpler and the so far artificial use case needs an extra escape char.

Hm.  I have to admit that I did not actually try your code, but there is
a loop in there until no further substitution takes place.  It seems to
me that this loop over process_macros thus does substitution _and_ uses
up the escape characters.  

So I fear that if in one iteration _some_ substitution takes place and
the escape character is stripped in the same iteration _another_ round
of substitution will be done and the escape character is gone.  So in
effect you will have to study the whole string in advance (plus all the
substitutions it will see) to tell how many rounds the loop will take
and how many escape characters will be needed.

Such a behaviour would be very odd in my eyes.  On the other hand, this
is only theory....

Cheers
  Detlev

-- 
Directories are added, deleted, and rearranged much as you would
expect, even if you don't know it's what you'd expect.
                                         -- Tom Lord in TLA Documentation
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

* [U-Boot] U-boot env variables parsing
  2010-04-08 16:06                         ` Detlev Zundel
@ 2010-04-08 17:49                           ` Joakim Tjernlund
  0 siblings, 0 replies; 15+ messages in thread
From: Joakim Tjernlund @ 2010-04-08 17:49 UTC (permalink / raw)
  To: u-boot

Detlev Zundel <dzu@denx.de> wrote on 2010-04-08 18:06:40:
>
> Hi Jocke,

Hi again :)

>
> [...]
>
> > Since an escape char appear to exist, one should be able to use it much like you
> > did above so I don't think that any use case disappears. Instead the common usage
> > becomes simpler and the so far artificial use case needs an extra escape char.
>
> Hm.  I have to admit that I did not actually try your code, but there is
> a loop in there until no further substitution takes place.  It seems to
> me that this loop over process_macros thus does substitution _and_ uses
> up the escape characters.

Possibly, I haven't tested that either but ...

>
> So I fear that if in one iteration _some_ substitution takes place and
> the escape character is stripped in the same iteration _another_ round
> of substitution will be done and the escape character is gone.  So in
> effect you will have to study the whole string in advance (plus all the
> substitutions it will see) to tell how many rounds the loop will take
> and how many escape characters will be needed.
>
> Such a behaviour would be very odd in my eyes.  On the other hand, this
> is only theory....

... I don't think you or WD has changed your mind even it it worked the
way we want it to so no point arguing over an impl. detail/bug when we
don't agree on the big picture.

This "feature" isn't that important to me so I rest my case.

  Jocke

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

end of thread, other threads:[~2010-04-08 17:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-01  5:27 [U-Boot] U-boot env variables parsing Nitin Mahajan
2010-04-01 12:15 ` Detlev Zundel
2010-04-01 12:31   ` Joakim Tjernlund
2010-04-01 12:47     ` Wolfgang Denk
2010-04-01 12:56       ` Joakim Tjernlund
2010-04-01 13:05         ` Wolfgang Denk
2010-04-01 13:11           ` Joakim Tjernlund
2010-04-01 14:56             ` Detlev Zundel
2010-04-01 17:13               ` Joakim Tjernlund
2010-04-01 18:27                 ` Wolfgang Denk
2010-04-01 20:08                   ` Joakim Tjernlund
2010-04-08 10:00                     ` Detlev Zundel
2010-04-08 15:41                       ` Joakim Tjernlund
2010-04-08 16:06                         ` Detlev Zundel
2010-04-08 17:49                           ` Joakim Tjernlund

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.