On Fri, Dec 12, 2025 at 02:25:54PM +0100, Ahmad Fatoum wrote: > Hello Sascha, > > On 12/12/25 10:37 AM, Sascha Hauer wrote: > > 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 <[email protected]> > >> --- > >> 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? > > In a later commit, we skip envfs_load if autoload_external_env() is > disabled. I thought that it's strange for nv -s to still load the > external environment to write variables into it.
Ok. I think a message for this would be good. 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 |
