Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file

[Wolfgang Denk <wd@denx.de>]

Dear Simon,

In message <CAPnjgZ1BuAJC6Vhp06HrifU9jZqbOEuC+zRauV1DH3rY1qZp3Q@mail.gmail.com> you wrote:
>
> > > i.e.
> > >   var+=value
> > > appends value to var, while
> > >   var\+=value
> > > sets variable with name "var+"
>
> My first preference is to disallow + at the end of an end var. Perhaps
> we can start printing a warning if people do it, for a few releases.

This might seem to be a harmless change, but it is actually a
fundamental one.  And it breaks backward compatiility.  And all this
without need, as a list of alternatives have been suggested.

> My distance second preference is what Marek has here, using a
> backslash to escape the + character.

Actually this has the same problem, as the backslash is also a legal
character in a variable name:

	=> setenv foo\\+ bar
	=> printenv foo\\+  
	foo\+=bar


Yes, it was probably not a good idea not to restrict the allowed
character set when I implemented this stuff 21 years ago, but then
code size was critical - we had U-Boot running from 128 kB EPROM
(you remember these huge chips which were erased under UV light?).

The fact is, '=' and NUL are the only characters that cannot be used
in a variable name.


> As for =+ ...while I can see how people might parse it (we are setting
> the var equal to what it has with an appending string) I think it is a
> terrible idea as it is just not what people expect.

What do people expect? This is a totally new feature, so people will
use what they find in the documentation and in example code.

> Also, putting the
> + after the = places (similarly unlikely) restrictions on the
> expression.

There is a fundamental difference here.

For the '+=' case, there is no way to escape the '+', as all
commonly used escapes are valid characters in the variable name,
too.

With '=+', the '=' defines where the variable name ends, and from
here you can define your own rules as where the value part begins -
this is just a matter how you implement your parser.

> The current format is basically the same as 'print'. So if I can't
> have the first preference, we could ensure that it prints a \ in the
> case that the var ends with +

But '\' is a legal character in the variable name, too. Anything but
'=' and NUL is a legal char. And this makes escaping impossible:

	=> setenv \'foo\\-\' foobar
	=> printenv \'foo\\-\'
	'foo\-'=foobar

> > Also, I think that it would be better if spaces and tabs were allowed
> > to indent the .env file, i.e.
> >
> >         var_a   =       3
> >         var_bcd =       7
> >
> > should set "var_a" to "3", "var_bcd" to "7".
> >
> > If special character are needed in either name or value, they could be
> > escaped and/or quoted.
>
> They are allowed in the value but are reduced to a single space in the
> front. We need this for multi-line strings (but I'm a bit worried
> about it).

You mean this automatically insert a newline between parts? ugh...
I didn't realize this. Did I miss it in the documentation?

> We could update it to skip any leading space after the = I think.

So what if you need a leading space?


> I don't like spaces before the = though. It doesn't match the 'print'
> output (which has no space) and it is confusing:

env print also does not add any spaces after the '='.

> I think we need strict rules so it is easy for people to get exactly
> the env they want.

Strict rules, proper documentation, and a set of examples.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Any time things appear to be going better, you have overlooked  some-
thing.