[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

Currently, there's no way to fetch the value of an environment
variable whose name is stored in some other variable, or generated from
such - in non-working pseudo-code,

  ${${varname}}
  ${array${index}}

This forces some scripts to needlessly duplicate logic and hardcode
assumptions. For example, in an A/B scheme with three variables

BOOT_ORDER # Either "A B" or "B A" depending on which slot was last updated
BOOT_A_LEFT # 0..3
BOOT_B_LEFT # 0..3

when one needs to determine the slot to boot from, one does something
like

setenv found
for slot in $BOOT_ORDER ; do
  if test "x$found" != "x" ; then
    # work around lack of break
  elif test "x$slot" = "xA" ; then
    if test $BOOT_A_LEFT -gt 0 ; then
      setexpr BOOT_A_LEFT $BOOT_A_LEFT - 1
      setenv found A
      setenv bootargs ${bootargs_A}
      setenv ubivol ${ubivol_A}
      # more setup based on A
    fi
  elif test "x$slot" = "xB" ; then
    if test $BOOT_B_LEFT -gt 0 ; then
      # the same ...
    fi
  fi
done

This is already bad enough, but extending that to A/B/C is tedious and
prone to copy-pastos.

So this is an attempt at allowing one to do "env set -E var value1 value2"
with the effect that, of course, normal variable expansion happens on
the command line, the valueX are joined with spaces as usual, and then
one more pass is done over that string replacing occurrences of
${foo}.

The above would become

setenv found
for slot in $BOOT_ORDER ; do
  if test "x$found" != "x" ; then
    # work around lack of break
  else
    env set -E boot_left "\${BOOT_${slot}_LEFT}"
    if test $boot_left -gt 0 ; then
      setexpr BOOT_${slot}_LEFT $boot_left - 1
      env set found $slot
      env set -E bootargs "\${bootargs_${slot}}"
      env set -E ubivol "\${ubivol_${slot}}"
    fi
  fi
done

I'm pleasantly surprised it was that easy to implement, but of course
I'm cheating a bit (cli_simple_process_macros is only available if
CONFIG_CMDLINE, though I think cli_simple.o could be unconditionally
built and then link-time GC should get rid of the excess
functions).

This has been lightly tested in the sandbox. I'll add some proper unit
tests, update the help texts and try to handle the Kconfig issue if
this is something that might be accepted.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 cmd/nvedit.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 81d94cd193..ff6ffcb674 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -224,7 +224,7 @@ DONE:
  */
 static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
 {
-	int   i, len;
+	int   i, len, expand = 0;
 	char  *name, *value, *s;
 	struct env_entry e, *ep;
 
@@ -244,6 +244,9 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
 			case 'f':		/* force */
 				env_flag |= H_FORCE;
 				break;
+			case 'E':
+				expand = 1;
+				break;
 			default:
 				return CMD_RET_USAGE;
 			}
@@ -287,6 +290,18 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
 	if (s != value)
 		*--s = '\0';
 
+	if (expand) {
+		char *expanded = malloc(CONFIG_SYS_CBSIZE);
+
+		if (expanded == NULL) {
+			printf("## Can't malloc %d bytes\n", CONFIG_SYS_CBSIZE);
+			free(value);
+			return 1;
+		}
+		cli_simple_process_macros(value, expanded);
+		free(value);
+		value = expanded;
+	}
 	e.key	= name;
 	e.data	= value;
 	hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag);
-- 
2.23.0

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Simon Glass]

Hi Rasmus,

On Tue, 4 Feb 2020 at 18:08, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> Currently, there's no way to fetch the value of an environment
> variable whose name is stored in some other variable, or generated from
> such - in non-working pseudo-code,
>
>   ${${varname}}
>   ${array${index}}
>
> This forces some scripts to needlessly duplicate logic and hardcode
> assumptions. For example, in an A/B scheme with three variables
>
> BOOT_ORDER # Either "A B" or "B A" depending on which slot was last updated
> BOOT_A_LEFT # 0..3
> BOOT_B_LEFT # 0..3
>
> when one needs to determine the slot to boot from, one does something
> like
>
> setenv found
> for slot in $BOOT_ORDER ; do
>   if test "x$found" != "x" ; then
>     # work around lack of break
>   elif test "x$slot" = "xA" ; then
>     if test $BOOT_A_LEFT -gt 0 ; then
>       setexpr BOOT_A_LEFT $BOOT_A_LEFT - 1
>       setenv found A
>       setenv bootargs ${bootargs_A}
>       setenv ubivol ${ubivol_A}
>       # more setup based on A
>     fi
>   elif test "x$slot" = "xB" ; then
>     if test $BOOT_B_LEFT -gt 0 ; then
>       # the same ...
>     fi
>   fi
> done
>
> This is already bad enough, but extending that to A/B/C is tedious and
> prone to copy-pastos.
>
> So this is an attempt at allowing one to do "env set -E var value1 value2"
> with the effect that, of course, normal variable expansion happens on
> the command line, the valueX are joined with spaces as usual, and then
> one more pass is done over that string replacing occurrences of
> ${foo}.
>
> The above would become
>
> setenv found
> for slot in $BOOT_ORDER ; do
>   if test "x$found" != "x" ; then
>     # work around lack of break
>   else
>     env set -E boot_left "\${BOOT_${slot}_LEFT}"
>     if test $boot_left -gt 0 ; then
>       setexpr BOOT_${slot}_LEFT $boot_left - 1
>       env set found $slot
>       env set -E bootargs "\${bootargs_${slot}}"
>       env set -E ubivol "\${ubivol_${slot}}"
>     fi
>   fi
> done
>
> I'm pleasantly surprised it was that easy to implement, but of course
> I'm cheating a bit (cli_simple_process_macros is only available if
> CONFIG_CMDLINE, though I think cli_simple.o could be unconditionally
> built and then link-time GC should get rid of the excess
> functions).
>
> This has been lightly tested in the sandbox. I'll add some proper unit
> tests, update the help texts and try to handle the Kconfig issue if
> this is something that might be accepted.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  cmd/nvedit.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

Seems OK to me. I suppose we don't want to implement bash's nested
expansion? ${var${suffix}}

Regards,
Simon

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 05/02/2020 18.59, Simon Glass wrote:
> Hi Rasmus,
> 

>> This has been lightly tested in the sandbox. I'll add some proper unit
>> tests, update the help texts and try to handle the Kconfig issue if
>> this is something that might be accepted.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>>  cmd/nvedit.c | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> Seems OK to me. 

Thanks. I'll go ahead and write some tests.

> I suppose we don't want to implement bash's nested
> expansion? ${var${suffix}}

It's not that easy to implement inside-out expansion, one needs to
juggle a lot of temporary buffers. So I went with the rather simple
one-extra-pass, which can mostly achieve the same things (although
perhaps the script needs to do a few extra steps).

Out of curiosity, what bash version supports the above?

$ echo $BASH_VERSION
4.4.20(1)-release
$ foo_a=3
$ foo_b=7
$ x=a
$ echo ${foo_${x}}
bash: ${foo_${x}}: bad substitution

however, this variant is supported:

$ var=foo_$x
$ echo ${var}
foo_a
$ echo ${!var}
3



> Regards,
> Simon
> 


-- 
Rasmus Villemoes
Software Developer
Prevas A/S
Hedeager 3
DK-8200 Aarhus N
+45 51210274
rasmus.villemoes at prevas.dk
www.prevas.dk

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus Villemoes,

In message <20200205010812.20373-1-rasmus.villemoes@prevas.dk> you wrote:
> Currently, there's no way to fetch the value of an environment
> variable whose name is stored in some other variable, or generated from
> such - in non-working pseudo-code,
>
>   ${${varname}}
>   ${array${index}}

HUSH does not support arrays anyway...

> This forces some scripts to needlessly duplicate logic and hardcode
> assumptions. For example, in an A/B scheme with three variables
>
> BOOT_ORDER # Either "A B" or "B A" depending on which slot was last updated
> BOOT_A_LEFT # 0..3
> BOOT_B_LEFT # 0..3
>
> when one needs to determine the slot to boot from, one does something
> like

Well, there _are_ other ways...

> This has been lightly tested in the sandbox. I'll add some proper unit
> tests, update the help texts and try to handle the Kconfig issue if
> this is something that might be accepted.

I'm pretty sure this will blow up in your face in real life, because
if side effects on existing shell scripts even if you don't
expect this.  This needs _lots_ of testing with existing code /
scripts.

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: wd at denx.de
"The greatest warriors are the ones who fight for peace."
- Holly Near

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Simon Glass]

Hi Rasmus,

On Tue, 11 Feb 2020 at 08:38, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 05/02/2020 18.59, Simon Glass wrote:
> > Hi Rasmus,
> >
>
> >> This has been lightly tested in the sandbox. I'll add some proper unit
> >> tests, update the help texts and try to handle the Kconfig issue if
> >> this is something that might be accepted.
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> >> ---
> >>  cmd/nvedit.c | 17 ++++++++++++++++-
> >>  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > Seems OK to me.
>
> Thanks. I'll go ahead and write some tests.
>
> > I suppose we don't want to implement bash's nested
> > expansion? ${var${suffix}}
>
> It's not that easy to implement inside-out expansion, one needs to
> juggle a lot of temporary buffers. So I went with the rather simple
> one-extra-pass, which can mostly achieve the same things (although
> perhaps the script needs to do a few extra steps).
>
> Out of curiosity, what bash version supports the above?
>
> $ echo $BASH_VERSION
> 4.4.20(1)-release
> $ foo_a=3
> $ foo_b=7
> $ x=a
> $ echo ${foo_${x}}
> bash: ${foo_${x}}: bad substitution
>
> however, this variant is supported:
>
> $ var=foo_$x
> $ echo ${var}
> foo_a
> $ echo ${!var}
> 3

Er, yes, sorry I was thinking of Makefiles.

Regards,
Simon

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 11/02/2020 17.30, Wolfgang Denk wrote:
> Dear Rasmus Villemoes,
> 
> In message <20200205010812.20373-1-rasmus.villemoes@prevas.dk> you wrote:
>> Currently, there's no way to fetch the value of an environment
>> variable whose name is stored in some other variable, or generated from
>> such - in non-working pseudo-code,
>>
>>   ${${varname}}
>>   ${array${index}}
> 
> HUSH does not support arrays anyway...

Of course not, but they can be emulated by having variables foo0, foo1,
foo2 and programmatically accessing the variable foo$index, if only
there's a way to do that... In a sense, my BOOT_A_LEFT/BOOT_B_LEFT is
also just an array with keys "A" and "B".

>> This forces some scripts to needlessly duplicate logic and hardcode
>> assumptions. For example, in an A/B scheme with three variables
>>
>> BOOT_ORDER # Either "A B" or "B A" depending on which slot was last updated
>> BOOT_A_LEFT # 0..3
>> BOOT_B_LEFT # 0..3
>>
>> when one needs to determine the slot to boot from, one does something
>> like
> 
> Well, there _are_ other ways...

Please do tell. How can I avoid code duplication and access a variable
whose name I generate by string concatenation/variable interpolation?
I.e., I don't want anything like "if test $slot = "A" ; then setenv
result BOOT_A_LEFT ; elif test $slot = "B" ; then setenv result
BOOT_B_LEFT ; fi", because that doesn't scale.

>> This has been lightly tested in the sandbox. I'll add some proper unit
>> tests, update the help texts and try to handle the Kconfig issue if
>> this is something that might be accepted.
> 
> I'm pretty sure this will blow up in your face in real life, because
> if side effects on existing shell scripts even if you don't
> expect this. 

How so? The behaviour is completely opt-in per "env set", so nothing at
all changes for existing scripts, and it's only supposed to be used
where one would otherwise use some eval-like construct in a "normal"
shell. So

  env set -E result "\${BOOT_${x}_LEFT}"

corresponds to

  eval "result=\${BOOT_${x}_LEFT}"

And just as "eval" is used sparingly in shell scripts, I expect "env set
-E" to be used only when necessary.

> This needs _lots_ of testing with existing code /
> scripts.

I'm not proposing changing semantics of existing scripts at all.

Thanks,
Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 11/02/2020 22.20, Rasmus Villemoes wrote:
> On 11/02/2020 17.30, Wolfgang Denk wrote:

>>> This forces some scripts to needlessly duplicate logic and hardcode
>>> assumptions. For example, in an A/B scheme with three variables
>>>
>>> BOOT_ORDER # Either "A B" or "B A" depending on which slot was last updated
>>> BOOT_A_LEFT # 0..3
>>> BOOT_B_LEFT # 0..3
>>>
>>> when one needs to determine the slot to boot from, one does something
>>> like
>>
>> Well, there _are_ other ways...
> 
> Please do tell. How can I avoid code duplication and access a variable
> whose name I generate by string concatenation/variable interpolation?

Btw., I also considered implementing this as a separate subcommand "env
get <dest> <varname>", then I could do "env get result
BOOT_${slot}_LEFT". That wouldn't be quite as powerful as "env set -E",
but I think sufficient for my use case(s).

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus,

In message <c48f9de9-ad02-3b45-ae2e-50c3086d6a68@prevas.dk> you wrote:
>
> > HUSH does not support arrays anyway...
> 
> Of course not, but they can be emulated by having variables foo0, foo1,
> foo2 and programmatically accessing the variable foo$index, if only
> there's a way to do that... In a sense, my BOOT_A_LEFT/BOOT_B_LEFT is
> also just an array with keys "A" and "B".

Actually the port to U-Boot cripples HUSH in many more aspects.
I've always hoped someone would some day volunteer and (1)( update
HUSH to a more recent (and less buggy) version and address a few of
the missing parts, like Command Substitution, which would be really
handy in many cases - here as well.


> > Well, there _are_ other ways...
> 
> Please do tell. How can I avoid code duplication and access a variable
> whose name I generate by string concatenation/variable interpolation?
> I.e., I don't want anything like "if test $slot = "A" ; then setenv
> result BOOT_A_LEFT ; elif test $slot = "B" ; then setenv result
> BOOT_B_LEFT ; fi", because that doesn't scale.

=> slot=A
=> setenv result BOOT_${slot}_LEFT
=> printenv result
result=BOOT_A_LEFT
=> setenv foo 'setenv result BOOT_${slot}_LEFT; printenv result'
=> slot=B
=> run foo
result=BOOT_B_LEFT
=> slot=X
=> run foo
result=BOOT_X_LEFT

What exactly is your question?


>   env set -E result "\${BOOT_${x}_LEFT}"
> 
> corresponds to
> 
>   eval "result=\${BOOT_${x}_LEFT}"

...and things become already very tricky here, as you can see from
the need of having "\$" and "$" mixed. Now assume you have more of
this in the embedded variables...

I consider this change a bad idea.  Instead of adding such a
hack we should add what you really want...

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: wd at denx.de
ATTENTION: Despite Any Other Listing of Product Contents Found  Here-
on, the Consumer is Advised That, in Actuality, This Product Consists
Of 99.9999999999% Empty Space.

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus,

In message <2e076ba8-6ffe-66dc-ecb9-ebb62e47f414@prevas.dk> you wrote:
> 
> >> Well, there _are_ other ways...
> > 
> > Please do tell. How can I avoid code duplication and access a variable
> > whose name I generate by string concatenation/variable interpolation?
> 
> Btw., I also considered implementing this as a separate subcommand "env
> get <dest> <varname>", then I could do "env get result
> BOOT_${slot}_LEFT". That wouldn't be quite as powerful as "env set -E",
> but I think sufficient for my use case(s).

Sorry, but I still don;t understand what exactly your problem is.

Se previous message, or, slightly extended:

=> setenv BOOT_A_LEFT boot a left
=> setenv BOOT_B_LEFT boot b left
=> setenv BOOT_X_LEFT boot x left
=> setenv foo 'setenv result BOOT_${slot}_LEFT; printenv result; print $result'
=> slot=B
=> run foo
result=BOOT_B_LEFT
BOOT_B_LEFT=boot b left
=> slot=X
=> run foo
result=BOOT_X_LEFT
BOOT_X_LEFT=boot x left
=> slot=A
=> run foo
result=BOOT_A_LEFT
BOOT_A_LEFT=boot a left


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: wd at denx.de
No man knows what true happiness is until he gets married.  By  then,
of course, its too late.

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 12/02/2020 12.38, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <c48f9de9-ad02-3b45-ae2e-50c3086d6a68@prevas.dk> you wrote:
>>
>>> HUSH does not support arrays anyway...
>>
>> Of course not, but they can be emulated by having variables foo0, foo1,
>> foo2 and programmatically accessing the variable foo$index, if only
>> there's a way to do that... In a sense, my BOOT_A_LEFT/BOOT_B_LEFT is
>> also just an array with keys "A" and "B".
> 
> Actually the port to U-Boot cripples HUSH in many more aspects.
> I've always hoped someone would some day volunteer and (1)( update
> HUSH to a more recent (and less buggy) version and address a few of
> the missing parts, like Command Substitution, which would be really
> handy in many cases - here as well.

I'm certainly also missing break (and to a lesser extent continue) in loops.

>>> Well, there _are_ other ways...
>>
>> Please do tell. How can I avoid code duplication and access a variable
>> whose name I generate by string concatenation/variable interpolation?
>> I.e., I don't want anything like "if test $slot = "A" ; then setenv
>> result BOOT_A_LEFT ; elif test $slot = "B" ; then setenv result
>> BOOT_B_LEFT ; fi", because that doesn't scale.
> 
> => slot=A
> => setenv result BOOT_${slot}_LEFT
> => printenv result
> result=BOOT_A_LEFT
> => setenv foo 'setenv result BOOT_${slot}_LEFT; printenv result'
> => slot=B
> => run foo
> result=BOOT_B_LEFT
> => slot=X
> => run foo
> result=BOOT_X_LEFT
> 
> What exactly is your question?

I'm sorry, I see I mistyped in my example above, it should have been

  if test $slot = "A" ; setenv result $BOOT_A_LEFT ...

as should hopefully be clear from the original post and the eval
examples. So to reiterate, the problem is to get the contents (or value,
if you will) of the BOOT_A_LEFT variable into the result variable, not
setting result to the string "BOOT_A_LEFT" - but with the wrinkle that
BOOT_A_LEFT is generated programmatically, so the code cannot literally
mention BOOT_A_LEFT anywhere.

>>   env set -E result "\${BOOT_${x}_LEFT}"
>>
>> corresponds to
>>
>>   eval "result=\${BOOT_${x}_LEFT}"
> 
> ...and things become already very tricky here, as you can see from
> the need of having "\$" and "$" mixed. Now assume you have more of
> this in the embedded variables...

Eh, yes, exactly as one needs to do in ordinary shells when using the
eval construct. I don't see how one can on the one hand trust
programmers to have arbitrary read and write access to all of physical
memory but not trust them to get such rather basic escaping right (and
testing should immediately show if one got it wrong).

I also proposed an escape-less solution, namely "env get". That would be
slightly less powerful, since

env get dest whatever

could be implemented in terms of "env set -E" as

env set -E dest "\${whatever}"

but as I said, I think that would be sufficient for my purposes.

So just as print[env] takes the name of a variable and shows the
name=value string, and one can thus say "printenv BOOT_${slot}_LEFT" as
you did in your extended example, I could do

  env get result BOOT_${slot}_LEFT

and get the value of the BOOT_${slot}_LEFT variable into result.

Would you be ok with adding such an "env get" with less foot-gun potential?

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus,

In message <f9aa247b-4b9b-9196-4de7-b352d25766fe@prevas.dk> you wrote:
>
> I'm sorry, I see I mistyped in my example above, it should have been
> 
>   if test $slot = "A" ; setenv result $BOOT_A_LEFT ...
> 
> as should hopefully be clear from the original post and the eval
> examples. So to reiterate, the problem is to get the contents (or value,
> if you will) of the BOOT_A_LEFT variable into the result variable, not
> setting result to the string "BOOT_A_LEFT" - but with the wrinkle that
> BOOT_A_LEFT is generated programmatically, so the code cannot literally
> mention BOOT_A_LEFT anywhere.

Didn't I show this in my second, expanded example?

I suggest you provide a working example of shell code (say, bash, if
you like) to demonstrate what you really have in mind.  It seems
I have problems understanding your verbal description.

> So just as print[env] takes the name of a variable and shows the
> name=value string, and one can thus say "printenv BOOT_${slot}_LEFT" as
> you did in your extended example, I could do
> 
>   env get result BOOT_${slot}_LEFT
> 
> and get the value of the BOOT_${slot}_LEFT variable into result.

I still fail to see why you think this cannot be done with just the
already existing code. Just use setenv instead of printenv in my
example?

> Would you be ok with adding such an "env get" with less foot-gun potential?

I'm not OK with adding any special-purpose code which can easily
be implemented with existing scripting capabilites.  And so far I
don't see the limitation you are running into.

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: wd at denx.de
Gew?hnlich glaubt der Mensch,  wenn er nur Worte h?rt,  es m?sse sich
dabei doch auch was denken lassen.                 -- Goethe, Faust I

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 13/02/2020 16.55, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <f9aa247b-4b9b-9196-4de7-b352d25766fe@prevas.dk> you wrote:
>>
>> I'm sorry, I see I mistyped in my example above, it should have been
>>
>>   if test $slot = "A" ; setenv result $BOOT_A_LEFT ...
>>
>> as should hopefully be clear from the original post and the eval
>> examples. So to reiterate, the problem is to get the contents (or value,
>> if you will) of the BOOT_A_LEFT variable into the result variable, not
>> setting result to the string "BOOT_A_LEFT" - but with the wrinkle that
>> BOOT_A_LEFT is generated programmatically, so the code cannot literally
>> mention BOOT_A_LEFT anywhere.
> 
> Didn't I show this in my second, expanded example?

I'm sorry, but no, you did not. You used the capability of the print (or
rather printenv) command to take the name of a variable and print
"name=value" to the terminal.

In your example, result had the value BOOT_A_LEFT, so doing "print
$result" meant executing the command "print BOOT_A_LEFT", and of course
the output of that was then "BOOT_A_LEFT=boot a left".

However, what I need is to get that "boot a left" value stored in some
other variable so I can actually do further logic based on that value.

> I suggest you provide a working example of shell code (say, bash, if
> you like) to demonstrate what you really have in mind.  It seems
> I have problems understanding your verbal description.

Assume BOOT_ORDER contains some permutation of "A B C", and for each
letter x, there's a BOOT_x_LEFT counter telling how many boot attempts
that slot has left. Now I want to find the first x in $BOOT_ORDER for
which $BOOT_x_LEFT is positive. If all BOOT_x_LEFT are 0, say I want the
sentinel value 'none'.

So in bash, that might be written

slot=none
for x in $BOOT_ORDER ; do
  eval "left=\${BOOT_${x}_LEFT}"
  if [ $left -gt 0 ] ; then
    slot=$x
    break
  fi
done

Now we can work around the lack of break in the busybox shell by writing
the loop so that the main body is skipped if we've found a valid slot:

slot=none
for x in $BOOT_ORDER ; do
  if [ $slot != 'none' ] ; then
    true
  else
    eval "left=\${BOOT_${x}_LEFT}"
    if [ $left -gt 0 ] ; then
      slot=$x
    fi
  fi
done

But we still can't translate this to busybox shell, because there's no
eval. Now, I can do this with a hypothetical "env get" command which I
just implemented to test that it works, and then the above becomes

env set slot none;
for x in $BOOT_ORDER ; do
  if test $slot != 'none' ; then
    true ;
  else
    env get left BOOT_${x}_LEFT ; # magic
    if test $left -gt 0 ; then
      env set slot $x ;
    fi;
  fi;
done;

Now, if you can implement the line marked #magic with existing
functions, I'm all ears. Or if you can implement the semantics of this
snippet in some other way, which does not open-code explicit references
to BOOT_A_LEFT, BOOT_B_LEFT etc. This is what I meant when I said I'd
prefer not to write the loop like this:

env set slot none;
for x in $BOOT_ORDER ; do
  if test $slot != 'none' ; then
    true ;
  else
    if test $x = A && test $BOOT_A_LEFT -gt 0 ; then
      env set slot A
      env set left $BOOT_A_LEFT
    elif test $x = B && test $BOOT_B_LEFT -gt 0 ; then
      env set slot B
      env set left $BOOT_B_LEFT
    elif test $x = C && test $BOOT_C_LEFT -gt 0 ; then
      env set slot C
      env set left $BOOT_C_LEFT
    fi
  fi;
done;

[yes, I also want left set as a side effect to the current value of the
appropriate BOOT_x_LEFT].

>> So just as print[env] takes the name of a variable and shows the
>> name=value string, and one can thus say "printenv BOOT_${slot}_LEFT" as
>> you did in your extended example, I could do
>>
>>   env get result BOOT_${slot}_LEFT
>>
>> and get the value of the BOOT_${slot}_LEFT variable into result.
> 
> I still fail to see why you think this cannot be done with just the
> already existing code. Just use setenv instead of printenv in my
> example?
> 
>> Would you be ok with adding such an "env get" with less foot-gun potential?
> 
> I'm not OK with adding any special-purpose code which can easily
> be implemented with existing scripting capabilites. 

Of course not. But _I'm_ not seeing how one can actually fetch the value
of one variable into another, just given the first variable's name -
when that name is programmatically generated, e.g. as BOOT_${x}_LEFT or
whatnot.

PS: Implementation of "env get" is just:

--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -386,6 +386,20 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag,
int argc, char * const argv[])
        return _do_env_set(flag, argc, argv, H_INTERACTIVE);
 }

+static int do_env_get(cmd_tbl_t *cmdtp, int flag, int argc, char *
const argv[])
+{
+       char *local_args[4];
+
+       if (argc != 3)
+               return CMD_RET_USAGE;
+       local_args[0] = argv[0];
+       local_args[1] = argv[1];
+       local_args[2] = env_get(argv[2]);
+       local_args[3] = NULL;
+
+       return _do_env_set(flag, argc, local_args, H_INTERACTIVE);
+}
+
 /*
  * Prompt for environment variable
  */
@@ -1334,6 +1348,7 @@ static cmd_tbl_t cmd_env_sub[] = {
 #endif
 #endif
        U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
+       U_BOOT_CMD_MKENT(get, 3, 0, do_env_get, "", ""),
 #if defined(CONFIG_CMD_ENV_EXISTS)
        U_BOOT_CMD_MKENT(exists, 2, 0, do_env_exists, "", ""),
 #endif

So it differs from "env set" (or setenv) in that

  env set foo bar

sets foo to the value "bar"

while

  env get foo bar

"gets" the value of the bar variable and stores it in foo, i.e. this is
essentially

  env set foo "$bar"

But the power of "env get" comes when bar is not just an explicit bare
word such as bar - namely, when I can use the shell to do one expansion
of the command line

  env get foo BOOT_${x}_LEFT

and thus choose which variable's value I'm fetching into foo.

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 14/02/2020 12.54, Rasmus Villemoes wrote:

> Now we can work around the lack of break in the busybox shell by writing
                                                  ^^^^^^^^^^^^^
> But we still can't translate this to busybox shell, because there's no
                                       ^^^^^^^^^^^^^
I of course meant "U-Boot shell", sorry about that.

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Martin Hundebøll]

Hi,

I chose to implement boot count / selection functionality as a command instead:
https://patchwork.ozlabs.org/patch/943549/

I do plan to extend it a bit in the coming weeks though:
 * move the env-get and -set to weak functions, so that board files can put the info wherever.
* add support for multiple lists of boot slots, so that the kernel can be updated independently of the rootfs.

// Martin

On 14 February 2020 12.54.35 CET, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
>On 13/02/2020 16.55, Wolfgang Denk wrote:
>> Dear Rasmus,
>> 
>> In message <f9aa247b-4b9b-9196-4de7-b352d25766fe@prevas.dk> you
>wrote:
>>>
>>> I'm sorry, I see I mistyped in my example above, it should have been
>>>
>>>   if test $slot = "A" ; setenv result $BOOT_A_LEFT ...
>>>
>>> as should hopefully be clear from the original post and the eval
>>> examples. So to reiterate, the problem is to get the contents (or
>value,
>>> if you will) of the BOOT_A_LEFT variable into the result variable,
>not
>>> setting result to the string "BOOT_A_LEFT" - but with the wrinkle
>that
>>> BOOT_A_LEFT is generated programmatically, so the code cannot
>literally
>>> mention BOOT_A_LEFT anywhere.
>> 
>> Didn't I show this in my second, expanded example?
>
>I'm sorry, but no, you did not. You used the capability of the print
>(or
>rather printenv) command to take the name of a variable and print
>"name=value" to the terminal.
>
>In your example, result had the value BOOT_A_LEFT, so doing "print
>$result" meant executing the command "print BOOT_A_LEFT", and of course
>the output of that was then "BOOT_A_LEFT=boot a left".
>
>However, what I need is to get that "boot a left" value stored in some
>other variable so I can actually do further logic based on that value.
>
>> I suggest you provide a working example of shell code (say, bash, if
>> you like) to demonstrate what you really have in mind.  It seems
>> I have problems understanding your verbal description.
>
>Assume BOOT_ORDER contains some permutation of "A B C", and for each
>letter x, there's a BOOT_x_LEFT counter telling how many boot attempts
>that slot has left. Now I want to find the first x in $BOOT_ORDER for
>which $BOOT_x_LEFT is positive. If all BOOT_x_LEFT are 0, say I want
>the
>sentinel value 'none'.
>
>So in bash, that might be written
>
>slot=none
>for x in $BOOT_ORDER ; do
>  eval "left=\${BOOT_${x}_LEFT}"
>  if [ $left -gt 0 ] ; then
>    slot=$x
>    break
>  fi
>done
>
>Now we can work around the lack of break in the busybox shell by
>writing
>the loop so that the main body is skipped if we've found a valid slot:
>
>slot=none
>for x in $BOOT_ORDER ; do
>  if [ $slot != 'none' ] ; then
>    true
>  else
>    eval "left=\${BOOT_${x}_LEFT}"
>    if [ $left -gt 0 ] ; then
>      slot=$x
>    fi
>  fi
>done
>
>But we still can't translate this to busybox shell, because there's no
>eval. Now, I can do this with a hypothetical "env get" command which I
>just implemented to test that it works, and then the above becomes
>
>env set slot none;
>for x in $BOOT_ORDER ; do
>  if test $slot != 'none' ; then
>    true ;
>  else
>    env get left BOOT_${x}_LEFT ; # magic
>    if test $left -gt 0 ; then
>      env set slot $x ;
>    fi;
>  fi;
>done;
>
>Now, if you can implement the line marked #magic with existing
>functions, I'm all ears. Or if you can implement the semantics of this
>snippet in some other way, which does not open-code explicit references
>to BOOT_A_LEFT, BOOT_B_LEFT etc. This is what I meant when I said I'd
>prefer not to write the loop like this:
>
>env set slot none;
>for x in $BOOT_ORDER ; do
>  if test $slot != 'none' ; then
>    true ;
>  else
>    if test $x = A && test $BOOT_A_LEFT -gt 0 ; then
>      env set slot A
>      env set left $BOOT_A_LEFT
>    elif test $x = B && test $BOOT_B_LEFT -gt 0 ; then
>      env set slot B
>      env set left $BOOT_B_LEFT
>    elif test $x = C && test $BOOT_C_LEFT -gt 0 ; then
>      env set slot C
>      env set left $BOOT_C_LEFT
>    fi
>  fi;
>done;
>
>[yes, I also want left set as a side effect to the current value of the
>appropriate BOOT_x_LEFT].
>
>>> So just as print[env] takes the name of a variable and shows the
>>> name=value string, and one can thus say "printenv BOOT_${slot}_LEFT"
>as
>>> you did in your extended example, I could do
>>>
>>>   env get result BOOT_${slot}_LEFT
>>>
>>> and get the value of the BOOT_${slot}_LEFT variable into result.
>> 
>> I still fail to see why you think this cannot be done with just the
>> already existing code. Just use setenv instead of printenv in my
>> example?
>> 
>>> Would you be ok with adding such an "env get" with less foot-gun
>potential?
>> 
>> I'm not OK with adding any special-purpose code which can easily
>> be implemented with existing scripting capabilites. 
>
>Of course not. But _I'm_ not seeing how one can actually fetch the
>value
>of one variable into another, just given the first variable's name -
>when that name is programmatically generated, e.g. as BOOT_${x}_LEFT or
>whatnot.
>
>PS: Implementation of "env get" is just:
>
>--- a/cmd/nvedit.c
>+++ b/cmd/nvedit.c
>@@ -386,6 +386,20 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag,
>int argc, char * const argv[])
>        return _do_env_set(flag, argc, argv, H_INTERACTIVE);
> }
>
>+static int do_env_get(cmd_tbl_t *cmdtp, int flag, int argc, char *
>const argv[])
>+{
>+       char *local_args[4];
>+
>+       if (argc != 3)
>+               return CMD_RET_USAGE;
>+       local_args[0] = argv[0];
>+       local_args[1] = argv[1];
>+       local_args[2] = env_get(argv[2]);
>+       local_args[3] = NULL;
>+
>+       return _do_env_set(flag, argc, local_args, H_INTERACTIVE);
>+}
>+
> /*
>  * Prompt for environment variable
>  */
>@@ -1334,6 +1348,7 @@ static cmd_tbl_t cmd_env_sub[] = {
> #endif
> #endif
>      U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
>+       U_BOOT_CMD_MKENT(get, 3, 0, do_env_get, "", ""),
> #if defined(CONFIG_CMD_ENV_EXISTS)
>        U_BOOT_CMD_MKENT(exists, 2, 0, do_env_exists, "", ""),
> #endif
>
>So it differs from "env set" (or setenv) in that
>
>  env set foo bar
>
>sets foo to the value "bar"
>
>while
>
>  env get foo bar
>
>"gets" the value of the bar variable and stores it in foo, i.e. this is
>essentially
>
>  env set foo "$bar"
>
>But the power of "env get" comes when bar is not just an explicit bare
>word such as bar - namely, when I can use the shell to do one expansion
>of the command line
>
>  env get foo BOOT_${x}_LEFT
>
>and thus choose which variable's value I'm fetching into foo.
>
>Rasmus

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus,

In message <509a01fb-ba01-33b9-33bc-7323544d290a@prevas.dk> you wrote:
>
> So in bash, that might be written
> 
> slot=none
> for x in $BOOT_ORDER ; do
>   eval "left=\${BOOT_${x}_LEFT}"
>   if [ $left -gt 0 ] ; then
>     slot=$x
>     break
>   fi
> done

OK, now I get the context.

You mean something like this in U-Boot ?

	setenv BOOT_ORDER A B C D
	setenv BOOT_A_LEFT 0
	setenv BOOT_B_LEFT 0
	setenv BOOT_C_LEFT 2
	setenv BOOT_D_LEFT 5

	slot=none
	for i in $BOOT_ORDER ; do
	setenv tmp_cmd 'setexpr tmp_val sub '^' "" $'BOOT_${i}_LEFT
	run tmp_cmd
	test $slot = none && test $tmp_val -gt 0 && slot=$i
	done
	echo "## Chosen Slot = $slot"

This returns

	## Chosen Slot = C

as you want.  The only inelegance of this approach is that it will
also print

	tmp_val=0
	tmp_val=0
	tmp_val=2
	tmp_val=5

on the console, which is not really nice.  I wonder if it's worth
adding a "-q" flag to the setexpr sommand?

And - I even beat your bash script which has 8 lines, while my
script has only 6 :-) :-)

> Now we can work around the lack of break in the busybox shell by writing
> the loop so that the main body is skipped if we've found a valid slot:

Things like this are usually easier done using && and ||

> But we still can't translate this to busybox shell, because there's no
> eval. Now, I can do this with a hypothetical "env get" command which I
> just implemented to test that it works, and then the above becomes

There is no eval, but there is still a zillion of simple tricks you
can pull to get your stuff done :-)

> Now, if you can implement the line marked #magic with existing
> functions, I'm all ears. Or if you can implement the semantics of this
> snippet in some other way, which does not open-code explicit references
> to BOOT_A_LEFT, BOOT_B_LEFT etc. This is what I meant when I said I'd
> prefer not to write the loop like this:

See above...

> [yes, I also want left set as a side effect to the current value of the
> appropriate BOOT_x_LEFT].

Oh, this is a new requirement...

So lets change my little script to add setting "left":

	slot=none
	for i in $BOOT_ORDER ; do
	setenv tmp_cmd 'setexpr tmp_val sub '^' "" $'BOOT_${i}_LEFT
	run tmp_cmd
	test $slot = none && test $tmp_val -gt 0 && slot=$i && left=$tmp_val
	done
	echo "## Chosen Slot = $slot ; Left = $left"

Result:

	## Chosen Slot = C ; Left = 2

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: wd at denx.de
Build a system that even a fool can use and only a fool will want  to
use it.

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Martin,

In message <2CD28D15-B5BF-4C5B-A57C-B9A24AD6D49E@geanix.com> you wrote:
>
>
> I chose to implement boot count / selection functionality as a command instead:
> https://patchwork.ozlabs.org/patch/943549/

Thanks for pointing out, I think I should NAK this patch, too. It
can be implemented by simple scripting, so there is no need to add
code that has to be maintained by all but is used by a very small
minoryty, if at all.

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: wd at denx.de
Abstainer: A weak person who yields to the temptation of denying him-
self a pleasure. A total abstainer is one who  abstains  from  every-
thing  but  abstention, and especially from inactivity in the affairs
of others.                                           - Ambrose Bierce

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Wolfgang Denk]

Dear Rasmus,

In message <20200216152427.E80C7240036@gemini.denx.de> I wrote:
>
> So lets change my little script to add setting "left":
>
> 	slot=none
> 	for i in $BOOT_ORDER ; do
> 	setenv tmp_cmd 'setexpr tmp_val sub '^' "" $'BOOT_${i}_LEFT
> 	run tmp_cmd
> 	test $slot = none && test $tmp_val -gt 0 && slot=$i && left=$tmp_val
> 	done
> 	echo "## Chosen Slot = $slot ; Left = $left"
>
> Result:
>
> 	## Chosen Slot = C ; Left = 2

Actually I'm stupid.  It's much easier this way, and without the
ugly printed messages:

	setenv BOOT_ORDER A B C D
	setenv BOOT_A_LEFT 0
	setenv BOOT_B_LEFT 0
	setenv BOOT_C_LEFT 2
	setenv BOOT_D_LEFT 5

	slot=none
	for i in $BOOT_ORDER ; do
	setenv tmp_cmd 'setenv tmp_val $'BOOT_${i}_LEFT
	run tmp_cmd
	test $slot = none && test $tmp_val -gt 0 && slot=$i && left=$tmp_val
	done
	echo "## Chosen Slot = $slot ; Left = $left"

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: wd at denx.de
A modem is a baudy house.

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 16/02/2020 18.25, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20200216152427.E80C7240036@gemini.denx.de> I wrote:
>>
>> So lets change my little script to add setting "left":
>>
>> 	slot=none
>> 	for i in $BOOT_ORDER ; do
>> 	setenv tmp_cmd 'setexpr tmp_val sub '^' "" $'BOOT_${i}_LEFT
>> 	run tmp_cmd
>> 	test $slot = none && test $tmp_val -gt 0 && slot=$i && left=$tmp_val
>> 	done
>> 	echo "## Chosen Slot = $slot ; Left = $left"
>>
>> Result:
>>
>> 	## Chosen Slot = C ; Left = 2
> 
> Actually I'm stupid. 

No, but I did notice the above seemed needlessly obfuscated :)

 It's much easier this way, and without the
> ugly printed messages:
> 
> 	setenv BOOT_ORDER A B C D
> 	setenv BOOT_A_LEFT 0
> 	setenv BOOT_B_LEFT 0
> 	setenv BOOT_C_LEFT 2
> 	setenv BOOT_D_LEFT 5
> 
> 	slot=none
> 	for i in $BOOT_ORDER ; do
> 	setenv tmp_cmd 'setenv tmp_val $'BOOT_${i}_LEFT
> 	run tmp_cmd

Nice. So the trick I was missing was to get a literal $, followed by the
("computed") name of the env var I wanted, all embedded in a command to
be run (to invoke the second expansion).

It's a bit tricky, but it does get the job done. There should be some
catalogue of things like this, mentioning "U-Boot shell doesn't directly
have $that feature, but you can often emulate it with something like $this".

> 	test $slot = none && test $tmp_val -gt 0 && slot=$i && left=$tmp_val

If performance matters, one can move the tmp_cmd handling after the
slot=none test - then one can also use left directly instead of tmp_val,
so the line only grows by one clause:

 	test $slot = none && setenv tmp_cmd 'setenv left $'BOOT_${i}_LEFT &&
run tmp_cmd && test $left -gt 0 && slot=$i

Or as a more readable alternative that still avoids the "run" overhead
and saves one line (and the tmp_var)

        setenv tmp_cmd 'setenv left $'BOOT_${i}_LEFT
 	test $slot = none && run tmp_cmd && test $left -gt 0 && slot=$i

Thanks, Wolfgang. Consider both "env set -E" and the alternative "env
get" withdrawn.

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 18/02/2020 09.11, Rasmus Villemoes wrote:

> Thanks, Wolfgang. Consider both "env set -E" and the alternative "env
> get" withdrawn.

So, if I wanted to change the status of such a patch in patchwork, what
would be the appropriate status? There's no "Withdrawn" or "Retracted".
So "Not applicable" or "Rejected"?

Rasmus

[PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Tom Rini]

On Fri, Feb 21, 2020 at 11:32:16PM +0000, Rasmus Villemoes wrote:
> On 18/02/2020 09.11, Rasmus Villemoes wrote:
> 
> > Thanks, Wolfgang. Consider both "env set -E" and the alternative "env
> > get" withdrawn.
> 
> So, if I wanted to change the status of such a patch in patchwork, what
> would be the appropriate status? There's no "Withdrawn" or "Retracted".
> So "Not applicable" or "Rejected"?

No strong preference, you can even just put it at / leave it at RFC.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200221/9f2860c2/attachment.sig>

Re: [PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 14/02/2020 12.54, Rasmus Villemoes wrote:
> 
> Assume BOOT_ORDER contains some permutation of "A B C", and for each
> letter x, there's a BOOT_x_LEFT counter telling how many boot attempts
> that slot has left. Now I want to find the first x in $BOOT_ORDER for
> which $BOOT_x_LEFT is positive. If all BOOT_x_LEFT are 0, say I want the
> sentinel value 'none'.
> 
> So in bash, that might be written
> 
> slot=none
> for x in $BOOT_ORDER ; do
>   eval "left=\${BOOT_${x}_LEFT}"
>   if [ $left -gt 0 ] ; then
>     slot=$x
>     break
>   fi
> done
> 
> Now we can work around the lack of break in the [U-Boot] shell by writing
> the loop so that the main body is skipped if we've found a valid slot:
> 
> slot=none
> for x in $BOOT_ORDER ; do
>   if [ $slot != 'none' ] ; then
>     true
>   else
>     eval "left=\${BOOT_${x}_LEFT}"
>     if [ $left -gt 0 ] ; then
>       slot=$x
>     fi
>   fi
> done
> 
> But we still can't translate this to [U-Boot] shell, because there's no
> eval. Now, I can do this with a hypothetical "env get" command which I
> just implemented to test that it works, and then the above becomes
> 
> env set slot none;
> for x in $BOOT_ORDER ; do
>   if test $slot != 'none' ; then
>     true ;
>   else
>     env get left BOOT_${x}_LEFT ; # magic
>     if test $left -gt 0 ; then
>       env set slot $x ;
>     fi;
>   fi;
> done;

For the benefit of people stumbling on this thread in the future, that
#magic line is actually possible as of v2022.07 and is called "env
indirect", guarded by CONFIG_CMD_NVEDIT_INDIRECT .

Rasmus