[U-Boot] [RFC] env: Group environment variables

[U-Boot] [RFC] env: Group environment variables

[John Schmoller]

This patch groups environment variables using a non-invasive protocol.
Grouping is achieved by setting a "grouping" variable to a string of
variables, and setting the master grouping variable, "env_groups" to
the list of these grouping variables.

For instance,
setenv net ipaddr netmask gatewayip serverip
setenv boot bootcmd bootdelay bootargs
setenv env_groups net boot

would print 4 variables grouped under net, 3 variables grouped under
boot, and the rest of the variables grouped under "other". If env_groups
is not defined, print behaves normally.

Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
---
I'm interesetd in seeing peoples opinions of this implementation of grouping
environment variables.  My major concerns about this implementation are

1) Using parse_line() requires placing several potentially large char array
(CONFIG_SYS_CBSIZE in size) on the stack.  Parse_line() does seem to be the
right tool for the job, though.

2) Trying to figure out which enviroment variables have already been printed
in groups is less than elegant.  Currently, it's a brute-force approach of
looking through every entry until a variable is found in a group or not.
Suggestions for cleaner algorithms here would be appreciated.

Implementation notes:
If env_groups is defined, none of the grouping variables will be printed.
This seemed to clutter up the printenv output.

Grouping environment variables will almost certainly lead to a reqirement for
bumping up CONFIG_SYS_MAXARGS.

Example output:
 => printenv
 === pci variables ===
 pci1inboundmembus=0x00000000
 pci1inboundmemphys=0x00000000
 pci1inboundmemsize=0x08000000
 pci1outboundmembus=0x80000000
 pci1outboundmemphys=0x80000000
 pci1outboundmemsize=0x40000000
 pci1outboundiobus=0x00000000
 pci1outboundiophys=0xe8000000
 pci1outboundiosize=0x00800000
 pci2outboundmembus=0xc0000000
 pci2outboundmemphys=0xc0000000
 pci2outboundmemsize=0x10000000

 === boot variables ===
 bootcmd_flash1=run set_bootargs; bootm 0xfef00000 - 0xfff00000
 bootcmd_flash2=run set_bootargs; bootm 0xf6f00000 - 0xf7f00000
 bootcmd=tftp 10000000 home/jschmoller/vxWorks.st;bootvx
 bootdelay=3
 bootfile=/home/shared/pxe/pxelinux.0
 bootcmd_net=run set_bootargs; $download_cmd $osaddr $osfile; if test $? -eq 0; then if test -n $fdtaddr; then $download_cmd $fdtaddr $fdtfile; if test $? -eq 0; then bootm $osaddr - $fdtaddr; else; echo FDT DOWNLOAD FAILED; fi; else; bootm $osaddr; fi; else; echo OS DOWNLOAD FAILED; fi;

 === prog variables ===
 prog_uboot1=$download_cmd $loadaddr $ubootfile; if test $? -eq 0; then protect off 0xfff80000 +80000; erase 0xfff80000 +80000; cp.w $loadaddr 0xfff80000 40000; protect on 0xfff80000 +80000; cmp.b $loadaddr 0xfff80000 80000; if test $? -ne 0; then echo PROGRAM FAILED; else; echo PROGRAM SUCCEEDED; fi; else; echo DOWNLOAD FAILED; fi;
 prog_uboot2=$download_cmd $loadaddr $ubootfile; if test $? -eq 0; then protect off 0xf7f80000 +80000; erase 0xf7f80000 +80000; cp.w $loadaddr 0xf7f80000 40000; protect on 0xf7f80000 +80000; cmp.b $loadaddr 0xf7f80000 80000; if test $? -ne 0; then echo PROGRAM FAILED; else; echo PROGRAM SUCCEEDED; fi; else; echo DOWNLOAD FAILED; fi;
 ubootfile=home/jschmoller/u-boot.bin
 prog_os1=$download_cmd $osaddr $osfile; if test $? -eq 0; then erase 0xfef00000 +$filesize; cp.b $osaddr 0xfef00000 $filesize; cmp.b $osaddr 0xfef00000 $filesize; if test $? -ne 0; then echo OS PROGRAM FAILED; else; echo OS PROGRAM SUCCEEDED; fi; else; echo OS DOWNLOAD FAILED; fi;
 prog_os2=$download_cmd $osaddr $osfile; if test $? -eq 0; then erase 0xf6f00000 +$filesize; cp.b $osaddr 0xf6f00000 $filesize; cmp.b $osaddr 0xf6f00000 $filesize; if test $? -ne 0; then echo OS PROGRAM FAILED; else; echo OS PROGRAM SUCCEEDED; fi; else; echo OS DOWNLOAD FAILED; fi;
 osfile=/home/user/board.uImage
 osaddr=0x1000000
 prog_fdt1=$download_cmd $fdtaddr $fdtfile; if test $? -eq 0; then erase 0xfff00000 +$filesize;cp.b $fdtaddr 0xfff00000 $filesize; cmp.b $fdtaddr 0xfff00000 $filesize; if test $? -ne 0; then echo FDT PROGRAM FAILED; else; echo FDT PROGRAM SUCCEEDED; fi; else; echo FDT DOWNLOAD FAILED; fi;
 prog_fdt2=$download_cmd $fdtaddr $fdtfile; if test $? -eq 0; then erase 0xf7f00000 +$filesize;cp.b $fdtaddr 0xf7f00000 $filesize; cmp.b $fdtaddr 0xf7f00000 $filesize; if test $? -ne 0; then echo FDT PROGRAM FAILED; else; echo FDT PROGRAM SUCCEEDED; fi; else; echo FDT DOWNLOAD FAILED; fi;
 fdtfile=/home/user/board.dtb
 fdtaddr=c00000

 === net variables ===
 ## Error: "ipaddr" not defined
 netmask=255.255.0.0
 serverip=10.52.0.33
 gatewayip=10.52.0.1
 ethaddr=00:17:3c:00:5f:30
 ethact=eTSEC1

 === board variables ===
 serial#=02091009
 board_cfg=90030065B-1
 board_rev=SB

 === junk variables ===
 ## Error: group "junk" not defined

 === other variables ===
 baudrate=115200
 loads_echo=1
 preboot=
 autoload=yes
 download_cmd=tftp
 console_args=console=ttyS0,115200
 root_args=root=/dev/nfs rw
 misc_args=ip=on
 set_bootargs=setenv bootargs ${console_args} ${root_args} ${misc_args}
 loadaddr=0x1000000
 pci2outboundiobus=0x00000000
 pci2outboundiophys=0xe8800000
 pci2outboundiosize=0x00800000
 eth1addr=00:17:3c:00:5f:31
 dnsip=10.52.0.1
 skip_nand_bbt=yes
 pcidelay=1000
 loadaddr=100000
 bootargs=motetsec(0,0)host:/home/mstarzewski/xes8572-vxWorks.st h=10.52.0.33 e=10.52.143.154:ffff0000 u=drives pw=drives f=0x0
 filesize=869D2
 fileaddr=10000000
 stdin=serial
 stdout=serial
 stderr=serial

 Environment size: 4148/32764 bytes
 => 

 common/cmd_nvedit.c |  120 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 110 insertions(+), 10 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index eb89e9e..6420e17 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -71,6 +71,54 @@ SPI_FLASH|MG_DISK|NVRAM|NOWHERE}
 #define XMK_STR(x)	#x
 #define MK_STR(x)	XMK_STR(x)
 
+enum {
+	PRINTENV_STATE_MATCHED = 0,
+	PRINTENV_STATE_ALL,
+	PRINTENV_STATE_SEARCH,
+	PRINTENV_STATE_GROUP
+};
+
+static int env_in_group(char *env)
+{
+	char *s;
+	char group_buf[CONFIG_SYS_CBSIZE];
+	char *group_argv[CONFIG_SYS_MAXARGS + 1];
+	int group_argc;
+	char env_buf[CONFIG_SYS_CBSIZE];
+	char *env_argv[CONFIG_SYS_MAXARGS + 1];
+	int env_argc;
+	int i, j;
+
+	if (strcmp("env_groups", env) == 0)
+		return 1;
+
+	if ((s = getenv("env_groups")) == NULL)
+		return 0;
+
+	strcpy(group_buf, s);
+	group_argc = parse_line(group_buf, group_argv);
+
+	/* Spin through all group variables specified in "env_groups" */
+	for (i = 0; i < group_argc; i++) {
+		if ((s = getenv(group_argv[i])) == NULL)
+			continue;
+
+		if (strcmp(group_argv[i], env) == 0)
+			return 1;
+
+		strcpy(env_buf, s);
+		env_argc = parse_line(env_buf, env_argv);
+
+		/* Spin through all vars contained in each group variable */
+		for (j = 0; j < env_argc; j++) {
+			if (strcmp(env_argv[j], env) == 0)
+				return 1;
+		}
+	}
+
+	return 0;
+}
+
 /************************************************************************
 ************************************************************************/
 
@@ -101,30 +149,48 @@ int get_env_id (void)
  * state 0: finish printing this string and return (matched!)
  * state 1: no matching to be done; print everything
  * state 2: continue searching for matched name
+ * state 3: print all only if not found in a group
  */
 static int printenv(char *name, int state)
 {
-	int i, j;
-	char c, buf[17];
+	int i, j, prev_state, size;
+	char c, buf[17], *str, *addr;
 
 	i = 0;
 	buf[16] = '\0';
+	prev_state = state;
 
 	while (state && env_get_char(i) != '\0') {
-		if (state == 2 && envmatch((uchar *)name, i) >= 0)
-			state = 0;
+		if (state == PRINTENV_STATE_SEARCH &&
+		    envmatch((uchar *)name, i) >= 0)
+			state = PRINTENV_STATE_MATCHED;
+
+		if (prev_state == PRINTENV_STATE_GROUP) {
+			addr = (char *)env_get_addr(i);
+			size = strchr(addr, '=') - addr;
+			str = malloc((size + 1) * sizeof(char));
+			strncpy(str, addr, size);
+			str[size] = '\0';
+
+			if (env_in_group(str))
+				state = PRINTENV_STATE_SEARCH;
+			else
+				state = PRINTENV_STATE_ALL;
+
+			free(str);
+		}
 
 		j = 0;
 		do {
 			buf[j++] = c = env_get_char(i++);
 			if (j == sizeof(buf) - 1) {
-				if (state <= 1)
+				if (state != PRINTENV_STATE_SEARCH)
 					puts(buf);
 				j = 0;
 			}
 		} while (c != '\0');
 
-		if (state <= 1) {
+		if (state != PRINTENV_STATE_SEARCH) {
 			if (j)
 				puts(buf);
 			putc('\n');
@@ -134,7 +200,7 @@ static int printenv(char *name, int state)
 			return -1;
 	}
 
-	if (state == 0)
+	if (state == PRINTENV_STATE_MATCHED)
 		i = 0;
 	return i;
 }
@@ -143,10 +209,44 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	int i;
 	int rcode = 0;
+	char *s;
+	char group_buf[CONFIG_SYS_CBSIZE];
+	char *group_argv[CONFIG_SYS_MAXARGS + 1];
+	int group_argc;
+	char env_buf[CONFIG_SYS_CBSIZE];
+	char *env_argv[CONFIG_SYS_MAXARGS + 1];
+	int env_argc;
 
 	if (argc == 1) {
-		/* print all env vars */
-		rcode = printenv(NULL, 1);
+		if ((s = getenv("env_groups")) == NULL) {
+			/* print all env vars */
+			rcode = printenv(NULL, PRINTENV_STATE_ALL);
+		} else {
+			/* print all env vars, but group them */
+			strcpy(group_buf, s);
+			group_argc = parse_line(group_buf, group_argv);
+
+			/* Cycle through all the groups in env_groups */
+			for (i = 0; i < group_argc; i++) {
+				printf("=== %s variables ===\n", group_argv[i]);
+				if ((s = getenv(group_argv[i])) == NULL) {
+					printf("## Error: group \"%s",
+					       group_argv[i]);
+					printf("\" not defined\n\n");
+					continue;
+				}
+
+				/* Recursively call printenv for each group */
+				sprintf(env_buf, "printenv %s", s);
+				env_argc = parse_line(env_buf, env_argv);
+				do_printenv(cmdtp, flag, env_argc, env_argv);
+				putc('\n');
+			}
+
+			puts("=== other variables ===\n");
+			rcode = printenv(NULL, PRINTENV_STATE_GROUP);
+		}
+
 		if (rcode < 0)
 			return 1;
 		printf("\nEnvironment size: %d/%ld bytes\n",
@@ -157,7 +257,7 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	/* print selected env vars */
 	for (i = 1; i < argc; ++i) {
 		char *name = argv[i];
-		if (printenv(name, 2)) {
+		if (printenv(name, PRINTENV_STATE_SEARCH)) {
 			printf("## Error: \"%s\" not defined\n", name);
 			++rcode;
 		}
-- 
1.6.0.4

[U-Boot] [RFC] env: Group environment variables

[Mike Frysinger]

On Wednesday 04 November 2009 11:34:12 John Schmoller wrote:
> This patch groups environment variables using a non-invasive protocol.
> Grouping is achieved by setting a "grouping" variable to a string of
> variables, and setting the master grouping variable, "env_groups" to
> the list of these grouping variables.
> 
> For instance,
> setenv net ipaddr netmask gatewayip serverip
> setenv boot bootcmd bootdelay bootargs
> setenv env_groups net boot
> 
> would print 4 variables grouped under net, 3 variables grouped under
> boot, and the rest of the variables grouped under "other". If env_groups
> is not defined, print behaves normally.
> 
> Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
> ---
> I'm interesetd in seeing peoples opinions of this implementation of
>  grouping environment variables.  My major concerns about this
>  implementation are

my main concern is bloat.  while i guess it would make nicer `printenv`, i'm 
not going to use it.  so please put it behind a config option so it doesnt 
waste space on all boards.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091104/6c19b3be/attachment.pgp 

[U-Boot] [RFC] env: Group environment variables

[John Schmoller]

On Wed, 2009-11-04 at 13:55 -0400, Mike Frysinger wrote:
> On Wednesday 04 November 2009 11:34:12 John Schmoller wrote:
> > This patch groups environment variables using a non-invasive protocol.
> > Grouping is achieved by setting a "grouping" variable to a string of
> > variables, and setting the master grouping variable, "env_groups" to
> > the list of these grouping variables.
> > 
> > For instance,
> > setenv net ipaddr netmask gatewayip serverip
> > setenv boot bootcmd bootdelay bootargs
> > setenv env_groups net boot
> > 
> > would print 4 variables grouped under net, 3 variables grouped under
> > boot, and the rest of the variables grouped under "other". If env_groups
> > is not defined, print behaves normally.
> > 
> > Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
> > ---
> > I'm interesetd in seeing peoples opinions of this implementation of
> >  grouping environment variables.  My major concerns about this
> >  implementation are
> 
> my main concern is bloat.  while i guess it would make nicer `printenv`, i'm 
> not going to use it.  so please put it behind a config option so it doesnt 
> waste space on all boards.

I can certainly do that.  It is a requested feature on the U-Boot task
list[1], so I didn't think that was needed, but I'll do it if that's
what people want.

John

[1] http://www.denx.de/wiki/U-Boot/TaskEnvironmentGroups

[U-Boot] [RFC] env: Group environment variables

[Mike Frysinger]

On Wednesday 04 November 2009 13:17:12 John Schmoller wrote:
> On Wed, 2009-11-04 at 13:55 -0400, Mike Frysinger wrote:
> > On Wednesday 04 November 2009 11:34:12 John Schmoller wrote:
> > > This patch groups environment variables using a non-invasive protocol.
> > > Grouping is achieved by setting a "grouping" variable to a string of
> > > variables, and setting the master grouping variable, "env_groups" to
> > > the list of these grouping variables.
> > >
> > > For instance,
> > > setenv net ipaddr netmask gatewayip serverip
> > > setenv boot bootcmd bootdelay bootargs
> > > setenv env_groups net boot
> > >
> > > would print 4 variables grouped under net, 3 variables grouped under
> > > boot, and the rest of the variables grouped under "other". If
> > > env_groups is not defined, print behaves normally.
> > >
> > > Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
> > > ---
> > > I'm interesetd in seeing peoples opinions of this implementation of
> > >  grouping environment variables.  My major concerns about this
> > >  implementation are
> >
> > my main concern is bloat.  while i guess it would make nicer `printenv`,
> > i'm not going to use it.  so please put it behind a config option so it
> > doesnt waste space on all boards.
> 
> I can certainly do that.  It is a requested feature on the U-Boot task
> list[1], so I didn't think that was needed, but I'll do it if that's
> what people want.

sure, i'm not saying people other than me dont want it, just that i dont think 
it's something you can call "core required functionality".  Wolfgang's call of 
course, but i'd like to see it behind a CONFIG (preferably disabled by 
default).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091104/ea9018bf/attachment.pgp 

[U-Boot] [RFC] env: Group environment variables

[John Schmoller]

On Wed, 2009-11-04 at 16:36 -0400, Mike Frysinger wrote:
> On Wednesday 04 November 2009 13:17:12 John Schmoller wrote:
> > On Wed, 2009-11-04 at 13:55 -0400, Mike Frysinger wrote:
> > > On Wednesday 04 November 2009 11:34:12 John Schmoller wrote:
> > > > This patch groups environment variables using a non-invasive protocol.
> > > > Grouping is achieved by setting a "grouping" variable to a string of
> > > > variables, and setting the master grouping variable, "env_groups" to
> > > > the list of these grouping variables.
> > > >
> > > > For instance,
> > > > setenv net ipaddr netmask gatewayip serverip
> > > > setenv boot bootcmd bootdelay bootargs
> > > > setenv env_groups net boot
> > > >
> > > > would print 4 variables grouped under net, 3 variables grouped under
> > > > boot, and the rest of the variables grouped under "other". If
> > > > env_groups is not defined, print behaves normally.
> > > >
> > > > Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
> > > > ---
> > > > I'm interesetd in seeing peoples opinions of this implementation of
> > > >  grouping environment variables.  My major concerns about this
> > > >  implementation are
> > >
> > > my main concern is bloat.  while i guess it would make nicer `printenv`,
> > > i'm not going to use it.  so please put it behind a config option so it
> > > doesnt waste space on all boards.
> > 
> > I can certainly do that.  It is a requested feature on the U-Boot task
> > list[1], so I didn't think that was needed, but I'll do it if that's
> > what people want.
> 
> sure, i'm not saying people other than me dont want it, just that i dont think 
> it's something you can call "core required functionality".  Wolfgang's call of 
> course, but i'd like to see it behind a CONFIG (preferably disabled by 
> default).

Will do.  I'll wait to see if anyone has any other comments about
implementation before resending.

John

[U-Boot] [RFC] env: Group environment variables

[Wolfgang Denk]

Dear Mike,

In message <200911041536.02492.vapier@gentoo.org> you wrote:
>
> > I can certainly do that.  It is a requested feature on the U-Boot task
> > list[1], so I didn't think that was needed, but I'll do it if that's
> > what people want.
>
> sure, i'm not saying people other than me dont want it, just that i dont think 
> it's something you can call "core required functionality".  Wolfgang's call of 
> course, but i'd like to see it behind a CONFIG (preferably disabled by 
> default).

Agreed on both accounts (i. e. it shall be configurable, default off).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Wenn Du ein' weise Antwort verlangst, Mu?t Du vern?nftig fragen.
                                                -- Goethe, Invektiven

[U-Boot] [RFC] env: Group environment variables

[Wolfgang Denk]

Dear John,

In message <1257352452-11748-1-git-send-email-jschmoller@xes-inc.com> you wrote:
> This patch groups environment variables using a non-invasive protocol.
> Grouping is achieved by setting a "grouping" variable to a string of
> variables, and setting the master grouping variable, "env_groups" to
> the list of these grouping variables.
> 
> For instance,
> setenv net ipaddr netmask gatewayip serverip
> setenv boot bootcmd bootdelay bootargs
> setenv env_groups net boot
> 
> would print 4 variables grouped under net, 3 variables grouped under
> boot, and the rest of the variables grouped under "other". If env_groups
> is not defined, print behaves normally.

First of all: thanks for the patch.

> I'm interesetd in seeing peoples opinions of this implementation of grouping
> environment variables.  My major concerns about this implementation are

I have to admit that I did not actually test the code yet. I just read
it a bit...

> 1) Using parse_line() requires placing several potentially large char array
> (CONFIG_SYS_CBSIZE in size) on the stack.  Parse_line() does seem to be the
> right tool for the job, though.

I am not sure about this. Keep in mind that parse_line() processes a
maximum of CONFIG_SYS_MAXARGS arguments only, which on most boards is
only 16 (and on some boards even less).

> 2) Trying to figure out which enviroment variables have already been printed
> in groups is less than elegant.  Currently, it's a brute-force approach of
> looking through every entry until a variable is found in a group or not.
> Suggestions for cleaner algorithms here would be appreciated.

The repeated scanning and comparing doesn't make much sense to me.
Probably it makes more sense to use a more suitable data structure
here.  How about performing a linear scan of the environment only
once and convert it into a more easily processable data structure, say
a hash table or a binary tree or whatever, and then operate on this.
Here you can easily add additional flags like a pointer which group the
variable belongs to (if any).

Also, this would make it easier for example to print a sorted list.

Eventually we should _always_ do that, i. e. replace the standard
copy operation as done in env_relocate*() by a function that not only
copies the environment, but converts it into a new internal
representation. This might be beneficial to accelerate access to
variables, too.



=> print only a list of groups

> If env_groups is defined, none of the grouping variables will be printed.
> This seemed to clutter up the printenv output.

I don't think this is a wise decision. Having "magic" variables which
cannot be seen an idea I dislike. I think the standard "printenv"
(without args) should print the grouping variables as first block.

Also, it would be nice if "prontenv" now would allow to print a group,
i. e. in your example something as "printenv net pci" should be
supported.

> Grouping environment variables will almost certainly lead to a reqirement for
> bumping up CONFIG_SYS_MAXARGS.

...which raises the question why there is such a static limit in the
first place. Yes, it was trivial to implement, but maybe this can be
improved? Artificial limits on line lengths and numbers of arguments
are a nice thing - to remove :-)


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The question of whether a computer can think is no more  interesting
than the question of whether a submarine can swim"
                                                - Edsgar W.  Dijkstra

[U-Boot] [RFC] env: Group environment variables

[John Schmoller]

On Thu, 2009-11-05 at 20:57 +0100, Wolfgang Denk wrote:
<snip>
> 
> > 2) Trying to figure out which enviroment variables have already been printed
> > in groups is less than elegant.  Currently, it's a brute-force approach of
> > looking through every entry until a variable is found in a group or not.
> > Suggestions for cleaner algorithms here would be appreciated.
> 
> The repeated scanning and comparing doesn't make much sense to me.
> Probably it makes more sense to use a more suitable data structure
> here.  How about performing a linear scan of the environment only
> once and convert it into a more easily processable data structure, say
> a hash table or a binary tree or whatever, and then operate on this.
> Here you can easily add additional flags like a pointer which group the
> variable belongs to (if any).
> 
> Also, this would make it easier for example to print a sorted list.
> 
> Eventually we should _always_ do that, i. e. replace the standard
> copy operation as done in env_relocate*() by a function that not only
> copies the environment, but converts it into a new internal
> representation. This might be beneficial to accelerate access to
> variables, too.

This is an excellent suggestion.  I'll see what I can whip up.

> 
> => print only a list of groups
> 
> > If env_groups is defined, none of the grouping variables will be printed.
> > This seemed to clutter up the printenv output.
> 
> I don't think this is a wise decision. Having "magic" variables which
> cannot be seen an idea I dislike. I think the standard "printenv"
> (without args) should print the grouping variables as first block.

Ok, I'll group the grouping variables into a group of their own.

> Also, it would be nice if "prontenv" now would allow to print a group,
> i. e. in your example something as "printenv net pci" should be
> supported.

This is already (accidentally :) supported.  a "printenv $net" would do
just what you state.

> > Grouping environment variables will almost certainly lead to a reqirement for
> > bumping up CONFIG_SYS_MAXARGS.
> 
> ...which raises the question why there is such a static limit in the
> first place. Yes, it was trivial to implement, but maybe this can be
> improved? Artificial limits on line lengths and numbers of arguments
> are a nice thing - to remove :-)
> 
> 
> Best regards,
> 
> Wolfgang Denk

John

[U-Boot] [RFC] env: Group environment variables

[Wolfgang Denk]

Dear John,

In message <1257452739.8937.1166.camel@johns> you wrote:
>
> > Also, it would be nice if "prontenv" now would allow to print a group,
> > i. e. in your example something as "printenv net pci" should be
> > supported.
> 
> This is already (accidentally :) supported.  a "printenv $net" would do
> just what you state.

Um.. this is not what I mean.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Humanity has the  stars  in  its  future,  and  that  future  is  too
important  to be lost under the burden of juvenile folly and ignorant
superstition.                                          - Isaac Asimov

[U-Boot] [RFC] env: Group environment variables

[John Schmoller]

On Thu, 2009-11-05 at 23:37 +0100, Wolfgang Denk wrote:
> Dear John,
> 
> In message <1257452739.8937.1166.camel@johns> you wrote:
> >
> > > Also, it would be nice if "prontenv" now would allow to print a group,
> > > i. e. in your example something as "printenv net pci" should be
> > > supported.
> > 
> > This is already (accidentally :) supported.  a "printenv $net" would do
> > just what you state.
> 
> Um.. this is not what I mean.

So, a "printenv net" would print 
 ipaddr netmask gatewayip serverip

A "printenv $net" would print
 ipaddr=10.52.0.133
 netmask=255.255.0.0
 gatewayip=10.52.0.1
 serverip=10.52.0.33

Do you mean something other than either of those?

John

[U-Boot] [RFC] env: Group environment variables

[Wolfgang Denk]

Dear John Schmoller,

In message <1257460973.8937.1170.camel@johns> you wrote:
>
> > > > Also, it would be nice if "prontenv" now would allow to print a group,
> > > > i. e. in your example something as "printenv net pci" should be
> > > > supported.
> > > 
> > > This is already (accidentally :) supported.  a "printenv $net" would do
> > > just what you state.
> > 
> > Um.. this is not what I mean.
> 
> So, a "printenv net" would print 
>  ipaddr netmask gatewayip serverip
> 
> A "printenv $net" would print
>  ipaddr=10.52.0.133
>  netmask=255.255.0.0
>  gatewayip=10.52.0.1
>  serverip=10.52.0.33
> 
> Do you mean something other than either of those?

Please ignore my question. My brain was offline when I asked this one.
You are right, of course.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I wrote my name at the top of the page. I wrote down  the  number  of
the  question  ``1''.  After much reflection I put a bracket round it
thus ``(1)''. But thereafter I could not think of anything  connected
with it that was either relevant or true.
                        - Sir Winston Churchill _My Early Life_ ch. 2