[U-Boot] [PATCH v3 1/2] cmd: nvedit: env import can now import only variables passed as parameters
Quentin Schulz
quentin.schulz at bootlin.com
Fri May 25 12:12:14 UTC 2018
Hi Lothar and Alex,
On Fri, May 25, 2018 at 01:26:20PM +0200, Lothar Waßmann wrote:
> 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.schulz at 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.schulz at 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!
> ^^^^
Exactly, with the added ternary conditions, the line exceeds the 80
characters just for the opening bracket. I just replaced the == 0 with a
! to gain a few precious characters and avoid having to do a:
ret = himport_r();
if (!ret)
But I could do it if wanted.
If I had changed the behaviour of the function, I would have made a
separate commit for it so it's not hidden in some unrelated code, so,
that's just a cosmetic fix here :)
Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180525/89cdd355/attachment.sig>
More information about the U-Boot
mailing list