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@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