Dear Takahiro, In message <20190717082525.891-4-takahiro.aka...@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.aka...@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; > e.data = NULL; > hsearch_r(e, FIND, &ep, &env_htab, flag); > if (ep == NULL) > -#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); > } else { > const char * const _argv[4] = { "setenv", argv[1], buffer, > NULL }; > > - 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 -- 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: w...@denx.de I've got to get something inside me. Some coffee or something. And then the world will somehow be better. - Terry Pratchett, _Men at Arms_ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot