[PATCH 1/2] env: Complete generic support for writable list

[Jan Kiszka]

On 07.02.23 05:02, Simon Glass wrote:
> Hi Jan,
> 
> On Fri, 3 Feb 2023 at 05:23, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>
>> From: Jan Kiszka <jan.kiszka at siemens.com>
>>
>> This completes what 890feecaab72 started by selecting ENV_APPEND and
>> loading the default env before any other sources. This ensures that load
>> operations pick up all non-writable vars from the default env and only
>> permitted parts from other locations according to the regular
>> priorities.
>>
>> With this change, boards only need to define the list of writable
>> variables but no longer have to provide a custom env_get_location
>> implementation.
>>
>> CC: Joe Hershberger <joe.hershberger at ni.com>
>> CC: Marek Vasut <marex at denx.de>
>> CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss at weidmueller.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>> Reviewed-by: Marek Vasut <marex at denx.de>
>> ---
>>  env/Kconfig | 1 +
>>  env/env.c   | 8 ++++++++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/env/Kconfig b/env/Kconfig
>> index c409ea71fe5..6e24eee55f2 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -733,6 +733,7 @@ config ENV_APPEND
>>
>>  config ENV_WRITEABLE_LIST
>>         bool "Permit write access only to listed variables"
>> +       select ENV_APPEND
>>         help
>>           If defined, only environment variables which explicitly set the 'w'
>>           writeable flag can be written and modified at runtime. No variables
>> diff --git a/env/env.c b/env/env.c
>> index 06078c7f374..45e638fcd1f 100644
>> --- a/env/env.c
>> +++ b/env/env.c
>> @@ -195,6 +195,14 @@ int env_load(void)
>>         int best_prio = -1;
>>         int prio;
>>
>> +       if (CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)) {
>> +               /*
>> +                * When using a list of writeable variables, the baseline comes
>> +                * from the built-in default env. So load this first.
>> +                */
>> +               env_set_default(NULL, 0);
>> +       }
>> +
>>         for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) {
>>                 int ret;
>>
>> --
>> 2.35.3
>>
> 
> This looks OK, but please can you write some tests in test/env ?

Looking at those, it seems there is nothing at all regarding access
flags yet. Any suggestions how to first of all build up that
infrastructure are welcome. Then I could add this aspect here on top.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux