[U-Boot] [PATCH v6 1/6] env: add the same prefix to error messages to make it detectable by tests

[U-Boot] [PATCH v6 1/6] env: add the same prefix to error messages to make it detectable by tests

[Quentin Schulz]

The error message should start with `## Error: ` so that it's easily
detectable by tests without needing to have a complex regexp for
matching all possible error message patterns.

Let's add the `## Error: ` prefix to the error messages since it's the
one already in use.

Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---

added in v5

 cmd/nvedit.c | 12 ++++++++----
 env/common.c |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index ddc888a..70d7068 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -959,7 +959,8 @@ NXTARG:		;
 				H_MATCH_KEY | H_MATCH_IDENT,
 				&ptr, size, argc, argv);
 		if (len < 0) {
-			pr_err("Cannot export environment: errno = %d\n", errno);
+			pr_err("## Error: Cannot export environment: errno = %d\n",
+			       errno);
 			return 1;
 		}
 		sprintf(buf, "%zX", (size_t)len);
@@ -979,7 +980,8 @@ NXTARG:		;
 			H_MATCH_KEY | H_MATCH_IDENT,
 			&res, ENV_SIZE, argc, argv);
 	if (len < 0) {
-		pr_err("Cannot export environment: errno = %d\n", errno);
+		pr_err("## Error: Cannot export environment: errno = %d\n",
+		       errno);
 		return 1;
 	}
 
@@ -994,7 +996,8 @@ NXTARG:		;
 	return 0;
 
 sep_err:
-	printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",	cmd);
+	printf("## Error: %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
+	       cmd);
 	return 1;
 }
 #endif
@@ -1114,7 +1117,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 
 	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
 			crlf_is_lf, 0, NULL) == 0) {
-		pr_err("Environment import failed: errno = %d\n", errno);
+		pr_err("## Error: Environment import failed: errno = %d\n",
+		       errno);
 		return 1;
 	}
 	gd->flags |= GD_FLG_ENV_READY;
diff --git a/env/common.c b/env/common.c
index dc8a14f..7bd2790 100644
--- a/env/common.c
+++ b/env/common.c
@@ -83,7 +83,8 @@ void set_default_env(const char *s)
 	if (himport_r(&env_htab, (char *)default_environment,
 			sizeof(default_environment), '\0', flags, 0,
 			0, NULL) == 0)
-		pr_err("Environment import failed: errno = %d\n", errno);
+		pr_err("## Error: Environment import failed: errno = %d\n",
+		       errno);
 
 	gd->flags |= GD_FLG_ENV_READY;
 	gd->flags |= GD_FLG_ENV_DEFAULT;

base-commit: 22d58e60ffb5484d912f26b9c3533eff1d3d3de9
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 2/6] test/py: return a RAM address different from 0 as it can be interpreted as NULL

[Quentin Schulz]

Some functions test that the given address is not NULL (0) and fail or
have a different behaviour if that's the case (e.g. hexport_r).

Let's make the RAM base address to be not zero by setting it to 2MiB if
that's the case.

2MiB is chosen because it represents the size of an ARM LPAE/v8 section.

Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---

added in v5

 test/py/u_boot_utils.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index bb31e57..07520ac 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -236,6 +236,12 @@ def find_ram_base(u_boot_console):
             ram_base = -1
             raise Exception('Failed to find RAM bank start in `bdinfo`')
 
+    # We don't want ram_base to be zero as some functions test if the given
+    # address is NULL (0). Let's add 2MiB then (size of an ARM LPAE/v8 section).
+
+    if ram_base == 0:
+        ram_base += 1024 * 1024 * 2
+
     return ram_base
 
 class PersistentFileHelperCtxMgr(object):
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 3/6] test/py: remove hacks for non-zero RAM base address in tests

[Quentin Schulz]

Some functions have different behaviour when the given address is 0
(assumed to be NULL by the function).

find_ram_base() does not return 0 anymore so it's safe to remove those
offsets.

Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---

added in v5

 test/py/tests/test_efi_loader.py | 2 +-
 test/py/tests/test_net.py        | 4 ++--
 test/py/tests/test_tpm2.py       | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/py/tests/test_efi_loader.py b/test/py/tests/test_efi_loader.py
index 35bd419..a66c6e6 100644
--- a/test/py/tests/test_efi_loader.py
+++ b/test/py/tests/test_efi_loader.py
@@ -118,7 +118,7 @@ def fetch_tftp_file(u_boot_console, env_conf):
 
     addr = f.get('addr', None)
     if not addr:
-        addr = u_boot_utils.find_ram_base(u_boot_console) + (1024 * 1024 * 4)
+        addr = u_boot_utils.find_ram_base(u_boot_console)
 
     fn = f['fn']
     output = u_boot_console.run_command('tftpboot %x %s' % (addr, fn))
diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
index f2e432b..2821ce6 100644
--- a/test/py/tests/test_net.py
+++ b/test/py/tests/test_net.py
@@ -146,7 +146,7 @@ def test_net_tftpboot(u_boot_console):
 
     addr = f.get('addr', None)
     if not addr:
-        addr = u_boot_utils.find_ram_base(u_boot_console) + (1024 * 1024 * 4)
+        addr = u_boot_utils.find_ram_base(u_boot_console)
 
     fn = f['fn']
     output = u_boot_console.run_command('tftpboot %x %s' % (addr, fn))
@@ -186,7 +186,7 @@ def test_net_nfs(u_boot_console):
 
     addr = f.get('addr', None)
     if not addr:
-        addr = u_boot_utils.find_ram_base(u_boot_console) + (1024 * 1024 * 4)
+        addr = u_boot_utils.find_ram_base(u_boot_console)
 
     fn = f['fn']
     output = u_boot_console.run_command('nfs %x %s' % (addr, fn))
diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
index 01ffb31..ef7b86a 100644
--- a/test/py/tests/test_tpm2.py
+++ b/test/py/tests/test_tpm2.py
@@ -183,7 +183,7 @@ def test_tpm2_pcr_read(u_boot_console):
     """
 
     force_init(u_boot_console)
-    ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
+    ram = u_boot_utils.find_ram_base(u_boot_console)
 
     read_pcr = u_boot_console.run_command('tpm pcr_read 0 0x%x' % ram)
     output = u_boot_console.run_command('echo $?')
@@ -210,7 +210,7 @@ def test_tpm2_pcr_extend(u_boot_console):
     """
 
     force_init(u_boot_console)
-    ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
+    ram = u_boot_utils.find_ram_base(u_boot_console)
 
     u_boot_console.run_command('tpm pcr_extend 0 0x%x' % ram)
     output = u_boot_console.run_command('echo $?')
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 4/6] hashtable: do not recreate whole hash table if vars are passed to himport_r

[Quentin Schulz]

When vars are passed to the himport_r function with H_NOCLEAR flag,
those vars will be overridden in the current environment and if one of
those vars is not in the imported environment, it'll be deleted in the
current environment whatever the flag passed to himport_r.

The H_NOCLEAR flag is used to clear the whole environment whether vars
are passed to the function or not.

This leads to incoherent behaviour. If one passes vars to himport_r
with the H_NOCLEAR flag, if a var in vars is not in the imported env,
that var will be removed from the current env.

If one passes vars to himport_r without the H_NOCLEAR flag, the whole
environment will be removed and vars will be imported from the
environment in RAM.

It makes more sense to keep the variable that is in the current
environment but not in the imported environment if the H_NOCLEAR flag is
set and remove only that variable if the H_NOCLEAR flag is not set.

Let's clear the whole environment only if H_NOCLEAR and vars are not
passed to himport_r.

Let's remove variables that are in the current environment but not in
the imported env only if the H_NOCLEAR flag is not passed.

Suggested-by: Wolfgang Denk <wd@denx.de>
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---

added in v6

 lib/hashtable.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/hashtable.c b/lib/hashtable.c
index 52aab6d..ffaa5b6 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -749,8 +749,11 @@ static int drop_var_from_set(const char *name, int nvars, char * vars[])
  *
  * The "flag" argument can be used to control the behaviour: when the
  * H_NOCLEAR bit is set, then an existing hash table will kept, i. e.
- * new data will be added to an existing hash table; otherwise, old
- * data will be discarded and a new hash table will be created.
+ * new data will be added to an existing hash table; otherwise, if no
+ * vars are passed, old data will be discarded and a new hash table
+ * will be created. If vars are passed, passed vars that are not in
+ * the linear list of "name=value" pairs will be removed from the
+ * current hash table.
  *
  * The separator character for the "name=value" pairs can be selected,
  * so we both support importing from externally stored environment
@@ -801,7 +804,7 @@ int himport_r(struct hsearch_data *htab,
 	if (nvars)
 		memcpy(localvars, vars, sizeof(vars[0]) * nvars);
 
-	if ((flag & H_NOCLEAR) == 0) {
+	if ((flag & H_NOCLEAR) == 0 && !nvars) {
 		/* Destroy old hash table if one exists */
 		debug("Destroy Hash Table: %p table = %p\n", htab,
 		       htab->table);
@@ -933,6 +936,9 @@ int himport_r(struct hsearch_data *htab,
 	debug("INSERT: free(data = %p)\n", data);
 	free(data);
 
+	if (flag & H_NOCLEAR)
+		goto end;
+
 	/* process variables which were not considered */
 	for (i = 0; i < nvars; i++) {
 		if (localvars[i] == NULL)
@@ -951,6 +957,7 @@ int himport_r(struct hsearch_data *htab,
 			printf("WARNING: '%s' not in imported env, deleting it!\n", localvars[i]);
 	}
 
+end:
 	debug("INSERT: done\n");
 	return 1;		/* everything OK */
 }
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 5/6] cmd: nvedit: env import can now import only variables passed as parameters

[Quentin Schulz]

While the `env export` can take as parameters variables to be exported,
`env import` does not have such a mechanism of variable selection.

Let's add the ability to add parameters at the end of the command for
variables to be imported.

Every env variable from the env to be imported passed by parameter to
this command will override the value of the variable in the current env.

If a variable exists in the current env but not in the imported env, if
this variable is passed as a parameter to env import, the variable will
be unset ONLY if the -d option is passed to env import, otherwise the
current value of the variable is kept.

If a variable exists in the imported env, the variable in the current
env will be set to the value of the one from the imported env.

All the remaining variables are left untouched.

As the size parameter of env import is positional but optional, let's
add the possibility to use the sentinel '-' for when we don't want to
give the size parameter (when the env is '\0' terminated) but we pass a
list of variables at the end of the command.

env import addr
env import addr -
env import addr size
env import addr - foo1 foo2
env import addr size foo1 foo2

are all valid.

env import -c addr
env import -c addr -
env import -c addr - foo1 foo2

are all invalid because they don't pass the size parameter required for
checking, while the following are valid.

env import addr size
env import addr size foo1 foo2

Nothing's changed for the other parameters or the overall behaviour.

One of its use case could be to load a secure environment from the
signed U-Boot binary and load only a handful of variables from an
other, unsecure, environment without completely losing control of
U-Boot.

Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Tested-by: Alex Kiernan <alex.kiernan@gmail.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---

v6:
  - fix commit log and add comment on -d option to reflect changes introduced by
  patch 4,

v4:
  - add tested-by by Alex,

v3:
  - migrate to env import addr size var... instead of env import -w addr
  size so that the list of variables to load is more explicit and the
  behaviour of env import is closer to the one of env export,

v2:
  - use strdup instead of malloc + strcpy,
  - NULL-check the result of strdup,
  - add common exit path for freeing memory in one unique place,
  - store token pointer from strtok within the char** array instead of
  strdup-ing token within elements of array,

 cmd/nvedit.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 70d7068..5de9d38 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1004,8 +1004,11 @@ sep_err:
 
 #ifdef CONFIG_CMD_IMPORTENV
 /*
- * env import [-d] [-t [-r] | -b | -c] addr [size]
- *	-d:	delete existing environment before importing;
+ * env import [-d] [-t [-r] | -b | -c] addr [size] [var ...]
+ *	-d:	delete existing environment before importing if no var is
+ *		passed; if vars are passed, if one var is in the current
+ *		environment but not in the environment at addr, delete var from
+ *		current environment;
  *		otherwise overwrite / append to existing definitions
  *	-t:	assume text format; either "size" must be given or the
  *		text data must be '\0' terminated
@@ -1018,6 +1021,11 @@ sep_err:
  *	addr:	memory address to read from
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
+ *		if var is set and size should be missing (i.e. '\0'
+ *		termination), set size to '-'
+ *	var...	List of the names of the only variables that get imported from
+ *		the environment at address 'addr'. Without arguments, the whole
+ *		environment gets imported.
  */
 static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 			 int argc, char * const argv[])
@@ -1029,6 +1037,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	int	fmt = 0;
 	int	del = 0;
 	int	crlf_is_lf = 0;
+	int	wl = 0;
 	size_t	size;
 
 	cmd = *argv;
@@ -1077,9 +1086,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	addr = simple_strtoul(argv[0], NULL, 16);
 	ptr = map_sysmem(addr, 0);
 
-	if (argc == 2) {
+	if (argc >= 2 && strcmp(argv[1], "-")) {
 		size = simple_strtoul(argv[1], NULL, 16);
-	} else if (argc == 1 && chk) {
+	} else if (chk) {
 		puts("## Error: external checksum format must pass size\n");
 		return CMD_RET_FAILURE;
 	} else {
@@ -1101,6 +1110,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 		printf("## Info: input data size = %zu = 0x%zX\n", size, size);
 	}
 
+	if (argc > 2)
+		wl = 1;
+
 	if (chk) {
 		uint32_t crc;
 		env_t *ep = (env_t *)ptr;
@@ -1115,8 +1127,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 		ptr = (char *)ep->data;
 	}
 
-	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
-			crlf_is_lf, 0, NULL) == 0) {
+	if (!himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
+		       crlf_is_lf, wl ? argc - 2 : 0, wl ? &argv[2] : NULL)) {
 		pr_err("## Error: Environment import failed: errno = %d\n",
 		       errno);
 		return 1;
@@ -1246,7 +1258,7 @@ static char env_help_text[] =
 #endif
 #endif
 #if defined(CONFIG_CMD_IMPORTENV)
-	"env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n"
+	"env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n"
 #endif
 	"env print [-a | name ...] - print environment\n"
 #if defined(CONFIG_CMD_RUN)
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 6/6] test/py: add test for whitelist of variables while importing environment

[Quentin Schulz]

This tests that the importing of an environment with a specified
whitelist works as intended.

If there are variables passed as parameter to the env import command,
those only should be imported in the current environment.

For each variable passed as parameter, if
 - foo is bar in current env and bar2 in exported env, after importing
 exported env, foo shall be bar2,
 - foo does not exist in current env and foo is bar2 in exported env,
 after importing exported env, foo shall be bar2,
 - foo is bar in current env and does not exist in exported env (but is
 passed as parameter), after importing exported env, foo shall be empty
 ONLY if the -d option is passed to env import, otherwise foo shall be
 bar,

Any variable not passed as parameter should be left untouched.

Two other tests are made to test that size cannot be '-' if the checksum
protection is enabled.

Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---

v6:
  - fix commit log to reflect changes introduced by patch 4,
  - fix existing test to reflect changes introduced by patch 4,
  - add test to test env import with the -d option,
  - undo what's done in the test at the end of it so we have a clean
  environment,

v5:
  - add reviewed-by by Stephen,

v4:
  - add reviewed-by by Simon,
  - fix double quotes instead of simple ones for strings,
  - fix missing space after # starting a comment,

v3:
  - update whitelist test to reflect changes made in patch 1,
  - add two tests for no size parameter passed but checksum protection is
  enabled because I added the possibility to have a sentinel in place of
  size parameter,
  - I intentionally didn't put the Reviewed and Acked-by of Simon and
  Stephen since the code changed since v2,

added in v2

 test/py/tests/test_env.py | 97 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+)

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index bfb5fc0..9bdaef9 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -5,6 +5,7 @@
 # Test operation of shell commands relating to environment variables.
 
 import pytest
+import u_boot_utils
 
 # FIXME: This might be useful for other tests;
 # perhaps refactor it into ConsoleBase or some other state object?
@@ -239,3 +240,99 @@ def test_env_expansion_spaces(state_test_env):
             unset_var(state_test_env, var_space)
         if var_test:
             unset_var(state_test_env, var_test)
+
+ at pytest.mark.buildconfigspec('cmd_importenv')
+def test_env_import_checksum_no_size(state_test_env):
+    """Test that omitted ('-') size parameter with checksum validation fails the
+       env import function.
+    """
+    c = state_test_env.u_boot_console
+    ram_base = u_boot_utils.find_ram_base(state_test_env.u_boot_console)
+    addr = '%08x' % ram_base
+
+    with c.disable_check('error_notification'):
+        response = c.run_command('env import -c %s -' % addr)
+    assert(response == '## Error: external checksum format must pass size')
+
+ at pytest.mark.buildconfigspec('cmd_importenv')
+def test_env_import_whitelist_checksum_no_size(state_test_env):
+    """Test that omitted ('-') size parameter with checksum validation fails the
+       env import function when variables are passed as parameters.
+    """
+    c = state_test_env.u_boot_console
+    ram_base = u_boot_utils.find_ram_base(state_test_env.u_boot_console)
+    addr = '%08x' % ram_base
+
+    with c.disable_check('error_notification'):
+        response = c.run_command('env import -c %s - foo1 foo2 foo4' % addr)
+    assert(response == '## Error: external checksum format must pass size')
+
+ at pytest.mark.buildconfigspec('cmd_exportenv')
+ at pytest.mark.buildconfigspec('cmd_importenv')
+def test_env_import_whitelist(state_test_env):
+    """Test importing only a handful of env variables from an environment."""
+    c = state_test_env.u_boot_console
+    ram_base = u_boot_utils.find_ram_base(state_test_env.u_boot_console)
+    addr = '%08x' % ram_base
+
+    set_var(state_test_env, 'foo1', 'bar1')
+    set_var(state_test_env, 'foo2', 'bar2')
+    set_var(state_test_env, 'foo3', 'bar3')
+
+    c.run_command('env export %s' % addr)
+
+    unset_var(state_test_env, 'foo1')
+    set_var(state_test_env, 'foo2', 'test2')
+    set_var(state_test_env, 'foo4', 'bar4')
+
+    # no foo1 in current env, foo2 overridden, foo3 should be of the value
+    # before exporting and foo4 should be of the value before importing.
+    c.run_command('env import %s - foo1 foo2 foo4' % addr)
+
+    validate_set(state_test_env, 'foo1', 'bar1')
+    validate_set(state_test_env, 'foo2', 'bar2')
+    validate_set(state_test_env, 'foo3', 'bar3')
+    validate_set(state_test_env, 'foo4', 'bar4')
+
+    # Cleanup test environment
+    unset_var(state_test_env, 'foo1')
+    unset_var(state_test_env, 'foo2')
+    unset_var(state_test_env, 'foo3')
+    unset_var(state_test_env, 'foo4')
+
+ at pytest.mark.buildconfigspec('cmd_exportenv')
+ at pytest.mark.buildconfigspec('cmd_importenv')
+def test_env_import_whitelist_delete(state_test_env):
+
+    """Test importing only a handful of env variables from an environment, with.
+       deletion if a var A that is passed to env import is not in the
+       environment to be imported.
+    """
+    c = state_test_env.u_boot_console
+    ram_base = u_boot_utils.find_ram_base(state_test_env.u_boot_console)
+    addr = '%08x' % ram_base
+
+    set_var(state_test_env, 'foo1', 'bar1')
+    set_var(state_test_env, 'foo2', 'bar2')
+    set_var(state_test_env, 'foo3', 'bar3')
+
+    c.run_command('env export %s' % addr)
+
+    unset_var(state_test_env, 'foo1')
+    set_var(state_test_env, 'foo2', 'test2')
+    set_var(state_test_env, 'foo4', 'bar4')
+
+    # no foo1 in current env, foo2 overridden, foo3 should be of the value
+    # before exporting and foo4 should be empty.
+    c.run_command('env import -d %s - foo1 foo2 foo4' % addr)
+
+    validate_set(state_test_env, 'foo1', 'bar1')
+    validate_set(state_test_env, 'foo2', 'bar2')
+    validate_set(state_test_env, 'foo3', 'bar3')
+    validate_empty(state_test_env, 'foo4')
+
+    # Cleanup test environment
+    unset_var(state_test_env, 'foo1')
+    unset_var(state_test_env, 'foo2')
+    unset_var(state_test_env, 'foo3')
+    unset_var(state_test_env, 'foo4')
-- 
git-series 0.9.1

[U-Boot] [PATCH v6 1/6] env: add the same prefix to error messages to make it detectable by tests

[Quentin Schulz]

Hi all,

This series was tested with Travis:

https://travis-ci.org/QSchulz/u-boot/builds/401644733

I can't explain why 45, 46 and 49 are failing but they have the
following error message:
The command "sudo apt-get install libisl15 -y" failed and exited with
100 during .

It's unrelated to this patch series (happened multiple times without my
patch series) as far as I can tell.

Quentin

On Mon, Jul 09, 2018 at 07:16:25PM +0200, Quentin Schulz wrote:
> The error message should start with `## Error: ` so that it's easily
> detectable by tests without needing to have a complex regexp for
> matching all possible error message patterns.
> 
> Let's add the `## Error: ` prefix to the error messages since it's the
> one already in use.
> 
> Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> 
> added in v5
> 
>  cmd/nvedit.c | 12 ++++++++----
>  env/common.c |  3 ++-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index ddc888a..70d7068 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -959,7 +959,8 @@ NXTARG:		;
>  				H_MATCH_KEY | H_MATCH_IDENT,
>  				&ptr, size, argc, argv);
>  		if (len < 0) {
> -			pr_err("Cannot export environment: errno = %d\n", errno);
> +			pr_err("## Error: Cannot export environment: errno = %d\n",
> +			       errno);
>  			return 1;
>  		}
>  		sprintf(buf, "%zX", (size_t)len);
> @@ -979,7 +980,8 @@ NXTARG:		;
>  			H_MATCH_KEY | H_MATCH_IDENT,
>  			&res, ENV_SIZE, argc, argv);
>  	if (len < 0) {
> -		pr_err("Cannot export environment: errno = %d\n", errno);
> +		pr_err("## Error: Cannot export environment: errno = %d\n",
> +		       errno);
>  		return 1;
>  	}
>  
> @@ -994,7 +996,8 @@ NXTARG:		;
>  	return 0;
>  
>  sep_err:
> -	printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",	cmd);
> +	printf("## Error: %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
> +	       cmd);
>  	return 1;
>  }
>  #endif
> @@ -1114,7 +1117,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
>  
>  	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
>  			crlf_is_lf, 0, NULL) == 0) {
> -		pr_err("Environment import failed: errno = %d\n", errno);
> +		pr_err("## Error: Environment import failed: errno = %d\n",
> +		       errno);
>  		return 1;
>  	}
>  	gd->flags |= GD_FLG_ENV_READY;
> diff --git a/env/common.c b/env/common.c
> index dc8a14f..7bd2790 100644
> --- a/env/common.c
> +++ b/env/common.c
> @@ -83,7 +83,8 @@ void set_default_env(const char *s)
>  	if (himport_r(&env_htab, (char *)default_environment,
>  			sizeof(default_environment), '\0', flags, 0,
>  			0, NULL) == 0)
> -		pr_err("Environment import failed: errno = %d\n", errno);
> +		pr_err("## Error: Environment import failed: errno = %d\n",
> +		       errno);
>  
>  	gd->flags |= GD_FLG_ENV_READY;
>  	gd->flags |= GD_FLG_ENV_DEFAULT;
> 
> base-commit: 22d58e60ffb5484d912f26b9c3533eff1d3d3de9
> -- 
> git-series 0.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180709/1b75dc16/attachment.sig>

[U-Boot] [U-Boot, v6, 1/6] env: add the same prefix to error messages to make it detectable by tests

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:25PM +0200, Quentin Schulz wrote:

> The error message should start with `## Error: ` so that it's easily
> detectable by tests without needing to have a complex regexp for
> matching all possible error message patterns.
> 
> Let's add the `## Error: ` prefix to the error messages since it's the
> one already in use.
> 
> Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/06ab1aaa/attachment.sig>

[U-Boot] [U-Boot, v6, 2/6] test/py: return a RAM address different from 0 as it can be interpreted as NULL

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:26PM +0200, Quentin Schulz wrote:

> Some functions test that the given address is not NULL (0) and fail or
> have a different behaviour if that's the case (e.g. hexport_r).
> 
> Let's make the RAM base address to be not zero by setting it to 2MiB if
> that's the case.
> 
> 2MiB is chosen because it represents the size of an ARM LPAE/v8 section.
> 
> Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/5b432a85/attachment.sig>

[U-Boot] [U-Boot, v6, 3/6] test/py: remove hacks for non-zero RAM base address in tests

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:27PM +0200, Quentin Schulz wrote:

> Some functions have different behaviour when the given address is 0
> (assumed to be NULL by the function).
> 
> find_ram_base() does not return 0 anymore so it's safe to remove those
> offsets.
> 
> Suggested-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/39813634/attachment.sig>

[U-Boot] [U-Boot, v6, 4/6] hashtable: do not recreate whole hash table if vars are passed to himport_r

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:28PM +0200, Quentin Schulz wrote:

> When vars are passed to the himport_r function with H_NOCLEAR flag,
> those vars will be overridden in the current environment and if one of
> those vars is not in the imported environment, it'll be deleted in the
> current environment whatever the flag passed to himport_r.
> 
> The H_NOCLEAR flag is used to clear the whole environment whether vars
> are passed to the function or not.
> 
> This leads to incoherent behaviour. If one passes vars to himport_r
> with the H_NOCLEAR flag, if a var in vars is not in the imported env,
> that var will be removed from the current env.
> 
> If one passes vars to himport_r without the H_NOCLEAR flag, the whole
> environment will be removed and vars will be imported from the
> environment in RAM.
> 
> It makes more sense to keep the variable that is in the current
> environment but not in the imported environment if the H_NOCLEAR flag is
> set and remove only that variable if the H_NOCLEAR flag is not set.
> 
> Let's clear the whole environment only if H_NOCLEAR and vars are not
> passed to himport_r.
> 
> Let's remove variables that are in the current environment but not in
> the imported env only if the H_NOCLEAR flag is not passed.
> 
> Suggested-by: Wolfgang Denk <wd@denx.de>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/879de707/attachment.sig>

[U-Boot] [U-Boot, v6, 5/6] cmd: nvedit: env import can now import only variables passed as parameters

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:29PM +0200, Quentin Schulz wrote:

> While the `env export` can take as parameters variables to be exported,
> `env import` does not have such a mechanism of variable selection.
> 
> Let's add the ability to add parameters at the end of the command for
> variables to be imported.
> 
> Every env variable from the env to be imported passed by parameter to
> this command will override the value of the variable in the current env.
> 
> If a variable exists in the current env but not in the imported env, if
> this variable is passed as a parameter to env import, the variable will
> be unset ONLY if the -d option is passed to env import, otherwise the
> current value of the variable is kept.
> 
> If a variable exists in the imported env, the variable in the current
> env will be set to the value of the one from the imported env.
> 
> All the remaining variables are left untouched.
> 
> As the size parameter of env import is positional but optional, let's
> add the possibility to use the sentinel '-' for when we don't want to
> give the size parameter (when the env is '\0' terminated) but we pass a
> list of variables at the end of the command.
> 
> env import addr
> env import addr -
> env import addr size
> env import addr - foo1 foo2
> env import addr size foo1 foo2
> 
> are all valid.
> 
> env import -c addr
> env import -c addr -
> env import -c addr - foo1 foo2
> 
> are all invalid because they don't pass the size parameter required for
> checking, while the following are valid.
> 
> env import addr size
> env import addr size foo1 foo2
> 
> Nothing's changed for the other parameters or the overall behaviour.
> 
> One of its use case could be to load a secure environment from the
> signed U-Boot binary and load only a handful of variables from an
> other, unsecure, environment without completely losing control of
> U-Boot.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Tested-by: Alex Kiernan <alex.kiernan@gmail.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/b4b83bd0/attachment.sig>

[U-Boot] [U-Boot, v6, 6/6] test/py: add test for whitelist of variables while importing environment

[Tom Rini]

On Mon, Jul 09, 2018 at 07:16:30PM +0200, Quentin Schulz wrote:

> This tests that the importing of an environment with a specified
> whitelist works as intended.
> 
> If there are variables passed as parameter to the env import command,
> those only should be imported in the current environment.
> 
> For each variable passed as parameter, if
>  - foo is bar in current env and bar2 in exported env, after importing
>  exported env, foo shall be bar2,
>  - foo does not exist in current env and foo is bar2 in exported env,
>  after importing exported env, foo shall be bar2,
>  - foo is bar in current env and does not exist in exported env (but is
>  passed as parameter), after importing exported env, foo shall be empty
>  ONLY if the -d option is passed to env import, otherwise foo shall be
>  bar,
> 
> Any variable not passed as parameter should be left untouched.
> 
> Two other tests are made to test that size cannot be '-' if the checksum
> protection is enabled.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/c8085f49/attachment.sig>