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

Simon Glass sjg at chromium.org
Wed Feb 5 18:59:07 CET 2020


Hi Rasmus,

On Tue, 4 Feb 2020 at 18:08, Rasmus Villemoes
<rasmus.villemoes at 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 at 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


More information about the U-Boot mailing list