[U-Boot] [PATCH] Tools: set multiple variable with fw_setenv utility

[Wolfgang Denk]

Dear Stefano Babic,

In message <4BF6A381.7020200 at denx.de> you wrote:
> Wolfgang Denk wrote:
> 
> >> a config file with a list of pairs <variable, value> to be set,
> >> separated by a TAB character.
> > 
> > I think we should be less restrictive here. Please split at the first
> > white space, and allow for multiple white spaces.
> 
> There is a reason why I set to this format. There should be a case where
> a variable is set with a leading (or more as one) white space. For
> example, I have seen something related to board names, and adding some
> spaces makes a sort of formatting without changing the code.

Adding spaces to the variable value? This makes little sense to me.
It's just a waste of storage space and boot time.

If you feel your environment is so complicated to read that you want
such "formatting", then rather structure your envrionment (see
proposals on the list), and/or help improving the printenv
capabilities to add sorting etc.

> Because the TAB character is not allowed inside a variable, this assures
> that everything except TAB is taken as part of the variables's value.

TAB can be embedded in a variable value (if you manage to enter it).

> See my comment above. With my proposal, something as
> 
> board_name<TAB>    my product made by my company

> works flawlessy. We can add some delimiters (as ', "), but we could have
> such delimiters inside the value itself and we have then to use escapes.
> Because <TAB> is not allowed as character inside a variable (or I have
> not yet seen this case...), it makes thing easier.

Who says that TAB would not be allowed? Of course it is. There is
virtually no restriction on the content of the value of a variable.
The only character that cannot be part of the value is '\0'.

[Of course it is not a wise decision to add non-printing or control
characters, but you can do this, if you like.]

> >> +	char dump[128];
> > 
> > Ouch! That's short! Why do we need such an arbitrary limit?
> 
> We do not need a small value, but I have to set a value for fgets. A
> bigger value should be enough, reporting an error if the line is too long.

But your code is not set up to handle the remainder of too long lines.
It would be read as the next line and cause confusion.

> >> +typedef struct {
> >> +	char name[255];
> >> +	char value[255];
> >> +} fw_env_list;
> >
> > Again we have arbitrary limits. And even different ones from the one
> > above (128).
>
> Another possibility is to dinamically allocate memory for name/value on
> demand. Theoretically a value could be a very long string and the only
> limit I see in u-boot is the environment size.

Why do you have to run this in two spearate passes at all - first
scan, then process.

Why cannot you process each variable when you read it? The we would
not need any array or list at all.

> > The command should also accept '-' as file name, and then read from
> > stdin.
>
> Ok, understood. So we can allow something like "fw_setenv -s - <
> variables_file".

Right.

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
I must follow the people.  Am I not their leader? - Benjamin Disraeli