From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lothar =?UTF-8?B?V2HDn21hbm4=?= Date: Fri, 25 May 2018 13:26:20 +0200 Subject: [U-Boot] [PATCH v3 1/2] cmd: nvedit: env import can now import only variables passed as parameters In-Reply-To: References: Message-ID: <20180525132620.1b6ef929@karo-electronics.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi, On Fri, 25 May 2018 11:52:17 +0100 Alex Kiernan wrote: > On Fri, May 25, 2018 at 9:38 AM Quentin Schulz > wrote: >=20 > > While the `env export` can take as parameters variables to be exported, > > `env import` does not have such a mechanism of variable selection. >=20 > > Let's add the ability to add parameters at the end of the command for > > variables to be imported. >=20 > > 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. >=20 > > 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. >=20 > > 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. >=20 > > All the remaining variables are left untouched. >=20 > > 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. >=20 > > env import addr > > env import addr - > > env import addr size > > env import addr - foo1 foo2 > > env import addr size foo1 foo2 >=20 > > are all valid. >=20 > > env import -c addr > > env import -c addr - > > env import -c addr - foo1 foo2 >=20 > > are all invalid because they don't pass the size parameter required for > > checking, while the following are valid. >=20 > > env import addr size > > env import addr size foo1 foo2 >=20 > > Nothing's changed for the other parameters or the overall behaviour. >=20 > > 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. >=20 > > Signed-off-by: Quentin Schulz > > --- >=20 > > v3: > > - migrate to env import addr size var... instead of env import -w ad= dr > > 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, >=20 > > 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, >=20 > > cmd/nvedit.c | 22 ++++++++++++++++------ > > 1 file changed, 16 insertions(+), 6 deletions(-) >=20 > > 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: >=20 > > #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 import= ed > from > > + * the environment at address 'addr'. Without arguments, t= he > 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 fl= ag, > > int fmt =3D 0; > > int del =3D 0; > > int crlf_is_lf =3D 0; > > + int wl =3D 0; > > size_t size; >=20 > > cmd =3D *argv; > > @@ -1074,9 +1080,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int fl= ag, > > addr =3D simple_strtoul(argv[0], NULL, 16); > > ptr =3D map_sysmem(addr, 0); >=20 > > - if (argc =3D=3D 2) { > > + if (argc >=3D 2 && strcmp(argv[1], "-")) { > > size =3D simple_strtoul(argv[1], NULL, 16); > > - } else if (argc =3D=3D 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 fl= ag, > > printf("## Info: input data size =3D %zu =3D 0x%zX\n",= size, > size); > > } >=20 > > + if (argc > 2) > > + wl =3D 1; > > + > > if (chk) { > > uint32_t crc; > > env_t *ep =3D (env_t *)ptr; > > @@ -1112,9 +1121,10 @@ static int do_env_import(cmd_tbl_t *cmdtp, int > flag, > > ptr =3D (char *)ep->data; > > } >=20 > > - if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, > > - crlf_is_lf, 0, NULL) =3D=3D 0) { > > + if (!himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, >=20 > 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() =3D=3D 0) before! ^^^^ Lothar Wa=C3=9Fmann