[U-Boot] [RFC PATCH 0/7] "Transient" (= export-restricted) environment vars

[U-Boot] [RFC PATCH 0/7] "Transient" (= export-restricted) environment vars

[Bernhard Nortmann]

This series evolved around the idea of introducing a new env_op
type to control (and possibly restrict) the export of variables.
This is especially useful if one wants to prevent dynamic
configuration information from ending up in a saved environment -
the 'classic' example being network setup with "dhcp" followed
by the "saveenv" command.

(The networking case is even further complicated by the fact
that users may actually wish to setup and save a static IP
configuration manually, which means that "locking up" the
corresponding variables right away isn't a viable solution.)

See also:
http://lists.denx.de/pipermail/u-boot/2015-September/227611.html
http://lists.denx.de/pipermail/u-boot/2016-April/250237.html

Regards, B. Nortmann

BTW: What the correct 'subsystem'/prefix for the "core" changes
related to env vars? patman isn't happy with my "env:" choice...


Bernhard Nortmann (7):
  env: Allow unconditional access if H_PROGRAMMATIC is set
  net: dm: Ignore unknown env_op_* constants
  env: Introduce "export" operation and (access flag) restriction
  env: Introduce "transient" and "system" access flags
  sunxi: env: flag fel_* environment vars as "system"
  env: Introduce setenv_transient() helper function
  env: Automatically mark dynamic configuration info as "do not export"

 cmd/net.c                      | 34 +++++++++++++++++-----------------
 cmd/nvedit.c                   | 24 +++++++++++++++++++++++-
 common/env_flags.c             | 21 ++++++++++++++++++---
 include/common.h               |  1 +
 include/configs/sunxi-common.h |  8 ++++++++
 include/env_flags.h            |  5 ++++-
 include/search.h               |  1 +
 lib/hashtable.c                |  4 ++++
 net/eth-uclass.c               |  2 ++
 9 files changed, 78 insertions(+), 22 deletions(-)

-- 
2.7.3

[U-Boot] [RFC PATCH 1/7] env: Allow unconditional access if H_PROGRAMMATIC is set

[Bernhard Nortmann]

This patch modifies env_flags_validate() in such a way that any
operation will *always* be allowed if H_PROGRAMMATIC is present.

Without this change, programmatically changing environment vars
may fail depending on their flags, e.g. when trying to setenv*()
a variable that is marked "read-only".
http://lists.denx.de/pipermail/u-boot/2016-April/250237.html

Notes: H_FORCE may be insufficient for this purpose, as it can be
disabled by CONFIG_ENV_ACCESS_IGNORE_FORCE.
H_PROGRAMMATIC indicates "U-Boot internal" use. By contrast, any
user interaction (from U-Boot prompt or scripts) is expected to
be marked H_INTERACTIVE.

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>

---

 common/env_flags.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/env_flags.c b/common/env_flags.c
index 921d377..1087f4e 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -523,6 +523,8 @@ int env_flags_validate(const ENTRY *item, const char *newval, enum env_op op,
 	}
 
 	/* check for access permission */
+	if (flag & H_PROGRAMMATIC)
+		return 0;
 #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
 	if (flag & H_FORCE)
 		return 0;
-- 
2.7.3

[U-Boot] [RFC PATCH 2/7] net: dm: Ignore unknown env_op_* constants

[Bernhard Nortmann]

This prevents a possible compiler warning similar to
"net/eth-uclass.c:<line>:<pos>: warning: enumeration value
'env_op_*' not handled in switch [-Wswitch]".

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 net/eth-uclass.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index c15cc4d..e38edd7 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -230,6 +230,8 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
 			break;
 		case env_op_delete:
 			memset(pdata->enetaddr, 0, 6);
+		default:
+			break; /* ignore */
 		}
 	}
 
-- 
2.7.3

[U-Boot] [RFC PATCH 3/7] env: Introduce "export" operation and (access flag) restriction

[Bernhard Nortmann]

This patch introduces a new "export" environment operation
(env_op_export) and the corresponding access flag
ENV_FLAGS_VARACCESS_PREVENT_EXPORT; so that env_flags_validate()
may now check requests to export specific variables.

In turn, hexport_r() makes uses of this ability to suppress the
export of variables that are flagged accordingly.

Note that env_flags_validate() and hexport_r() will respect H_FORCE
and H_PROGRAMMATIC flags, allowing to bypass the export filtering.
H_PROGRAMMATIC gets used within env_print() to make sure all
variables are listed. This is necessary because env_print() is
essentially an "export to text" operation.

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 cmd/nvedit.c        | 3 ++-
 common/env_flags.c  | 8 +++++++-
 include/env_flags.h | 3 ++-
 include/search.h    | 1 +
 lib/hashtable.c     | 4 ++++
 5 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index b67563b..88dbcb9 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -101,7 +101,8 @@ static int env_print(char *name, int flag)
 	}
 
 	/* print whole list */
-	len = hexport_r(&env_htab, '\n', flag, &res, 0, 0, NULL);
+	len = hexport_r(&env_htab, '\n',
+			flag | H_PROGRAMMATIC, &res, 0, 0, NULL);
 
 	if (len > 0) {
 		puts(res);
diff --git a/common/env_flags.c b/common/env_flags.c
index 1087f4e..f39d952 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -510,7 +510,7 @@ int env_flags_validate(const ENTRY *item, const char *newval, enum env_op op,
 	newval = newval ? : "";
 
 	/* validate the value to match the variable type */
-	if (op != env_op_delete) {
+	if (op != env_op_delete && op != env_op_export) {
 		enum env_flags_vartype type = (enum env_flags_vartype)
 			(ENV_FLAGS_VARTYPE_BIN_MASK & item->flags);
 
@@ -560,6 +560,12 @@ int env_flags_validate(const ENTRY *item, const char *newval, enum env_op op,
 			return 1;
 		}
 		break;
+	case env_op_export:
+		if (item->flags & ENV_FLAGS_VARACCESS_PREVENT_EXPORT) {
+			printf("## Don't export \"%s\"\n", name);
+			return 1;
+		}
+		break;
 	}
 
 	return 0;
diff --git a/include/env_flags.h b/include/env_flags.h
index 0dcec06..7e2362a 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -173,6 +173,7 @@ int env_flags_validate(const ENTRY *item, const char *newval, enum env_op op,
 #define ENV_FLAGS_VARACCESS_PREVENT_CREATE		0x00000010
 #define ENV_FLAGS_VARACCESS_PREVENT_OVERWR		0x00000020
 #define ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR	0x00000040
-#define ENV_FLAGS_VARACCESS_BIN_MASK			0x00000078
+#define ENV_FLAGS_VARACCESS_PREVENT_EXPORT		0x00000080
+#define ENV_FLAGS_VARACCESS_BIN_MASK			0x000000F8
 
 #endif /* __ENV_FLAGS_H__ */
diff --git a/include/search.h b/include/search.h
index 343dbc3..bb95265 100644
--- a/include/search.h
+++ b/include/search.h
@@ -23,6 +23,7 @@ enum env_op {
 	env_op_create,
 	env_op_delete,
 	env_op_overwrite,
+	env_op_export,
 };
 
 /* Action which shall be performed in the call the hsearch.  */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 02b4105..708319d 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -621,6 +621,10 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
 			if ((flag & H_HIDE_DOT) && ep->key[0] == '.')
 				continue;
 
+			if (env_flags_validate(ep, NULL, env_op_export,
+					       flag & H_FORCE) != 0)
+				continue;
+
 			list[n++] = ep;
 
 			totlen += strlen(ep->key) + 2;
-- 
2.7.3

[U-Boot] [RFC PATCH 4/7] env: Introduce "transient" and "system" access flags

[Bernhard Nortmann]

"transient" (='t') is like "any", but requests that a variable
should not be exported (ENV_FLAGS_VARACCESS_PREVENT_EXPORT).

"system" (='S') is meant for 'internal' variables that
aren't supposed to be changed by the user. It corresponds
to "transient" plus "read-only".

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 common/env_flags.c  | 11 +++++++++--
 include/env_flags.h |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/common/env_flags.c b/common/env_flags.c
index f39d952..2c30c7f 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -28,7 +28,7 @@
 #endif
 
 static const char env_flags_vartype_rep[] = "sdxb" ENV_FLAGS_NET_VARTYPE_REPS;
-static const char env_flags_varaccess_rep[] = "aroc";
+static const char env_flags_varaccess_rep[] = "aroctS";
 static const int env_flags_varaccess_mask[] = {
 	0,
 	ENV_FLAGS_VARACCESS_PREVENT_DELETE |
@@ -37,7 +37,12 @@ static const int env_flags_varaccess_mask[] = {
 	ENV_FLAGS_VARACCESS_PREVENT_DELETE |
 		ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
 	ENV_FLAGS_VARACCESS_PREVENT_DELETE |
-		ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR};
+		ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR,
+	ENV_FLAGS_VARACCESS_PREVENT_EXPORT,
+	ENV_FLAGS_VARACCESS_PREVENT_DELETE |
+		ENV_FLAGS_VARACCESS_PREVENT_CREATE |
+		ENV_FLAGS_VARACCESS_PREVENT_OVERWR |
+		ENV_FLAGS_VARACCESS_PREVENT_EXPORT};
 
 #ifdef CONFIG_CMD_ENV_FLAGS
 static const char * const env_flags_vartype_names[] = {
@@ -55,6 +60,8 @@ static const char * const env_flags_varaccess_names[] = {
 	"read-only",
 	"write-once",
 	"change-default",
+	"transient",	/* do not export/save */
+	"system",	/* = "transient" plus "read-only" */
 };
 
 /*
diff --git a/include/env_flags.h b/include/env_flags.h
index 7e2362a..9997a25 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -25,6 +25,8 @@ enum env_flags_varaccess {
 	env_flags_varaccess_readonly,
 	env_flags_varaccess_writeonce,
 	env_flags_varaccess_changedefault,
+	env_flags_varaccess_transient,
+	env_flags_varaccess_locked,
 	env_flags_varaccess_end
 };
 
-- 
2.7.3

[U-Boot] [RFC PATCH 5/7] sunxi: env: flag fel_* environment vars as "system"

[Bernhard Nortmann]

"fel_booted" and "fel_scriptaddr" have a special meaning and get
used by U-Boot internally.

This patch aims at both preventing user modification of these
values, and at excluding them from being stored on "saveenv".

As this is achieved by setting specialized access flags,
also enable the "env flags" command.

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 include/configs/sunxi-common.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 94275a7..1b48cf2 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -433,6 +433,14 @@ extern int soft_i2c_gpio_scl;
 	"fdt ram " FDT_ADDR_R " 0x100000;" \
 	"ramdisk ram " RAMDISK_ADDR_R " 0x4000000\0"
 
+/*
+ * flag environment vars for FEL mode ("usb boot") as special. "system" access
+ * means they're marked read-only, and shouldn't be written on "saveenv".
+ */
+#define CONFIG_ENV_FLAGS_LIST_STATIC "fel_booted:bS,fel_scriptaddr:xS"
+/* support "env flags" command */
+#define CONFIG_CMD_ENV_FLAGS
+
 #ifdef CONFIG_MMC
 #define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0)
 #if CONFIG_MMC_SUNXI_SLOT_EXTRA != -1
-- 
2.7.3

[U-Boot] [RFC PATCH 6/7] env: Introduce setenv_transient() helper function

[Bernhard Nortmann]

Like setenv(), but automatically marks the entry as "don't export".

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 cmd/nvedit.c     | 21 +++++++++++++++++++++
 include/common.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 88dbcb9..3c408f6 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -300,6 +300,27 @@ int setenv(const char *varname, const char *varvalue)
 }
 
 /**
+ * Set a "transient" environment variable
+ *
+ * Like setenv(), but this automatically marks the
+ * resulting entry as transient (= "do not export").
+ */
+int setenv_transient(const char *varname, const char *varvalue)
+{
+	int rc = setenv(varname, varvalue);
+	if (rc == 0) {
+		ENTRY e, *ep;
+
+		e.key = varname;
+		e.data = NULL;
+		hsearch_r(e, FIND, &ep, &env_htab, 0);
+		if (ep)
+			ep->flags |= ENV_FLAGS_VARACCESS_PREVENT_EXPORT;
+	}
+	return rc;
+}
+
+/**
  * Set an environment variable to an integer value
  *
  * @param varname	Environment variable to set
diff --git a/include/common.h b/include/common.h
index 3feaae6..a8e019a 100644
--- a/include/common.h
+++ b/include/common.h
@@ -380,6 +380,7 @@ ulong getenv_hex(const char *varname, ulong default_val);
 int getenv_yesno(const char *var);
 int	saveenv	     (void);
 int	setenv	     (const char *, const char *);
+int	setenv_transient(const char *, const char *);
 int setenv_ulong(const char *varname, ulong value);
 int setenv_hex(const char *varname, ulong value);
 /**
-- 
2.7.3

[U-Boot] [RFC PATCH 7/7] env: Automatically mark dynamic configuration info as "do not export"

[Bernhard Nortmann]

This is an attempt to prevent such information from ending up
in exported environment data, especially when doing "saveenv".
(http://lists.denx.de/pipermail/u-boot/2015-September/227611.html)

The patch makes use of the new setenv_transient() helper for
environment variables that get updated via network configuration:
BOOTP/DHCP (netboot_update_env), CDP (cdp_update_env) and
link-local protocol (do_link_local).

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 cmd/net.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/cmd/net.c b/cmd/net.c
index b2f3c7b..84b34ce 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -116,23 +116,23 @@ static void netboot_update_env(void)
 
 	if (net_gateway.s_addr) {
 		ip_to_string(net_gateway, tmp);
-		setenv("gatewayip", tmp);
+		setenv_transient("gatewayip", tmp);
 	}
 
 	if (net_netmask.s_addr) {
 		ip_to_string(net_netmask, tmp);
-		setenv("netmask", tmp);
+		setenv_transient("netmask", tmp);
 	}
 
 	if (net_hostname[0])
-		setenv("hostname", net_hostname);
+		setenv_transient("hostname", net_hostname);
 
 	if (net_root_path[0])
-		setenv("rootpath", net_root_path);
+		setenv_transient("rootpath", net_root_path);
 
 	if (net_ip.s_addr) {
 		ip_to_string(net_ip, tmp);
-		setenv("ipaddr", tmp);
+		setenv_transient("ipaddr", tmp);
 	}
 #if !defined(CONFIG_BOOTP_SERVERIP)
 	/*
@@ -141,32 +141,32 @@ static void netboot_update_env(void)
 	 */
 	if (net_server_ip.s_addr) {
 		ip_to_string(net_server_ip, tmp);
-		setenv("serverip", tmp);
+		setenv_transient("serverip", tmp);
 	}
 #endif
 	if (net_dns_server.s_addr) {
 		ip_to_string(net_dns_server, tmp);
-		setenv("dnsip", tmp);
+		setenv_transient("dnsip", tmp);
 	}
 #if defined(CONFIG_BOOTP_DNS2)
 	if (net_dns_server2.s_addr) {
 		ip_to_string(net_dns_server2, tmp);
-		setenv("dnsip2", tmp);
+		setenv_transient("dnsip2", tmp);
 	}
 #endif
 	if (net_nis_domain[0])
-		setenv("domain", net_nis_domain);
+		setenv_transient("domain", net_nis_domain);
 
 #if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_TIMEOFFSET)
 	if (net_ntp_time_offset) {
 		sprintf(tmp, "%d", net_ntp_time_offset);
-		setenv("timeoffset", tmp);
+		setenv_transient("timeoffset", tmp);
 	}
 #endif
 #if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_NTPSERVER)
 	if (net_ntp_server.s_addr) {
 		ip_to_string(net_ntp_server, tmp);
-		setenv("ntpserverip", tmp);
+		setenv_transient("ntpserverip", tmp);
 	}
 #endif
 }
@@ -294,14 +294,14 @@ static void cdp_update_env(void)
 		printf("CDP offered appliance VLAN %d\n",
 		       ntohs(cdp_appliance_vlan));
 		vlan_to_string(cdp_appliance_vlan, tmp);
-		setenv("vlan", tmp);
+		setenv_transient("vlan", tmp);
 		net_our_vlan = cdp_appliance_vlan;
 	}
 
 	if (cdp_native_vlan != htons(-1)) {
 		printf("CDP offered native VLAN %d\n", ntohs(cdp_native_vlan));
 		vlan_to_string(cdp_native_vlan, tmp);
-		setenv("nvlan", tmp);
+		setenv_transient("nvlan", tmp);
 		net_native_vlan = cdp_native_vlan;
 	}
 }
@@ -426,14 +426,14 @@ static int do_link_local(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	net_gateway.s_addr = 0;
 	ip_to_string(net_gateway, tmp);
-	setenv("gatewayip", tmp);
+	setenv_transient("gatewayip", tmp);
 
 	ip_to_string(net_netmask, tmp);
-	setenv("netmask", tmp);
+	setenv_transient("netmask", tmp);
 
 	ip_to_string(net_ip, tmp);
-	setenv("ipaddr", tmp);
-	setenv("llipaddr", tmp); /* store this for next time */
+	setenv_transient("ipaddr", tmp);
+	setenv_transient("llipaddr", tmp); /* store this for next time */
 
 	return CMD_RET_SUCCESS;
 }
-- 
2.7.3

[U-Boot] [RFC PATCH 0/7] "Transient" (= export-restricted) environment vars

[Joe Hershberger]

Hi Bernhard,

On Mon, Jul 11, 2016 at 1:14 PM, Bernhard Nortmann
<bernhard.nortmann@web.de> wrote:
> This series evolved around the idea of introducing a new env_op
> type to control (and possibly restrict) the export of variables.
> This is especially useful if one wants to prevent dynamic
> configuration information from ending up in a saved environment -
> the 'classic' example being network setup with "dhcp" followed
> by the "saveenv" command.
>
> (The networking case is even further complicated by the fact
> that users may actually wish to setup and save a static IP
> configuration manually, which means that "locking up" the
> corresponding variables right away isn't a viable solution.)
>
> See also:
> http://lists.denx.de/pipermail/u-boot/2015-September/227611.html
> http://lists.denx.de/pipermail/u-boot/2016-April/250237.html

Also see:

http://lists.denx.de/pipermail/u-boot/2010-May/071315.html
http://lists.denx.de/pipermail/u-boot/2010-June/073031.html

> Regards, B. Nortmann
>
> BTW: What the correct 'subsystem'/prefix for the "core" changes
> related to env vars? patman isn't happy with my "env:" choice...

I still use "env:" and just silence patman.

>
> Bernhard Nortmann (7):
>   env: Allow unconditional access if H_PROGRAMMATIC is set
>   net: dm: Ignore unknown env_op_* constants
>   env: Introduce "export" operation and (access flag) restriction
>   env: Introduce "transient" and "system" access flags
>   sunxi: env: flag fel_* environment vars as "system"
>   env: Introduce setenv_transient() helper function
>   env: Automatically mark dynamic configuration info as "do not export"
>
>  cmd/net.c                      | 34 +++++++++++++++++-----------------
>  cmd/nvedit.c                   | 24 +++++++++++++++++++++++-
>  common/env_flags.c             | 21 ++++++++++++++++++---
>  include/common.h               |  1 +
>  include/configs/sunxi-common.h |  8 ++++++++
>  include/env_flags.h            |  5 ++++-
>  include/search.h               |  1 +
>  lib/hashtable.c                |  4 ++++
>  net/eth-uclass.c               |  2 ++
>  9 files changed, 78 insertions(+), 22 deletions(-)
>
> --
> 2.7.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC PATCH 4/7] env: Introduce "transient" and "system" access flags

[Bernhard Nortmann]

Am 11.07.2016 um 20:14 schrieb Bernhard Nortmann:
> "transient" (='t') is like "any", but requests that a variable
> should not be exported (ENV_FLAGS_VARACCESS_PREVENT_EXPORT).
>
> "system" (='S') is meant for 'internal' variables that
> aren't supposed to be changed by the user. It corresponds
> to "transient" plus "read-only".
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> ---
>
> [...]
>
> diff --git a/include/env_flags.h b/include/env_flags.h
> index 7e2362a..9997a25 100644
> --- a/include/env_flags.h
> +++ b/include/env_flags.h
> @@ -25,6 +25,8 @@ enum env_flags_varaccess {
>   	env_flags_varaccess_readonly,
>   	env_flags_varaccess_writeonce,
>   	env_flags_varaccess_changedefault,
> +	env_flags_varaccess_transient,
> +	env_flags_varaccess_locked,
>   	env_flags_varaccess_end
>   };
>   

The "env_flags_varaccess_locked" is a remnant of a previous iteration and
should of course read "env_flags_varaccess_system" instead. I've fixed this
in my local branch, and the correction will be part of a future v2.

Regards, B. Nortmann

[U-Boot] [RFC PATCH 3/7] env: Introduce "export" operation and (access flag) restriction

[Bernhard Nortmann]

Am 11.07.2016 um 20:14 schrieb Bernhard Nortmann:
> This patch introduces a new "export" environment operation
> (env_op_export) and the corresponding access flag
> ENV_FLAGS_VARACCESS_PREVENT_EXPORT; so that env_flags_validate()
> may now check requests to export specific variables.
>
> In turn, hexport_r() makes uses of this ability to suppress the
> export of variables that are flagged accordingly.
>
> Note that env_flags_validate() and hexport_r() will respect H_FORCE
> and H_PROGRAMMATIC flags, allowing to bypass the export filtering.
> H_PROGRAMMATIC gets used within env_print() to make sure all
> variables are listed. This is necessary because env_print() is
> essentially an "export to text" operation.
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> ---
>
> [...]
>
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 02b4105..708319d 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -621,6 +621,10 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
>   			if ((flag & H_HIDE_DOT) && ep->key[0] == '.')
>   				continue;
>   
> +			if (env_flags_validate(ep, NULL, env_op_export,
> +					       flag & H_FORCE) != 0)
> +				continue;
> +
>   			list[n++] = ep;
>   
>   			totlen += strlen(ep->key) + 2;

This is also a remnant of previous experiments, and a too narrow condition
that doesn't match the commit message's description. If "flag" is to be
fully respected (H_FORCE | H_PROGRAMMATIC bits), then the H_FORCE mask
(bitwise and operation) needs to be removed. Fixed in my local branch.

Regards, B. Nortmann

[U-Boot] [RFC PATCH 2/7] net: dm: Ignore unknown env_op_* constants

[Simon Glass]

Hi Bernhard,

On 11 July 2016 at 12:14, Bernhard Nortmann <bernhard.nortmann@web.de> wrote:
> This prevents a possible compiler warning similar to
> "net/eth-uclass.c:<line>:<pos>: warning: enumeration value
> 'env_op_*' not handled in switch [-Wswitch]".
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> ---
>
>  net/eth-uclass.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index c15cc4d..e38edd7 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -230,6 +230,8 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
>                         break;
>                 case env_op_delete:
>                         memset(pdata->enetaddr, 0, 6);
> +               default:
> +                       break; /* ignore */

Presumably hitting the default would be an internal error. So in that
case there is no point in returning an error, I think, and what you
have is best.

>                 }
>         }
>
> --
> 2.7.3
>

Regards,
Simon

[U-Boot] [RFC PATCH 2/7] net: dm: Ignore unknown env_op_* constants

[Bernhard Nortmann]

Hi Simon!

Am 15.07.2016 um 05:19 schrieb Simon Glass:
> [...]
> Presumably hitting the default would be an internal error. So in that
> case there is no point in returning an error, I think, and what you
> have is best.
>
> Regards,
> Simon


It's not pretty, but a "catch all" solution. The alternative is to 
handle all
possible env_op_* constants explicitly, even if they don't affect (or relate
to) eth*_addr at all. But that would mean this code has to be touched and
adjusted every time someone comes up with a new env_op_*...

Regards, B. Nortmann