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

Marek Vasut
2012-08-27T09:57:57Z

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