[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Gerlando Falauto]

This is a resubmission (after removing remove checkpatch errors) of
http://lists.denx.de/pipermail/u-boot/2011-September/102875.html

Here I am proposing a set of changes in the behaviour of the environment 
import/set_to_default functions.

PATCH 1:
Add a "new" himport_ex() function (reworking of himport_r which is now a 
wrapper around it), which has 3 new arguments:

"nvars", "vars":, number and list of variables to take into account 
(0 means ALL)

"apply" callback function which is in charge of checking whether a 
variable can be overwritten, and possibly immediately apply the changes. 
This parameter would be either set to NULL (in which case nothing should 
change wrt to the past -- i.e. environment is blindly imported) or to 
"env_check_apply()" function, whose code was taken away from _do_env_set().
This would be useful, for instance, for "baudrate" or "stdin,stderr,stdout", 
whose changes would not otherwise be effective until the next reboot.

The idea is that there should be a single place where all the checks are
to be performed. So the same function env_check_apply() is called from 
_do_env_set() as well (thus keeping the previous behavior).

PATCH 2:
Add the same behaviour when deleting variables.
Not quite sure whether this makes sense or not.

PATCH 3:
Add the code for setting individual variables to default

PATCH 4:
Impolement importing of  individual variables
(also taking into account "-f" for forcing).

Gerlando Falauto (4):
  Groundwork for generalization of env interface
  env: check and apply changes on delete/destroy
  env: implement selective "env default"
  env: implement "env import -n var[,var...]"

 README                   |    2 +
 common/cmd_nvedit.c      |  231 ++++++++++++++++++++++++++++++++--------------
 common/env_common.c      |   29 ++++++-
 include/config_cmd_all.h |    1 +
 include/environment.h    |   12 +++
 include/search.h         |   19 ++++-
 lib/hashtable.c          |   68 ++++++++++++-
 7 files changed, 284 insertions(+), 78 deletions(-)

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c   |  152 +++++++++++++++++++++++++++++++------------------
 common/env_common.c   |   15 ++++-
 include/environment.h |    7 ++
 include/search.h      |   13 ++++
 lib/hashtable.c       |   50 ++++++++++++++++
 5 files changed, 179 insertions(+), 58 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index fa99c44..10b9552 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -194,32 +194,23 @@ static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 #endif
 
 /*
- * Set a new environment variable,
- * or replace or delete an existing one.
+ * Performs consistency checking before setting, replacing,
+ * or deleting an environment variable, then (if successful)
+ * apply the changes to internals so to make them effective.
+ * Code for this function was taken out of _do_env_set(),
+ * which now calls it.
+ * Also called as a callback function by himport_ex().
+ * Returns 0 in case of success, 1 in case of failure.
+ * When (flag & H_FORCE) is set, force overwriting of
+ * write-once variables.
  */
 
-int _do_env_set (int flag, int argc, char * const argv[])
+int env_check_apply(const char *name, const char *oldval,
+		    const char *newval, int flag)
 {
 	bd_t  *bd = gd->bd;
-	int   i, len;
+	int   i;
 	int   console = -1;
-	char  *name, *value, *s;
-	ENTRY e, *ep;
-
-	name = argv[1];
-
-	if (strchr(name, '=')) {
-		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
-		return 1;
-	}
-
-	env_id++;
-	/*
-	 * search if variable with this name already exists
-	 */
-	e.key = name;
-	e.data = NULL;
-	hsearch_r(e, FIND, &ep, &env_htab);
 
 	/* Check for console redirection */
 	if (strcmp(name, "stdin") == 0)
@@ -230,22 +221,23 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		console = stderr;
 
 	if (console != -1) {
-		if (argc < 3) {		/* Cannot delete it! */
+		if ((newval == NULL) || (*newval == '\0')) {
+			/* We cannot delete stdin/stdout/stderr */
 			printf("Can't delete \"%s\"\n", name);
 			return 1;
 		}
 
 #ifdef CONFIG_CONSOLE_MUX
-		i = iomux_doenv(console, argv[2]);
+		i = iomux_doenv(console, newval);
 		if (i)
 			return i;
 #else
 		/* Try assigning specified device */
-		if (console_assign(console, argv[2]) < 0)
+		if (console_assign(console, newval) < 0)
 			return 1;
 
 #ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign(argv[2]) < 0)
+		if (serial_assign(newval) < 0)
 			return 1;
 #endif
 #endif /* CONFIG_CONSOLE_MUX */
@@ -255,23 +247,29 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 * Some variables like "ethaddr" and "serial#" can be set only
 	 * once and cannot be deleted; also, "ver" is readonly.
 	 */
-	if (ep) {		/* variable exists */
+	if ((oldval != NULL) /* variable exists */
+	 && ((flag & H_FORCE) == 0)) { /* and we are not forced */
 #ifndef CONFIG_ENV_OVERWRITE
 		if ((strcmp(name, "serial#") == 0) ||
 		    ((strcmp(name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
-		     && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
+		     && (strcmp(oldval, MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
 		    ) ) {
 			printf("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}
 #endif
+	}
+	/* if variable exists... */
+	if ((oldval != NULL)
+	/* or we are in a scratched-out environment */
+	|| ((flag & H_NOCLEAR) == 0)) {
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
 		if (strcmp(name, "baudrate") == 0) {
-			int baudrate = simple_strtoul(argv[2], NULL, 10);
+			int baudrate = simple_strtoul(newval, NULL, 10);
 			int i;
 			for (i = 0; i < N_BAUDRATES; ++i) {
 				if (baudrate == baudrate_table[i])
@@ -282,6 +280,10 @@ int _do_env_set (int flag, int argc, char * const argv[])
 					baudrate);
 				return 1;
 			}
+			if (gd->baudrate == baudrate) {
+				/* If unchanged, we just say it's OK */
+				return 0;
+			}
 			printf ("## Switch baudrate to %d bps and press ENTER ...\n",
 				baudrate);
 			udelay(50000);
@@ -299,6 +301,72 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		}
 	}
 
+	/*
+	 * Some variables should be updated when the corresponding
+	 * entry in the environment is changed
+	 */
+
+	if (strcmp(name, "ipaddr") == 0) {
+		const char *s = newval;	/* always use only one arg */
+		char *e;
+		unsigned long addr;
+		bd->bi_ip_addr = 0;
+		for (addr = 0, i = 0; i < 4; ++i) {
+			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
+			addr <<= 8;
+			addr  |= (val & 0xFF);
+			if (s)
+				s = (*e) ? e+1 : e;
+		}
+		bd->bi_ip_addr = htonl(addr);
+		return 0;
+	} else if (strcmp(newval, "loadaddr") == 0) {
+		load_addr = simple_strtoul(newval, NULL, 16);
+		return 0;
+	}
+#if defined(CONFIG_CMD_NET)
+	else if (strcmp(newval, "bootfile") == 0) {
+		copy_filename(BootFile, newval, sizeof(BootFile));
+		return 0;
+	}
+#endif
+	return 0;
+}
+
+/*
+ * Set a new environment variable,
+ * or replace or delete an existing one.
+*/
+int _do_env_set(int flag, int argc, char * const argv[])
+{
+	int   i, len;
+	char  *name, *value, *s;
+	ENTRY e, *ep;
+
+	name = argv[1];
+	value = argv[2];
+
+	if (strchr(name, '=')) {
+		printf("## Error: illegal character '='"
+		       "in variable name \"%s\"\n", name);
+		return 1;
+	}
+
+	env_id++;
+	/*
+	 * search if variable with this name already exists
+	 */
+	e.key = name;
+	e.data = NULL;
+	hsearch_r(e, FIND, &ep, &env_htab);
+
+	/* Perform requested checks. Notice how since we are overwriting
+	 * a single variable, we need to set H_NOCLEAR */
+	if (env_check_apply(name, ep ? ep->data : NULL, value, H_NOCLEAR)) {
+		debug("check function did not approve, refusing\n");
+		return 1;
+	}
+
 	/* Delete only ? */
 	if ((argc < 3) || argv[2] == NULL) {
 		int rc = hdelete_r(name, &env_htab);
@@ -336,34 +404,6 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	/*
-	 * Some variables should be updated when the corresponding
-	 * entry in the environment is changed
-	 */
-
-	if (strcmp(name, "ipaddr") == 0) {
-		char *s = argv[2];	/* always use only one arg */
-		char *e;
-		unsigned long addr;
-		bd->bi_ip_addr = 0;
-		for (addr = 0, i = 0; i < 4; ++i) {
-			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
-			addr <<= 8;
-			addr  |= (val & 0xFF);
-			if (s) s = (*e) ? e+1 : e;
-		}
-		bd->bi_ip_addr = htonl(addr);
-		return 0;
-	} else if (strcmp(argv[1], "loadaddr") == 0) {
-		load_addr = simple_strtoul(argv[2], NULL, 16);
-		return 0;
-	}
-#if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1], "bootfile") == 0) {
-		copy_filename(BootFile, argv[2], sizeof(BootFile));
-		return 0;
-	}
-#endif
 	return 0;
 }
 
diff --git a/common/env_common.c b/common/env_common.c
index c7e9bea..db607e1 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -172,6 +172,9 @@ const uchar *env_get_addr (int index)
 
 void set_default_env(const char *s)
 {
+	/* By default, do not apply changes as they will eventually
+	 * be applied by someone else */
+	apply_cb apply_function = NULL;
 	if (sizeof(default_environment) > ENV_SIZE) {
 		puts("*** Error - default environment is too large\n\n");
 		return;
@@ -183,14 +186,22 @@ void set_default_env(const char *s)
 				"using default environment\n\n",
 				s+1);
 		} else {
+			/* This set_to_default was explicitly asked for
+			 * by the user, as opposed to being a recovery
+			 * mechanism. Therefore we chack every single
+			 * variable and apply changes to the system
+			 * right away (e.g. baudrate, console).
+			 */
+			apply_function = env_check_apply;
 			puts(s);
 		}
 	} else {
 		puts("Using default environment\n\n");
 	}
 
-	if (himport_r(&env_htab, (char *)default_environment,
-		    sizeof(default_environment), '\0', 0) == 0) {
+	if (himport_ex(&env_htab, (char *)default_environment,
+		    sizeof(default_environment), '\0', 0,
+		      0, NULL, apply_function) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 	}
 	gd->flags |= GD_FLG_ENV_READY;
diff --git a/include/environment.h b/include/environment.h
index 6394a96..cd98f91 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -174,6 +174,13 @@ void set_default_env(const char *s);
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
+/*
+ * Check if variable "name" can be changed from oldval to newval,
+ * and if so, apply the changes (e.g. baudrate)
+ */
+int env_check_apply(const char *name, const char *oldval,
+		    const char *newval, int flag);
+
 #endif
 
 #endif	/* _ENVIRONMENT_H_ */
diff --git a/include/search.h b/include/search.h
index b4edd43..4ac7171 100644
--- a/include/search.h
+++ b/include/search.h
@@ -46,6 +46,11 @@ typedef struct entry {
 /* Opaque type for internal use.  */
 struct _ENTRY;
 
+/* Callback function to be called for checking whether the given change may
+ * be applied or not. Must return 0 for approval, 1 for denial. */
+typedef int (*apply_cb)(const char *name, const char *oldval,
+			const char *newval, int flag);
+
 /*
  * Family of hash table handling functions.  The functions also
  * have reentrant counterparts ending with _r.  The non-reentrant
@@ -97,7 +102,15 @@ extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
 		     int __flag);
 
+extern int himport_ex(struct hsearch_data *__htab,
+		     const char *__env, size_t __size, const char __sep,
+		     int __flag,
+		     int __argc, char * const __argv[],
+		     apply_cb apply);
+
+
 /* Flags for himport_r() */
 #define	H_NOCLEAR	1	/* do not clear hash table before importing */
+#define H_FORCE		2	/* overwrite read-only/write-once variables */
 
 #endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 6895550..a2c268f 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -630,6 +630,33 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep,
 int himport_r(struct hsearch_data *htab,
 	      const char *env, size_t size, const char sep, int flag)
 {
+	return himport_ex(htab, env, size, sep, flag, 0, NULL, NULL);
+}
+
+/* Check whether variable name is amongst vars[] */
+static int process_var(const char *name, int nvars, char * const vars[])
+{
+	int i = 0;
+	/* No variables specified means process all of them*/
+	if (nvars == 0)
+		return 1;
+
+	for (i = 0; i < nvars; i++) {
+		if (!strcmp(name, vars[i]))
+			return 1;
+	}
+	debug("Skipping non-listed variable %s\n", name);
+	return 0;
+}
+
+
+
+int himport_ex(struct hsearch_data *htab,
+	      const char *env, size_t size, const char sep, int flag,
+	      int nvars, char * const vars[],
+	      int(*apply)(const char *, const char *, const char *, int)
+	      )
+{
 	char *data, *sp, *dp, *name, *value;
 
 	/* Test for correct arguments.  */
@@ -715,6 +742,8 @@ int himport_r(struct hsearch_data *htab,
 			*dp++ = '\0';	/* terminate name */
 
 			debug("DELETE CANDIDATE: \"%s\"\n", name);
+			if (!process_var(name, nvars, vars))
+				continue;
 
 			if (hdelete_r(name, htab) == 0)
 				debug("DELETE ERROR ##############################\n");
@@ -732,10 +761,31 @@ int himport_r(struct hsearch_data *htab,
 		*sp++ = '\0';	/* terminate value */
 		++dp;
 
+		/* Skip variables which are not supposed to be treated */
+		if (!process_var(name, nvars, vars))
+			continue;
+
 		/* enter into hash table */
 		e.key = name;
 		e.data = value;
 
+		/* if there is an apply function, check what it has to say */
+		if (apply != NULL) {
+			debug("searching before calling cb function"
+			      " for  %s\n", name);
+			/*
+			 * Search for variable in existing env, so to pass
+			 * its previous value to the apply callback
+			 */
+			hsearch_r(e, FIND, &rv, htab);
+			debug("previous value was %s\n", rv ? rv->data : "");
+			if (apply(name, rv ? rv->data : NULL, value, flag)) {
+				debug("callback function refused to set"
+				      " variable %s, skipping it!\n", name);
+				continue;
+			}
+		}
+
 		hsearch_r(e, ENTER, &rv, htab);
 		if (rv == NULL) {
 			printf("himport_r: can't insert \"%s=%s\" into hash table\n",
-- 
1.7.1

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-2-git-send-email-gerlando.falauto@keymile.com> you wrote:
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c   |  152 +++++++++++++++++++++++++++++++------------------
>  common/env_common.c   |   15 ++++-
>  include/environment.h |    7 ++
>  include/search.h      |   13 ++++
>  lib/hashtable.c       |   50 ++++++++++++++++
>  5 files changed, 179 insertions(+), 58 deletions(-)
...
> -int _do_env_set (int flag, int argc, char * const argv[])
> +int env_check_apply(const char *name, const char *oldval,
> +		    const char *newval, int flag)

Please use only TAB for indentation.  Please fix globally.


> -	if (ep) {		/* variable exists */
> +	if ((oldval != NULL) /* variable exists */
> +	 && ((flag & H_FORCE) == 0)) { /* and we are not forced */

Incorrect indentation.


> +/*
> + * Set a new environment variable,
> + * or replace or delete an existing one.
> +*/

Incorrect multiline comment style.

> +	/* Perform requested checks. Notice how since we are overwriting
> +	 * a single variable, we need to set H_NOCLEAR */

Incorrect multiline comment style.

>  void set_default_env(const char *s)
>  {
> +	/* By default, do not apply changes as they will eventually
> +	 * be applied by someone else */

Incorrect multiline comment style.

Please fix globally!

> +	if (himport_ex(&env_htab, (char *)default_environment,
> +		    sizeof(default_environment), '\0', 0,
> +		      0, NULL, apply_function) == 0) {

Incorrect / inconsistent indentation.  Please fix globally.


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
Applying computer technology is simply finding the  right  wrench  to
pound in the correct screw.

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Gerlando Falauto]

On 11/05/2011 05:09 PM, Wolfgang Denk wrote:

[NOTE: I removed the quoting from the hunks as it would not make any sense]

-int _do_env_set (int flag, int argc, char * const argv[])
+int env_check_apply(const char *name, const char *oldval,
+		    const char *newval, int flag)

> Please use only TAB for indentation.  Please fix globally.

 From fs/ubibfs/ubifs.h:

int ubifs_decompress(const void *buf, int len, void *out, int *out_len,
		     int compr_type);

Here there are 2 tabs + 5 spaces to align arguments from the first line 
with those on the second line. I did the same (except that in a patch 
hunk the plus sign before the first tab shifts it to the right).

Could you please provide some examples as to what would be the correct 
coding style for function declarations and/or function calls that spawn 
on multiple lines? I could not find anything on the topic.

As for:

-	if (ep) {		/* variable exists */
+	if ((oldval != NULL) /* variable exists */
+	 && ((flag & H_FORCE) == 0)) { /* and we are not forced */

Ditto. Here I was trying to align parentheses. Could you please provide 
an example? In my opinion, using only tabs would definitely make the two 
lines not aligned and make the code much harder to read.

As for:

+	if (himport_ex(&env_htab, (char *)default_environment,
+		    sizeof(default_environment), '\0', 0,
+		      0, NULL, apply_function) == 0) {

What should be the right indentation?

Perhaps for function calls you would want something like:

+	if (himport_ex(&env_htab, (char *)default_environment,
+		sizeof(default_environment), '\0', 0,
+		0, NULL, apply_function) == 0) {

As soon as I have these questions answered, I will apply all the other 
changes you requested, and submit a new patchset.
I would very much appreciate some feedback about the idea though.

Thank you,
Gerlando Falauto

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <4EB84859.6000906@keymile.com> you wrote:
> 
> -int _do_env_set (int flag, int argc, char * const argv[])
> +int env_check_apply(const char *name, const char *oldval,
> +		    const char *newval, int flag)
> 
> > Please use only TAB for indentation.  Please fix globally.
> 
>  From fs/ubibfs/ubifs.h:

Never ever use examples from other code to argument your's was right -
the example you chose might be wrong as well.

> Could you please provide some examples as to what would be the correct 
> coding style for function declarations and/or function calls that spawn 
> on multiple lines? I could not find anything on the topic.

http://www.denx.de/wiki/U-Boot/CodingStyle:

	Use TAB characters for indentation and vertical alignment, not
	spaces 

> +	if (himport_ex(&env_htab, (char *)default_environment,
> +		    sizeof(default_environment), '\0', 0,
> +		      0, NULL, apply_function) == 0) {
> 
> What should be the right indentation?

In any case it makse no sense to have the 2nd and 3rd line indented
differently, right?

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
"A child is a person who can't understand why someone would give away
a perfectly good kitten."                               - Doug Larson

[U-Boot] Continuation line alignment

[Scott Wood]

On 11/07/2011 04:05 PM, Wolfgang Denk wrote:
> Dear Gerlando Falauto,
> 
> In message <4EB84859.6000906@keymile.com> you wrote:
>>
>> -int _do_env_set (int flag, int argc, char * const argv[])
>> +int env_check_apply(const char *name, const char *oldval,
>> +		    const char *newval, int flag)
>>
>>> Please use only TAB for indentation.  Please fix globally.
>>
>>  From fs/ubibfs/ubifs.h:
> 
> Never ever use examples from other code to argument your's was right -
> the example you chose might be wrong as well.
> 
>> Could you please provide some examples as to what would be the correct 
>> coding style for function declarations and/or function calls that spawn 
>> on multiple lines? I could not find anything on the topic.
> 
> http://www.denx.de/wiki/U-Boot/CodingStyle:
> 
> 	Use TAB characters for indentation and vertical alignment, not
> 	spaces 

It is impossible to align this nicely with tabs alone.  Such alignment
is not just an isolated example, but quite common in both the Linux
kernel and U-Boot.

Grep for a tab followed by a space...

>> +	if (himport_ex(&env_htab, (char *)default_environment,
>> +		    sizeof(default_environment), '\0', 0,
>> +		      0, NULL, apply_function) == 0) {
>>
>> What should be the right indentation?
> 
> In any case it makse no sense to have the 2nd and 3rd line indented
> differently, right?

They generally shouldn't be different from each other, but that doesn't
answer the question of what it should look like.

Documentation/CodingStyle calls for something like this:

	if (himport_ex(&env_htab, (char *)default_environment,
				sizeof(default_environment), '\0',
				0, 0, NULL, apply_function) == 0) {

...but judging by how common it is, many people find this nicer:

	if (himport_ex(&env_htab, (char *)default_environment,
		       sizeof(default_environment), '\0',
		       0, 0, NULL, apply_function) == 0) {

I vote for allowing it, if anyone's counting.

-Scott

[U-Boot] Continuation line alignment

[Gerlando Falauto]

On 11/08/2011 12:02 AM, Scott Wood wrote:
> On 11/07/2011 04:05 PM, Wolfgang Denk wrote:
>> Dear Gerlando Falauto,
>>
>> In message<4EB84859.6000906@keymile.com>  you wrote:
>>>
>>> -int _do_env_set (int flag, int argc, char * const argv[])
>>> +int env_check_apply(const char *name, const char *oldval,
>>> +		    const char *newval, int flag)
>>>
>>>> Please use only TAB for indentation.  Please fix globally.
>>>
>>>    From fs/ubibfs/ubifs.h:
>>
>> Never ever use examples from other code to argument your's was right -
>> the example you chose might be wrong as well.
>>
>>> Could you please provide some examples as to what would be the correct
>>> coding style for function declarations and/or function calls that spawn
>>> on multiple lines? I could not find anything on the topic.
>>
>> http://www.denx.de/wiki/U-Boot/CodingStyle:
>>
>> 	Use TAB characters for indentation and vertical alignment, not
>> 	spaces
>
> It is impossible to align this nicely with tabs alone.  Such alignment
> is not just an isolated example, but quite common in both the Linux
> kernel and U-Boot.
>
> Grep for a tab followed by a space...
>
>>> +	if (himport_ex(&env_htab, (char *)default_environment,
>>> +		    sizeof(default_environment), '\0', 0,
>>> +		      0, NULL, apply_function) == 0) {
>>>
>>> What should be the right indentation?
>>
>> In any case it makse no sense to have the 2nd and 3rd line indented
>> differently, right?
>
> They generally shouldn't be different from each other, but that doesn't
> answer the question of what it should look like.

Thank you! :-)

> Documentation/CodingStyle calls for something like this:
>
> 	if (himport_ex(&env_htab, (char *)default_environment,
> 				sizeof(default_environment), '\0',
> 				0, 0, NULL, apply_function) == 0) {

I would have thought this would make more sense (offset by 1, not 9):

	if (himport_ex(&env_htab, (char *)default_environment,
			sizeof(default_environment), '\0',
			0, 0, NULL, apply_function) == 0) {

Or maybe even:

	if (himport_ex(&env_htab, (char *)default_environment,
		sizeof(default_environment), '\0',
		0, 0, NULL, apply_function) == 0) {

which would save a lot of screen real estate.
Now we can also argue that the argument list is really too big...

 > ...but judging by how common it is, many people find this nicer:
>
> 	if (himport_ex(&env_htab, (char *)default_environment,
> 		       sizeof(default_environment), '\0',
> 		       0, 0, NULL, apply_function) == 0) {

Actually I don't have that much of a preference in this case, it's just 
a matter of aesthetics, and I think it doesn't affect readabilty that 
much. As long as we agree on what is OK and what is not (or, rather, 
what is the preferred way).

What bothers me more is, for instance, the condition under which my 
smartphone will work correctly:

	if (((day_of_week() % 2 == 0) &&
	     (temperature() < 14.4 || temperature() > 15.3))
           || ((sky_color() == E_BLUE) && (sim_credit() % 100 != 27))
	  || (uptime() < 3600) ) {
		work = 1;
	} else if (  ((received_calls() > 1) &&
		      (zenith_angle() == 0))
                   || (call_is_important())
		  ) {
		work = 0;
	} else {
		udelay(rand());
		work = ((rand() % 2) == 1);
	}

Any ideas how to align that? Please don't answer: get a real phone. :-)

Thanks!
Gerlando Falauto

[U-Boot] Continuation line alignment

[Scott Wood]

On 11/07/2011 05:32 PM, Gerlando Falauto wrote:
> On 11/08/2011 12:02 AM, Scott Wood wrote:
>> Documentation/CodingStyle calls for something like this:
>>
>> 	if (himport_ex(&env_htab, (char *)default_environment,
>> 				sizeof(default_environment), '\0',
>> 				0, 0, NULL, apply_function) == 0) {
> 
> I would have thought this would make more sense (offset by 1, not 9):
> 
> 	if (himport_ex(&env_htab, (char *)default_environment,
> 			sizeof(default_environment), '\0',
> 			0, 0, NULL, apply_function) == 0) {

Documentation/CodingStyle simply says continuation lines "are always
substantially shorter than the parent and are placed substantially to
the right".

> Or maybe even:
> 
> 	if (himport_ex(&env_htab, (char *)default_environment,
> 		sizeof(default_environment), '\0',
> 		0, 0, NULL, apply_function) == 0) {
> 
> which would save a lot of screen real estate.

It would also make it harder to distinguish the continuation lines from
the if-body:

	if (himport_ex(...
		sizeof(...
		0, 0, ...) {
		body goes here, at the same column
		have to look for the brace to distinguish
	}

-Scott

[U-Boot] Continuation line alignment

[Scott Wood]

On 11/07/2011 05:32 PM, Gerlando Falauto wrote:
> What bothers me more is, for instance, the condition under which my 
> smartphone will work correctly:
> 
> 	if (((day_of_week() % 2 == 0) &&
> 	     (temperature() < 14.4 || temperature() > 15.3))
>            || ((sky_color() == E_BLUE) && (sim_credit() % 100 != 27))
> 	  || (uptime() < 3600) ) {
> 		work = 1;
> 	} else if (  ((received_calls() > 1) &&
> 		      (zenith_angle() == 0))
>                    || (call_is_important())
> 		  ) {
> 		work = 0;
> 	} else {
> 		udelay(rand());
> 		work = ((rand() % 2) == 1);
> 	}

I like aligning based on which level of nested parens the line break is
in (and removing unnecessary parens when precedence is obvious, to make
it easier to track the relevant ones):

	if ((day_of_week() % 2 == 0 &&
	     (temperature() < 14.4 || temperature() > 15.3)) ||
	    (sky_color() == E_BLUE && sim_credit() % 100 != 27) ||
	    uptime() < 3600) {
		work = 1;
	} else if ((received_calls() > 1 &&
		    zenith_angle() == 0) ||
		   call_is_important()) {
		work = 0;
	} else {
		udelay(rand());
		work = ((rand() % 2) == 1);
	}

-Scott

[U-Boot] Continuation line alignment

[Gerlando Falauto]

On 11/08/2011 12:58 AM, Scott Wood wrote:
 > I like aligning based on which level of nested parens the line break is
 > in (and removing unnecessary parens when precedence is obvious, to make
 > it easier to track the relevant ones):
 >
 > 	if ((day_of_week() % 2 == 0&&
 > 	(temperature()<  14.4 || temperature()>  15.3)) ||
 > 	    (sky_color() == E_BLUE&&  sim_credit() % 100 != 27) ||
 > 	    uptime()<  3600) {
 > 		work = 1;
 > 	} else if ((received_calls()>  1&&
 > 		zenith_angle() == 0) ||
 > 		   call_is_important()) {
 > 		work = 0;
 > 	} else {
 > 		udelay(rand());
 > 		work = ((rand() % 2) == 1);
 > 	}

I like that too.

Anyway... Are there are any guidelines for indenting comments on the 
right side of the code? Like:

	if ((day_of_week() % 2 == 0 &&		/* even days are OK */
	     (temperature() < 14.4 || temperature() > 15.3)) ||
						/*
						 * but only outside of a
						 * certain temp. range
						 */
	    (sky_color() == E_BLUE && sim_credit() % 100 != 27) ||
						/*
						 * or, it's a nice a day
						 * but balance does not
						 * have 27 cents as
						 * decimal part
						 */
	    uptime() < 3600) {			/* work 4 the 1st hr! */
		work = 1;
	} else if ((received_calls() > 1 &&
		    zenith_angle() == 0) ||
		   call_is_important()) {
		work = 0;
	} else {
		udelay(rand());
		work = ((rand() % 2) == 1);
	}

... becuase git-gui will interpret all tabs which are on the right of 
some non-whitespace text in a very weird way.
Is using tabs on the right of text forbidden/not recommended for some 
reason?

Thanks!
Gerlando Falauto

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Gerlando Falauto]

On 11/07/2011 11:05 PM, Wolfgang Denk wrote:
> Dear Gerlando Falauto,
>
> In message<4EB84859.6000906@keymile.com>  you wrote:
>>
>> -int _do_env_set (int flag, int argc, char * const argv[])
>> +int env_check_apply(const char *name, const char *oldval,
>> +		    const char *newval, int flag)
>>
>>> Please use only TAB for indentation.  Please fix globally.
>>
>>    From fs/ubibfs/ubifs.h:
>
> Never ever use examples from other code to argument your's was right -
> the example you chose might be wrong as well.

The purpose was not to argument mine was right.

>> Could you please provide some examples as to what would be the correct
>> coding style for function declarations and/or function calls that spawn
>> on multiple lines? I could not find anything on the topic.
>
> http://www.denx.de/wiki/U-Boot/CodingStyle:
>
> 	Use TAB characters for indentation and vertical alignment, not
> 	spaces

That's exactly what you told me in your reply, and doesn't answer my 
question.
The only way I could think of to achieve vertical alignment in a complex 
if statement without recurring to spaces is by adding extra tabs between 
parentheses, with an enormous waste of space.
Your answer might as well be: "forget about alignment altogether, nobody 
wants that, just indent it somehow".

>> +	if (himport_ex(&env_htab, (char *)default_environment,
>> +		    sizeof(default_environment), '\0', 0,
>> +		      0, NULL, apply_function) == 0) {
>>
>> What should be the right indentation?
>
> In any case it makse no sense to have the 2nd and 3rd line indented
> differently, right?

That's absolutely right.
Once again, though, you did not help me understand what The Right Thing 
(tm) is. I also made a shy attempt, but you're not telling me whether 
it's good or not.
It's hard to follow some guidelines when they're not clearly stated.

Thank you,
Gerlando Falauto

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <4EB86424.7000508@keymile.com> you wrote:
>
> > http://www.denx.de/wiki/U-Boot/CodingStyle:
> >
> > 	Use TAB characters for indentation and vertical alignment, not
> > 	spaces
> 
> That's exactly what you told me in your reply, and doesn't answer my 
> question.

Sory, but I don;t know how else to put it.

> The only way I could think of to achieve vertical alignment in a complex 
> if statement without recurring to spaces is by adding extra tabs between 
> parentheses, with an enormous waste of space.

In the first step you should try and avoid complex if statements.

> Your answer might as well be: "forget about alignment altogether, nobody 
> wants that, just indent it somehow".
> 
> >> +	if (himport_ex(&env_htab, (char *)default_environment,
> >> +		    sizeof(default_environment), '\0', 0,
> >> +		      0, NULL, apply_function) == 0) {
> >>
> >> What should be the right indentation?
> >
> > In any case it makse no sense to have the 2nd and 3rd line indented
> > differently, right?
> 
> That's absolutely right.
> Once again, though, you did not help me understand what The Right Thing 
> (tm) is. I also made a shy attempt, but you're not telling me whether 
> it's good or not.
> It's hard to follow some guidelines when they're not clearly stated.

Well, my suggestion is to align by TABs:

	if (himport_ex(&env_htab, (char *)default_environment,
			sizeof(default_environment), '\0', 0,
			0, NULL, apply_function) == 0) {
		...
	}

Yes, the 's' and the '0' don't start exactly below the '&'.  But who
says they should?  We also don't align the closing ')' below the
opeing '(' ...

And does above code look difficult to read? 

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 optimum committee has no members.
                                                   - Norman Augustine

[U-Boot] [PATCH v0 1/4] Groundwork for generalization of env interface

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-2-git-send-email-gerlando.falauto@keymile.com> you wrote:
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c   |  152 +++++++++++++++++++++++++++++++------------------
>  common/env_common.c   |   15 ++++-
>  include/environment.h |    7 ++
>  include/search.h      |   13 ++++
>  lib/hashtable.c       |   50 ++++++++++++++++
>  5 files changed, 179 insertions(+), 58 deletions(-)

This patch introduces new build warnings:

cmd_nvedit.c: In function 'env_check_apply':
cmd_nvedit.c:242:3: warning: passing argument 1 of 'serial_assign'
discards 'const' qualifier from pointer target type [enabled by
default]
/home/wd/git/u-boot/env-substitute/include/serial.h:95:12: note:
expected 'char *' but argument is of type 'const char *'

Please fix.

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
A year spent in artificial intelligence is enough to make one believe
in God.

[U-Boot] [PATCH v0 2/4] env: check and apply changes on delete/destroy

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c |    2 +-
 include/search.h    |    6 ++++--
 lib/hashtable.c     |   18 ++++++++++++------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 10b9552..905d3be 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -369,7 +369,7 @@ int _do_env_set(int flag, int argc, char * const argv[])
 
 	/* Delete only ? */
 	if ((argc < 3) || argv[2] == NULL) {
-		int rc = hdelete_r(name, &env_htab);
+		int rc = hdelete_r(name, &env_htab, NULL);
 		return !rc;
 	}
 
diff --git a/include/search.h b/include/search.h
index 4ac7171..cc620bd 100644
--- a/include/search.h
+++ b/include/search.h
@@ -68,7 +68,8 @@ struct hsearch_data {
 extern int hcreate_r(size_t __nel, struct hsearch_data *__htab);
 
 /* Destroy current internal hashing table.  */
-extern void hdestroy_r(struct hsearch_data *__htab);
+extern void hdestroy_r(struct hsearch_data *__htab,
+		       apply_cb apply);
 
 /*
  * Search for entry matching ITEM.key in internal hash table.  If
@@ -93,7 +94,8 @@ extern int hstrstr_r(const char *__match, int __last_idx, ENTRY ** __retval,
 		    struct hsearch_data *__htab);
 
 /* Search and delete entry matching ITEM.key in internal hash table. */
-extern int hdelete_r(const char *__key, struct hsearch_data *__htab);
+extern int hdelete_r(const char *__key, struct hsearch_data *__htab,
+		     apply_cb apply);
 
 extern ssize_t hexport_r(struct hsearch_data *__htab,
 		     const char __sep, char **__resp, size_t __size);
diff --git a/lib/hashtable.c b/lib/hashtable.c
index a2c268f..d84882f 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -142,7 +142,8 @@ int hcreate_r(size_t nel, struct hsearch_data *htab)
  * be freed and the local static variable can be marked as not used.
  */
 
-void hdestroy_r(struct hsearch_data *htab)
+void hdestroy_r(struct hsearch_data *htab,
+		int(*apply)(const char *, const char *, const char *, int))
 {
 	int i;
 
@@ -156,7 +157,10 @@ void hdestroy_r(struct hsearch_data *htab)
 	for (i = 1; i <= htab->size; ++i) {
 		if (htab->table[i].used > 0) {
 			ENTRY *ep = &htab->table[i].entry;
-
+			if (apply != NULL) {
+				/* deletion is always forced */
+				apply(ep->key, ep->data, NULL, H_FORCE);
+			}
 			free((void *)ep->key);
 			free(ep->data);
 		}
@@ -401,7 +405,8 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
  * do that.
  */
 
-int hdelete_r(const char *key, struct hsearch_data *htab)
+int hdelete_r(const char *key, struct hsearch_data *htab,
+	      int(*apply)(const char *, const char *, const char *, int))
 {
 	ENTRY e, *ep;
 	int idx;
@@ -417,7 +422,8 @@ int hdelete_r(const char *key, struct hsearch_data *htab)
 
 	/* free used ENTRY */
 	debug("hdelete: DELETING key \"%s\"\n", key);
-
+	if (apply != NULL)
+		apply(ep->key, ep->data, NULL, H_FORCE);
 	free((void *)ep->key);
 	free(ep->data);
 	htab->table[idx].used = -1;
@@ -679,7 +685,7 @@ int himport_ex(struct hsearch_data *htab,
 		debug("Destroy Hash Table: %p table = %p\n", htab,
 		       htab->table);
 		if (htab->table)
-			hdestroy_r(htab);
+			hdestroy_r(htab, apply);
 	}
 
 	/*
@@ -745,7 +751,7 @@ int himport_ex(struct hsearch_data *htab,
 			if (!process_var(name, nvars, vars))
 				continue;
 
-			if (hdelete_r(name, htab) == 0)
+			if (hdelete_r(name, htab, apply) == 0)
 				debug("DELETE ERROR ##############################\n");
 
 			continue;
-- 
1.7.1

[U-Boot] [PATCH v0 2/4] env: check and apply changes on delete/destroy

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-3-git-send-email-gerlando.falauto@keymile.com> you wrote:
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c |    2 +-
>  include/search.h    |    6 ++++--
>  lib/hashtable.c     |   18 ++++++++++++------
>  3 files changed, 17 insertions(+), 9 deletions(-)

Are you sure these changes are really independent from the previous
and following ones?  Ordo we introduc bisectability issues here?


> -extern void hdestroy_r(struct hsearch_data *__htab);
> +extern void hdestroy_r(struct hsearch_data *__htab,
> +		       apply_cb apply);

See provious comments - indentation always by TAB. Please fix
globally, i. e. in all your patches.


> +			if (apply != NULL) {
> +				/* deletion is always forced */

Um.... why is this the case?

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
>  Is there a way to determine Yesterday's date using Unix utilities?
         echo "what is yesterday's date?" | /bin/mail root
         -- Randal L. Schwartz in <ukbuh2y982.fsf@julie.teleport.com>

[U-Boot] [PATCH v0 3/4] env: implement selective "env default"

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 README                   |    2 ++
 common/cmd_nvedit.c      |   42 ++++++++++++++++++++++++++++++++++++------
 common/env_common.c      |   14 ++++++++++++++
 include/config_cmd_all.h |    1 +
 include/environment.h    |    5 +++++
 5 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/README b/README
index c6b179c..b5b316f 100644
--- a/README
+++ b/README
@@ -724,6 +724,8 @@ The following options need to be configured:
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...
+		CONFIG_CMD_DEFAULTENV_VARS
+					* Reset individual variables to default
 		CONFIG_CMD_DHCP		* DHCP support
 		CONFIG_CMD_DIAG		* Diagnostics
 		CONFIG_CMD_DS4510	* ds4510 I2C gpio commands
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 905d3be..07cd062 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -638,13 +638,40 @@ int envmatch(uchar *s1, int i2)
 	return -1;
 }
 
-static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_env_default(cmd_tbl_t *cmdtp, int __flag, int argc,
+			  char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
-		return cmd_usage(cmdtp);
-
-	set_default_env("## Resetting to default environment\n");
-	return 0;
+	int all = 0, flag = 0;
+	debug("Initial value for argc=%d\n", argc);
+	while (--argc > 0 && **++argv == '-') {
+		char *arg = *argv;
+		while (*++arg) {
+			switch (*arg) {
+			case 'a':		/* default all */
+				all = 1;
+				break;
+			case 'f':		/* force */
+				flag |= H_FORCE;
+				break;
+			default:
+				return cmd_usage(cmdtp);
+			}
+		}
+	}
+	debug("Final value for argc=%d\n", argc);
+	if (all && (argc == 0)) {
+		/* Reset the whole environment */
+		set_default_env("## Resetting to default environment\n");
+		return 0;
+	}
+#if CONFIG_CMD_DEFAULTENV_VARS
+	if (!all && (argc > 0)) {
+		/* Reset individual variables */
+		env_default_vars(argc, argv);
+		return 0;
+	}
+#endif
+	return cmd_usage(cmdtp);
 }
 
 static int do_env_delete(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
@@ -970,6 +997,9 @@ U_BOOT_CMD(
 	"ask name [message] [size] - ask for environment variable\nenv "
 #endif
 	"default -f - reset default environment\n"
+#if defined(CONFIG_CMD_DEFAULTENV_VARS)
+	"env default var [...] - reset variable(s) to their default value\n"
+#endif
 #if defined(CONFIG_CMD_EDITENV)
 	"env edit name - edit environment variable\n"
 #endif
diff --git a/common/env_common.c b/common/env_common.c
index db607e1..af28d47 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -207,6 +207,20 @@ void set_default_env(const char *s)
 	gd->flags |= GD_FLG_ENV_READY;
 }
 
+#ifdef CONFIG_CMD_DEFAULTENV_VARS
+
+/* [re]set individual variables to their value in the default environment */
+int env_default_vars(int nvars, char * const vars[])
+{
+	/* Special use-case: import from default environment
+	   (and use \0 as a separator) */
+	return himport_ex(&env_htab, (const char *)default_environment,
+			  sizeof(default_environment), '\0', H_NOCLEAR,
+			  nvars, vars, env_check_apply);
+}
+
+#endif /* CONFIG_CMD_DEFAULTENV_VARS */
+
 /*
  * Check if CRC is valid and (if yes) import the environment.
  * Note that "buf" may or may not be aligned.
diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
index 9716f9c..e728eae 100644
--- a/include/config_cmd_all.h
+++ b/include/config_cmd_all.h
@@ -25,6 +25,7 @@
 #define CONFIG_CMD_CDP		/* Cisco Discovery Protocol	*/
 #define CONFIG_CMD_CONSOLE	/* coninfo			*/
 #define CONFIG_CMD_DATE		/* support for RTC, date/time...*/
+#define CONFIG_CMD_DEFAULTENV_VARS /* default individ variables */
 #define CONFIG_CMD_DHCP		/* DHCP Support			*/
 #define CONFIG_CMD_DIAG		/* Diagnostics			*/
 #define CONFIG_CMD_DISPLAY	/* Display support		*/
diff --git a/include/environment.h b/include/environment.h
index cd98f91..b9201d3 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -171,6 +171,11 @@ void env_crc_update (void);
 /* [re]set to the default environment */
 void set_default_env(const char *s);
 
+#ifdef CONFIG_CMD_DEFAULTENV_VARS
+/* [re]set individual variables to their value in the default environment */
+int env_default_vars(int nvars, char * const vars[]);
+#endif
+
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
-- 
1.7.1

[U-Boot] [PATCH v0 3/4] env: implement selective "env default"

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-4-git-send-email-gerlando.falauto@keymile.com> you wrote:
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  README                   |    2 ++
>  common/cmd_nvedit.c      |   42 ++++++++++++++++++++++++++++++++++++------
>  common/env_common.c      |   14 ++++++++++++++
>  include/config_cmd_all.h |    1 +
>  include/environment.h    |    5 +++++
>  5 files changed, 58 insertions(+), 6 deletions(-)

Did you actually ever try to compile your code, and test it?

...
> +#if CONFIG_CMD_DEFAULTENV_VARS

When I try and enable your code by adding

	#define CONFIG_CMD_DEFAULTENV_VARS

to a board config file, this will result in a an error:

cmd_nvedit.c:699:31: error: #if with no expression


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
Copy from one, it's plagiarism; copy from two, it's research.

[U-Boot] [PATCH v0 4/4] env: implement "env import -n var[, var...]"

[Gerlando Falauto]

Implemented selective importing of variables in
env import

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c |   35 ++++++++++++++++++++++++++++++-----
 1 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 07cd062..0984a53 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -831,15 +831,19 @@ sep_err:
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
  */
-static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+#define MAX_NVARS 16
+static int do_env_import(cmd_tbl_t *cmdtp, int __flag, int argc,
+			 char * const argv[])
 {
 	char	*cmd, *addr;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
-	int	del = 0;
+	int	flag = H_NOCLEAR;
 	size_t	size;
-
+	char	*vars[MAX_NVARS];
+	int	nvars = 0;
+	char	*s;
 	cmd = *argv;
 
 	while (--argc > 0 && **++argv == '-') {
@@ -863,7 +867,27 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 				sep = '\n';
 				break;
 			case 'd':
-				del = 1;
+				flag &= ~H_NOCLEAR;
+				break;
+			case 'f':
+				flag |= H_FORCE;
+				break;
+			case 'n':
+				s = *++argv;
+				argc--;
+				do {
+					if (nvars >= MAX_NVARS) {
+						printf("Cannot import more "
+						       "than %d variables at a"
+						       " time!", MAX_NVARS);
+						return 1;
+					}
+					vars[nvars++] = strsep(&s, ",");
+					debug("Considering variable %s\n",
+					      vars[nvars-1]);
+				} while (s);
+				debug("Importing a total of %d variables\n",
+				      nvars);
 				break;
 			default:
 				return cmd_usage(cmdtp);
@@ -914,7 +938,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		addr = (char *)ep->data;
 	}
 
-	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) == 0) {
+	if (himport_ex(&env_htab, addr, size, sep, flag,
+		       nvars, vars, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
-- 
1.7.1

[U-Boot] [PATCH] env: fix "env ask" command

[Wolfgang Denk]

The "env ask" traditionally uses a somewhat awkward syntax:

	env ask name [message ...] [size]

So far, when a mesage was given, you always also had to enter a size.
If you forgot to do that, the command would terminate without any
indication of the problem.

To avoid incompatible changes of the interface, we now check the last
argument if it can be converted into a decimal number.  If this is the
case, we assume it is a size; otherwise we treat it as part of the
message.

Also, add a space after the message fore easier reading.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
Note: this patch was generated on top of Gerlando Falauto's
"generalization of env interface" patch series.  THis should be
applied before.

 common/cmd_nvedit.c |   46 ++++++++++++++++++++++++++++------------------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 5507922..2dd4eba 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -465,40 +465,50 @@ int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	extern char console_buffer[CONFIG_SYS_CBSIZE];
 	char message[CONFIG_SYS_CBSIZE];
-	int size = CONFIG_SYS_CBSIZE - 1;
-	int i, len, pos;
+	int i, len, pos, size;
 	char *local_args[4];
+	char *endptr;
 
 	local_args[0] = argv[0];
 	local_args[1] = argv[1];
 	local_args[2] = NULL;
 	local_args[3] = NULL;
 
-	/* Check the syntax */
-	switch (argc) {
-	case 1:
+	/*
+	 * Check the syntax:
+	 *
+	 * env_ask envname [message1 ...] [size]
+	 */
+	if (argc == 1)
 		return cmd_usage(cmdtp);
 
-	case 2:		/* env_ask envname */
-		sprintf(message, "Please enter '%s':", argv[1]);
-		break;
-
-	case 3:		/* env_ask envname size */
-		sprintf(message, "Please enter '%s':", argv[1]);
-		size = simple_strtoul(argv[2], NULL, 10);
-		break;
-
-	default:	/* env_ask envname message1 ... messagen size */
-		for (i = 2, pos = 0; i < argc - 1; i++) {
+	/*
+	 * We test the last argument if it can be converted
+	 * into a decimal number.  If yes, we assume it's
+	 * the size.  Otherwise we echo it as part of the
+	 * message.
+	 */
+	i = simple_strtoul(argv[argc - 1], &endptr, 10);
+	if (*endptr != '\0') {			/* no size */
+		size = CONFIG_SYS_CBSIZE - 1;
+	} else {				/* size given */
+		size = i;
+		--argc;
+	}
+	
+	if (argc <= 2) {
+		sprintf(message, "Please enter '%s': ", argv[1]);
+	} else {
+		/* env_ask envname message1 ... messagen [size] */
+		for (i = 2, pos = 0; i < argc; i++) {
 			if (pos)
 				message[pos++] = ' ';
 
 			strcpy(message+pos, argv[i]);
 			pos += strlen(argv[i]);
 		}
+		message[pos++] = ' ';
 		message[pos] = '\0';
-		size = simple_strtoul(argv[argc - 1], NULL, 10);
-		break;
 	}
 
 	if (size >= CONFIG_SYS_CBSIZE)
-- 
1.7.6.4

[U-Boot] [PATCH 1/2] Add SLRE - Super Light Regular Expression library

[Wolfgang Denk]

Downloaded from http://slre.sourceforge.net/
and adapted for U-Boot environment.

Used to implement regex operations on environment variables.
Code size is ~ 3.5 KiB on PPC.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
Note: This patch was generated on top of my ``env: fix "env ask"
command'' patch. This should be applied before.

 include/slre.h |  100 ++++++++
 lib/slre.c     |  709 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 809 insertions(+), 0 deletions(-)
 create mode 100644 include/slre.h
 create mode 100644 lib/slre.c

diff --git a/include/slre.h b/include/slre.h
new file mode 100644
index 0000000..4b41a4b
--- /dev/null
+++ b/include/slre.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2004-2005 Sergey Lyubka <valenok@gmail.com>
+ * All rights reserved
+ *
+ * "THE BEER-WARE LICENSE" (Revision 42):
+ * Sergey Lyubka wrote this file.  As long as you retain this notice you
+ * can do whatever you want with this stuff. If we meet some day, and you think
+ * this stuff is worth it, you can buy me a beer in return.
+ */
+
+/*
+ * Downloaded Sat Nov  5 17:42:08 CET 2011 at
+ * http://slre.sourceforge.net/1.0/slre.h
+ */
+
+/*
+ * This is a regular expression library that implements a subset of Perl RE.
+ * Please refer to http://slre.sourceforge.net for detailed description.
+ *
+ * Usage example (parsing HTTP request):
+ *
+ * struct slre	slre;
+ * struct cap	captures[4 + 1];  // Number of braket pairs + 1
+ * ...
+ *
+ * slre_compile(&slre,"^(GET|POST) (\S+) HTTP/(\S+?)\r\n");
+ *
+ * if (slre_match(&slre, buf, len, captures)) {
+ *	printf("Request line length: %d\n", captures[0].len);
+ *	printf("Method: %.*s\n", captures[1].len, captures[1].ptr);
+ *	printf("URI: %.*s\n", captures[2].len, captures[2].ptr);
+ * }
+ *
+ * Supported syntax:
+ *	^		Match beginning of a buffer
+ *	$		Match end of a buffer
+ *	()		Grouping and substring capturing
+ *	[...]		Match any character from set
+ *	[^...]		Match any character but ones from set
+ *	\s		Match whitespace
+ *	\S		Match non-whitespace
+ *	\d		Match decimal digit
+ *	\r		Match carriage return
+ *	\n		Match newline
+ *	+		Match one or more times (greedy)
+ *	+?		Match one or more times (non-greedy)
+ *	*		Match zero or more times (greedy)
+ *	*?		Match zero or more times (non-greedy)
+ *	?		Match zero or once
+ *	\xDD		Match byte with hex value 0xDD
+ *	\meta		Match one of the meta character: ^$().[*+?\
+ */
+
+#ifndef SLRE_HEADER_DEFINED
+#define	SLRE_HEADER_DEFINED
+
+/*
+ * Compiled regular expression
+ */
+struct slre {
+	unsigned char	code[256];
+	unsigned char	data[256];
+	int		code_size;
+	int		data_size;
+	int		num_caps;	/* Number of bracket pairs	*/
+	int		anchored;	/* Must match from string start	*/
+	const char	*err_str;	/* Error string			*/
+};
+
+/*
+ * Captured substring
+ */
+struct cap {
+	const char	*ptr;		/* Pointer to the substring	*/
+	int		len;		/* Substring length		*/
+};
+
+/*
+ * Compile regular expression. If success, 1 is returned.
+ * If error, 0 is returned and slre.err_str points to the error message.
+ */
+int slre_compile(struct slre *, const char *re);
+
+/*
+ * Return 1 if match, 0 if no match.
+ * If `captured_substrings' array is not NULL, then it is filled with the
+ * values of captured substrings. captured_substrings[0] element is always
+ * a full matched substring. The round bracket captures start from
+ * captured_substrings[1].
+ * It is assumed that the size of captured_substrings array is enough to
+ * hold all captures. The caller function must make sure it is! So, the
+ * array_size = number_of_round_bracket_pairs + 1
+ */
+int slre_match(const struct slre *, const char *buf, int buf_len,
+	struct cap *captured_substrings);
+
+#ifdef SLRE_TEST
+void slre_dump(const struct slre *r, FILE *fp);
+#endif /* SLRE_TEST */
+#endif /* SLRE_HEADER_DEFINED */
diff --git a/lib/slre.c b/lib/slre.c
new file mode 100644
index 0000000..3069c6d
--- /dev/null
+++ b/lib/slre.c
@@ -0,0 +1,709 @@
+/*
+ * Copyright (c) 2004-2005 Sergey Lyubka <valenok@gmail.com>
+ * All rights reserved
+ *
+ * "THE BEER-WARE LICENSE" (Revision 42):
+ * Sergey Lyubka wrote this file.  As long as you retain this notice you
+ * can do whatever you want with this stuff. If we meet some day, and you think
+ * this stuff is worth it, you can buy me a beer in return.
+ */
+
+/*
+ * Downloaded Sat Nov  5 17:43:06 CET 2011 at
+ * http://slre.sourceforge.net/1.0/slre.c
+ */
+
+#ifdef SLRE_TEST
+#include <stdio.h>
+#include <assert.h>
+#include <ctype.h>
+#include <stdlib.h>
+#include <string.h>
+#else
+#include <common.h>
+#include <linux/ctype.h>
+#endif /* SLRE_TEST */
+
+#include <errno.h>
+
+#include <slre.h>
+
+enum {END, BRANCH, ANY, EXACT, ANYOF, ANYBUT, OPEN, CLOSE, BOL, EOL,
+	STAR, PLUS, STARQ, PLUSQ, QUEST, SPACE, NONSPACE, DIGIT};
+
+#ifdef SLRE_TEST
+static struct {
+	const char	*name;
+	int		narg;
+	const char	*flags;
+} opcodes[] = {
+	{"END",		0, ""},		/* End of code block or program	*/
+	{"BRANCH",	2, "oo"},	/* Alternative operator, "|"	*/
+	{"ANY",		0, ""},		/* Match any character, "."	*/
+	{"EXACT",	2, "d"},	/* Match exact string		*/
+	{"ANYOF",	2, "D"},	/* Match any from set, "[]"	*/
+	{"ANYBUT",	2, "D"},	/* Match any but from set, "[^]"*/
+	{"OPEN ",	1, "i"},	/* Capture start, "("		*/
+	{"CLOSE",	1, "i"},	/* Capture end, ")"		*/
+	{"BOL",		0, ""},		/* Beginning of string, "^"	*/
+	{"EOL",		0, ""},		/* End of string, "$"		*/
+	{"STAR",	1, "o"},	/* Match zero or more times "*"	*/
+	{"PLUS",	1, "o"},	/* Match one or more times, "+"	*/
+	{"STARQ",	1, "o"},	/* Non-greedy STAR,  "*?"	*/
+	{"PLUSQ",	1, "o"},	/* Non-greedy PLUS, "+?"	*/
+	{"QUEST",	1, "o"},	/* Match zero or one time, "?"	*/
+	{"SPACE",	0, ""},		/* Match whitespace, "\s"	*/
+	{"NONSPACE",	0, ""},		/* Match non-space, "\S"	*/
+	{"DIGIT",	0, ""}		/* Match digit, "\d"		*/
+};
+#endif /* SLRE_TEST */
+
+/*
+ * Commands and operands are all unsigned char (1 byte long). All code offsets
+ * are relative to current address, and positive (always point forward). Data
+ * offsets are absolute. Commands with operands:
+ *
+ * BRANCH offset1 offset2
+ *	Try to match the code block that follows the BRANCH instruction
+ *	(code block ends with END). If no match, try to match code block that
+ *	starts at offset1. If either of these match, jump to offset2.
+ *
+ * EXACT data_offset data_length
+ *	Try to match exact string. String is recorded in data section from
+ *	data_offset, and has length data_length.
+ *
+ * OPEN capture_number
+ * CLOSE capture_number
+ *	If the user have passed 'struct cap' array for captures, OPEN
+ *	records the beginning of the matched substring (cap->ptr), CLOSE
+ *	sets the length (cap->len) for respective capture_number.
+ *
+ * STAR code_offset
+ * PLUS code_offset
+ * QUEST code_offset
+ *	*, +, ?, respectively. Try to gobble as much as possible from the
+ *	matched buffer, until code block that follows these instructions
+ *	matches. When the longest possible string is matched,
+ *	jump to code_offset
+ *
+ * STARQ, PLUSQ are non-greedy versions of STAR and PLUS.
+ */
+
+static const char *meta_chars = "|.^$*+?()[\\";
+
+#ifdef SLRE_TEST
+
+static void
+print_character_set(FILE *fp, const unsigned char *p, int len)
+{
+	int	i;
+
+	for (i = 0; i < len; i++) {
+		if (i > 0)
+			(void) fputc(',', fp);
+		if (p[i] == 0) {
+			i++;
+			if (p[i] == 0)
+				(void) fprintf(fp, "\\x%02x", p[i]);
+			else
+				(void) fprintf(fp, "%s", opcodes[p[i]].name);
+		} else if (isprint(p[i])) {
+			(void) fputc(p[i], fp);
+		} else {
+			(void) fprintf(fp, "\\x%02x", p[i]);
+		}
+	}
+}
+
+void
+slre_dump(const struct slre *r, FILE *fp)
+{
+	int	i, j, ch, op, pc;
+
+	for (pc = 0; pc < r->code_size; pc++) {
+
+		op = r->code[pc];
+		(void) fprintf(fp, "%3d %s ", pc, opcodes[op].name);
+
+		for (i = 0; opcodes[op].flags[i] != '\0'; i++)
+			switch (opcodes[op].flags[i]) {
+			case 'i':
+				(void) fprintf(fp, "%d ", r->code[pc + 1]);
+				pc++;
+				break;
+			case 'o':
+				(void) fprintf(fp, "%d ",
+				    pc + r->code[pc + 1] - i);
+				pc++;
+				break;
+			case 'D':
+				print_character_set(fp, r->data +
+				    r->code[pc + 1], r->code[pc + 2]);
+				pc += 2;
+				break;
+			case 'd':
+				(void) fputc('"', fp);
+				for (j = 0; j < r->code[pc + 2]; j++) {
+					ch = r->data[r->code[pc + 1] + j];
+					if (isprint(ch)) {
+						(void) fputc(ch, fp);
+					} else {
+						(void) fprintf(fp,
+							"\\x%02x", ch);
+					}
+				}
+				(void) fputc('"', fp);
+				pc += 2;
+				break;
+			}
+
+		(void) fputc('\n', fp);
+	}
+}
+#endif /* SLRE_TEST */
+
+static void
+set_jump_offset(struct slre *r, int pc, int offset)
+{
+	assert(offset < r->code_size);
+
+	if (r->code_size - offset > 0xff)
+		r->err_str = "Jump offset is too big";
+	else
+		r->code[pc] = (unsigned char) (r->code_size - offset);
+}
+
+static void
+emit(struct slre *r, int code)
+{
+	if (r->code_size >= (int) (sizeof(r->code) / sizeof(r->code[0])))
+		r->err_str = "RE is too long (code overflow)";
+	else
+		r->code[r->code_size++] = (unsigned char) code;
+}
+
+static void
+store_char_in_data(struct slre *r, int ch)
+{
+	if (r->data_size >= (int) sizeof(r->data))
+		r->err_str = "RE is too long (data overflow)";
+	else
+		r->data[r->data_size++] = ch;
+}
+
+static void
+exact(struct slre *r, const char **re)
+{
+	int	old_data_size = r->data_size;
+
+	while (**re != '\0' && (strchr(meta_chars, **re)) == NULL)
+		store_char_in_data(r, *(*re)++);
+
+	emit(r, EXACT);
+	emit(r, old_data_size);
+	emit(r, r->data_size - old_data_size);
+}
+
+static int
+get_escape_char(const char **re)
+{
+	int	res;
+
+	switch (*(*re)++) {
+	case 'n':
+		res = '\n';
+		break;
+	case 'r':
+		res = '\r';
+		break;
+	case 't':
+		res = '\t';
+		break;
+	case '0':
+		res = 0;
+		break;
+	case 'S':
+		res = NONSPACE << 8;
+		break;
+	case 's':
+		res = SPACE << 8;
+		break;
+	case 'd':
+		res = DIGIT << 8;
+		break;
+	default:
+		res = (*re)[-1];
+		break;
+	}
+
+	return res;
+}
+
+static void
+anyof(struct slre *r, const char **re)
+{
+	int	esc, old_data_size = r->data_size, op = ANYOF;
+
+	if (**re == '^') {
+		op = ANYBUT;
+		(*re)++;
+	}
+
+	while (**re != '\0')
+
+		switch (*(*re)++) {
+		case ']':
+			emit(r, op);
+			emit(r, old_data_size);
+			emit(r, r->data_size - old_data_size);
+			return;
+			/* NOTREACHED */
+			break;
+		case '\\':
+			esc = get_escape_char(re);
+			if ((esc & 0xff) == 0) {
+				store_char_in_data(r, 0);
+				store_char_in_data(r, esc >> 8);
+			} else {
+				store_char_in_data(r, esc);
+			}
+			break;
+		default:
+			store_char_in_data(r, (*re)[-1]);
+			break;
+		}
+
+	r->err_str = "No closing ']' bracket";
+}
+
+static void
+relocate(struct slre *r, int begin, int shift)
+{
+	emit(r, END);
+	memmove(r->code + begin + shift, r->code + begin, r->code_size - begin);
+	r->code_size += shift;
+}
+
+static void
+quantifier(struct slre *r, int prev, int op)
+{
+	if (r->code[prev] == EXACT && r->code[prev + 2] > 1) {
+		r->code[prev + 2]--;
+		emit(r, EXACT);
+		emit(r, r->code[prev + 1] + r->code[prev + 2]);
+		emit(r, 1);
+		prev = r->code_size - 3;
+	}
+	relocate(r, prev, 2);
+	r->code[prev] = op;
+	set_jump_offset(r, prev + 1, prev);
+}
+
+static void
+exact_one_char(struct slre *r, int ch)
+{
+	emit(r, EXACT);
+	emit(r, r->data_size);
+	emit(r, 1);
+	store_char_in_data(r, ch);
+}
+
+static void
+fixup_branch(struct slre *r, int fixup)
+{
+	if (fixup > 0) {
+		emit(r, END);
+		set_jump_offset(r, fixup, fixup - 2);
+	}
+}
+
+static void
+compile(struct slre *r, const char **re)
+{
+	int	op, esc, branch_start, last_op, fixup, cap_no, level;
+
+	fixup = 0;
+	level = r->num_caps;
+	branch_start = last_op = r->code_size;
+
+	for (;;)
+		switch (*(*re)++) {
+		case '\0':
+			(*re)--;
+			return;
+			/* NOTREACHED */
+			break;
+		case '^':
+			emit(r, BOL);
+			break;
+		case '$':
+			emit(r, EOL);
+			break;
+		case '.':
+			last_op = r->code_size;
+			emit(r, ANY);
+			break;
+		case '[':
+			last_op = r->code_size;
+			anyof(r, re);
+			break;
+		case '\\':
+			last_op = r->code_size;
+			esc = get_escape_char(re);
+			if (esc & 0xff00)
+				emit(r, esc >> 8);
+			else
+				exact_one_char(r, esc);
+			break;
+		case '(':
+			last_op = r->code_size;
+			cap_no = ++r->num_caps;
+			emit(r, OPEN);
+			emit(r, cap_no);
+
+			compile(r, re);
+			if (*(*re)++ != ')') {
+				r->err_str = "No closing bracket";
+				return;
+			}
+
+			emit(r, CLOSE);
+			emit(r, cap_no);
+			break;
+		case ')':
+			(*re)--;
+			fixup_branch(r, fixup);
+			if (level == 0) {
+				r->err_str = "Unbalanced brackets";
+				return;
+			}
+			return;
+			/* NOTREACHED */
+			break;
+		case '+':
+		case '*':
+			op = (*re)[-1] == '*' ? STAR : PLUS;
+			if (**re == '?') {
+				(*re)++;
+				op = op == STAR ? STARQ : PLUSQ;
+			}
+			quantifier(r, last_op, op);
+			break;
+		case '?':
+			quantifier(r, last_op, QUEST);
+			break;
+		case '|':
+			fixup_branch(r, fixup);
+			relocate(r, branch_start, 3);
+			r->code[branch_start] = BRANCH;
+			set_jump_offset(r, branch_start + 1, branch_start);
+			fixup = branch_start + 2;
+			r->code[fixup] = 0xff;
+			break;
+		default:
+			(*re)--;
+			last_op = r->code_size;
+			exact(r, re);
+			break;
+		}
+}
+
+int
+slre_compile(struct slre *r, const char *re)
+{
+	r->err_str = NULL;
+	r->code_size = r->data_size = r->num_caps = r->anchored = 0;
+
+	if (*re == '^')
+		r->anchored++;
+
+	emit(r, OPEN);	/* This will capture what matches full RE */
+	emit(r, 0);
+
+	while (*re != '\0')
+		compile(r, &re);
+
+	if (r->code[2] == BRANCH)
+		fixup_branch(r, 4);
+
+	emit(r, CLOSE);
+	emit(r, 0);
+	emit(r, END);
+
+	return (r->err_str == NULL ? 1 : 0);
+}
+
+static int match(const struct slre *, int,
+		const char *, int, int *, struct cap *);
+
+static void
+loop_greedy(const struct slre *r, int pc, const char *s, int len, int *ofs)
+{
+	int	saved_offset, matched_offset;
+
+	saved_offset = matched_offset = *ofs;
+
+	while (match(r, pc + 2, s, len, ofs, NULL)) {
+		saved_offset = *ofs;
+		if (match(r, pc + r->code[pc + 1], s, len, ofs, NULL))
+			matched_offset = saved_offset;
+		*ofs = saved_offset;
+	}
+
+	*ofs = matched_offset;
+}
+
+static void
+loop_non_greedy(const struct slre *r, int pc, const char *s, int len, int *ofs)
+{
+	int	saved_offset = *ofs;
+
+	while (match(r, pc + 2, s, len, ofs, NULL)) {
+		saved_offset = *ofs;
+		if (match(r, pc + r->code[pc + 1], s, len, ofs, NULL))
+			break;
+	}
+
+	*ofs = saved_offset;
+}
+
+static int
+is_any_of(const unsigned char *p, int len, const char *s, int *ofs)
+{
+	int	i, ch;
+
+	ch = s[*ofs];
+
+	for (i = 0; i < len; i++)
+		if (p[i] == ch) {
+			(*ofs)++;
+			return 1;
+		}
+
+	return 0;
+}
+
+static int
+is_any_but(const unsigned char *p, int len, const char *s, int *ofs)
+{
+	int	i, ch;
+
+	ch = s[*ofs];
+
+	for (i = 0; i < len; i++) {
+		if (p[i] == ch)
+			return 0;
+	}
+
+	(*ofs)++;
+	return 1;
+}
+
+static int
+match(const struct slre *r, int pc, const char *s, int len,
+		int *ofs, struct cap *caps)
+{
+	int	n, saved_offset, res = 1;
+
+	while (res && r->code[pc] != END) {
+
+		assert(pc < r->code_size);
+		assert(pc < (int) (sizeof(r->code) / sizeof(r->code[0])));
+
+		switch (r->code[pc]) {
+		case BRANCH:
+			saved_offset = *ofs;
+			res = match(r, pc + 3, s, len, ofs, caps);
+			if (res == 0) {
+				*ofs = saved_offset;
+				res = match(r, pc + r->code[pc + 1],
+				    s, len, ofs, caps);
+			}
+			pc += r->code[pc + 2];
+			break;
+		case EXACT:
+			res = 0;
+			n = r->code[pc + 2];	/* String length */
+			if (n <= len - *ofs && !memcmp(s + *ofs, r->data +
+			    r->code[pc + 1], n)) {
+				(*ofs) += n;
+				res = 1;
+			}
+			pc += 3;
+			break;
+		case QUEST:
+			res = 1;
+			saved_offset = *ofs;
+			if (!match(r, pc + 2, s, len, ofs, caps))
+				*ofs = saved_offset;
+			pc += r->code[pc + 1];
+			break;
+		case STAR:
+			res = 1;
+			loop_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case STARQ:
+			res = 1;
+			loop_non_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case PLUS:
+			res = match(r, pc + 2, s, len, ofs, caps);
+			if (res == 0)
+				break;
+
+			loop_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case PLUSQ:
+			res = match(r, pc + 2, s, len, ofs, caps);
+			if (res == 0)
+				break;
+
+			loop_non_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case SPACE:
+			res = 0;
+			if (*ofs < len && isspace(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case NONSPACE:
+			res = 0;
+			if (*ofs < len &&
+					!isspace(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case DIGIT:
+			res = 0;
+			if (*ofs < len && isdigit(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case ANY:
+			res = 0;
+			if (*ofs < len) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case ANYOF:
+			res = 0;
+			if (*ofs < len)
+				res = is_any_of(r->data + r->code[pc + 1],
+					r->code[pc + 2], s, ofs);
+			pc += 3;
+			break;
+		case ANYBUT:
+			res = 0;
+			if (*ofs < len)
+				res = is_any_but(r->data + r->code[pc + 1],
+					r->code[pc + 2], s, ofs);
+			pc += 3;
+			break;
+		case BOL:
+			res = *ofs == 0 ? 1 : 0;
+			pc++;
+			break;
+		case EOL:
+			res = *ofs == len ? 1 : 0;
+			pc++;
+			break;
+		case OPEN:
+			if (caps != NULL)
+				caps[r->code[pc + 1]].ptr = s + *ofs;
+			pc += 2;
+			break;
+		case CLOSE:
+			if (caps != NULL)
+				caps[r->code[pc + 1]].len = (s + *ofs) -
+				    caps[r->code[pc + 1]].ptr;
+			pc += 2;
+			break;
+		case END:
+			pc++;
+			break;
+		default:
+			printf("unknown cmd (%d) at %d\n", r->code[pc], pc);
+			assert(0);
+			break;
+		}
+	}
+
+	return res;
+}
+
+int
+slre_match(const struct slre *r, const char *buf, int len,
+		struct cap *caps)
+{
+	int	i, ofs = 0, res = 0;
+
+	if (r->anchored) {
+		res = match(r, 0, buf, len, &ofs, caps);
+	} else {
+		for (i = 0; i < len && res == 0; i++) {
+			ofs = i;
+			res = match(r, 0, buf, len, &ofs, caps);
+		}
+	}
+
+	return res;
+}
+
+#ifdef SLRE_TEST
+int main(int argc, char *argv[])
+{
+	struct slre	slre;
+	struct cap	caps[20];
+	char		data[1 * 1024 * 1024];
+	FILE		*fp;
+	int		i, count, res, len;
+
+	if (argc < 3) {
+		printf("Usage: %s 'slre' <file> [count]\n", argv[0]);
+	} else {
+		fp = fopen(argv[2], "rb");
+		if (fp == NULL) {
+			printf("Error: cannot open %s:%s\n",
+				argv[2], strerror(errno));
+		} else if (!slre_compile(&slre, argv[1])) {
+			printf("Error compiling slre: %s\n", slre.err_str);
+		} else {
+			slre_dump(&slre, stderr);
+
+			(void) memset(caps, 0, sizeof(caps));
+
+			/* Read first 128K of file */
+			len = fread(data, 1, sizeof(data), fp);
+			(void) fclose(fp);
+
+			res = 0;
+			count = argc > 3 ? atoi(argv[3]) : 1;
+			for (i = 0; i < count; i++)
+				res = slre_match(&slre, data, len, caps);
+
+			printf("Result: %d\n", res);
+
+			for (i = 0; i < 20; i++) {
+				if (caps[i].len > 0) {
+					printf("Substring %d: [%.*s]\n", i,
+						caps[i].len, caps[i].ptr);
+				}
+			}
+		}
+	}
+
+	return 0;
+}
+#endif /* SLRE_TEST */
-- 
1.7.6.4

[U-Boot] [PATCH 2/2] env: add regex support for environment variables

[Wolfgang Denk]

Syntax:  env regex [-g] [-s subst] regex name [...]

The code is based on SLRE (http://slre.sourceforge.net/)
which provides a tiny subset of Perl regular expressions.

Without options, this will implement regex pattern matching on
environment variables.  Variables with matching values will be printd
as with "env print", so this basicly performs a "grep" on the given
list of variables.

With "-s subst", the matching pattern gets replaced with the string
given in "subst".  Back references '\0' ... '\9' are allowed, where
'\0' stands for the whole matched string, and '\1', '\2', ... are
replaced with the first, second, ... sub-pattern.

"-g" allows for global replacement.

Examples:
	=> setenv foo abcdefghijklmnop
	=> env reg 'A' '[bdgmo]' foo
	=> env reg -s 'A' '[bdgmo]' foo
	foo=aAcdefghijklmnop
	=> env reg -g -s 'B' '[bdgmo]' foo
	foo=aAcBefBhijklBnBp
	=> env reg -g -s '\\2--\\1' '(Be).*(kl)' foo
	foo=aAckl--BeBnBp

[Note: the double backslashes are needed by U-Boot's "shell" so one
backslash gets passed to the actual command.]

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c |  288 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Makefile        |    1 +
 2 files changed, 289 insertions(+), 0 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 2dd4eba..4307d27 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -164,6 +164,288 @@ int do_env_print (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return rcode;
 }
 
+#ifdef CONFIG_CMD_REGEX
+
+/*
+ * memstr - Find the first substring in memory
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ *
+ * Similar to and based on strstr(),
+ * but strings do not need to be NUL terminated.
+ */
+static char *memstr(const char *s1, int l1, const char *s2, int l2)
+{
+	if (!l2)
+		return (char *) s1;
+
+	while (l1 >= l2) {
+		l1--;
+		if (!memcmp(s1, s2, l2))
+			return (char *) s1;
+		s1++;
+	}
+	return NULL;
+}
+
+static char *substitute(char *string,	/* string buffer */
+			int *slen,	/* current string length */
+			int ssize,	/* string bufer size */
+			const char *old,/* old (replaced) string */
+			int olen,	/* length of old string */
+			const char *new,/* new (replacement) string */
+			int nlen)	/* length of new string */
+{
+	char *p = memstr(string, *slen, old, olen);
+
+	if (p == NULL)
+		return NULL;
+
+	debug("## Match at pos %ld: match len %d, subst len %d\n",
+		(long)(p - string), olen, nlen);
+
+	/* make sure replacement matches */
+	if (*slen + nlen - olen > ssize) {
+		printf("## error: substitution buffer overflow\n");
+		return NULL;
+	}
+
+	/* move tail if needed */
+	if (olen != nlen) {
+		int tail, len;
+
+		len = (olen > nlen) ? olen : nlen;
+
+		tail = ssize - (p + len - string);
+
+		debug("## tail len %d\n", tail);
+
+		memmove(p + nlen, p + olen, tail);
+	}
+
+	/* insert substitue */
+	memcpy(p, new, nlen);
+
+	*slen += nlen - olen;
+
+	return p + nlen;
+}
+
+#include "slre.h"
+
+#define	SLRE_BUFSZ	16384
+#define	SLRE_PATSZ	4096
+
+/*
+ * Perform regex operations on a environment variable
+ *
+ * Returns 0 in case of error, or length of printed string
+ */
+static int env_regex(const char *name, const char *regexp, const char *subst,
+			int global)
+{
+	ENTRY e, *ep;
+	struct slre slre;
+	struct cap caps[32];
+	char data[SLRE_BUFSZ];
+	char *datap = data;
+	int res, len, nlen, loop;
+
+	if (name == NULL)
+		return 0;
+
+	if (slre_compile(&slre, regexp) == 0) {
+		printf("Error compiling regex: %s\n", slre.err_str);
+		return 1;
+	}
+
+	e.key = name;
+	e.data = NULL;
+	hsearch_r(e, FIND, &ep, &env_htab);
+	if (ep == NULL) {
+		printf("## Error: \"%s\" not defined\n", name);
+		return 1;
+	}
+
+	debug("REGEX on %s=%s\n", ep->key, ep->data);
+	debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n",
+		regexp, subst ? subst : "<NULL>", global);
+
+	len = strlen(ep->data);
+	if (len + 1 > SLRE_BUFSZ) {
+		printf("## error: substitution buffer overflow\n");
+		return 1;
+	}
+
+	strcpy(data, ep->data);
+
+	if (subst == NULL)
+		nlen = 0;
+	else
+		nlen = strlen(subst);
+
+	for (loop = 0; ;loop++) {
+		char nbuf[SLRE_PATSZ];
+		const char *old;;
+		char *np;
+		int i, olen;
+
+		(void) memset(caps, 0, sizeof(caps));
+
+		res = slre_match(&slre, datap, len, caps);
+
+		debug("Result: %d\n", res);
+
+		for (i = 0; i < 20; i++) {
+			if (caps[i].len > 0) {
+				debug("Substring %d: [%.*s]\n", i,
+					caps[i].len, caps[i].ptr);
+			}
+		}
+
+		if (res == 0) {
+			if (loop == 0) {
+				printf("%s: No match\n", ep->key);
+				return 1;
+			} else {
+				break;
+			}
+		}
+
+		debug("## MATCH ## %s\n", data);
+
+		if (subst == NULL) {
+			printf("%s=%s\n", ep->key, ep->data);
+			return 0;
+		}
+
+		old = caps[0].ptr;
+		olen = caps[0].len;
+
+		if (nlen + 1 >= SLRE_PATSZ) {
+			printf("## error: pattern buffer overflow\n");
+			return 1;
+		}
+		strcpy(nbuf, subst);
+
+		debug("## SUBST(1) ## %s\n", nbuf);
+
+		/*
+		 * Handle back references
+		 *
+		 * Support for \0 ... \9, where \0 is the
+		 * whole matched pattern (similar to &).
+		 *
+		 * Implementation is a bit simpleminded as
+		 * backrefs are substituted sequentially, one
+		 * by one.  This will lead to somewhat
+		 * unexpected results if the replacement
+		 * strings contain any \N strings then then
+		 * may get substitued, too.  We accept this
+		 * restriction for the sake of simplicity.
+		 */
+		for (i = 0; i < 10; ++i) {
+			char backref[2] = {
+				'\\',
+				'0',
+			};
+
+			if (caps[i].len == 0)
+				break;
+
+			backref[1] += i;
+
+			debug("## BACKREF %d: replace \"%.*s\" by \"%.*s\" in \"%s\"\n",
+				i,
+				2, backref,
+				caps[i].len, caps[i].ptr,
+				nbuf);
+
+			for (np = nbuf;;) {
+				char *p = memstr(np, nlen,
+						backref, 2);
+
+				if (p == NULL)
+					break;
+
+				np = substitute(np, &nlen,
+					SLRE_PATSZ,
+					backref, 2,
+					caps[i].ptr, caps[i].len);
+
+				if (np == NULL)
+					return 1;
+			}
+		}
+		debug("## SUBST(2) ## %s\n", nbuf);
+
+		datap = substitute(datap, &len, SLRE_BUFSZ,
+				old, olen,
+				nbuf, nlen);
+
+		if (datap == NULL)
+			return 1;
+
+		debug("## REMAINDER: %s\n", datap);
+
+		debug("## RESULT: %s\n", data);
+
+		if (!global)
+			break;
+	}
+	debug("## FINAL (now setenv()) :  %s\n", data);
+	
+	printf("%s=%s\n", ep->key, data);
+
+	return setenv(ep->key, data);
+}
+
+static int do_env_regex(cmd_tbl_t *cmdtp, int flag, int argc,
+			char *const argv[])
+{
+	int i;
+	int rcode = 0;
+	int global = 0;
+	const char *regex, *subst = NULL;
+
+	while (--argc > 0 && **++argv == '-') {
+		char *arg = *argv;
+		while (*++arg) {
+			switch (*arg) {
+			case 'e':
+				/* allow regex starting with '-' */
+				++argv;
+				goto DONE;
+			case 'g':
+				global = 1;
+				break;
+			case 's':
+				if (--argc <= 0)
+					return cmd_usage(cmdtp);
+				subst = *++argv;
+				goto NXTARG;
+			default:
+				return cmd_usage(cmdtp);
+			}
+		}
+NXTARG:		;
+	}
+DONE:
+	if (argc < 2)
+		return cmd_usage(cmdtp);
+
+	regex = argv[0];
+
+	/* process selected env vars */
+	for (i = 1; i < argc; ++i) {
+		if (env_regex(argv[i], regex, subst, global) != 0)
+			rcode = 1;
+	}
+
+	return rcode;
+}
+#endif
+
 #ifdef CONFIG_CMD_GREPENV
 static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
@@ -1022,6 +1304,9 @@ static cmd_tbl_t cmd_env_sub[] = {
 	U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""),
 #endif
 	U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""),
+#if defined(CONFIG_CMD_REGEX)
+	U_BOOT_CMD_MKENT(regex, CONFIG_SYS_MAXARGS, 1, do_env_regex, "", ""),
+#endif
 #if defined(CONFIG_CMD_RUN)
 	U_BOOT_CMD_MKENT(run, CONFIG_SYS_MAXARGS, 1, do_run, "", ""),
 #endif
@@ -1076,6 +1361,9 @@ U_BOOT_CMD(
 #endif
 	"env import [-d] [-t | -b | -c] addr [size] - import environment\n"
 	"env print [name ...] - print environment\n"
+#ifdef CONFIG_CMD_REGEX
+	"env regex [-g] [-s subst] regex name [...] - search and substitute regular expression\n"
+#endif
 #if defined(CONFIG_CMD_RUN)
 	"env run var [...] - run commands in an environment variable\n"
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index 54708c2..346409c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -48,6 +48,7 @@ COBJS-y += net_utils.o
 COBJS-y += qsort.o
 COBJS-$(CONFIG_SHA1) += sha1.o
 COBJS-$(CONFIG_SHA256) += sha256.o
+COBJS-$(CONFIG_CMD_REGEX) += slre.o
 COBJS-y	+= strmhz.o
 COBJS-$(CONFIG_RBTREE)	+= rbtree.o
 endif
-- 
1.7.6.4

[U-Boot] [PATCH 2/2] env: add regex support for environment variables

[Detlev Zundel]

Hi Wolfgang,

this really is an interesting addition!

> Syntax:  env regex [-g] [-s subst] regex name [...]
>
> The code is based on SLRE (http://slre.sourceforge.net/)
> which provides a tiny subset of Perl regular expressions.
>
> Without options, this will implement regex pattern matching on
> environment variables.  Variables with matching values will be printd
> as with "env print", so this basicly performs a "grep" on the given
> list of variables.

Ok, this usage looks fine.

> With "-s subst", the matching pattern gets replaced with the string
> given in "subst".  Back references '\0' ... '\9' are allowed, where
> '\0' stands for the whole matched string, and '\1', '\2', ... are
> replaced with the first, second, ... sub-pattern.
>
> "-g" allows for global replacement.

But IMHO this usage doesn't really belong into the "env" command.  It
much rather is a further operation of the setexpr command:

  set environment variable as the result of eval expression",
  name value1 <op> value2\n"
      - set environment variable 'name' to the result of the evaluated\n"
        express specified by <op>.  <op> can be &, |, ^, +, -, *, /, %"


We could add the <op>erations for regsubst and regsubstg.  For actual
names for the operations, I'm somewhat unsure.  Maybe "function like",
i.e. "regsubst(string, pattern, replacement)" and "regsubstg(string,
pattern, replacement)"?

> Examples:
> 	=> setenv foo abcdefghijklmnop
> 	=> env reg 'A' '[bdgmo]' foo
> 	=> env reg -s 'A' '[bdgmo]' foo
> 	foo=aAcdefghijklmnop
> 	=> env reg -g -s 'B' '[bdgmo]' foo
> 	foo=aAcBefBhijklBnBp
> 	=> env reg -g -s '\\2--\\1' '(Be).*(kl)' foo
> 	foo=aAckl--BeBnBp

So I'd vote for

=> setenv result regsubst($foo, '[bdgmo]', 'A')

What do you think?

Cheers
  Detlev

-- 
To you I'm an atheist; to God, I'm the Loyal Opposition.
                                        -- Woody Allen
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 0/9] Add Regular Expressions Support

[Wolfgang Denk]

The following patch series adds the SLRE "Super Light Regular
Expression" library and uses this to add regex support for 
the "env grep" (aka "grepenv") command, and new functions (or
operators?) "gsub" and "sub" to the "setexpr" command.

The rework to "env grep" also fixed a few bugs (which cuased it to
dump always _all_ environment varioables on some systems), and adds
the capability to grep in either the variable name, or the value, or
in both (the old version olways did the latter).  Instead of special
functions we now use common code (i. e. hexport_r()) for the variable
lookup, which gives us sorted output as a free additional benefit.

This allows to do things like

- print all MAC addresses:

	=> env grep -e eth.*addr
	eth1addr=00:10:ec:80:c5:15
	ethaddr=00:10:ec:00:c5:15

- print all variables that have at least 2 colons in their value:

	=> env grep -v -e :.*:
	addip=setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}:${netdev}:off panic=1
	eth1addr=00:10:ec:80:c5:15
	ethaddr=00:10:ec:00:c5:15
	ver=U-Boot 2013.04-rc1-00289-g497746b-dirty (Mar 22 2013 - 12:50:25)

- Generate broadcast address by substituting the last two numbers of
  the IP address by "255.255":

  	=> print ipaddr
	ipaddr=192.168.1.104
	=> setexpr broadcast sub "(.*\\.).*\\..*" "\\1255.255" $ipaddr
	broadcast=192.168.255.255

- Depending on keyboard configuration (German vs. US keyboard) a
  barcode scanner may initialize the MAC address as C0:E5:4E:02:06:DC
  or as C0>E5>4E>02>06>DC.  Make sure we always have a correct value:

  	=> print ethaddr
	ethaddr=C0>E5>4E>02>06>DC
	=> setexpr ethaddr gsub > :
	ethaddr=C0:E5:4E:02:06:DC

etc.

	
Regex support can be enabled by defining  CONFIG_REGEX  in the board
config file.


Wolfgang Denk (9):
  hashtable: preparations to use hexport_r() for "env grep"
  "env grep" - reimplement command using hexport_r()
  "env grep" - add options to grep in name, value, or both.
  Add SLRE - Super Light Regular Expression library
  "env grep" - add support for regular expression matches
  setexpr: simplify code, improve help message
  setexpr: add regex substring matching and substitution
  m28evk: enable "env grep" and regexp support
  amcc-common.h: enable support for "env grep", "setexpr", and regex.

 README                        |   7 +
 common/cmd_nvedit.c           |  87 +++--
 common/cmd_setexpr.c          | 296 ++++++++++++++++-
 include/configs/amcc-common.h |   9 +-
 include/configs/m28evk.h      |   3 +
 include/search.h              |  15 +-
 include/slre.h                | 100 ++++++
 lib/Makefile                  |   1 +
 lib/hashtable.c               |  93 ++++--
 lib/slre.c                    | 724 ++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 1263 insertions(+), 72 deletions(-)
 create mode 100644 include/slre.h
 create mode 100644 lib/slre.c

-- 
1.8.1.4

[U-Boot] [PATCH 1/9] hashtable: preparations to use hexport_r() for "env grep"

[Wolfgang Denk]

The output of "env grep" is unsorted, and printing is done by a
private implementation to parse the hash table.  We have all the
needed code in place in hexport_r() alsready, so let's use this
instead.  Here we prepare the code for this, without any functional
changes yet.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c | 10 +++++++---
 include/search.h    |  8 +++++++-
 lib/hashtable.c     | 32 ++++++++++++++++++++++++--------
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 947d6c4..5c84795 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2000-2010
+ * (C) Copyright 2000-2013
  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
  *
  * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH <www.elinos.com>
@@ -871,7 +871,9 @@ NXTARG:		;
 	argv++;
 
 	if (sep) {		/* export as text file */
-		len = hexport_r(&env_htab, sep, 0, &addr, size, argc, argv);
+		len = hexport_r(&env_htab, sep,
+				H_MATCH_KEY | H_MATCH_IDENT,
+				&addr, size, argc, argv);
 		if (len < 0) {
 			error("Cannot export environment: errno = %d\n", errno);
 			return 1;
@@ -889,7 +891,9 @@ NXTARG:		;
 	else			/* export as raw binary data */
 		res = addr;
 
-	len = hexport_r(&env_htab, '\0', 0, &res, ENV_SIZE, argc, argv);
+	len = hexport_r(&env_htab, '\0',
+			H_MATCH_KEY | H_MATCH_IDENT, 
+			&res, ENV_SIZE, argc, argv);
 	if (len < 0) {
 		error("Cannot export environment: errno = %d\n", errno);
 		return 1;
diff --git a/include/search.h b/include/search.h
index 13d3be6..9d9abd6 100644
--- a/include/search.h
+++ b/include/search.h
@@ -22,7 +22,7 @@
 /*
  * Based on code from uClibc-0.9.30.3
  * Extensions for use within U-Boot
- * Copyright (C) 2010 Wolfgang Denk <wd@denx.de>
+ * Copyright (C) 2010-2013 Wolfgang Denk <wd@denx.de>
  */
 
 #ifndef _SEARCH_H
@@ -131,5 +131,11 @@ extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
 #define H_FORCE		(1 << 1) /* overwrite read-only/write-once variables */
 #define H_INTERACTIVE	(1 << 2) /* indicate that an import is user directed */
 #define H_HIDE_DOT	(1 << 3) /* don't print env vars that begin with '.' */
+#define H_MATCH_KEY	(1 << 4) /* search/grep key  = variable names	     */
+#define H_MATCH_DATA	(1 << 5) /* search/grep data = variable values	     */
+#define H_MATCH_BOTH	(H_MATCH_KEY | H_MATCH_DATA) /* search/grep both     */
+#define H_MATCH_IDENT	(1 << 6) /* search for indentical strings	     */
+#define H_MATCH_SUBSTR  (1 << 7) /* search for substring matches	     */
+#define H_MATCH_METHOD	(H_MATCH_IDENT | H_MATCH_SUBSTR)
 
 #endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 07ebfb2..1d18d15 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -2,7 +2,7 @@
  * This implementation is based on code from uClibc-0.9.30.3 but was
  * modified and extended for use within U-Boot.
  *
- * Copyright (C) 2010 Wolfgang Denk <wd@denx.de>
+ * Copyright (C) 2010-2013 Wolfgang Denk <wd@denx.de>
  *
  * Original license header:
  *
@@ -563,6 +563,28 @@ static int cmpkey(const void *p1, const void *p2)
 	return (strcmp(e1->key, e2->key));
 }
 
+static int match_strings(ENTRY *ep, int flag,
+		 int argc, char * const argv[])
+{
+	int arg;
+
+	for (arg = 0; arg < argc; ++arg) {
+		if (flag & H_MATCH_KEY) {
+			switch (flag & H_MATCH_METHOD) {
+			case H_MATCH_IDENT:
+				if (strcmp(argv[arg], ep->key) == 0) 
+					return 1;
+				break;
+			default:
+				printf("## ERROR: unsupported match method: 0x%02x\n",
+					flag & H_MATCH_METHOD);
+				break;
+			}
+		}
+	}
+	return 0;
+}
+
 ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
 		 char **resp, size_t size,
 		 int argc, char * const argv[])
@@ -589,14 +611,8 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
 
 		if (htab->table[i].used > 0) {
 			ENTRY *ep = &htab->table[i].entry;
-			int arg, found = 0;
+			int found = match_strings(ep, flag, argc, argv);
 
-			for (arg = 0; arg < argc; ++arg) {
-				if (strcmp(argv[arg], ep->key) == 0) {
-					found = 1;
-					break;
-				}
-			}
 			if ((argc > 0) && (found == 0))
 				continue;
 
-- 
1.8.1.4

[U-Boot] [PATCH 2/9] "env grep" - reimplement command using hexport_r()

[Wolfgang Denk]

Also drop hstrstr_r() which is not needed any more.
The new code is way more flexible.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c | 30 ++++++++++---------------
 include/search.h    |  6 -----
 lib/hashtable.c     | 64 +++++++++++++++++++++++------------------------------
 3 files changed, 40 insertions(+), 60 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 5c84795..40d9f7a 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -162,31 +162,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc,
 static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 		       int argc, char * const argv[])
 {
-	ENTRY *match;
-	unsigned char matched[env_htab.size / 8];
-	int rcode = 1, arg = 1, idx;
+	char *res = NULL;
+	int len;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	memset(matched, 0, env_htab.size / 8);
+	len = hexport_r(&env_htab, '\n',
+			flag | H_MATCH_BOTH | H_MATCH_SUBSTR,
+			&res, 0, argc, argv);
 
-	while (arg <= argc) {
-		idx = 0;
-		while ((idx = hstrstr_r(argv[arg], idx, &match, &env_htab))) {
-			if (!(matched[idx / 8] & (1 << (idx & 7)))) {
-				puts(match->key);
-				puts("=");
-				puts(match->data);
-				puts("\n");
-			}
-			matched[idx / 8] |= 1 << (idx & 7);
-			rcode = 0;
-		}
-		arg++;
+	if (len > 0) {
+		puts(res);
+		free(res);
 	}
 
-	return rcode;
+	if (len < 2)
+		return 1;
+
+	return 0;
 }
 #endif
 #endif /* CONFIG_SPL_BUILD */
diff --git a/include/search.h b/include/search.h
index 9d9abd6..d06a201 100644
--- a/include/search.h
+++ b/include/search.h
@@ -98,12 +98,6 @@ extern int hsearch_r(ENTRY __item, ACTION __action, ENTRY ** __retval,
  */
 extern int hmatch_r(const char *__match, int __last_idx, ENTRY ** __retval,
 		    struct hsearch_data *__htab);
-/*
- * Search for an entry whose key or data contains `MATCH'.  Otherwise,
- * Same semantics as hsearch_r().
- */
-extern int hstrstr_r(const char *__match, int __last_idx, ENTRY ** __retval,
-		    struct hsearch_data *__htab);
 
 /* Search and delete entry matching ITEM.key in internal hash table. */
 extern int hdelete_r(const char *__key, struct hsearch_data *__htab,
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 1d18d15..8ad56ec 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -210,29 +210,6 @@ void hdestroy_r(struct hsearch_data *htab)
  *   example for functions like hdelete().
  */
 
-/*
- * hstrstr_r - return index to entry whose key and/or data contains match
- */
-int hstrstr_r(const char *match, int last_idx, ENTRY ** retval,
-	      struct hsearch_data *htab)
-{
-	unsigned int idx;
-
-	for (idx = last_idx + 1; idx < htab->size; ++idx) {
-		if (htab->table[idx].used <= 0)
-			continue;
-		if (strstr(htab->table[idx].entry.key, match) ||
-		    strstr(htab->table[idx].entry.data, match)) {
-			*retval = &htab->table[idx].entry;
-			return idx;
-		}
-	}
-
-	__set_errno(ESRCH);
-	*retval = NULL;
-	return 0;
-}
-
 int hmatch_r(const char *match, int last_idx, ENTRY ** retval,
 	     struct hsearch_data *htab)
 {
@@ -563,23 +540,38 @@ static int cmpkey(const void *p1, const void *p2)
 	return (strcmp(e1->key, e2->key));
 }
 
-static int match_strings(ENTRY *ep, int flag,
+static int match_string(int flag, const char * str, const char * pat)
+{
+	switch (flag & H_MATCH_METHOD) {
+	case H_MATCH_IDENT:
+		if (strcmp(str, pat) == 0) 
+			return 1;
+		break;
+	case H_MATCH_SUBSTR:
+		if (strstr(str, pat))
+			return 1;
+		break;
+	default:
+		printf("## ERROR: unsupported match method: 0x%02x\n",
+			flag & H_MATCH_METHOD);
+		break;
+	}
+	return 0;
+}
+
+static int match_entry(ENTRY *ep, int flag,
 		 int argc, char * const argv[])
 {
 	int arg;
 
-	for (arg = 0; arg < argc; ++arg) {
+	for (arg = 1; arg < argc; ++arg) {
 		if (flag & H_MATCH_KEY) {
-			switch (flag & H_MATCH_METHOD) {
-			case H_MATCH_IDENT:
-				if (strcmp(argv[arg], ep->key) == 0) 
-					return 1;
-				break;
-			default:
-				printf("## ERROR: unsupported match method: 0x%02x\n",
-					flag & H_MATCH_METHOD);
-				break;
-			}
+			if (match_string(flag, ep->key, argv[arg]))
+				return 1;
+		}
+		if (flag & H_MATCH_DATA) {
+			if (match_string(flag, ep->data, argv[arg]))
+				return 1;
 		}
 	}
 	return 0;
@@ -611,7 +603,7 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
 
 		if (htab->table[i].used > 0) {
 			ENTRY *ep = &htab->table[i].entry;
-			int found = match_strings(ep, flag, argc, argv);
+			int found = match_entry(ep, flag, argc, argv);
 
 			if ((argc > 0) && (found == 0))
 				continue;
-- 
1.8.1.4

[U-Boot] [PATCH 3/9] "env grep" - add options to grep in name, value, or both.

[Wolfgang Denk]

Add options to "env grep" command:

-n : search only the envrironment variable names
-v : search only their values
-b : search both names and values (= default)

An option "--" will stop parsing options, so to print variables that
contain the striing "- " please use:

	env grep -- "- "

Or to print all environment varioables which have a '-' in their name,
use:

	env grep -n -- -

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 40d9f7a..441bf40 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -163,13 +163,39 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 		       int argc, char * const argv[])
 {
 	char *res = NULL;
-	int len;
+	int len, grep_flags;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
+	grep_flags = H_MATCH_BOTH;
+
+	while (argc > 1 && **(argv + 1) == '-') {
+		char *arg = *++argv;
+
+		--argc;
+		while (*++arg) {
+			switch (*arg) {
+			case 'n':		/* grep for name */
+				grep_flags = H_MATCH_KEY;
+				break;
+			case 'v':		/* grep for value */
+				grep_flags = H_MATCH_DATA;
+				break;
+			case 'b':		/* grep for both */
+				grep_flags = H_MATCH_BOTH;
+				break;
+			case '-':
+				goto DONE;
+			default:
+				return CMD_RET_USAGE;
+			}
+		}
+	}
+
+DONE:
 	len = hexport_r(&env_htab, '\n',
-			flag | H_MATCH_BOTH | H_MATCH_SUBSTR,
+			flag | grep_flags | H_MATCH_SUBSTR,
 			&res, 0, argc, argv);
 
 	if (len > 0) {
@@ -1106,7 +1132,7 @@ static char env_help_text[] =
 	"env flags - print variables that have non-default flags\n"
 #endif
 #if defined(CONFIG_CMD_GREPENV)
-	"env grep string [...] - search environment\n"
+	"env grep [-n | -v | -b] string [...] - search environment\n"
 #endif
 #if defined(CONFIG_CMD_IMPORTENV)
 	"env import [-d] [-t | -b | -c] addr [size] - import environment\n"
@@ -1153,8 +1179,10 @@ U_BOOT_CMD_COMPLETE(
 U_BOOT_CMD_COMPLETE(
 	grepenv, CONFIG_SYS_MAXARGS, 0,  do_env_grep,
 	"search environment variables",
-	"string ...\n"
-	"    - list environment name=value pairs matching 'string'",
+	"[-n | -v | -b] string ...\n"
+	"    - list environment name=value pairs matching 'string'\n"
+	"      \"-n\": search variable names; \"-v\": search values;\n"
+	"      \"-b\": search both names and values (default)",
 	var_complete
 );
 #endif
-- 
1.8.1.4

[U-Boot] [PATCH 4/9] Add SLRE - Super Light Regular Expression library

[Wolfgang Denk]

Downloaded from http://slre.sourceforge.net/
and adapted for U-Boot environment.

Used to implement regex operations on environment variables.
Code size is ~ 3.5 KiB on PPC.

To enable this code, define the  CONFIG_REGEX  option in your board
config file.

Note:  There are more recent versions of the SLRE library available at
http://slre.googlecode.com ; unfortunately, the new code has a heavily
reorked API which makes it less usable for our purposes:
- the return code is strings, which are more difficult to process
- we don't get any information any more which sub-string of the data
  was matched by the given regex
- it is much more cumbersome to work with arbitrary expressions, where
  for example the number of substrings for capturing are not known at
  compile time
Also, there does not seem to be any real changes or improvements of
the functionality.

Because of this, we deliberately stick with the older code.

Note 2: the test code (built when SLRE_TEST is defined) was modified
to allow for more extensive testing; now we can test the regexp
matching on all lines on a text file (instead of the whole data in the
file as a single block).

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 README         |   7 +
 include/slre.h | 100 ++++++++
 lib/Makefile   |   1 +
 lib/slre.c     | 724 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 832 insertions(+)
 create mode 100644 include/slre.h
 create mode 100644 lib/slre.c

diff --git a/README b/README
index 7f2506a..f088d28 100644
--- a/README
+++ b/README
@@ -930,6 +930,13 @@ The following options need to be configured:
 
 		XXX - this list needs to get updated!
 
+- Regular expression support:
+		CONFIG_REGEX
+                If this variable is defined, U-Boot is linked against
+                the SLRE (Super Light Regular Expression) library,
+                which adds regex support to some commands, as for
+                example "env grep" and "setexpr".
+
 - Device tree:
 		CONFIG_OF_CONTROL
 		If this variable is defined, U-Boot will use a device tree
diff --git a/include/slre.h b/include/slre.h
new file mode 100644
index 0000000..4b41a4b
--- /dev/null
+++ b/include/slre.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2004-2005 Sergey Lyubka <valenok@gmail.com>
+ * All rights reserved
+ *
+ * "THE BEER-WARE LICENSE" (Revision 42):
+ * Sergey Lyubka wrote this file.  As long as you retain this notice you
+ * can do whatever you want with this stuff. If we meet some day, and you think
+ * this stuff is worth it, you can buy me a beer in return.
+ */
+
+/*
+ * Downloaded Sat Nov  5 17:42:08 CET 2011 at
+ * http://slre.sourceforge.net/1.0/slre.h
+ */
+
+/*
+ * This is a regular expression library that implements a subset of Perl RE.
+ * Please refer to http://slre.sourceforge.net for detailed description.
+ *
+ * Usage example (parsing HTTP request):
+ *
+ * struct slre	slre;
+ * struct cap	captures[4 + 1];  // Number of braket pairs + 1
+ * ...
+ *
+ * slre_compile(&slre,"^(GET|POST) (\S+) HTTP/(\S+?)\r\n");
+ *
+ * if (slre_match(&slre, buf, len, captures)) {
+ *	printf("Request line length: %d\n", captures[0].len);
+ *	printf("Method: %.*s\n", captures[1].len, captures[1].ptr);
+ *	printf("URI: %.*s\n", captures[2].len, captures[2].ptr);
+ * }
+ *
+ * Supported syntax:
+ *	^		Match beginning of a buffer
+ *	$		Match end of a buffer
+ *	()		Grouping and substring capturing
+ *	[...]		Match any character from set
+ *	[^...]		Match any character but ones from set
+ *	\s		Match whitespace
+ *	\S		Match non-whitespace
+ *	\d		Match decimal digit
+ *	\r		Match carriage return
+ *	\n		Match newline
+ *	+		Match one or more times (greedy)
+ *	+?		Match one or more times (non-greedy)
+ *	*		Match zero or more times (greedy)
+ *	*?		Match zero or more times (non-greedy)
+ *	?		Match zero or once
+ *	\xDD		Match byte with hex value 0xDD
+ *	\meta		Match one of the meta character: ^$().[*+?\
+ */
+
+#ifndef SLRE_HEADER_DEFINED
+#define	SLRE_HEADER_DEFINED
+
+/*
+ * Compiled regular expression
+ */
+struct slre {
+	unsigned char	code[256];
+	unsigned char	data[256];
+	int		code_size;
+	int		data_size;
+	int		num_caps;	/* Number of bracket pairs	*/
+	int		anchored;	/* Must match from string start	*/
+	const char	*err_str;	/* Error string			*/
+};
+
+/*
+ * Captured substring
+ */
+struct cap {
+	const char	*ptr;		/* Pointer to the substring	*/
+	int		len;		/* Substring length		*/
+};
+
+/*
+ * Compile regular expression. If success, 1 is returned.
+ * If error, 0 is returned and slre.err_str points to the error message.
+ */
+int slre_compile(struct slre *, const char *re);
+
+/*
+ * Return 1 if match, 0 if no match.
+ * If `captured_substrings' array is not NULL, then it is filled with the
+ * values of captured substrings. captured_substrings[0] element is always
+ * a full matched substring. The round bracket captures start from
+ * captured_substrings[1].
+ * It is assumed that the size of captured_substrings array is enough to
+ * hold all captures. The caller function must make sure it is! So, the
+ * array_size = number_of_round_bracket_pairs + 1
+ */
+int slre_match(const struct slre *, const char *buf, int buf_len,
+	struct cap *captured_substrings);
+
+#ifdef SLRE_TEST
+void slre_dump(const struct slre *r, FILE *fp);
+#endif /* SLRE_TEST */
+#endif /* SLRE_HEADER_DEFINED */
diff --git a/lib/Makefile b/lib/Makefile
index 1bfd3ee..57a1cae 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -71,6 +71,7 @@ COBJS-y += crc32.o
 COBJS-y += ctype.o
 COBJS-y += div64.o
 COBJS-y += linux_string.o
+COBJS-$(CONFIG_REGEX) += slre.o
 COBJS-y += string.o
 COBJS-y += time.o
 COBJS-$(CONFIG_BOOTP_PXE) += uuid.o
diff --git a/lib/slre.c b/lib/slre.c
new file mode 100644
index 0000000..8cdd192
--- /dev/null
+++ b/lib/slre.c
@@ -0,0 +1,724 @@
+/*
+ * Copyright (c) 2004-2005 Sergey Lyubka <valenok@gmail.com>
+ * All rights reserved
+ *
+ * "THE BEER-WARE LICENSE" (Revision 42):
+ * Sergey Lyubka wrote this file.  As long as you retain this notice you
+ * can do whatever you want with this stuff. If we meet some day, and you think
+ * this stuff is worth it, you can buy me a beer in return.
+ */
+
+/*
+ * Downloaded Sat Nov  5 17:43:06 CET 2011 at
+ * http://slre.sourceforge.net/1.0/slre.c
+ */
+
+#ifdef SLRE_TEST
+#include <stdio.h>
+#include <assert.h>
+#include <ctype.h>
+#include <stdlib.h>
+#include <string.h>
+#else
+#include <common.h>
+#include <linux/ctype.h>
+#endif /* SLRE_TEST */
+
+#include <errno.h>
+
+#include <slre.h>
+
+enum {END, BRANCH, ANY, EXACT, ANYOF, ANYBUT, OPEN, CLOSE, BOL, EOL,
+	STAR, PLUS, STARQ, PLUSQ, QUEST, SPACE, NONSPACE, DIGIT};
+
+#ifdef SLRE_TEST
+static struct {
+	const char	*name;
+	int		narg;
+	const char	*flags;
+} opcodes[] = {
+	{"END",		0, ""},		/* End of code block or program	*/
+	{"BRANCH",	2, "oo"},	/* Alternative operator, "|"	*/
+	{"ANY",		0, ""},		/* Match any character, "."	*/
+	{"EXACT",	2, "d"},	/* Match exact string		*/
+	{"ANYOF",	2, "D"},	/* Match any from set, "[]"	*/
+	{"ANYBUT",	2, "D"},	/* Match any but from set, "[^]"*/
+	{"OPEN ",	1, "i"},	/* Capture start, "("		*/
+	{"CLOSE",	1, "i"},	/* Capture end, ")"		*/
+	{"BOL",		0, ""},		/* Beginning of string, "^"	*/
+	{"EOL",		0, ""},		/* End of string, "$"		*/
+	{"STAR",	1, "o"},	/* Match zero or more times "*"	*/
+	{"PLUS",	1, "o"},	/* Match one or more times, "+"	*/
+	{"STARQ",	1, "o"},	/* Non-greedy STAR,  "*?"	*/
+	{"PLUSQ",	1, "o"},	/* Non-greedy PLUS, "+?"	*/
+	{"QUEST",	1, "o"},	/* Match zero or one time, "?"	*/
+	{"SPACE",	0, ""},		/* Match whitespace, "\s"	*/
+	{"NONSPACE",	0, ""},		/* Match non-space, "\S"	*/
+	{"DIGIT",	0, ""}		/* Match digit, "\d"		*/
+};
+#endif /* SLRE_TEST */
+
+/*
+ * Commands and operands are all unsigned char (1 byte long). All code offsets
+ * are relative to current address, and positive (always point forward). Data
+ * offsets are absolute. Commands with operands:
+ *
+ * BRANCH offset1 offset2
+ *	Try to match the code block that follows the BRANCH instruction
+ *	(code block ends with END). If no match, try to match code block that
+ *	starts at offset1. If either of these match, jump to offset2.
+ *
+ * EXACT data_offset data_length
+ *	Try to match exact string. String is recorded in data section from
+ *	data_offset, and has length data_length.
+ *
+ * OPEN capture_number
+ * CLOSE capture_number
+ *	If the user have passed 'struct cap' array for captures, OPEN
+ *	records the beginning of the matched substring (cap->ptr), CLOSE
+ *	sets the length (cap->len) for respective capture_number.
+ *
+ * STAR code_offset
+ * PLUS code_offset
+ * QUEST code_offset
+ *	*, +, ?, respectively. Try to gobble as much as possible from the
+ *	matched buffer, until code block that follows these instructions
+ *	matches. When the longest possible string is matched,
+ *	jump to code_offset
+ *
+ * STARQ, PLUSQ are non-greedy versions of STAR and PLUS.
+ */
+
+static const char *meta_chars = "|.^$*+?()[\\";
+
+#ifdef SLRE_TEST
+
+static void
+print_character_set(FILE *fp, const unsigned char *p, int len)
+{
+	int	i;
+
+	for (i = 0; i < len; i++) {
+		if (i > 0)
+			(void) fputc(',', fp);
+		if (p[i] == 0) {
+			i++;
+			if (p[i] == 0)
+				(void) fprintf(fp, "\\x%02x", p[i]);
+			else
+				(void) fprintf(fp, "%s", opcodes[p[i]].name);
+		} else if (isprint(p[i])) {
+			(void) fputc(p[i], fp);
+		} else {
+			(void) fprintf(fp, "\\x%02x", p[i]);
+		}
+	}
+}
+
+void
+slre_dump(const struct slre *r, FILE *fp)
+{
+	int	i, j, ch, op, pc;
+
+	for (pc = 0; pc < r->code_size; pc++) {
+
+		op = r->code[pc];
+		(void) fprintf(fp, "%3d %s ", pc, opcodes[op].name);
+
+		for (i = 0; opcodes[op].flags[i] != '\0'; i++)
+			switch (opcodes[op].flags[i]) {
+			case 'i':
+				(void) fprintf(fp, "%d ", r->code[pc + 1]);
+				pc++;
+				break;
+			case 'o':
+				(void) fprintf(fp, "%d ",
+				    pc + r->code[pc + 1] - i);
+				pc++;
+				break;
+			case 'D':
+				print_character_set(fp, r->data +
+				    r->code[pc + 1], r->code[pc + 2]);
+				pc += 2;
+				break;
+			case 'd':
+				(void) fputc('"', fp);
+				for (j = 0; j < r->code[pc + 2]; j++) {
+					ch = r->data[r->code[pc + 1] + j];
+					if (isprint(ch)) {
+						(void) fputc(ch, fp);
+					} else {
+						(void) fprintf(fp,
+							"\\x%02x", ch);
+					}
+				}
+				(void) fputc('"', fp);
+				pc += 2;
+				break;
+			}
+
+		(void) fputc('\n', fp);
+	}
+}
+#endif /* SLRE_TEST */
+
+static void
+set_jump_offset(struct slre *r, int pc, int offset)
+{
+	assert(offset < r->code_size);
+
+	if (r->code_size - offset > 0xff)
+		r->err_str = "Jump offset is too big";
+	else
+		r->code[pc] = (unsigned char) (r->code_size - offset);
+}
+
+static void
+emit(struct slre *r, int code)
+{
+	if (r->code_size >= (int) (sizeof(r->code) / sizeof(r->code[0])))
+		r->err_str = "RE is too long (code overflow)";
+	else
+		r->code[r->code_size++] = (unsigned char) code;
+}
+
+static void
+store_char_in_data(struct slre *r, int ch)
+{
+	if (r->data_size >= (int) sizeof(r->data))
+		r->err_str = "RE is too long (data overflow)";
+	else
+		r->data[r->data_size++] = ch;
+}
+
+static void
+exact(struct slre *r, const char **re)
+{
+	int	old_data_size = r->data_size;
+
+	while (**re != '\0' && (strchr(meta_chars, **re)) == NULL)
+		store_char_in_data(r, *(*re)++);
+
+	emit(r, EXACT);
+	emit(r, old_data_size);
+	emit(r, r->data_size - old_data_size);
+}
+
+static int
+get_escape_char(const char **re)
+{
+	int	res;
+
+	switch (*(*re)++) {
+	case 'n':
+		res = '\n';
+		break;
+	case 'r':
+		res = '\r';
+		break;
+	case 't':
+		res = '\t';
+		break;
+	case '0':
+		res = 0;
+		break;
+	case 'S':
+		res = NONSPACE << 8;
+		break;
+	case 's':
+		res = SPACE << 8;
+		break;
+	case 'd':
+		res = DIGIT << 8;
+		break;
+	default:
+		res = (*re)[-1];
+		break;
+	}
+
+	return res;
+}
+
+static void
+anyof(struct slre *r, const char **re)
+{
+	int	esc, old_data_size = r->data_size, op = ANYOF;
+
+	if (**re == '^') {
+		op = ANYBUT;
+		(*re)++;
+	}
+
+	while (**re != '\0')
+
+		switch (*(*re)++) {
+		case ']':
+			emit(r, op);
+			emit(r, old_data_size);
+			emit(r, r->data_size - old_data_size);
+			return;
+			/* NOTREACHED */
+			break;
+		case '\\':
+			esc = get_escape_char(re);
+			if ((esc & 0xff) == 0) {
+				store_char_in_data(r, 0);
+				store_char_in_data(r, esc >> 8);
+			} else {
+				store_char_in_data(r, esc);
+			}
+			break;
+		default:
+			store_char_in_data(r, (*re)[-1]);
+			break;
+		}
+
+	r->err_str = "No closing ']' bracket";
+}
+
+static void
+relocate(struct slre *r, int begin, int shift)
+{
+	emit(r, END);
+	memmove(r->code + begin + shift, r->code + begin, r->code_size - begin);
+	r->code_size += shift;
+}
+
+static void
+quantifier(struct slre *r, int prev, int op)
+{
+	if (r->code[prev] == EXACT && r->code[prev + 2] > 1) {
+		r->code[prev + 2]--;
+		emit(r, EXACT);
+		emit(r, r->code[prev + 1] + r->code[prev + 2]);
+		emit(r, 1);
+		prev = r->code_size - 3;
+	}
+	relocate(r, prev, 2);
+	r->code[prev] = op;
+	set_jump_offset(r, prev + 1, prev);
+}
+
+static void
+exact_one_char(struct slre *r, int ch)
+{
+	emit(r, EXACT);
+	emit(r, r->data_size);
+	emit(r, 1);
+	store_char_in_data(r, ch);
+}
+
+static void
+fixup_branch(struct slre *r, int fixup)
+{
+	if (fixup > 0) {
+		emit(r, END);
+		set_jump_offset(r, fixup, fixup - 2);
+	}
+}
+
+static void
+compile(struct slre *r, const char **re)
+{
+	int	op, esc, branch_start, last_op, fixup, cap_no, level;
+
+	fixup = 0;
+	level = r->num_caps;
+	branch_start = last_op = r->code_size;
+
+	for (;;)
+		switch (*(*re)++) {
+		case '\0':
+			(*re)--;
+			return;
+			/* NOTREACHED */
+			break;
+		case '^':
+			emit(r, BOL);
+			break;
+		case '$':
+			emit(r, EOL);
+			break;
+		case '.':
+			last_op = r->code_size;
+			emit(r, ANY);
+			break;
+		case '[':
+			last_op = r->code_size;
+			anyof(r, re);
+			break;
+		case '\\':
+			last_op = r->code_size;
+			esc = get_escape_char(re);
+			if (esc & 0xff00)
+				emit(r, esc >> 8);
+			else
+				exact_one_char(r, esc);
+			break;
+		case '(':
+			last_op = r->code_size;
+			cap_no = ++r->num_caps;
+			emit(r, OPEN);
+			emit(r, cap_no);
+
+			compile(r, re);
+			if (*(*re)++ != ')') {
+				r->err_str = "No closing bracket";
+				return;
+			}
+
+			emit(r, CLOSE);
+			emit(r, cap_no);
+			break;
+		case ')':
+			(*re)--;
+			fixup_branch(r, fixup);
+			if (level == 0) {
+				r->err_str = "Unbalanced brackets";
+				return;
+			}
+			return;
+			/* NOTREACHED */
+			break;
+		case '+':
+		case '*':
+			op = (*re)[-1] == '*' ? STAR : PLUS;
+			if (**re == '?') {
+				(*re)++;
+				op = op == STAR ? STARQ : PLUSQ;
+			}
+			quantifier(r, last_op, op);
+			break;
+		case '?':
+			quantifier(r, last_op, QUEST);
+			break;
+		case '|':
+			fixup_branch(r, fixup);
+			relocate(r, branch_start, 3);
+			r->code[branch_start] = BRANCH;
+			set_jump_offset(r, branch_start + 1, branch_start);
+			fixup = branch_start + 2;
+			r->code[fixup] = 0xff;
+			break;
+		default:
+			(*re)--;
+			last_op = r->code_size;
+			exact(r, re);
+			break;
+		}
+}
+
+int
+slre_compile(struct slre *r, const char *re)
+{
+	r->err_str = NULL;
+	r->code_size = r->data_size = r->num_caps = r->anchored = 0;
+
+	if (*re == '^')
+		r->anchored++;
+
+	emit(r, OPEN);	/* This will capture what matches full RE */
+	emit(r, 0);
+
+	while (*re != '\0')
+		compile(r, &re);
+
+	if (r->code[2] == BRANCH)
+		fixup_branch(r, 4);
+
+	emit(r, CLOSE);
+	emit(r, 0);
+	emit(r, END);
+
+	return (r->err_str == NULL ? 1 : 0);
+}
+
+static int match(const struct slre *, int,
+		const char *, int, int *, struct cap *);
+
+static void
+loop_greedy(const struct slre *r, int pc, const char *s, int len, int *ofs)
+{
+	int	saved_offset, matched_offset;
+
+	saved_offset = matched_offset = *ofs;
+
+	while (match(r, pc + 2, s, len, ofs, NULL)) {
+		saved_offset = *ofs;
+		if (match(r, pc + r->code[pc + 1], s, len, ofs, NULL))
+			matched_offset = saved_offset;
+		*ofs = saved_offset;
+	}
+
+	*ofs = matched_offset;
+}
+
+static void
+loop_non_greedy(const struct slre *r, int pc, const char *s, int len, int *ofs)
+{
+	int	saved_offset = *ofs;
+
+	while (match(r, pc + 2, s, len, ofs, NULL)) {
+		saved_offset = *ofs;
+		if (match(r, pc + r->code[pc + 1], s, len, ofs, NULL))
+			break;
+	}
+
+	*ofs = saved_offset;
+}
+
+static int
+is_any_of(const unsigned char *p, int len, const char *s, int *ofs)
+{
+	int	i, ch;
+
+	ch = s[*ofs];
+
+	for (i = 0; i < len; i++)
+		if (p[i] == ch) {
+			(*ofs)++;
+			return 1;
+		}
+
+	return 0;
+}
+
+static int
+is_any_but(const unsigned char *p, int len, const char *s, int *ofs)
+{
+	int	i, ch;
+
+	ch = s[*ofs];
+
+	for (i = 0; i < len; i++) {
+		if (p[i] == ch)
+			return 0;
+	}
+
+	(*ofs)++;
+	return 1;
+}
+
+static int
+match(const struct slre *r, int pc, const char *s, int len,
+		int *ofs, struct cap *caps)
+{
+	int	n, saved_offset, res = 1;
+
+	while (res && r->code[pc] != END) {
+
+		assert(pc < r->code_size);
+		assert(pc < (int) (sizeof(r->code) / sizeof(r->code[0])));
+
+		switch (r->code[pc]) {
+		case BRANCH:
+			saved_offset = *ofs;
+			res = match(r, pc + 3, s, len, ofs, caps);
+			if (res == 0) {
+				*ofs = saved_offset;
+				res = match(r, pc + r->code[pc + 1],
+				    s, len, ofs, caps);
+			}
+			pc += r->code[pc + 2];
+			break;
+		case EXACT:
+			res = 0;
+			n = r->code[pc + 2];	/* String length */
+			if (n <= len - *ofs && !memcmp(s + *ofs, r->data +
+			    r->code[pc + 1], n)) {
+				(*ofs) += n;
+				res = 1;
+			}
+			pc += 3;
+			break;
+		case QUEST:
+			res = 1;
+			saved_offset = *ofs;
+			if (!match(r, pc + 2, s, len, ofs, caps))
+				*ofs = saved_offset;
+			pc += r->code[pc + 1];
+			break;
+		case STAR:
+			res = 1;
+			loop_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case STARQ:
+			res = 1;
+			loop_non_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case PLUS:
+			res = match(r, pc + 2, s, len, ofs, caps);
+			if (res == 0)
+				break;
+
+			loop_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case PLUSQ:
+			res = match(r, pc + 2, s, len, ofs, caps);
+			if (res == 0)
+				break;
+
+			loop_non_greedy(r, pc, s, len, ofs);
+			pc += r->code[pc + 1];
+			break;
+		case SPACE:
+			res = 0;
+			if (*ofs < len && isspace(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case NONSPACE:
+			res = 0;
+			if (*ofs < len &&
+					!isspace(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case DIGIT:
+			res = 0;
+			if (*ofs < len && isdigit(((unsigned char *)s)[*ofs])) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case ANY:
+			res = 0;
+			if (*ofs < len) {
+				(*ofs)++;
+				res = 1;
+			}
+			pc++;
+			break;
+		case ANYOF:
+			res = 0;
+			if (*ofs < len)
+				res = is_any_of(r->data + r->code[pc + 1],
+					r->code[pc + 2], s, ofs);
+			pc += 3;
+			break;
+		case ANYBUT:
+			res = 0;
+			if (*ofs < len)
+				res = is_any_but(r->data + r->code[pc + 1],
+					r->code[pc + 2], s, ofs);
+			pc += 3;
+			break;
+		case BOL:
+			res = *ofs == 0 ? 1 : 0;
+			pc++;
+			break;
+		case EOL:
+			res = *ofs == len ? 1 : 0;
+			pc++;
+			break;
+		case OPEN:
+			if (caps != NULL)
+				caps[r->code[pc + 1]].ptr = s + *ofs;
+			pc += 2;
+			break;
+		case CLOSE:
+			if (caps != NULL)
+				caps[r->code[pc + 1]].len = (s + *ofs) -
+				    caps[r->code[pc + 1]].ptr;
+			pc += 2;
+			break;
+		case END:
+			pc++;
+			break;
+		default:
+			printf("unknown cmd (%d) at %d\n", r->code[pc], pc);
+			assert(0);
+			break;
+		}
+	}
+
+	return res;
+}
+
+int
+slre_match(const struct slre *r, const char *buf, int len,
+		struct cap *caps)
+{
+	int	i, ofs = 0, res = 0;
+
+	if (r->anchored) {
+		res = match(r, 0, buf, len, &ofs, caps);
+	} else {
+		for (i = 0; i < len && res == 0; i++) {
+			ofs = i;
+			res = match(r, 0, buf, len, &ofs, caps);
+		}
+	}
+
+	return res;
+}
+
+#ifdef SLRE_TEST
+#define N_CAPS	5
+
+int main(int argc, char *argv[])
+{
+	struct slre	slre;
+	struct cap	caps[N_CAPS];
+	unsigned char	data[1 * 1024 * 1024];
+	FILE		*fp;
+	int		i, res, len;
+
+	if (argc < 2) {
+		fprintf(stderr, "Usage: %s 'slre' <file>\n", argv[0]);
+		return 1;
+	}
+
+	fp = fopen(argv[2], "rb");
+	if (fp == NULL) {
+		fprintf(stderr, "Error: cannot open %s:%s\n",
+			argv[2], strerror(errno));
+		return 1;
+	}
+
+	if (!slre_compile(&slre, argv[1])) {
+		fprintf(stderr, "Error compiling slre: %s\n", slre.err_str);
+		return 1;
+	}
+	
+	slre_dump(&slre, stderr);
+
+	while (fgets(data, sizeof(data), fp) != NULL) {
+		len = strlen(data);
+
+		if ((len > 0) && (data[len-1] == '\n')) {
+			data[len-1] = '\0';
+			--len;
+		}
+
+		printf("Data = \"%s\"\n", data);
+
+		(void) memset(caps, 0, sizeof(caps));
+
+		res = 0;
+
+		res = slre_match(&slre, data, len, caps);
+		printf("Result [%d]: %d\n", i, res);
+
+		for (i = 0; i < N_CAPS; i++) {
+			if (caps[i].len > 0) {
+				printf("Substring %d: len=%d  [%.*s]\n", i,
+					caps[i].len,
+					caps[i].len, caps[i].ptr);
+			}
+		}
+		printf("----------------------------------------------------\n");
+	}
+	(void) fclose(fp);
+
+	return 0;
+}
+#endif /* SLRE_TEST */
-- 
1.8.1.4

[U-Boot] [PATCH 5/9] "env grep" - add support for regular expression matches

[Wolfgang Denk]

When CONFIG_REGEX is enabled, the new option "-e" becomes available
which causes regular expression matches to be used.  This allows for
example things like these:

- print all MAC addresses:

	=> env grep -e eth.*addr
	eth1addr=00:10:ec:80:c5:15
	ethaddr=00:10:ec:00:c5:15

- print all variables that have at least 2 colons in their value:

	=> env grep -v -e :.*:
	addip=setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}:${netdev}:off
	panic=1
	eth1addr=00:10:ec:80:c5:15
	ethaddr=00:10:ec:00:c5:15
	ver=U-Boot 2013.04-rc1-00289-g497746b-dirty (Mar 22 2013 - 12:50:25)

etc.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c | 29 +++++++++++++++++++++++------
 include/search.h    |  5 +++--
 lib/hashtable.c     | 29 ++++++++++++++++++++++++++---
 3 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 441bf40..aecc803 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -163,12 +163,13 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 		       int argc, char * const argv[])
 {
 	char *res = NULL;
-	int len, grep_flags;
+	int len, grep_how, grep_what;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	grep_flags = H_MATCH_BOTH;
+	grep_how  = H_MATCH_SUBSTR;	/* default: substring search	*/
+	grep_what = H_MATCH_BOTH;	/* default: grep names and values */
 
 	while (argc > 1 && **(argv + 1) == '-') {
 		char *arg = *++argv;
@@ -176,14 +177,19 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 		--argc;
 		while (*++arg) {
 			switch (*arg) {
+#ifdef CONFIG_REGEX
+			case 'e':		/* use regex matching */
+				grep_how  = H_MATCH_REGEX;
+				break;
+#endif
 			case 'n':		/* grep for name */
-				grep_flags = H_MATCH_KEY;
+				grep_what = H_MATCH_KEY;
 				break;
 			case 'v':		/* grep for value */
-				grep_flags = H_MATCH_DATA;
+				grep_what = H_MATCH_DATA;
 				break;
 			case 'b':		/* grep for both */
-				grep_flags = H_MATCH_BOTH;
+				grep_what = H_MATCH_BOTH;
 				break;
 			case '-':
 				goto DONE;
@@ -195,7 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 
 DONE:
 	len = hexport_r(&env_htab, '\n',
-			flag | grep_flags | H_MATCH_SUBSTR,
+			flag | grep_what | grep_how,
 			&res, 0, argc, argv);
 
 	if (len > 0) {
@@ -1132,8 +1138,12 @@ static char env_help_text[] =
 	"env flags - print variables that have non-default flags\n"
 #endif
 #if defined(CONFIG_CMD_GREPENV)
+#ifdef CONFIG_REGEX
+	"env grep [-e] [-n | -v | -b] string [...] - search environment\n"
+#else
 	"env grep [-n | -v | -b] string [...] - search environment\n"
 #endif
+#endif
 #if defined(CONFIG_CMD_IMPORTENV)
 	"env import [-d] [-t | -b | -c] addr [size] - import environment\n"
 #endif
@@ -1179,8 +1189,15 @@ U_BOOT_CMD_COMPLETE(
 U_BOOT_CMD_COMPLETE(
 	grepenv, CONFIG_SYS_MAXARGS, 0,  do_env_grep,
 	"search environment variables",
+#ifdef CONFIG_REGEX
+	"[-e] [-n | -v | -b] string ...\n"
+#else
 	"[-n | -v | -b] string ...\n"
+#endif
 	"    - list environment name=value pairs matching 'string'\n"
+#ifdef CONFIG_REGEX
+	"      \"-e\": enable regular expressions;\n"
+#endif
 	"      \"-n\": search variable names; \"-v\": search values;\n"
 	"      \"-b\": search both names and values (default)",
 	var_complete
diff --git a/include/search.h b/include/search.h
index d06a201..d9ac8df 100644
--- a/include/search.h
+++ b/include/search.h
@@ -129,7 +129,8 @@ extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
 #define H_MATCH_DATA	(1 << 5) /* search/grep data = variable values	     */
 #define H_MATCH_BOTH	(H_MATCH_KEY | H_MATCH_DATA) /* search/grep both     */
 #define H_MATCH_IDENT	(1 << 6) /* search for indentical strings	     */
-#define H_MATCH_SUBSTR  (1 << 7) /* search for substring matches	     */
-#define H_MATCH_METHOD	(H_MATCH_IDENT | H_MATCH_SUBSTR)
+#define H_MATCH_SUBSTR	(1 << 7) /* search for substring matches	     */
+#define H_MATCH_REGEX	(1 << 8) /* search for regular expression matches    */
+#define H_MATCH_METHOD	(H_MATCH_IDENT | H_MATCH_SUBSTR | H_MATCH_REGEX)
 
 #endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 8ad56ec..e0d3a19 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -57,6 +57,7 @@
 #include <env_callback.h>
 #include <env_flags.h>
 #include <search.h>
+#include <slre.h>
 
 /*
  * [Aho,Sethi,Ullman] Compilers: Principles, Techniques and Tools, 1986
@@ -540,7 +541,7 @@ static int cmpkey(const void *p1, const void *p2)
 	return (strcmp(e1->key, e2->key));
 }
 
-static int match_string(int flag, const char * str, const char * pat)
+static int match_string(int flag, const char *str, const char *pat, void *priv)
 {
 	switch (flag & H_MATCH_METHOD) {
 	case H_MATCH_IDENT:
@@ -551,6 +552,17 @@ static int match_string(int flag, const char * str, const char * pat)
 		if (strstr(str, pat))
 			return 1;
 		break;
+#ifdef CONFIG_REGEX
+	case H_MATCH_REGEX:
+		{
+			struct slre *slrep = (struct slre *)priv;
+			struct cap caps[slrep->num_caps + 2];
+
+			if (slre_match(slrep, str, strlen(str), caps))
+				return 1;
+		}	
+		break;
+#endif
 	default:
 		printf("## ERROR: unsupported match method: 0x%02x\n",
 			flag & H_MATCH_METHOD);
@@ -563,14 +575,25 @@ static int match_entry(ENTRY *ep, int flag,
 		 int argc, char * const argv[])
 {
 	int arg;
+	void *priv = NULL;
 
 	for (arg = 1; arg < argc; ++arg) {
+#ifdef CONFIG_REGEX
+		struct slre slre;
+
+		if (slre_compile(&slre, argv[arg]) == 0) {
+			printf("Error compiling regex: %s\n", slre.err_str);
+			return 0;
+		}
+
+		priv = (void *)&slre;
+#endif
 		if (flag & H_MATCH_KEY) {
-			if (match_string(flag, ep->key, argv[arg]))
+			if (match_string(flag, ep->key, argv[arg], priv))
 				return 1;
 		}
 		if (flag & H_MATCH_DATA) {
-			if (match_string(flag, ep->data, argv[arg]))
+			if (match_string(flag, ep->data, argv[arg], priv))
 				return 1;
 		}
 	}
-- 
1.8.1.4

[U-Boot] [PATCH 6/9] setexpr: simplify code, improve help message

[Wolfgang Denk]

Simplify the argument checking for the "setexpr" command.  This is
done mainly to make future extensions easier.

Also improve the help message for the one argument version of the
command - this does not "load an address", but a value, which in
this context may be a plain number or a pointer dereference.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_setexpr.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/common/cmd_setexpr.c b/common/cmd_setexpr.c
index 7a38e94..ccd87f4 100644
--- a/common/cmd_setexpr.c
+++ b/common/cmd_setexpr.c
@@ -56,22 +56,26 @@ static int do_setexpr(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	ulong value;
 	int w;
 
-	/* Validate arguments */
-	if (argc != 5 && argc != 3)
-		return CMD_RET_USAGE;
-	if (argc == 5 && strlen(argv[3]) != 1)
+	if (argc < 3)
 		return CMD_RET_USAGE;
 
 	w = cmd_get_data_size(argv[0], 4);
 
 	a = get_arg(argv[2], w);
 
+	/* plain assignment: "setexpr name value" */
 	if (argc == 3) {
 		setenv_hex(argv[1], a);
-
 		return 0;
 	}
 
+	/* standard operators: "setexpr name val1 op val2" */
+	if (argc != 5)
+		return CMD_RET_USAGE;
+
+	if (strlen(argv[3]) != 1)
+		return CMD_RET_USAGE;
+
 	b = get_arg(argv[4], w);
 
 	switch (argv[3][0]) {
@@ -117,6 +121,6 @@ U_BOOT_CMD(
 	"      express specified by <op>.  <op> can be &, |, ^, +, -, *, /, %\n"
 	"      size argument is only meaningful if value1 and/or value2 are\n"
 	"      memory addresses (*)\n"
-	"setexpr[.b, .w, .l] name *value\n"
-	"    - load a memory address into a variable"
+	"setexpr[.b, .w, .l] name [*]value\n"
+	"    - load a value into a variable"
 );
-- 
1.8.1.4

[U-Boot] [PATCH 7/9] setexpr: add regex substring matching and substitution

[Wolfgang Denk]

Add "setexpr name gsub r s [t]" and "setexpr name sub r s [t]"
commands which implement substring matching for the regular
expression <r> in the string <t>, and substitution of the string <s>.
The result is assigned to the environment variable <name>.  If <t> is
not supplied, the previous value of <name> is used instead.  "gsub"
performs global substitution, while "sub" will replace only the first
substring.

Both commands are closely modeled after the gawk functions with the
same names.

Examples:

- Generate broadcast address by substituting the last two numbers of
  the IP address by "255.255":

  	=> print ipaddr
	ipaddr=192.168.1.104
	=> setexpr broadcast sub "(.*\\.).*\\..*" "\\1255.255" $ipaddr
	broadcast=192.168.255.255

- Depending on keyboard configuration (German vs. US keyboard) a
  barcode scanner may initialize the MAC address as C0:E5:4E:02:06:DC
  or as C0>E5>4E>02>06>DC.  Make sure we always have a correct value:

	=> print ethaddr
	ethaddr=C0>E5>4E>02>06>DC
	=> setexpr ethaddr gsub > :
	ethaddr=C0:E5:4E:02:06:DC

- Do the same, but substitute one step at a time in a loop until no
  futher matches:

	=> setenv ethaddr C0>E5>4E>02>06>DC
	=> while setexpr ethaddr sub > :
	> do
	> echo -----
	> done
	ethaddr=C0:E5>4E>02>06>DC
	-----
	ethaddr=C0:E5:4E>02>06>DC
	-----
	ethaddr=C0:E5:4E:02>06>DC
	-----
	ethaddr=C0:E5:4E:02:06>DC
	-----
	ethaddr=C0:E5:4E:02:06:DC
	-----
	C0:E5:4E:02:06:DC: No match
	=> print ethaddr
	ethaddr=C0:E5:4E:02:06:DC

etc.

To enable this feature, the CONFIG_REGEX option has to be defined in
the board config file.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_setexpr.c | 280 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 277 insertions(+), 3 deletions(-)

diff --git a/common/cmd_setexpr.c b/common/cmd_setexpr.c
index ccd87f4..bfeec29 100644
--- a/common/cmd_setexpr.c
+++ b/common/cmd_setexpr.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2008 Freescale Semiconductor, Inc.
+ * Copyright 2013 Wolfgang Denk <wd@denx.de>
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -50,13 +51,263 @@ static ulong get_arg(char *s, int w)
 	}
 }
 
+#ifdef CONFIG_REGEX
+
+#include <slre.h>
+
+#define SLRE_BUFSZ	16384
+#define SLRE_PATSZ	4096
+
+/*
+ * memstr - Find the first substring in memory
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ *
+ * Similar to and based on strstr(),
+ * but strings do not need to be NUL terminated.
+ */
+static char *memstr(const char *s1, int l1, const char *s2, int l2)
+{
+	if (!l2)
+		return (char *) s1;
+
+	while (l1 >= l2) {
+		l1--;
+		if (!memcmp(s1, s2, l2))
+			return (char *) s1;
+		s1++;
+	}
+	return NULL;
+}
+
+static char *substitute(char *string,	/* string buffer */
+			int *slen,	/* current string length */
+			int ssize,	/* string bufer size */
+			const char *old,/* old (replaced) string */
+			int olen,	/* length of old string */
+			const char *new,/* new (replacement) string */
+			int nlen)	/* length of new string */
+{
+	char *p = memstr(string, *slen, old, olen);
+
+	if (p == NULL)
+		return NULL;
+
+	debug("## Match at pos %ld: match len %d, subst len %d\n",
+		(long)(p - string), olen, nlen);
+
+	/* make sure replacement matches */
+	if (*slen + nlen - olen > ssize) {
+		printf("## error: substitution buffer overflow\n");
+		return NULL;
+	}
+
+	/* move tail if needed */
+	if (olen != nlen) {
+		int tail, len;
+
+		len = (olen > nlen) ? olen : nlen;
+
+		tail = ssize - (p + len - string);
+
+		debug("## tail len %d\n", tail);
+
+		memmove(p + nlen, p + olen, tail);
+	}
+
+	/* insert substitue */
+	memcpy(p, new, nlen);
+
+	*slen += nlen - olen;
+
+	return p + nlen;
+}
+
+/*
+ * Perform regex operations on a environment variable
+ *
+ * Returns 0 if OK, 1 in case of errors.
+ */
+static int regex_sub(const char *name,
+	const char *r, const char *s, const char *t,
+	int global)
+{
+	struct slre slre;
+	char data[SLRE_BUFSZ];
+	char *datap = data;
+	const char *value;
+	int res, len, nlen, loop;
+
+	if (name == NULL)
+		return 1;
+
+	if (slre_compile(&slre, r) == 0) {
+		printf("Error compiling regex: %s\n", slre.err_str);
+		return 1;
+	}
+
+	if (t == NULL) {
+		value = getenv(name);
+
+		if (value == NULL) {
+			printf("## Error: variable \"%s\" not defined\n", name);
+			return 1;
+		}
+		t = value;
+	}
+
+	debug("REGEX on %s=%s\n", name, t);
+	debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n",
+		r, s ? s : "<NULL>", global);
+
+	len = strlen(t);
+	if (len + 1 > SLRE_BUFSZ) {
+		printf("## error: subst buffer overflow: have %d, need %d\n",
+			SLRE_BUFSZ, len + 1);
+		return 1;
+	}
+
+	strcpy(data, t);
+
+	if (s == NULL)
+		nlen = 0;
+	else
+		nlen = strlen(s);
+
+	for (loop = 0; ;loop++) {
+		struct cap caps[slre.num_caps + 2];
+		char nbuf[SLRE_PATSZ];
+		const char *old;
+		char *np;
+		int i, olen;
+
+		(void) memset(caps, 0, sizeof(caps));
+
+		res = slre_match(&slre, datap, len, caps);
+
+		debug("Result: %d\n", res);
+
+		for (i = 0; i < slre.num_caps; i++) {
+			if (caps[i].len > 0) {
+				debug("Substring %d: [%.*s]\n", i,
+					caps[i].len, caps[i].ptr);
+			}
+		}
+
+		if (res == 0) {
+			if (loop == 0) {
+				printf("%s: No match\n", t);
+				return 1;
+			} else {
+				break;
+			}
+		}
+
+		debug("## MATCH ## %s\n", data);
+
+		if (s == NULL) {
+			printf("%s=%s\n", name, t);
+			return 1;
+		}
+
+		old = caps[0].ptr;
+		olen = caps[0].len;
+
+		if (nlen + 1 >= SLRE_PATSZ) {
+			printf("## error: pattern buffer overflow: have %d, need %d\n",
+				SLRE_BUFSZ, nlen + 1);
+			return 1;
+		}
+		strcpy(nbuf, s);
+
+		debug("## SUBST(1) ## %s\n", nbuf);
+
+		/*
+		 * Handle back references
+		 *
+		 * Support for \0 ... \9, where \0 is the
+		 * whole matched pattern (similar to &).
+		 *
+		 * Implementation is a bit simpleminded as
+		 * backrefs are substituted sequentially, one
+		 * by one.  This will lead to somewhat
+		 * unexpected results if the replacement
+		 * strings contain any \N strings then then
+		 * may get substitued, too.  We accept this
+		 * restriction for the sake of simplicity.
+		 */
+		for (i = 0; i < 10; ++i) {
+			char backref[2] = {
+				'\\',
+				'0',
+			};
+
+			if (caps[i].len == 0)
+				break;
+
+			backref[1] += i;
+
+			debug("## BACKREF %d: replace \"%.*s\" by \"%.*s\" in \"%s\"\n",
+				i,
+				2, backref,
+				caps[i].len, caps[i].ptr,
+				nbuf);
+
+			for (np = nbuf;;) {
+				char *p = memstr(np, nlen, backref, 2);
+
+				if (p == NULL)
+					break;
+
+				np = substitute(np, &nlen,
+					SLRE_PATSZ,
+					backref, 2,
+					caps[i].ptr, caps[i].len);
+
+				if (np == NULL)
+					return 1;
+			}
+		}
+		debug("## SUBST(2) ## %s\n", nbuf);
+
+		datap = substitute(datap, &len, SLRE_BUFSZ,
+				old, olen,
+				nbuf, nlen);
+
+		if (datap == NULL)
+			return 1;
+
+		debug("## REMAINDER: %s\n", datap);
+
+		debug("## RESULT: %s\n", data);
+
+		if (!global)
+			break;
+	}
+	debug("## FINAL (now setenv()) :  %s\n", data);
+
+	printf("%s=%s\n", name, data);
+
+	return setenv(name, data);
+}
+#endif
+
 static int do_setexpr(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	ulong a, b;
 	ulong value;
 	int w;
 
-	if (argc < 3)
+	/*
+	 * We take 3, 5, or 6 arguments:
+	 * 3 : setexpr name value
+	 * 5 : setexpr name val1 op val2
+	 *     setexpr name [g]sub r s
+	 * 6 : setexpr name [g]sub r s t
+	 */
+
+	/* > 6 already tested by max command args */
+	if ((argc < 3) || (argc == 4))
 		return CMD_RET_USAGE;
 
 	w = cmd_get_data_size(argv[0], 4);
@@ -69,6 +320,19 @@ static int do_setexpr(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 0;
 	}
 
+	/* 5 or 6 args (6 args only with [g]sub) */
+#ifdef CONFIG_REGEX
+	/*
+	 * rexep handling: "setexpr name [g]sub r s [t]"
+	 * with 5 args, "t" will be NULL
+	 */
+	if (strcmp(argv[2], "gsub") == 0)
+		return regex_sub(argv[1], argv[3], argv[4], argv[5], 1);
+
+	if (strcmp(argv[2], "sub") == 0)
+		return regex_sub(argv[1], argv[3], argv[4], argv[5], 0);
+#endif
+
 	/* standard operators: "setexpr name val1 op val2" */
 	if (argc != 5)
 		return CMD_RET_USAGE;
@@ -114,13 +378,23 @@ static int do_setexpr(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 U_BOOT_CMD(
-	setexpr, 5, 0, do_setexpr,
+	setexpr, 6, 0, do_setexpr,
 	"set environment variable as the result of eval expression",
 	"[.b, .w, .l] name [*]value1 <op> [*]value2\n"
 	"    - set environment variable 'name' to the result of the evaluated\n"
-	"      express specified by <op>.  <op> can be &, |, ^, +, -, *, /, %\n"
+	"      expression specified by <op>.  <op> can be &, |, ^, +, -, *, /, %\n"
 	"      size argument is only meaningful if value1 and/or value2 are\n"
 	"      memory addresses (*)\n"
 	"setexpr[.b, .w, .l] name [*]value\n"
 	"    - load a value into a variable"
+#ifdef CONFIG_REGEX
+	"\n"
+	"setexpr name gsub r s [t]\n"
+	"    - For each substring matching the regular expression <r> in the\n"
+	"      string <t>, substitute the string <s>.  The result is\n"
+	"      assigned to <name>.  If <t> is not supplied, use the old\n"
+	"      value of <name>\n"
+	"setexpr name sub r s [t]\n"
+	"    - Just like gsub(), but replace only the first matching substring"
+#endif
 );
-- 
1.8.1.4

[U-Boot] [PATCH 7/9] setexpr: add regex substring matching and substitution

[Marek Vasut]

Dear Wolfgang Denk,

> Add "setexpr name gsub r s [t]" and "setexpr name sub r s [t]"
> commands which implement substring matching for the regular
> expression <r> in the string <t>, and substitution of the string <s>.
> The result is assigned to the environment variable <name>.  If <t> is
> not supplied, the previous value of <name> is used instead.  "gsub"
> performs global substitution, while "sub" will replace only the first
> substring.
> 
[...]

> +static char *memstr(const char *s1, int l1, const char *s2, int l2)
> +{
> +	if (!l2)
> +		return (char *) s1;
> +
> +	while (l1 >= l2) {
> +		l1--;
> +		if (!memcmp(s1, s2, l2))
> +			return (char *) s1;
> +		s1++;
> +	}
> +	return NULL;
> +}

Will this memstr() not crash with wrong parameters? Or is this safe?

btw. there is a checkpatch issue and two warnings:
ERROR: space required after that ';' (ctx:WxV)
#294: FILE: common/cmd_setexpr.c:177:
+       for (loop = 0; ;loop++) {
                       ^

I think the warnings are ok.

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/9] setexpr: add regex substring matching and substitution

[Wolfgang Denk]

Dear Marek,

In message <201303222337.00146.marex@denx.de> you wrote:
>
> > +static char *memstr(const char *s1, int l1, const char *s2, int l2)
> > +{
> > +	if (!l2)
> > +		return (char *) s1;
> > +
> > +	while (l1 >= l2) {
> > +		l1--;
> > +		if (!memcmp(s1, s2, l2))
> > +			return (char *) s1;
> > +		s1++;
> > +	}
> > +	return NULL;
> > +}
> 
> Will this memstr() not crash with wrong parameters? Or is this safe?

I'm not sure thwat you mean by "will it crash" / "is this safe"?

Of course you can pass bogus parameters that will cause issues - in
the very same way as you can do it with existing standard functions
like memcmp() or strstr().

Or do you see any specific problems in above code.

> btw. there is a checkpatch issue and two warnings:
> ERROR: space required after that ';' (ctx:WxV)
> #294: FILE: common/cmd_setexpr.c:177:
> +       for (loop = 0; ;loop++) {

Will fix that (though I'm not sure if the result is easier to read).

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 explanation requiring the fewest assumptions is the  most  likely
to be correct.                                    -- William of Occam

[U-Boot] [PATCH 8/9] m28evk: enable "env grep" and regexp support

[Wolfgang Denk]

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Marek Vasut <marek.vasut@gmail.com>
---
 include/configs/m28evk.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/m28evk.h b/include/configs/m28evk.h
index f2725cc..4540bf0 100644
--- a/include/configs/m28evk.h
+++ b/include/configs/m28evk.h
@@ -65,6 +65,7 @@
 #define	CONFIG_CMD_EXT2
 #define	CONFIG_CMD_FAT
 #define	CONFIG_CMD_GPIO
+#define CONFIG_CMD_GREPENV
 #define	CONFIG_CMD_I2C
 #define	CONFIG_CMD_MII
 #define	CONFIG_CMD_MMC
@@ -77,6 +78,8 @@
 #define	CONFIG_CMD_SPI
 #define	CONFIG_CMD_USB
 
+#define CONFIG_REGEX			/* Enable regular expression support */
+
 /*
  * Memory configurations
  */
-- 
1.8.1.4

[U-Boot] [PATCH 9/9] amcc-common.h: enable support for "env grep", "setexpr", and regex.

[Wolfgang Denk]

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Stefan Roese <sr@denx.de>
---
 include/configs/amcc-common.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/configs/amcc-common.h b/include/configs/amcc-common.h
index f2f522d..80e5735 100644
--- a/include/configs/amcc-common.h
+++ b/include/configs/amcc-common.h
@@ -71,6 +71,7 @@
 #define CONFIG_CMD_DIAG
 #define CONFIG_CMD_EEPROM
 #define CONFIG_CMD_ELF
+#define CONFIG_CMD_GREPENV
 #define CONFIG_CMD_I2C
 #define CONFIG_CMD_IRQ
 #define CONFIG_CMD_MII
@@ -78,6 +79,7 @@
 #define CONFIG_CMD_NFS
 #define CONFIG_CMD_PING
 #define CONFIG_CMD_REGINFO
+#define CONFIG_CMD_SETEXPR
 
 /*
  * Miscellaneous configurable options
@@ -108,13 +110,14 @@
 #define CONFIG_MX_CYCLIC		/* enable mdc/mwc commands      */
 #define CONFIG_ZERO_BOOTDELAY_CHECK	/* check for keypress on bootdelay==0 */
 #define CONFIG_VERSION_VARIABLE 	/* include version env variable */
-#define CONFIG_SYS_CONSOLE_INFO_QUIET		/* don't print console @ startup*/
+#define CONFIG_SYS_CONSOLE_INFO_QUIET	/* don't print console @ startup*/
 
-#define CONFIG_SYS_HUSH_PARSER			/* Use the HUSH parser		*/
+#define CONFIG_SYS_HUSH_PARSER		/* Use the HUSH parser		*/
 
 #define CONFIG_LOADS_ECHO		/* echo on for serial download	*/
-#define CONFIG_SYS_LOADS_BAUD_CHANGE		/* allow baudrate change	*/
+#define CONFIG_SYS_LOADS_BAUD_CHANGE	/* allow baudrate change	*/
 
+#define CONFIG_REGEX			/* Enable regular expression support */
 /*
  * BOOTP options
  */
-- 
1.8.1.4

[U-Boot] [PATCH 0/9] Add Regular Expressions Support

[Wolfgang Denk]

Hi,

In message <1363988699-6410-1-git-send-email-wd@denx.de> you wrote:
> The following patch series adds the SLRE "Super Light Regular
> Expression" library and uses this to add regex support for 
> the "env grep" (aka "grepenv") command, and new functions (or
> operators?) "gsub" and "sub" to the "setexpr" command.

I should probably add:

- This patch series has been compile-tested (and found to be clean
  with ELDK v5.3) on all of the following boards:

  ARM: m28evk

  PPC4xx: acadia bamboo bluestone bubinga canyonlands dlvision-10g
  dlvision ebony gdppc440etx icon intip io io64 iocon katmai kilauea
  luan makalu neo ocotea redwood sequoia t3corp taihu taishan walnut
  yosemite yucca

- Runtime / functional testing has been done mostly on PPC (Sequoia
  board).

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
PROGRAM - n.  A magic spell cast over a computer  allowing it to turn
one's input into error messages.
v. tr. - To engage in a pastime similar to banging one's head against
a wall, but with fewer opportunities for reward.

[U-Boot] [PATCH 0/9] Add Regular Expressions Support

[Otavio Salvador]

On Fri, Mar 22, 2013 at 6:44 PM, Wolfgang Denk <wd@denx.de> wrote:
> The following patch series adds the SLRE "Super Light Regular
> Expression" library and uses this to add regex support for
> the "env grep" (aka "grepenv") command, and new functions (or
> operators?) "gsub" and "sub" to the "setexpr" command.
>
> The rework to "env grep" also fixed a few bugs (which cuased it to
> dump always _all_ environment varioables on some systems), and adds
> the capability to grep in either the variable name, or the value, or
> in both (the old version olways did the latter).  Instead of special
> functions we now use common code (i. e. hexport_r()) for the variable
> lookup, which gives us sorted output as a free additional benefit.

It is awesome :-)

I just found some indenting formating changing in the code mixed with
code change, it'd be good to keep those in specific patches. It is
easy to see in the last patch of serie, for example.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH 0/9] Add Regular Expressions Support

[Wolfgang Denk]

Dear Otavio,

In message <CAP9ODKoDSwBRp7sweLwX-7LrQyeHaq0JP1fhe2GJ86E=_ev0fA@mail.gmail.com> you wrote:
>
> > The following patch series adds the SLRE "Super Light Regular
> > Expression" library and uses this to add regex support for
> > the "env grep" (aka "grepenv") command, and new functions (or
> > operators?) "gsub" and "sub" to the "setexpr" command.
...
> It is awesome :-)

I'm glad you like it.

> I just found some indenting formating changing in the code mixed with
> code change, it'd be good to keep those in specific patches. It is
> easy to see in the last patch of serie, for example.

Yes, I will do this.  Thanks!

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 universe contains any amount of horrible ways  to  be  woken  up,
such as the noise of the mob breaking down the front door, the scream
of fire engines, or the realization that today is the Monday which on
Friday night was a comfortably long way off.
                                 - Terry Pratchett, _Moving Pictures_

[U-Boot] [PATCH] env: fix "env ask" command

[Wolfgang Denk]

The "env ask" traditionally uses a somewhat awkward syntax:

	env ask name [message ...] [size]

So far, when a mesage was given, you always also had to enter a size.
If you forgot to do that, the command would terminate without any
indication of the problem.

To avoid incompatible changes of the interface, we now check the last
argument if it can be converted into a decimal number.  If this is the
case, we assume it is a size; otherwise we treat it as part of the
message.

Also, add a space after the message fore easier reading,
and clean up help mesage.

Signed-off-by: Wolfgang Denk <wd@denx.de>
---
 common/cmd_nvedit.c | 56 +++++++++++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 7633f0c..5f30f86 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -325,41 +325,50 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char message[CONFIG_SYS_CBSIZE];
-	int size = CONFIG_SYS_CBSIZE - 1;
-	int i, len, pos;
+	int i, len, pos, size;
 	char *local_args[4];
+	char *endptr;
 
 	local_args[0] = argv[0];
 	local_args[1] = argv[1];
 	local_args[2] = NULL;
 	local_args[3] = NULL;
 
-	/* Check the syntax */
-	switch (argc) {
-	case 1:
+	/*
+	 * Check the syntax:
+	 *
+	 * env_ask envname [message1 ...] [size]
+	 */
+	if (argc == 1)
 		return CMD_RET_USAGE;
 
-	case 2:		/* env_ask envname */
-		sprintf(message, "Please enter '%s':", argv[1]);
-		break;
-
-	case 3:		/* env_ask envname size */
-		sprintf(message, "Please enter '%s':", argv[1]);
-		size = simple_strtoul(argv[2], NULL, 10);
-		break;
-
-	default:	/* env_ask envname message1 ... messagen size */
-		for (i = 2, pos = 0; i < argc - 1; i++) {
+	/*
+	 * We test the last argument if it can be converted
+	 * into a decimal number.  If yes, we assume it's
+	 * the size.  Otherwise we echo it as part of the
+	 * message.
+	 */
+	i = simple_strtoul(argv[argc - 1], &endptr, 10);
+	if (*endptr != '\0') {			/* no size */
+		size = CONFIG_SYS_CBSIZE - 1;
+	} else {				/* size given */
+		size = i;
+		--argc;
+	}
+	
+	if (argc <= 2) {
+		sprintf(message, "Please enter '%s': ", argv[1]);
+	} else {
+		/* env_ask envname message1 ... messagen [size] */
+		for (i = 2, pos = 0; i < argc; i++) {
 			if (pos)
 				message[pos++] = ' ';
 
 			strcpy(message + pos, argv[i]);
 			pos += strlen(argv[i]);
 		}
-
+		message[pos++] = ' ';
 		message[pos] = '\0';
-		size = simple_strtoul(argv[argc - 1], NULL, 10);
-		break;
 	}
 
 	if (size >= CONFIG_SYS_CBSIZE)
@@ -1168,14 +1177,7 @@ U_BOOT_CMD(
 	askenv,	CONFIG_SYS_MAXARGS,	1,	do_env_ask,
 	"get environment variables from stdin",
 	"name [message] [size]\n"
-	"    - get environment variable 'name' from stdin (max 'size' chars)\n"
-	"askenv name\n"
-	"    - get environment variable 'name' from stdin\n"
-	"askenv name size\n"
-	"    - get environment variable 'name' from stdin (max 'size' chars)\n"
-	"askenv name [message] size\n"
-	"    - display 'message' string and get environment variable 'name'"
-	"from stdin (max 'size' chars)"
+	"    - get environment variable 'name' from stdin (max 'size' chars)"
 );
 #endif
 
-- 
1.7.11.7

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-1-git-send-email-gerlando.falauto@keymile.com> you wrote:
> This is a resubmission (after removing remove checkpatch errors) of
> http://lists.denx.de/pipermail/u-boot/2011-September/102875.html
> 
> Here I am proposing a set of changes in the behaviour of the environment 
> import/set_to_default functions.
> 
> PATCH 1:
> Add a "new" himport_ex() function (reworking of himport_r which is now a 
> wrapper around it), which has 3 new arguments:
> 
> "nvars", "vars":, number and list of variables to take into account 
> (0 means ALL)
> 
> "apply" callback function which is in charge of checking whether a 
> variable can be overwritten, and possibly immediately apply the changes. 
> This parameter would be either set to NULL (in which case nothing should 
> change wrt to the past -- i.e. environment is blindly imported) or to 
> "env_check_apply()" function, whose code was taken away from _do_env_set().
> This would be useful, for instance, for "baudrate" or "stdin,stderr,stdout", 
> whose changes would not otherwise be effective until the next reboot.
> 
> The idea is that there should be a single place where all the checks are
> to be performed. So the same function env_check_apply() is called from 
> _do_env_set() as well (thus keeping the previous behavior).

Would you please also update the help messages for the respective
commands so we can see their correct (new) usage there?

Thanks.

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
"There is such a fine line between genius and stupidity."
- David St. Hubbins, "Spinal Tap"

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1319647072-17504-1-git-send-email-gerlando.falauto@keymile.com> you wrote:
> This is a resubmission (after removing remove checkpatch errors) of
> http://lists.denx.de/pipermail/u-boot/2011-September/102875.html
> 
> Here I am proposing a set of changes in the behaviour of the environment 
> import/set_to_default functions.
> 
> PATCH 1:
> Add a "new" himport_ex() function (reworking of himport_r which is now a 
> wrapper around it), which has 3 new arguments:
> 
> "nvars", "vars":, number and list of variables to take into account 
> (0 means ALL)

Hmmm.... I wonder how much testing you did.  For me, env import is
broken now.

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 like your game but we have to change the rules."

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Gerlando Falauto]

On 11/06/2011 11:15 PM, Wolfgang Denk wrote:
 > Dear Gerlando Falauto,
 >
 > In 
message<1319647072-17504-1-git-send-email-gerlando.falauto@keymile.com> 
  you wrote:
 >> This is a resubmission (after removing remove checkpatch errors) of
 >> http://lists.denx.de/pipermail/u-boot/2011-September/102875.html
 >>
 >> Here I am proposing a set of changes in the behaviour of the environment
 >> import/set_to_default functions.
 >>
 >> PATCH 1:
 >> Add a "new" himport_ex() function (reworking of himport_r which is now a
 >> wrapper around it), which has 3 new arguments:
 >>
 >> "nvars", "vars":, number and list of variables to take into account
 >> (0 means ALL)
 >
 > Hmmm.... I wonder how much testing you did.

I tested "env import" with and without -n. Same for "env default".
Also tested special variables.

 > For me, env import is broken now.

I am not able to see how it's obviously broken.
Perhaps you mean that when you don't provide an argument to -n, the 
following argument (e.g., -t) is interpreted as a variable name (and 
therefore consumed)? That doesn't look like an error to me.
Could you please elaborate and/or provide a test case?

Thank you,
Gerlando Falauto

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <4EB8F762.1030508@keymile.com> you wrote:
>
> I tested "env import" with and without -n. Same for "env default".
> Also tested special variables.
> 
>  > For me, env import is broken now.
> 
> I am not able to see how it's obviously broken.

It just didn't work for me.

> Perhaps you mean that when you don't provide an argument to -n, the 
> following argument (e.g., -t) is interpreted as a variable name (and 
> therefore consumed)? That doesn't look like an error to me.

Yes, this is a serious bug. Aor design problem.

The whole interface is wrong. I have just extendend "env export" to
do a similar thing. The new syntax is:

         env export [-t | -b | -c] [-s size] addr [var ...]

We should do exactly the same for "env
import", i. e. make the size parameter an option to be passed with
"-s size", and pass the names of any variables to import as additional
arguments:

	env import [-t | -b | -c] [-s size] addr [var ...]

This also greatly simplifies the implementation.

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
Fools ignore complexity. Pragmatists suffer it. Some  can  avoid  it.
Geniuses remove it.
     - Perlis's Programming Proverb #58, SIGPLAN Notices, Sept.  1982

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Gerlando Falauto]

On 11/08/2011 12:46 PM, Wolfgang Denk wrote:
> Dear Gerlando Falauto,
>
> In message<4EB8F762.1030508@keymile.com>  you wrote:
>>
>> I tested "env import" with and without -n. Same for "env default".
>> Also tested special variables.
>>
>>   >  For me, env import is broken now.
>>
>> I am not able to see how it's obviously broken.
>
> It just didn't work for me.

Honestly, I didn't try it against the latest master branch.

>> Perhaps you mean that when you don't provide an argument to -n, the
>> following argument (e.g., -t) is interpreted as a variable name (and
>> therefore consumed)? That doesn't look like an error to me.
>
> Yes, this is a serious bug. Aor design problem.
>
> The whole interface is wrong. I have just extendend "env export" to
> do a similar thing. The new syntax is:
>
>           env export [-t | -b | -c] [-s size] addr [var ...]

When/where? I can't see that in the latest master branch.

> We should do exactly the same for "env
> import", i. e. make the size parameter an option to be passed with
> "-s size", and pass the names of any variables to import as additional
> arguments:
>
> 	env import [-t | -b | -c] [-s size] addr [var ...]
>
> This also greatly simplifies the implementation.

Totally agree. Question is: won't that break all the existing scripts???

If you agree, I can post an updated patch with the same behavior for 
import. Although I am bit concerned about our scripts.

Best,
Gerlando Falauto

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Wolfgang Denk]

Dear Gerlando,

In message <4EB91ABD.90206@keymile.com> you wrote:
>
> > The whole interface is wrong. I have just extendend "env export" to
> > do a similar thing. The new syntax is:
> >
> >           env export [-t | -b | -c] [-s size] addr [var ...]
> 
> When/where? I can't see that in the latest master branch.

Ah, I didn't actually send it yet.  Done now.

> > We should do exactly the same for "env
> > import", i. e. make the size parameter an option to be passed with
> > "-s size", and pass the names of any variables to import as additional
> > arguments:
> >
> > 	env import [-t | -b | -c] [-s size] addr [var ...]
> >
> > This also greatly simplifies the implementation.
> 
> Totally agree. Question is: won't that break all the existing scripts???

Yes.  It _is_ an incompatible change of the API.  I hate it, but
mostly because I didn't chose such a format right from the beginning.

My hope is that probably not too many people use these features in
scripts - at least so far it's only omap3_beagle and omap3_mvblx that
use "env import -t $loadaddr $filesize".  It is IMO better to change
this now, instead of waiting until we have more users and the problem
hits us in another place, hard.

> If you agree, I can post an updated patch with the same behavior for 
> import. Although I am bit concerned about our scripts.

We would fix these, too.

But agreed, incompatible API changes are always a bad thing.

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
About the use of language: it is impossible to sharpen a pencil  with
a  blunt  ax.  It is equally vain to try to do it with ten blunt axes
instead.                                           -- Edsger Dijkstra

[U-Boot] [PATCH v0 0/4] env: reworking + default/import individual vars

[Holger Brunck]

Hi,

On 11/08/2011 01:47 PM, Wolfgang Denk wrote:
>>>
>>> 	env import [-t | -b | -c] [-s size] addr [var ...]
>>>
>>> This also greatly simplifies the implementation.
>>
>> Totally agree. Question is: won't that break all the existing scripts???
> 
> Yes.  It _is_ an incompatible change of the API.  I hate it, but
> mostly because I didn't chose such a format right from the beginning.
> 
> My hope is that probably not too many people use these features in
> scripts - at least so far it's only omap3_beagle and omap3_mvblx that
> use "env import -t $loadaddr $filesize".  It is IMO better to change
> this now, instead of waiting until we have more users and the problem
> hits us in another place, hard.
> 

we use it too in keymile-common.h and in the scripts we load with this command
e.g. board/keymile/scripts/develop-ppc_82xx.txt

The resulting incompatibility is indeed not very nice. I don't see big issues
for the command line and keymile-common.h where we can live with different API
for different u-boot versions. But the usage in the scripts directory is more
complicated, because then we have to make sure that uboot version x loads
scripts x and u-boot version y loads scripts y. Ok we could prepare a second
scripts directory e.g. "scripts_v2" or similar in addition to the current
scripts directory in tftpboot, but this is not very nice.

Best regards
Holger