From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'
Date: Wed, 20 Nov 2019 21:21:36 +0100 [thread overview]
Message-ID: <8a392b32-9662-7296-3086-7e9c9f8fcbf9@gmail.com> (raw)
In-Reply-To: <0102016e8a263d9b-81e7eb33-ed84-442a-ae92-7ffa9b3333ff-000000@eu-west-1.amazonses.com>
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
>
next prev parent reply other threads:[~2019-11-20 20:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-19 17:31 [U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f' James Byrne
2019-11-19 20:30 ` Simon Goldschmidt
2019-11-19 20:56 ` Heinrich Schuchardt
2019-11-19 21:01 ` Simon Goldschmidt
2019-11-19 21:34 ` Joe Hershberger
2019-11-20 21:24 ` Tom Rini
2019-11-20 18:49 ` James Byrne
2019-11-20 20:21 ` Simon Goldschmidt [this message]
2019-11-20 0:10 ` AKASHI Takahiro
2019-11-19 21:33 ` Joe Hershberger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8a392b32-9662-7296-3086-7e9c9f8fcbf9@gmail.com \
--to=simon.k.r.goldschmidt@gmail.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox