[U-Boot] [PATCH 1/2] env: set individual variables to default

[Gerlando Falauto]

On 09/23/2011 11:55 AM, Wolfgang Denk wrote:

[...]
>> While we're on this subject, do you think it would make any sense to
>> *import* individual variables?
>
> Tough question.  In theory yes, it would make perfect sense. On the
> other hand, this is a boot loader, and we should not try to be
> feature-complete.  So far I haven't seen a use case for this.

Well, at Keymile we are essentially using "env import" as a way to have 
"default" variables stored in an external file (e.g., because they are 
only needed for development and not for production, and that not only 
reduces the site of the environments, but also makes updates easier).
To this extent, "env import" and "env default" sort of have the same 
purpose...

>> If yes, what could the syntax be?
>>
>>     env import [flags] addr [size]<vars...>
>
> env import -n name[,..] [other_flags] addr [size]
>
> ?

Uhm, wouldn't that make the syntax completely unrelated to all other 
commands, leading to confusion?

How about something like:

env import [-f] [flags] addr [size] [-n name[ ...]]
env default [-f] -a|-n name[ ...]
env set [-f] name [val ...]

?
Where:
  -a in "env default" would be the way to prevent the inadverent user
     from wrecking the environment by mistake.
  -f forces overwriting of read-only or write-once variables
  -n allows to be selective about which variables should be restored
     from default/external environment

>> would make the optional argument "size" ambiguous (unless we take for
>> granted that variable names cannot start with a digit...).
>
> But they can, at least until now.

Slightly off-topic: how about variables starting with "-"?

[...]
>> Anyway, the whole thing was based on the assumption that it could make
>> some sense at some point to import individual variables from a
>> downloaded file. If that's the case, code should be factored together
>> with himport_r() as you suggested. If not, most of this code (the part
>> checking for comments, tabs, '\0'...) can be safely deleted (and I don't
>> think factoring the rest would be necessary).
>
> I agree that it makes sense to generalize and clean up this interface.
> It makes sense to select individual variables, and it makes sense to
> unify the "-f" handling to enforce actions on protected variables
> (while without "-f" only actions on the "normal" variables should be
> done).
>
> I can even imagine introducing a new variable that contains the name
> of the write-protected variables (and probably other properties, like
> being excluded from saveenv, etc.) - this has been discussed a number
> of times before, now we have the code base in place to actually
> implement it.
>
> All we need to do is extend the struct entry (in "include/search.h")
> by an "int flags"), and we can there register properties like
> read-only, don't-save etc.  In a first step this could be added
> transparently - so we could remove all the special handling of
> "ethaddr", "serial#" etc. in common/cmd_nvedit.c; then we could unify
> this to include "eth1addr" etc as well; then  we could extend it to
> read the names of such variables and their properties from a variable,
> etc.
>
> ... there is plenty of ideas someone could pick up...
>

OK, one step at a time.
But it's definitely worth knowing your thoughts before starting.
Thank you for sharing them!

Best,
Gerlando Falauto