Re: [PATCH v2 1/3] globalvar: suppress nvvar_save when no external environment was loaded

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Dec 11, 2025 at 09:48:10PM +0100, Ahmad Fatoum wrote:
> nvvar_save will load the extenral environment before writing it back
> with nv changed, which we means we still end up parsing the environment
> in this case, even if we don't execute init scripts or import nv out of
> it.
> 
> Fix this to only parse the environment when we actually loaded it
> before.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/environment.c | 7 +++++++
>  common/globalvar.c   | 8 +++++++-
>  include/globalvar.h  | 1 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/common/environment.c b/common/environment.c
> index 0e551c90352e..ec14d0629a14 100644
> --- a/common/environment.c
> +++ b/common/environment.c
> @@ -453,6 +453,7 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
>  	int envfd;
>  	int ret = 0;
>  	size_t size, rsize;
> +	__maybe_unused const char *defenv_path;
>  
>  #ifdef __BAREBOX__
>  	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD))
> @@ -531,6 +532,12 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
>  
>  	ret = 0;
>  
> +#ifdef CONFIG_NVVAR
> +	defenv_path = default_environment_path_get();
> +	if (defenv_path && !strcmp(filename, defenv_path))
> +	    nv_var_set_persistable();
> +#endif
> +
>  out:
>  	close(envfd);
>  	free(buf);
> diff --git a/common/globalvar.c b/common/globalvar.c
> index 77af6733a6a0..1e06fb43775f 100644
> --- a/common/globalvar.c
> +++ b/common/globalvar.c
> @@ -15,6 +15,7 @@
>  #include <fnmatch.h>
>  
>  static int nv_dirty;
> +static bool nv_persistable;
>  
>  struct device global_device = {
>  	.name = "global",
> @@ -31,6 +32,11 @@ void nv_var_set_clean(void)
>  	nv_dirty = 0;
>  }
>  
> +void nv_var_set_persistable(void)
> +{
> +	nv_persistable = true;
> +}
> +
>  void globalvar_remove(const char *name)
>  {
>  	struct param_d *p, *tmp;
> @@ -713,7 +719,7 @@ int nvvar_save(void)
>  	const char *env = default_environment_path_get();
>  	int ret = 0;
>  #define TMPDIR "/.env.tmp"
> -	if (!nv_dirty || !env)
> +	if (!nv_dirty || !env || !nv_persistable)
>  		return 0;

With this "nv -s" or whatever calls this just silently does nothing.
This doesn't sound like a desired behaviour. At least a message would be
useful.

What's the purpose of this patch anyway?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |