[PATCH] env: introduce CONFIG_ENV_DOTVARS_TEMPORARY

[PATCH] env: introduce CONFIG_ENV_DOTVARS_TEMPORARY

[Rasmus Villemoes]

The printenv command already by default hides variables beginning with
a dot. It can be useful to take that convention even further, and
prevent such variables from ever being stored persistently (and
ignored if they happen to exist in stable storage).

This way, one can freely use such variable names in script logic,
without worrying about random temporary variables leaking to
persistent storage and/or to/from another U-boot "session".

Shell variables can be used somewhat similarly, but they are not as
flexible, since many helper commands (e.g. setexpr, fdt) offer to
store their output in an environment variable, not a shell variable.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 env/Kconfig     | 10 ++++++++++
 env/common.c    |  6 ++++--
 lib/hashtable.c |  3 +++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index 4661082f0e..69fd2cae03 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -559,6 +559,16 @@ config SYS_RELOC_GD_ENV_ADDR
 	  Relocate the early env_addr pointer so we know it is not inside
 	  the binary. Some systems need this and for the rest, it doesn't hurt.
 
+config ENV_DOTVARS_TEMPORARY
+	bool "Ignore variables beginning with . when saving/loading the environment"
+	help
+	  If you select this option, environment variable names
+	  beginning with a dot (.) are skipped when writing the
+	  environment to persistent storage. Similarly, should the
+	  persistent storage somehow contain such a variable, it is
+	  ignored (i.e. not added to the runtime environment) when
+	  loading.
+
 config USE_DEFAULT_ENV_FILE
 	bool "Create default environment from file"
 	help
diff --git a/env/common.c b/env/common.c
index 0da21ee081..c23b490364 100644
--- a/env/common.c
+++ b/env/common.c
@@ -116,6 +116,7 @@ int env_set_default_vars(int nvars, char * const vars[], int flags)
 int env_import(const char *buf, int check)
 {
 	env_t *ep = (env_t *)buf;
+	int flag = IS_ENABLED(CONFIG_ENV_DOTVARS_TEMPORARY) ? H_HIDE_DOT : 0;
 
 	if (check) {
 		uint32_t crc;
@@ -128,7 +129,7 @@ int env_import(const char *buf, int check)
 		}
 	}
 
-	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0,
+	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', flag, 0,
 			0, NULL)) {
 		gd->flags |= GD_FLG_ENV_READY;
 		return 0;
@@ -212,9 +213,10 @@ int env_export(env_t *env_out)
 {
 	char *res;
 	ssize_t	len;
+	int flag = IS_ENABLED(CONFIG_ENV_DOTVARS_TEMPORARY) ? H_HIDE_DOT : 0;
 
 	res = (char *)env_out->data;
-	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, 0, NULL);
+	len = hexport_r(&env_htab, '\0', flag, &res, ENV_SIZE, 0, NULL);
 	if (len < 0) {
 		pr_err("Cannot export environment: errno = %d\n", errno);
 		return 1;
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 907e8a642f..e05a097c75 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -928,6 +928,9 @@ int himport_r(struct hsearch_data *htab,
 		if (!drop_var_from_set(name, nvars, localvars))
 			continue;
 
+		if ((flag & H_HIDE_DOT) && *name == '.')
+			continue;
+
 		/* enter into hash table */
 		e.key = name;
 		e.data = value;
-- 
2.23.0

[PATCH] env: introduce CONFIG_ENV_DOTVARS_TEMPORARY

[Wolfgang Denk]

Dear Rasmus Villemoes,

In message <20200108134247.31443-1-rasmus.villemoes@prevas.dk> you wrote:
> The printenv command already by default hides variables beginning with
> a dot. It can be useful to take that convention even further, and
> prevent such variables from ever being stored persistently (and
> ignored if they happen to exist in stable storage).
>
> This way, one can freely use such variable names in script logic,
> without worrying about random temporary variables leaking to
> persistent storage and/or to/from another U-boot "session".

This is not a good idea. The decision whether a variable shall be
stored permanently or not, or wheter it is readonly or writable, and
other such properties should never based on their name.  There may
be many good reasons that some .name variable _shall_ be persistent.


Naked-by: Wolfgang Denk <wd@denx.de>


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A door is what a dog is perpetually on the wrong side of.
                                                        - Ogden Nash

[PATCH] env: introduce CONFIG_ENV_DOTVARS_TEMPORARY

[Rasmus Villemoes]

On 20/01/2020 17.44, Wolfgang Denk wrote:
> Dear Rasmus Villemoes,
> 
> In message <20200108134247.31443-1-rasmus.villemoes@prevas.dk> you wrote:
>> The printenv command already by default hides variables beginning with
>> a dot. It can be useful to take that convention even further, and
>> prevent such variables from ever being stored persistently (and
>> ignored if they happen to exist in stable storage).
>>
>> This way, one can freely use such variable names in script logic,
>> without worrying about random temporary variables leaking to
>> persistent storage and/or to/from another U-boot "session".
> 
> This is not a good idea. The decision whether a variable shall be
> stored permanently or not, or wheter it is readonly or writable, and
> other such properties should never based on their name. 

Sorry, but what other property of the variable could possibly determine
those things, then?

 There may
> be many good reasons that some .name variable _shall_ be persistent.

Sure, absolutely. Which is why this is entirely opt-in for those who
know they won't need that, but do have some semi-complicated script that
interacts with various commands that return their output via an
environment variable.

> Naked-by: Wolfgang Denk <wd@denx.de>

Ah, now I see how  env_flags_varaccess is actually implemented,
involving a .flags special variable. OK, then I can certainly see why
one would not want that to be excluded from the environment - I just
thought the idea behind "printenv" hiding dot-variables by default was
that those were considered temporary, and not special in this way.

So, would you accept introducing env_flags_varaccess_temporary, for
which I could then add tmp_.*:st ?

Thanks,
Rasmus

[PATCH] env: introduce CONFIG_ENV_DOTVARS_TEMPORARY

[Wolfgang Denk]

Dear Rasmus,

In message <2676be2b-2e4f-7aba-14e6-5659174ad011@prevas.dk> you wrote:
>
> > This is not a good idea. The decision whether a variable shall be
> > stored permanently or not, or wheter it is readonly or writable, and
> > other such properties should never based on their name. 
> 
> Sorry, but what other property of the variable could possibly determine
> those things, then?

Such properties are stored in the .flags settings, see env/flags.c

>  There may
> > be many good reasons that some .name variable _shall_ be persistent.
> 
> Sure, absolutely. Which is why this is entirely opt-in for those who
> know they won't need that, but do have some semi-complicated script that
> interacts with various commands that return their output via an
> environment variable.

This has been discussed several times before (for example in the
context of UEFI persistance handling); it should be implemented
using the existing .flags mechanism.

> Ah, now I see how  env_flags_varaccess is actually implemented,
> involving a .flags special variable. OK, then I can certainly see why
> one would not want that to be excluded from the environment - I just
> thought the idea behind "printenv" hiding dot-variables by default was
> that those were considered temporary, and not special in this way.

Not only that, .flags is exactly the mechanism that should be used
to implement what you want.

> So, would you accept introducing env_flags_varaccess_temporary, for
> which I could then add tmp_.*:st ?

Please look up the last discussion of this topic; see the thread
"efi_loader: implementing non-volatile UEFI variables" starting
here:

https://lists.denx.de/pipermail/u-boot/2019-June/373503.html

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anyone who knows history, particularly the history of Europe, will, I
think, recognize that the domination of education or of government by
any one particular religious faith is never a happy  arrangement  for
the people.                                       - Eleanor Roosevelt