[U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'

Simon Goldschmidt
2019-11-20T20:21:36Z

Am 20.11.2019 um 19:49 schrieb James Byrne:
> On 19/11/2019 21:01, Simon Goldschmidt wrote:
>>
>>
>> Heinrich Schuchardt <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
>> schrieb am Di., 19. Nov. 2019, 21:56:
>>
>> On 11/19/19 9:30 PM, Simon Goldschmidt wrote:
>> > Am 19.11.2019 um 18:31 schrieb James Byrne:
>> >> Add env_force() to provide an equivalent to 'setenv -f' that can
>> be used
>> >> programmatically.
>> >>
>> >> Also tighten up the definition of argv in _do_env_set() so that
>> >> 'const char *' pointers are used.
>> >>
>> >> Signed-off-by: James Byrne <james.byrne@origamienergy.com
>> <mailto:james.byrne@origamienergy.com>>
>> >
>> > OK, I'm on CC, so I'll give my two cent:
>> >
>> > I always thought this code to be messed up a bit: I think it's better
>> > programming style to have the "string argument parsing" code call
>> real C
>> > functions with typed arguments. The env code instead does it the
>> other
>> > way round (here, you add do_programmatic_env_set() that sets up an
>> > argv[] array to call another function).
>> >
>> > I'm not a maintainer for the ENV code, but maybe that should be
>> sorted
>> > out instead of adding yet more code that goes this way?
>>
>> There is no maintainer for the ENV code. Simon makes a valid point here.
>> By creating a function for setting variables and separating it from
>> parsing arguments you get the function you need for forcing the value of
>> a variable for free.
>>
>>
>> Right. I thought someone had volunteered but a look at the maintainers
>> file proves me wrong.
>>
>> In any way, I'd be more open to review a cleanup patch than a patch
>> continuing this messy code flow.
>
> Having looked at it again, I agree. I have now redone it, but I have
> ended up changing quite a lot more of the underlying code. I will
> resubmit a revised patch (probably tomorrow) in two parts, one to apply
> some tidying up to the env code, and one to add the new function. It
> will be a much bigger patch set though!

Cool. I wouldn't want to put this as a burdon on you (meaning to NACK
this patch like it is), btu a cleanup in that direction would certainly
be appreciated!

Thanks,
Simon

>
> James
>