* [PATCH v1] env: setenv add resolve value option
@ 2021-11-02 7:19 Artem Lapkin
2021-11-02 14:58 ` Simon Glass
2021-11-02 16:44 ` Tom Rini
0 siblings, 2 replies; 5+ messages in thread
From: Artem Lapkin @ 2021-11-02 7:19 UTC (permalink / raw)
To: sjg
Cc: trini, marek.behun, narmstrong, twarren, andre.przywara, u-boot,
u-boot-amlogic, art, nick, gouwa
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 <art@khadas.com>
---
cmd/nvedit.c | 42 +++++++++++++++++++++++++++++++++++++++++-
include/_exports.h | 1 +
include/env.h | 11 +++++++++++
include/exports.h | 1 +
4 files changed, 54 insertions(+), 1 deletion(-)
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
+ */
+ 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));
+ 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')
+ 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 ee5e30d036..c4efc5b7e5 100644
--- a/include/env.h
+++ b/include/env.h
@@ -133,6 +133,17 @@ int env_get_f(const char *name, char *buf, unsigned int len);
*/
int env_get_yesno(const char *var);
+/**
+ * env_resolve() - resolve to environment variable
+ *
+ * Same as env_set but make deep resolve for variable
+ *
+ * @varname: Variable to adjust
+ * @value: Value to resolve for the variable, or NULL or "" to delete the variable
+ * @return 0 if OK, 1 on error
+ */
+int env_resolve(const char *varname, const char *value);
+
/**
* env_set() - set an environment variable
*
diff --git a/include/exports.h b/include/exports.h
index 550cafdc7a..a54b997988 100644
--- a/include/exports.h
+++ b/include/exports.h
@@ -44,6 +44,7 @@ int vprintf(const char *, va_list);
unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
char *env_get(const char *name);
+int env_resolve(const char *varname, const char *value);
int env_set(const char *varname, const char *value);
long simple_strtol(const char *cp, char **endp, unsigned int base);
int strcmp(const char *cs, const char *ct);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1] env: setenv add resolve value option
2021-11-02 7:19 [PATCH v1] env: setenv add resolve value option Artem Lapkin
@ 2021-11-02 14:58 ` Simon Glass
2021-11-02 16:44 ` Tom Rini
1 sibling, 0 replies; 5+ messages in thread
From: Simon Glass @ 2021-11-02 14:58 UTC (permalink / raw)
To: Artem Lapkin
Cc: Tom Rini, Marek Behún, Neil Armstrong, Tom Warren,
Andre Przywara, U-Boot Mailing List, u-boot-amlogic,
Artem Lapkin, Nick Xie, Gouwa Wang
Hi Artem,
On Tue, 2 Nov 2021 at 01:19, Artem Lapkin <email2tema@gmail.com> 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 <art@khadas.com>
> ---
> 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-nm@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 ee5e30d036..c4efc5b7e5 100644
> --- a/include/env.h
> +++ b/include/env.h
> @@ -133,6 +133,17 @@ int env_get_f(const char *name, char *buf, unsigned int len);
> */
> int env_get_yesno(const char *var);
>
> +/**
> + * env_resolve() - resolve to environment variable
> + *
> + * Same as env_set but make deep resolve for variable
> + *
> + * @varname: Variable to adjust
> + * @value: Value to resolve for the variable, or NULL or "" to delete the variable
> + * @return 0 if OK, 1 on error
> + */
> +int env_resolve(const char *varname, const char *value);
> +
> /**
> * env_set() - set an environment variable
> *
> diff --git a/include/exports.h b/include/exports.h
> index 550cafdc7a..a54b997988 100644
> --- a/include/exports.h
> +++ b/include/exports.h
> @@ -44,6 +44,7 @@ int vprintf(const char *, va_list);
> unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
> int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
> char *env_get(const char *name);
> +int env_resolve(const char *varname, const char *value);
please add function comment
> int env_set(const char *varname, const char *value);
> long simple_strtol(const char *cp, char **endp, unsigned int base);
> int strcmp(const char *cs, const char *ct);
> --
> 2.25.1
>
Can you please add to the env tests?
test/py/tests/test_env.py
Regards,
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] env: setenv add resolve value option
2021-11-02 7:19 [PATCH v1] env: setenv add resolve value option Artem Lapkin
2021-11-02 14:58 ` Simon Glass
@ 2021-11-02 16:44 ` Tom Rini
2021-11-03 7:41 ` Art Nikpal
1 sibling, 1 reply; 5+ messages in thread
From: Tom Rini @ 2021-11-02 16:44 UTC (permalink / raw)
To: Artem Lapkin
Cc: sjg, marek.behun, narmstrong, twarren, andre.przywara, u-boot,
u-boot-amlogic, art, nick, gouwa
[-- Attachment #1: Type: text/plain, Size: 866 bytes --]
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 <art@khadas.com>
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] env: setenv add resolve value option
2021-11-02 16:44 ` Tom Rini
@ 2021-11-03 7:41 ` Art Nikpal
2021-11-05 2:02 ` Simon Glass
0 siblings, 1 reply; 5+ messages in thread
From: Art Nikpal @ 2021-11-03 7:41 UTC (permalink / raw)
To: Tom Rini
Cc: Simon Glass, marek.behun, Neil Armstrong, Tom Warren,
Andre Przywara, U-Boot Mailing List, u-boot-amlogic,
Artem Lapkin, Nick Xie, Gouwa Wang
> 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-nm@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 <trini@konsulko.com> 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 <art@khadas.com>
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] env: setenv add resolve value option
2021-11-03 7:41 ` Art Nikpal
@ 2021-11-05 2:02 ` Simon Glass
0 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2021-11-05 2:02 UTC (permalink / raw)
To: Art Nikpal
Cc: Tom Rini, marek.behun, Neil Armstrong, Tom Warren,
Andre Przywara, U-Boot Mailing List, u-boot-amlogic,
Artem Lapkin, Nick Xie, Gouwa Wang
Hi Art,
On Wed, 3 Nov 2021 at 01:41, Art Nikpal <email2tema@gmail.com> 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-nm@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 <trini@konsulko.com> 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 <art@khadas.com>
> >
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-05 2:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02 7:19 [PATCH v1] env: setenv add resolve value option Artem Lapkin
2021-11-02 14:58 ` Simon Glass
2021-11-02 16:44 ` Tom Rini
2021-11-03 7:41 ` Art Nikpal
2021-11-05 2:02 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.