[PATCH V2 01/13] env: Complete generic support for writable list

[Stefan Herbrechtsmeier]

Hi,

Am 05.10.2022 um 10:33 schrieb Jan Kiszka:
> From: Jan Kiszka <jan.kiszka at siemens.com>
>
> This completes what 890feecaab72 started by selecting ENV_APPEND and
> ENV_IS_NOWHERE and by moving this driver to top if the list. This
> ensures that load operations pick up both the default env and the
> permitted parts of the next-prio location. When writing though, we must
> use the regular ordering.
>
> 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>
> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
> ---
>   env/Kconfig |  2 ++
>   env/env.c   | 22 ++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
>
> diff --git a/env/Kconfig b/env/Kconfig
> index 24111dfaf47..05b5fbf17d1 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -715,6 +715,8 @@ config ENV_APPEND
>   
>   config ENV_WRITEABLE_LIST
>   	bool "Permit write access only to listed variables"
> +	select ENV_IS_NOWHERE
> +	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 69848fb0608..d0649f9540d 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -133,6 +133,28 @@ __weak enum env_location arch_env_get_location(enum env_operation op, int prio)
>   	if (prio >= ARRAY_SIZE(env_locations))
>   		return ENVL_UNKNOWN;
>   
> +	if (IS_ENABLED(CONFIG_ENV_WRITEABLE_LIST)) {
> +		/*
> +		 * In writeable-list mode, ENVL_NOWHERE gains highest prio by
> +		 * virtually injecting it at prio 0.
> +		 */
> +		if (prio == 0) {
> +			/*
> +			 * Avoid the injection for write operations as that
> +			 * would block it.
> +			 */
> +			if (op != ENVOP_SAVE && op != ENVOP_ERASE)
> +				return ENVL_NOWHERE;

Is it consensus now to use ENVL_NOWHERE as synonym for default 
environment? If I remember correct this was rejected in the past and 
ENVL_NOWHERE  should only be used if no enviroment is available.

Why don't you call env_set_default(NULL, 0) in env_load() before 
env_driver_lookup()?

> +		} else {
> +			/*
> +			 * always subtract 1, also for writing because
> +			 * env_load_prio, which is used for writing, was
> +			 * initialized with that offset.
> +			 */
> +			prio--;
> +		}
> +	}
> +
>   	return env_locations[prio];
>   }
>   

Regards

   Stefan