Dear Takahiro,
In message 20190717082525.891-4-takahiro.akashi@linaro.org you wrote:
The following interfaces are extended to allow for accepting an additional argument, env_context. env_get() -> env_get_ext() env_set() -> env_get_ext()
Please don't, see previous comments.
Relevant env commands are synced with this change to maintain the semantics of existing U-Boot environment.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/nvedit.c | 82 ++++++++++++++++++++++++++++++++++------------- include/exports.h | 3 ++ 2 files changed, 62 insertions(+), 23 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 49d3b5bdf466..cc80ba712767 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -87,7 +87,7 @@ int get_env_id(void)
- Returns 0 in case of error, or length of printed string
*/ -static int env_print(char *name, int flag) +static int env_print_ext(enum env_context ctx, char *name, int flag) { char *res = NULL; ssize_t len; @@ -96,6 +96,7 @@ static int env_print(char *name, int flag) ENTRY e, *ep;
e.key = name;
e.context = ctx;-#if defined(CONFIG_CMD_NVEDIT_EFI)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
return do_env_print_efi(cmdtp, flag, --argc, ++argv);-#endif
- if (ctx == ENVCTX_UEFI)
return do_env_print_efi(cmdtp, flag, argc, argv);
I don't like this. It doesn't scale. ENVCTX_UEFI is just one context among others; it should not need a "if" here to handle.
+{ +#if defined(CONFIG_CMD_NVEDIT_EFI)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
Please use proper argument handling, options can be combined, so the 'e' might not be the second character.
Also, this should probably changed to support a generic "context" specific handling, though "-c context" or such, with U-Boot being the default.
This again allows to get rid of all these "if"s
-#if CONFIG_IS_ENABLED(CMD_NVEDIT_EFI)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
return do_env_set_efi(NULL, flag, --argc, ++argv);-#endif
- if (ctx == ENVCTX_UEFI)
return do_env_set_efi(NULL, flag, argc, argv);
Ditto here.
+#if CONFIG_IS_ENABLED(CMD_NVEDIT_EFI)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
return _do_env_set(flag, --argc, ++argv, H_INTERACTIVE,ENVCTX_UEFI);- else
+#endif
And here.
const char * const _argv[3] = { "setenv", argv[1], NULL };
return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE);
return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE,ENVCTX_UBOOT);
return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE);
return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE,ENVCTX_UBOOT);
Also here. ENVCTX_UBOOT is not a special context and should not need special handling.
Best regards,
Wolfgang Denk