diff mbox series

[v3] env: remove vars that are not in default env

Message ID f1a2067c-942c-7e2d-ab28-7906a904a58e@marvell.com
State Accepted
Delegated to: Tom Rini
Headers show
Series [v3] env: remove vars that are not in default env | expand

Commit Message

Ravi Minnikanti Aug. 11, 2024, 6:44 p.m. UTC
current env_set_default_vars() doesn't delete
var that are not in the imported env. hashtable
removes vars that are not in the imported
env but present in the current env only if H_NOCLEAR
flag is not set.

This change is to avoid passing H_NOCLEAR flag if
specific vars are passed to env_set_default_vars()

Without this change:
Marvell>> env default boot_mode
Marvell>>

With the change:
Marvell>> env default boot_mode
WARNING: 'boot_mode' not in imported env, deleting it!

Signed-off-by: Ravi Minnikanti <rminnikanti@marvell.com>
---
Changes in v2:
- Added env ut to test the scenario
- Updated doc usage/cmd/env.rst

=> ut env
Running 5 environment tests
Test: env_test_attrs_lookup: attr.c
Test: env_test_attrs_lookup_regex: attr.c
Test: env_test_env_cmd: cmd_ut_env.c
Test: env_test_htab_deletes: hashtable.c
Test: env_test_htab_fill: hashtable.c
Failures: 0

Changes in v3:
- Fixed review comments
- Ran checkpatch.pl and fixed trailing whitespaces.
- Used UT_TESTF_CONSOLE_REC flag instead of console_record_reset_enable()
- Removed unnecessary ut_assert_nextline_empty() check
---
 doc/usage/cmd/env.rst |  4 +++-
 env/common.c          | 10 +++++++++-
 test/env/cmd_ut_env.c | 27 +++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

Comments

Simon Glass Aug. 13, 2024, 12:17 p.m. UTC | #1
On Sun, 11 Aug 2024 at 12:44, Ravi Minnikanti <rminnikanti@marvell.com> wrote:
>
> current env_set_default_vars() doesn't delete
> var that are not in the imported env. hashtable
> removes vars that are not in the imported
> env but present in the current env only if H_NOCLEAR
> flag is not set.
>
> This change is to avoid passing H_NOCLEAR flag if
> specific vars are passed to env_set_default_vars()
>
> Without this change:
> Marvell>> env default boot_mode
> Marvell>>
>
> With the change:
> Marvell>> env default boot_mode
> WARNING: 'boot_mode' not in imported env, deleting it!
>
> Signed-off-by: Ravi Minnikanti <rminnikanti@marvell.com>
> ---
> Changes in v2:
> - Added env ut to test the scenario
> - Updated doc usage/cmd/env.rst
>
> => ut env
> Running 5 environment tests
> Test: env_test_attrs_lookup: attr.c
> Test: env_test_attrs_lookup_regex: attr.c
> Test: env_test_env_cmd: cmd_ut_env.c
> Test: env_test_htab_deletes: hashtable.c
> Test: env_test_htab_fill: hashtable.c
> Failures: 0
>
> Changes in v3:
> - Fixed review comments
> - Ran checkpatch.pl and fixed trailing whitespaces.
> - Used UT_TESTF_CONSOLE_REC flag instead of console_record_reset_enable()
> - Removed unnecessary ut_assert_nextline_empty() check
> ---
>  doc/usage/cmd/env.rst |  4 +++-
>  env/common.c          | 10 +++++++++-
>  test/env/cmd_ut_env.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Aug. 15, 2024, 8:33 p.m. UTC | #2
On Sun, 11 Aug 2024 11:44:15 -0700, Ravi Minnikanti wrote:

> current env_set_default_vars() doesn't delete
> var that are not in the imported env. hashtable
> removes vars that are not in the imported
> env but present in the current env only if H_NOCLEAR
> flag is not set.
> 
> This change is to avoid passing H_NOCLEAR flag if
> specific vars are passed to env_set_default_vars()
> 
> [...]

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/doc/usage/cmd/env.rst b/doc/usage/cmd/env.rst
index 9629f97ffc..b65d85b668 100644
--- a/doc/usage/cmd/env.rst
+++ b/doc/usage/cmd/env.rst
@@ -79,7 +79,8 @@  The *env default* command resets the selected variables in the U-Boot
 environment to their default values.
 
     var
-        list of variable name.
+        list of variable names. If variable is not part of default
+        environment, it is deleted with a warning message on console.
     \-a
         all U-Boot environment.
     \-f
@@ -309,6 +310,7 @@  Delete environment variable in memory::
 Reset environment variable to default value, in memory::
 
     => env default bootcmd
+    => env default ipaddr serverip
     => env default -a
 
 Save current environment in persistent storage::
diff --git a/env/common.c b/env/common.c
index 8d47d72605..6cba7f1c18 100644
--- a/env/common.c
+++ b/env/common.c
@@ -401,7 +401,15 @@  int env_set_default_vars(int nvars, char * const vars[], int flags)
 	 * Special use-case: import from default environment
 	 * (and use \0 as a separator)
 	 */
-	flags |= H_NOCLEAR | H_DEFAULT;
+
+	/*
+	 * When vars are passed remove variables that are not in
+	 * the default environment.
+	 */
+	if (!nvars)
+		flags |= H_NOCLEAR;
+
+	flags |= H_DEFAULT;
 	return himport_r(&env_htab, default_environment,
 				sizeof(default_environment), '\0',
 				flags, 0, nvars, vars);
diff --git a/test/env/cmd_ut_env.c b/test/env/cmd_ut_env.c
index 13e0998341..238cf31036 100644
--- a/test/env/cmd_ut_env.c
+++ b/test/env/cmd_ut_env.c
@@ -9,6 +9,33 @@ 
 #include <test/suites.h>
 #include <test/ut.h>
 
+static int env_test_env_cmd(struct unit_test_state *uts)
+{
+	ut_assertok(run_command("setenv non_default_var1 1", 0));
+	ut_assert_console_end();
+
+	ut_assertok(run_command("setenv non_default_var2 1", 0));
+	ut_assert_console_end();
+
+	ut_assertok(run_command("env print non_default_var1", 0));
+	ut_assert_nextline("non_default_var1=1");
+	ut_assert_console_end();
+
+	ut_assertok(run_command("env default non_default_var1 non_default_var2", 0));
+	ut_assert_nextline("WARNING: 'non_default_var1' not in imported env, deleting it!");
+	ut_assert_nextline("WARNING: 'non_default_var2' not in imported env, deleting it!");
+	ut_assert_console_end();
+
+	ut_asserteq(1, run_command("env exists non_default_var1", 0));
+	ut_assert_console_end();
+
+	ut_asserteq(1, run_command("env exists non_default_var2", 0));
+	ut_assert_console_end();
+
+	return 0;
+}
+ENV_TEST(env_test_env_cmd, UT_TESTF_CONSOLE_REC);
+
 int do_ut_env(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	struct unit_test *tests = UNIT_TEST_SUITE_START(env_test);