[U-Boot] [PATCH v4 6/7] env: make "env default" selective, check and apply

[Marek Vasut]

Dear Gerlando Falauto,

> On 08/24/2012 11:10 PM, Marek Vasut wrote:
> > Dear Gerlando Falauto,
> > 
> >> On 08/24/2012 04:56 PM, Marek Vasut wrote:
> >>> Dear Gerlando Falauto,
> >>> 
> >>>> Change the syntax (user API) for "env default":
> >>>>     -f: override write-once variables
> >>>>     var... : accept individual variable(s)
> >>>>     -a: all (resetting the whole env is NOT the default behavior)
> >>>> 
> >>>> Enable variable checking and make changes effective by
> >>>> enabling do_apply argument to himport_r().
> >>>> 
> >>>> Signed-off-by: Gerlando Falauto<gerlando.falauto at keymile.com>
> >>>> ---
> >>>> 
> >>>>    common/cmd_nvedit.c   |   40
> >>>>    ++++++++++++++++++++++++++++++++++------ common/env_common.c   |  
> >>>>    28 +++++++++++++++++++++++++++-
> >>>>    include/environment.h |    3 +++
> >>>>    3 files changed, 64 insertions(+), 7 deletions(-)
> >>>> 
> >>>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> >>>> index b0860f3..ac2b985 100644
> >>>> --- a/common/cmd_nvedit.c
> >>>> +++ b/common/cmd_nvedit.c
> >>>> @@ -656,14 +656,41 @@ int envmatch(uchar *s1, int i2)
> >>>> 
> >>>>    	return -1;
> >>>>    
> >>>>    }
> >>>> 
> >>>> -static int do_env_default(cmd_tbl_t *cmdtp, int flag,
> >>>> +static int do_env_default(cmd_tbl_t *cmdtp, int __flag,
> >>>> 
> >>>>    			  int argc, char * const argv[])
> >>>>    
> >>>>    {
> >>>> 
> >>>> -	if (argc != 2 || strcmp(argv[1], "-f") != 0)
> >>>> -		return CMD_RET_USAGE;
> >>>> +	int all = 0, flag = 0;
> >>>> 
> >>>> -	set_default_env("## Resetting to default environment\n");
> >>>> -	return 0;
> >>>> +	debug("Initial value for argc=%d\n", argc);
> >>>> +	while (--argc>   0&&   **++argv == '-') {
> >>> 
> >>> mmmmm ... **++argv, yummy :) This might use some cleanup, to make more
> >>> readable.
> >> 
> >> Uhm, this pattern is being used all over the place on that file (that's
> >> where I copied it from).
> > 
> > That doesn't mean it's right, it just means the codebase is in a very sad
> > state
> > 
> > :-C
> > :
> >>> Don't we have some getopt or something too ?
> >> 
> >> Not that I (or "git grep") know of.
> > 
> > Even more :-C
> 
> You're absolutely right, but this patchset has been around for almost a
> year and we've gotten nowhere with it. I'm afraid that if we keep adding
> new (somewhat unrelated) improvement requests we'll get stuck with it
> for quite some time once again.

Definitelly, it was just a remark that it's sad state of the codebase, not that 
you should start implementing it !

> Could we keep this getopt topic in mind as an open point for a later,
> overall review of how command line options are parsed?

Yes!

> Thank you,
> Gerlando

Best regards,
Marek Vasut