Re: [PATCH v1] env: setenv add resolve value option
Hi Art, On Wed, 3 Nov 2021 at 01:41, Art Nikpal wrote: > > > The high level problem I have with this patch is that we keep going back > > to "we really need to update to a modern hush (or other shell) rather > > than patching new features in to our ancient fork". > > Yes it will be fine ! Does anybody have information about these plans ? Yes see here: https://docs.google.com/document/d/1YBOMsbM19uSFyoJWnt7-PsOLBaevzQUgV-hiR88a5-o/edit#bookmark=id.j6h2xzste5sy We could ask for an update on progress. > but in any case my patch didn't broke compatibility like next patch > > > See also this old patch: > > https://patchwork.ozlabs.org/project/uboot/patch/1449255744-25787-1-git-send-email...@ti.com/ > > > Can you please add to the env tests? > > please add function comment > > ... > > tnx for suggestions ... > i can make v2 variant for my patch , if no one is against this idea I am not against it. But see Tom's comment. > > On Wed, Nov 3, 2021 at 12:44 AM Tom Rini wrote: > > > > On Tue, Nov 02, 2021 at 03:19:14PM +0800, Artem Lapkin wrote: > > > > > Add possibility setup env variable with additional resolving vars inside > > > value. > > > > > > Usage examples > > > > > > => setenv a hello > > > => setenv b world > > > => setenv c '${a} ${b}' > > > => setenv -r d '${c}! ${a}...' > > > => printenv d > > > d=hello world! hello... > > > > > > /* internal usage example */ > > > env_resolve("d", "${c}! ${a}..."); > > > /* d="hello world! hello..." */ > > > > > > Notes > > > > > > Resolving works only for ${var} "bracket" and didn't workd for > > > "unbracket" $var > > > > > > => setenv -r d '$c! $a...' > > > => printenv d > > > d=$c! $a... > > > > > > Signed-off-by: Artem Lapkin > > > > The high level problem I have with this patch is that we keep going back > > to "we really need to update to a modern hush (or other shell) rather > > than patching new features in to our ancient fork". It is orthogonal to the shell, so far as I can tell. Regards, Simon
Re: [PATCH v1] env: setenv add resolve value option
> The high level problem I have with this patch is that we keep going back > to "we really need to update to a modern hush (or other shell) rather > than patching new features in to our ancient fork". Yes it will be fine ! Does anybody have information about these plans ? but in any case my patch didn't broke compatibility like next patch > See also this old patch: > https://patchwork.ozlabs.org/project/uboot/patch/1449255744-25787-1-git-send-email...@ti.com/ > Can you please add to the env tests? > please add function comment > ... tnx for suggestions ... i can make v2 variant for my patch , if no one is against this idea On Wed, Nov 3, 2021 at 12:44 AM Tom Rini wrote: > > On Tue, Nov 02, 2021 at 03:19:14PM +0800, Artem Lapkin wrote: > > > Add possibility setup env variable with additional resolving vars inside > > value. > > > > Usage examples > > > > => setenv a hello > > => setenv b world > > => setenv c '${a} ${b}' > > => setenv -r d '${c}! ${a}...' > > => printenv d > > d=hello world! hello... > > > > /* internal usage example */ > > env_resolve("d", "${c}! ${a}..."); > > /* d="hello world! hello..." */ > > > > Notes > > > > Resolving works only for ${var} "bracket" and didn't workd for > > "unbracket" $var > > > > => setenv -r d '$c! $a...' > > => printenv d > > d=$c! $a... > > > > Signed-off-by: Artem Lapkin > > The high level problem I have with this patch is that we keep going back > to "we really need to update to a modern hush (or other shell) rather > than patching new features in to our ancient fork". > > -- > Tom
Re: [PATCH v1] env: setenv add resolve value option
On Tue, Nov 02, 2021 at 03:19:14PM +0800, Artem Lapkin wrote: > Add possibility setup env variable with additional resolving vars inside > value. > > Usage examples > > => setenv a hello > => setenv b world > => setenv c '${a} ${b}' > => setenv -r d '${c}! ${a}...' > => printenv d > d=hello world! hello... > > /* internal usage example */ > env_resolve("d", "${c}! ${a}..."); > /* d="hello world! hello..." */ > > Notes > > Resolving works only for ${var} "bracket" and didn't workd for > "unbracket" $var > > => setenv -r d '$c! $a...' > => printenv d > d=$c! $a... > > Signed-off-by: Artem Lapkin The high level problem I have with this patch is that we keep going back to "we really need to update to a modern hush (or other shell) rather than patching new features in to our ancient fork". -- Tom signature.asc Description: PGP signature
Re: [PATCH v1] env: setenv add resolve value option
Hi Artem, On Tue, 2 Nov 2021 at 01:19, Artem Lapkin wrote: > > Add possibility setup env variable with additional resolving vars inside > value. > > Usage examples > > => setenv a hello > => setenv b world > => setenv c '${a} ${b}' > => setenv -r d '${c}! ${a}...' > => printenv d > d=hello world! hello... > > /* internal usage example */ > env_resolve("d", "${c}! ${a}..."); > /* d="hello world! hello..." */ > > Notes > > Resolving works only for ${var} "bracket" and didn't workd for > "unbracket" $var how come? > > => setenv -r d '$c! $a...' > => printenv d > d=$c! $a... > > Signed-off-by: Artem Lapkin > --- > cmd/nvedit.c | 42 +- > include/_exports.h | 1 + > include/env.h | 11 +++ > include/exports.h | 1 + > 4 files changed, 54 insertions(+), 1 deletion(-) See also this old patch: https://patchwork.ozlabs.org/project/uboot/patch/1449255744-25787-1-git-send-email...@ti.com/ > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > index 3bb6e764c0..6608932dc0 100644 > --- a/cmd/nvedit.c > +++ b/cmd/nvedit.c > @@ -229,6 +229,7 @@ static int _do_env_set(int flag, int argc, char *const > argv[], int env_flag) > int i, len; > char *name, *value, *s; > struct env_entry e, *ep; > + bool resolve = 0; > > debug("Initial value for argc=%d\n", argc); > > @@ -246,6 +247,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 'r': /* resolve */ > + resolve = 1; > + break; > default: > return CMD_RET_USAGE; > } > @@ -291,6 +295,26 @@ static int _do_env_set(int flag, int argc, char *const > argv[], int env_flag) > if (s != value) > *--s = '\0'; > > + /* > +* deep resolve value vars > +*/ single-line comment style is /*... */ > + if (resolve) { > + int max_loop = 32; > + char value2[CONFIG_SYS_CBSIZE]; > + > + do { > + cli_simple_process_macros(value, value2, > CONFIG_SYS_CBSIZE); > + if (!strcmp(value, value2)) > + break; > + value = realloc(value, strlen(value2)); strdup() ? > + if (!value) { > + printf("## Can't realloc %d bytes\n", len); > + return 1; > + } > + strcpy(value, value2); > + } while (max_loop--); > + } > + > e.key = name; > e.data = value; > hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag); > @@ -304,6 +328,20 @@ static int _do_env_set(int flag, int argc, char *const > argv[], int env_flag) > return 0; > } > > +int env_resolve(const char *varname, const char *varvalue) > +{ > + const char * const argv[5] = { "setenv", "-r", varname, varvalue, > NULL }; > + > + /* before import into hashtable */ > + if (!(gd->flags & GD_FLG_ENV_READY)) > + return 1; > + > + if (!varvalue || varvalue[0] == '\0') Could put that in a var to avoid repeating in the next 3 lines: > + return _do_env_set(0, 3, (char * const *)argv, > H_PROGRAMMATIC); > + else > + return _do_env_set(0, 4, (char * const *)argv, > H_PROGRAMMATIC); > +} > + > int env_set(const char *varname, const char *varvalue) > { > const char * const argv[4] = { "setenv", varname, varvalue, NULL }; > @@ -1371,7 +1409,9 @@ U_BOOT_CMD_COMPLETE( > "setenv [-f] name value ...\n" > "- [forcibly] set environment variable 'name' to 'value ...'\n" > "setenv [-f] name\n" > - "- [forcibly] delete environment variable 'name'", > + "- [forcibly] delete environment variable 'name'\n" > + "setenv [-r] name value ...\n" > + "- [resolve] resolve 'value ...' to environment variable\n", > var_complete > ); > > diff --git a/include/_exports.h b/include/_exports.h > index 8030d70c0b..86bc07f051 100644 > --- a/include/_exports.h > +++ b/include/_exports.h > @@ -32,6 +32,7 @@ > EXPORT_FUNC(do_reset, int, do_reset, struct cmd_tbl *, > int , int , char * const []) > EXPORT_FUNC(env_get, char *, env_get, const char*) > + EXPORT_FUNC(env_resolve, int, env_resolve, const char *, const char *) > EXPORT_FUNC(env_set, int, env_set, const char *, const char *) > EXPORT_FUNC(simple_strtoul, unsigned long, simple_strtoul, > const char *, char **, unsigned int) > diff --git a/include/env.h b/include/env.h > index ee5e30