From patchwork Fri Feb 14 11:54:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236361 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Fri, 14 Feb 2020 11:54:35 +0000 Subject: [PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set" In-Reply-To: <20200213155514.C4DA1240057@gemini.denx.de> References: <20200205010812.20373-1-rasmus.villemoes@prevas.dk> <20200211163055.30A262403FE@gemini.denx.de> <20200212113848.997CB240057@gemini.denx.de> <20200213155514.C4DA1240057@gemini.denx.de> Message-ID: <509a01fb-ba01-33b9-33bc-7323544d290a@prevas.dk> On 13/02/2020 16.55, Wolfgang Denk wrote: > Dear Rasmus, > > In message 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: * 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 --- 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); +} + /*