Hi, On Fri, 25 May 2018 11:52:17 +0100 Alex Kiernan wrote: > On Fri, May 25, 2018 at 9:38 AM Quentin Schulz <quentin.sch...@bootlin.com> > 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. > > > 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.sch...@bootlin.com> > > --- > > > 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 | 22 ++++++++++++++++------ > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > > index 11489b0..18cc4db 100644 > > --- a/cmd/nvedit.c > > +++ b/cmd/nvedit.c > > @@ -1001,7 +1001,7 @@ sep_err: > > > #ifdef CONFIG_CMD_IMPORTENV > > /* > > - * env import [-d] [-t [-r] | -b | -c] addr [size] > > + * env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] > > * -d: delete existing environment before importing; > > * otherwise overwrite / append to existing definitions > > * -t: assume text format; either "size" must be given or the > > @@ -1015,6 +1015,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[]) > > @@ -1026,6 +1031,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; > > @@ -1074,9 +1080,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 { > > @@ -1098,6 +1104,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; > > @@ -1112,9 +1121,10 @@ 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, > > The addition of a '!' seems like an odd change, but looking at what > himport_r() returns, I think you're correct and it's been broken forever. > No. It was if (h_import_r() == 0) before! ^^^^
Lothar Waßmann _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot