env: reworking + default/import individual vars
cover env: reworking + default/import individual vars
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(-)
PATCH 1/4 Groundwork for generalization of env interface
--- 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(-)
common/cmd_nvedit.c +96 -56
@@ -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 ; }
common/env_common.c +13 -2
@@ -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 ;
include/environment.h +7 -0
@@ -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_ */
include/search.h +13 -0
@@ -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 */
lib/hashtable.c +50 -0
72 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH 1/4 Groundwork for generalization of env interface
Dear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]>…
Signed-off-by Gerlando Falauto <[email protected]>--- 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(-)
...
common/cmd_nvedit.c static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ · line 209
Quoted 3 lines
common/cmd_nvedit.c +2
-1-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.
common/cmd_nvedit.c int _do_env_set (int flag, int argc, char * const argv[]) · line 251
Quoted 3 lines
common/cmd_nvedit.c +2
-1-if (ep) {/* variable exists */ +if ((oldval !=NULL )/* variable exists */ + && ((flag &H_FORCE ) ==0 )) {/* and we are not forced */
Incorrect indentation.
common/cmd_nvedit.c int _do_env_set (int flag, int argc, char * const argv[]) · line 339
Quoted 4 lines
common/cmd_nvedit.c +4
-0+/* +* Set a new environment variable, +* or replace or delete an existing one. +*/
Incorrect multiline comment style.
common/cmd_nvedit.c int _do_env_set (int flag, int argc, char * const argv[]) · line 364
Quoted 2 lines
common/cmd_nvedit.c +2
-0+/* Perform requested checks. Notice how since we are overwriting +* a single variable, we need to set H_NOCLEAR */
Incorrect multiline comment style.
common/env_common.c const uchar *env_get_addr (int index) · line 176
Quoted 4 lines
void
set_default_env(const char *s)
{common/env_common.c +2
-0+/* By default, do not apply changes as they will eventually +* be applied by someone else */
Incorrect multiline comment style.
Please fix globally!
common/env_common.c void set_default_env(const char *s) · line 204
Quoted 3 lines
common/env_common.c +3
-0+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
Signature · 5 lines
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.
PATCH 1/4 Groundwork for generalization of env interface
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)
Quoted 1 line
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
PATCH 1/4 Groundwork for generalization of env interface
Dear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]> you wrote:
common/cmd_nvedit.c +2
-1-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)Quoted 2 lines
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.
Quoted 3 lines
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
Quoted 5 lines
common/env_common.c +3
-0+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
Signature · 5 lines
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
Quoted 15 lines · Wolfgang Denk
Dear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]> you wrote:
common/cmd_nvedit.c +2
-1-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)Quoted 2 lines
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.
Quoted 8 lines
Quoted 3 lines
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...
Quoted 8 lines
Quoted 5 lines
common/env_common.c +3
-0+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
Quoted 35 lines · Scott Wood
Quoted 15 lines · Wolfgang Denk
Dear Gerlando Falauto,
Quoted 8 lines · In message<[email protected]> you wrote:
common/cmd_nvedit.c +2
-1-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)Quoted 2 lines
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.
Quoted 8 lines
Quoted 3 lines
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...
Quoted 4 lines
Quoted 4 lines
common/env_common.c +3
-0+if (himport_ex (&env_htab, (char *)default_environment, +sizeof (default_environment),'\0' ,0 , +0 ,NULL , apply_function) ==0 ) {
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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! :-)
Quoted 5 lines
Documentation/CodingStylecalls 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...
Quoted 5 lines
...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
Quoted 7 lines · Gerlando Falauto
Quoted 5 lines · Scott Wood
Documentation/CodingStylecalls for something like this:if (himport_ex(&env_htab, (char *)default_environment, sizeof(default_environment), '\0', 0, 0, NULL, apply_function) == 0) {
Quoted 5 lines
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".
Quoted 7 lines
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
In reply to Gerlando FalautoQuoted 17 lines · Gerlando Falauto
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
Quoted 4 lines · Scott Wood
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):
Quoted 13 lines
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
PATCH 1/4 Groundwork for generalization of env interface
In reply to Wolfgang DenkQuoted 8 lines · Wolfgang Denk
Dear Gerlando Falauto,
Quoted 5 lines · In message<[email protected]> you wrote:
common/cmd_nvedit.c +2
-1-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)
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 3 lines
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".
Quoted 8 lines
Quoted 5 lines
common/env_common.c +3
-0+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
PATCH 1/4 Groundwork for generalization of env interface
Dear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 5 lines
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.
Quoted 3 lines
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.
Quoted 12 lines
Your answer might as well be: "forget about alignment altogether, nobody wants that, just indent it somehow".
Quoted 8 lines
Quoted 5 lines
common/env_common.c +3
-0+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?
Quoted 5 lines
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
Signature · 5 lines
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 AugustinePATCH 1/4 Groundwork for generalization of env interface
In reply to Gerlando FalautoDear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]>…
Signed-off-by Gerlando Falauto <[email protected]>--- 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
Signature · 5 lines
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.
PATCH 2/4 env: check and apply changes on delete/destroy
In reply to Gerlando Falauto--- common/cmd_nvedit.c | 2 +- include/search.h | 6 ++++-- lib/hashtable.c | 18 ++++++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-)
common/cmd_nvedit.c +1 -1
@@ -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; }
include/search.h +4 -2
@@ -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);
lib/hashtable.c +12 -6
@@ -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 ;
Signature · 1 line
1.7.1
PATCH 2/4 env: check and apply changes on delete/destroy
Dear Gerlando Falauto,
Quoted 6 lines · In message <[email protected]>…
Signed-off-by Gerlando Falauto <[email protected]>--- 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?
include/search.h struct hsearch_data { · line 72
Quoted 3 lines
include/search.h +2
-1-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.
lib/hashtable.c void hdestroy_r(struct hsearch_data *htab) · line 161
Quoted 2 lines
lib/hashtable.c +2
-0+if (apply !=NULL ) { +/* deletion is always forced */
Um.... why is this the case?
Best regards,
Wolfgang Denk
Signature · 3 lines
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
Quoted 1 line
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 <[email protected]>
PATCH 3/4 env: implement selective "env default"
In reply to Gerlando Falauto--- 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(-)
README +2 -0
@@ -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
common/cmd_nvedit.c +36 -6
55 lines not on this page · raw message
common/env_common.c +14 -0
@@ -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.
include/config_cmd_all.h +1 -0
@@ -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 */
include/environment.h +5 -0
@@ -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);
Signature · 1 line
1.7.1
PATCH 3/4 env: implement selective "env default"
Dear Gerlando Falauto,
Quoted 8 lines · In message <[email protected]>…
Signed-off-by Gerlando Falauto <[email protected]>--- 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?
...
Quoted 1 line
+#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
Signature · 4 lines
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.
PATCH 4/4 env: implement "env import -n var[, var...]"
In reply to Gerlando FalautoImplemented selective importing of variables in
env import
--- common/cmd_nvedit.c | 35 ++++++++++++++++++++++++++++++----- 1 files changed, 30 insertions(+), 5 deletions(-)
common/cmd_nvedit.c +30 -5
@@ -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 ; }
Signature · 1 line
1.7.1
PATCH 1/1 env: fix "env ask" command
In reply to Gerlando FalautoThe "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.
---
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(-)
common/cmd_nvedit.c +28 -18
@@ -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 )
Signature · 1 line
1.7.6.4
PATCH 1/2 Add SLRE - Super Light Regular Expression library
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.
---
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
include/slre.h new file +100 -0
@@ -0,0 +1,100 @@ +/* +* Copyright (c) 2004-2005 Sergey Lyubka <[email protected]> +* 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 */
lib/slre.c new file +709 -0
709 lines not on this page · raw message
Signature · 1 line
1.7.6.4
PATCH 2/2 env: add regex support for environment variables
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.]
--- common/cmd_nvedit.c | 288 +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/Makefile | 1 + 2 files changed, 289 insertions(+), 0 deletions(-)
common/cmd_nvedit.c +288 -0
@@ -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
lib/Makefile +1 -0
@@ -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
Signature · 1 line
1.7.6.4
PATCH 2/2 env: add regex support for environment variables
Hi Wolfgang,
this really is an interesting addition!
Quoted 9 lines
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.
Quoted 6 lines
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)"?
Quoted 9 lines
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
Signature · 6 lines
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.dePATCH 1/1 env: fix "env ask" command
In reply to Wolfgang DenkThe "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.
--- common/cmd_nvedit.c | 56 +++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 27 deletions(-)
common/cmd_nvedit.c +29 -27
@@ -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
Signature · 1 line
1.7.11.7
cover Add Regular Expressions Support
In reply to Wolfgang DenkThe 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
Signature · 1 line
1.8.1.4
PATCH 1/9 hashtable: preparations to use hexport_r() for "env grep"
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.
--- common/cmd_nvedit.c | 10 +++++++--- include/search.h | 8 +++++++- lib/hashtable.c | 32 ++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 12 deletions(-)
common/cmd_nvedit.c +7 -3
@@ -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 ;
include/search.h +7 -1
@@ -22,7 +22,7 @@/* * Based on code from uClibc-0.9.30.3 * Extensions for use within U-Boot -* Copyright (C) 2010 Wolfgang Denk <[email protected]> +* Copyright (C) 2010-2013 Wolfgang Denk <[email protected]> */ #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 */
lib/hashtable.c +24 -8
@@ -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 <[email protected]> +* Copyright (C) 2010-2013 Wolfgang Denk <[email protected]> * * 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 ;
Signature · 1 line
1.8.1.4
PATCH 2/9 "env grep" - reimplement command using hexport_r()
In reply to Wolfgang DenkAlso drop hstrstr_r() which is not needed any more. The new code is way more flexible.
--- common/cmd_nvedit.c | 30 ++++++++++--------------- include/search.h | 6 ----- lib/hashtable.c | 64 +++++++++++++++++++++++------------------------------ 3 files changed, 40 insertions(+), 60 deletions(-)
common/cmd_nvedit.c +12 -18
@@ -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 */
include/search.h +0 -6
@@ -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,
lib/hashtable.c +28 -36
87 lines not on this page · raw message
Signature · 1 line
1.8.1.4
PATCH 3/9 "env grep" - add options to grep in name, value, or both.
In reply to Wolfgang DenkAdd 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 -- -
--- common/cmd_nvedit.c | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-)
common/cmd_nvedit.c +33 -5
@@ -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
Signature · 1 line
1.8.1.4
PATCH 4/9 Add SLRE - Super Light Regular Expression library
In reply to Wolfgang DenkDownloaded 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).
--- 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
README +7 -0
@@ -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
include/slre.h new file +100 -0
100 lines not on this page · raw message
lib/Makefile +1 -0
@@ -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
lib/slre.c new file +724 -0
724 lines not on this page · raw message
Signature · 1 line
1.8.1.4
PATCH 5/9 "env grep" - add support for regular expression matches
In reply to Wolfgang DenkWhen 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.
--- common/cmd_nvedit.c | 29 +++++++++++++++++++++++------ include/search.h | 5 +++-- lib/hashtable.c | 29 ++++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 11 deletions(-)
common/cmd_nvedit.c +23 -6
@@ -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
include/search.h +3 -2
@@ -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 */
lib/hashtable.c +26 -3
@@ -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 ; } }
Signature · 1 line
1.8.1.4
PATCH 6/9 setexpr: simplify code, improve help message
In reply to Wolfgang DenkSimplify 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.
--- common/cmd_setexpr.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
common/cmd_setexpr.c +11 -7
@@ -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" );
Signature · 1 line
1.8.1.4
PATCH 7/9 setexpr: add regex substring matching and substitution
In reply to Wolfgang DenkAdd "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.
--- common/cmd_setexpr.c | 280 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 277 insertions(+), 3 deletions(-)
common/cmd_setexpr.c +277 -3
@@ -1,5 +1,6 @@/* * Copyright 2008 Freescale Semiconductor, Inc. +* Copyright 2013 Wolfgang Denk <[email protected]> * * 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 );
Signature · 1 line
1.8.1.4
PATCH 7/9 setexpr: add regex substring matching and substitution
Dear Wolfgang Denk,
Quoted 8 lines
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.
[...]
common/cmd_setexpr.c static ulong get_arg(char *s, int w) · line 81
Quoted 13 lines
common/cmd_setexpr.c +13
-0+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
PATCH 7/9 setexpr: add regex substring matching and substitution
Dear Marek,
Quoted 16 lines · In message <[email protected]> you wrote:
Quoted 4 lines
common/cmd_setexpr.c +3
-0+static char *memstr (const char *s1,int l1,const char *s2,int l2) +{ +if (!l2)common/cmd_setexpr.c static ulong get_arg(char *s, int w) · line 81
Quoted 10 lines
common/cmd_setexpr.c +10
-0+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.
Quoted 4 lines
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
Signature · 5 lines
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
PATCH 8/9 m28evk: enable "env grep" and regexp support
In reply to Wolfgang Denk--- include/configs/m28evk.h | 3 +++ 1 file changed, 3 insertions(+)
include/configs/m28evk.h +3 -0
@@ -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 */
Signature · 1 line
1.8.1.4
PATCH 9/9 amcc-common.h: enable support for "env grep", "setexpr", and regex.
In reply to Wolfgang Denk--- include/configs/amcc-common.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
include/configs/amcc-common.h +6 -3
@@ -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 */
Signature · 1 line
1.8.1.4
cover Add Regular Expressions Support
In reply to Wolfgang DenkHi,
Quoted 4 lines · In message <[email protected]> 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
Signature · 7 lines
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.
cover Add Regular Expressions Support
In reply to Wolfgang DenkQuoted 11 lines · 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.
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
cover Add Regular Expressions Support
Dear Otavio,
Quoted 5 lines · In message <CAP9ODKoDSwBRp7sweLwX-7LrQyeHaq0JP1fhe2GJ86E=_ev0fA@mail.gmail.com>…
Quoted 5 lines
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.
...
Quoted 1 line
It is awesome :-)
I'm glad you like it.
Quoted 3 lines
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
Signature · 8 lines
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_cover env: reworking + default/import individual vars
In reply to Gerlando FalautoDear Gerlando Falauto,
Quoted 21 lines · In message <[email protected]>…
This is a resubmission (after removing remove checkpatch errors) of
http://lists.denx.de/pipermail/u-boot/2011-September/102875.htmlHere 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 ofhimport_rwhich 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.
Quoted 3 lines
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
Signature · 5 lines
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"
cover env: reworking + default/import individual vars
In reply to Gerlando FalautoDear Gerlando Falauto,
Quoted 6 lines · In message <[email protected]>…
This is a resubmission (after removing remove checkpatch errors) of
http://lists.denx.de/pipermail/u-boot/2011-September/102875.htmlHere I am proposing a set of changes in the behaviour of the environment import/set_to_default functions.
Quoted 6 lines
PATCH 1:
Add a "new"himport_ex()function (reworking ofhimport_rwhich 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
Signature · 4 lines
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."
cover env: reworking + default/import individual vars
Quoted 3 lines · Wolfgang Denk
Dear Gerlando Falauto,
In
message<[email protected]>
Quoted 6 lines · you
Quoted 6 lines
This is a resubmission (after removing remove checkpatch errors) of
http://lists.denx.de/pipermail/u-boot/2011-September/102875.htmlHere I am proposing a set of changes in the behaviour of the environment import/set_to_default functions.
Quoted 8 lines
Quoted 6 lines
PATCH 1:
Add a "new"himport_ex()function (reworking ofhimport_rwhich 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.
Quoted 1 line
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
cover env: reworking + default/import individual vars
Dear Gerlando Falauto,
Quoted 7 lines · In message <[email protected]> you wrote:
I tested "env import" with and without -n. Same for "env default". Also tested special variables.
Quoted 1 line
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.
Quoted 3 lines
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
Signature · 6 lines
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. 1982cover env: reworking + default/import individual vars
Quoted 4 lines · Wolfgang Denk
Dear Gerlando Falauto,
Quoted 1 line · In message<[email protected]> you wrote:
Quoted 8 lines
Quoted 6 lines
I tested "env import" with and without -n. Same for "env default". Also tested special variables.
Quoted 1 line
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.
Quoted 10 lines
Quoted 3 lines
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.
Quoted 8 lines
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
cover env: reworking + default/import individual vars
Dear Gerlando,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 5 lines
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.
Quoted 10 lines
Quoted 8 lines
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.
Quoted 2 lines
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
Signature · 6 lines
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
cover env: reworking + default/import individual vars
Hi,
Quoted 7 lines · Wolfgang Denk
Quoted 6 lines
Quoted 4 lines
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???
Quoted 9 lines
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_beagleandomap3_mvblxthat 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