From patchwork Wed Feb  5 01:08:20 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
X-Patchwork-Id: 235967
List-Id: U-Boot discussion <u-boot.lists.denx.de>
From: rasmus.villemoes at prevas.dk (Rasmus Villemoes)
Date: Wed, 5 Feb 2020 01:08:20 +0000
Subject: [PATCH] RFC: nvedit: support doing one (extra) expansion of the
 value in "env set"
Message-ID: <20200205010812.20373-1-rasmus.villemoes@prevas.dk>

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(-)

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);