[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 at prevas.dk> wrote:
>On 13/02/2020 16.55, Wolfgang Denk wrote:
>> Dear Rasmus,
>> 
>> In message <f9aa247b-4b9b-9196-4de7-b352d25766fe at 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.