[U-Boot] [PATCH] Handle environment variables like commands
The command interpreter checks always if an environment variable for that name exists and in this case the content of the variable is executed. It becomes possible to redefine all U-Boot commands. A new "builtin" command is added to be able to run builtin U-boot commands even if they are redefined.
Signed-off-by : Stefano Babic sbabic@denx.de --- common/hush.c | 9 ++++++++- common/main.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/common/hush.c b/common/hush.c index 093c428..68f8a6a 100644 --- a/common/hush.c +++ b/common/hush.c @@ -1677,9 +1677,16 @@ static int run_pipe_real(struct pipe *pi) child->argv[i]); return -1; } - /* Look up command in command table */
+ /* Check if exists a variable with that name */ + if ((p = getenv (child->argv[i])) != NULL ) { + int rcode; + rcode = (parse_string_outer(p, + FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0); + return rcode; + }
+ /* Look up command in command table */ if ((cmdtp = find_cmd(child->argv[i])) == NULL) { printf ("Unknown command '%s' - try 'help'\n", child->argv[i]); return -1; /* give up after bad command */ diff --git a/common/main.c b/common/main.c index c06ea07..b101b4f 100644 --- a/common/main.c +++ b/common/main.c @@ -1289,6 +1289,7 @@ int run_command (const char *cmd, int flag) int argc, inquotes; int repeatable = 1; int rc = 0; + char *runenv;
#ifdef DEBUG_PARSER printf ("[RUN_COMMAND] cmd[%p]="", cmd); @@ -1357,6 +1358,13 @@ int run_command (const char *cmd, int flag) continue; }
+ if ((runenv = getenv (argv[0])) != NULL ) { + if (run_command(runenv,flag) == -1) { + rc = -1; + } + continue; + } + /* Look up command in command table */ if ((cmdtp = find_cmd(argv[0])) == NULL) { printf ("Unknown command '%s' - try 'help'\n", argv[0]); @@ -1433,3 +1441,32 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) return 0; } #endif + +int do_builtin (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{ + int i; + + if (argc < 2) { + printf ("Usage:\n%s\n", cmdtp->usage); + return 1; + } + + /* Look up command in command table */ + if ((cmdtp = find_cmd(argv[1])) == NULL) { + printf ("Unknown builtin command '%s' - try 'help'\n", argv[1]); + return 1; + } + + /* Remove builtin from arg list */ + argc--; + + /* OK - call function to do the command */ + if ((cmdtp->cmd) (cmdtp, flag, argc, &argv[1]) != 0) { + return 1; + } +} + +U_BOOT_CMD( + builtin, CFG_MAXARGS, 1, do_builtin, + "builtin - run builtin commands\n", +);
On Wed, Oct 15, 2008 at 12:09:44PM +0200, Stefano Babic wrote:
The command interpreter checks always if an environment variable for that name exists and in this case the content of the variable is executed. It becomes possible to redefine all U-Boot commands. A new "builtin" command is added to be able to run builtin U-boot commands even if they are redefined.
Signed-off-by : Stefano Babic sbabic@denx.de
It seems to me that the "builtin" command itself can still be redefined. Is this an issue?
Dear Petri Lehtinen,
In message 20081015104550.GB6881@colossus you wrote:
The command interpreter checks always if an environment variable for that name exists and in this case the content of the variable is executed. It becomes possible to redefine all U-Boot commands. A new "builtin" command is added to be able to run builtin U-boot commands even if they are redefined.
...
It seems to me that the "builtin" command itself can still be redefined. Is this an issue?
Good catch. Yes, this must be prevented. Any variable named "builtin" must be ignored here - i. e. we should issue an erro when someone tries a "setenv", and ignore this variable explicitly (just in case someone sneaks this into the default environment, or uses un-fixed Linux tools to create such a variable).
Thanks.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Good catch. Yes, this must be prevented. Any variable named "builtin" must be ignored here - i. e. we should issue an erro when someone tries a "setenv", and ignore this variable explicitly (just in case someone sneaks this into the default environment, or uses un-fixed Linux tools to create such a variable).
I know, I wanted to know which is the preferred method to avoid that. One way is to avoid to set the variable, as you suggest. The other way is never execute a variable with the name "builtin" in the command interpreter: "setenv builtin <something>" remains possible but "builtin <command>" calls always the U-Boot commands.
Best regards, Stefano Babic
Dear Stefano,
In message 48F5DBB7.4070307@babic.homelinux.org you wrote:
Wolfgang Denk wrote:
Good catch. Yes, this must be prevented. Any variable named "builtin" must be ignored here - i. e. we should issue an erro when someone tries a "setenv", and ignore this variable explicitly (just in case someone sneaks this into the default environment, or uses un-fixed Linux tools to create such a variable).
I know, I wanted to know which is the preferred method to avoid that. One way is to avoid to set the variable, as you suggest. The other way is never execute a variable with the name "builtin" in the command interpreter: "setenv builtin <something>" remains possible but "builtin <command>" calls always the U-Boot commands.
We should do both: avoid that the variable gets set, and raise error messages (both in setenv() and in the fw_setenv Linux tool), but we should also be defensive and never execute it if we run into it (which could for example happen if we're being poresented an old environment where some user defined such a variable).
Best regards,
Wolfgang Denk
Dear Stefano,
In message 1224065384-24171-1-git-send-email-sbabic@denx.de you wrote:
The command interpreter checks always if an environment variable for that name exists and in this case the content of the variable is executed. It becomes possible to redefine all U-Boot commands. A new "builtin" command is added to be able to run builtin U-boot commands even if they are redefined.
Cool. That's just great.
If I had known it's suffcient to put ideas on a web page to have people put them into code I would have started doing this much earlier :-)
/* Check if exists a variable with that name */if ((p = getenv (child->argv[i])) != NULL ) {int rcode;rcode = (parse_string_outer(p,FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0);return rcode;}
I think this covers only the hush shell version. We need something like this instead:
#ifdef CFG_HUSH_PARSER rcode = parse_string_outer(p, FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP); #else rcode =run_command (p, 0); #endif
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
If I had known it's suffcient to put ideas on a web page to have people put them into code I would have started doing this much earlier :-)
That's a good idea ;).
I think this covers only the hush shell version. We need something like this instead:
This is not true, I think. The change you are talking about is in hush.c (so it must cover *only* the hush shell). The other change is in the run_command() function and it is called when CFG_HUSH_PARSER is not defined. So both cases are covered.
Best regards, Stefano Babic
Dear Stefano,
In message 48F5DA6A.3090101@denx.de you wrote:
I think this covers only the hush shell version. We need something like this instead:
This is not true, I think. The change you are talking about is in hush.c (so it must cover *only* the hush shell). The other change is in the run_command() function and it is called when CFG_HUSH_PARSER is not defined. So both cases are covered.
You are right, of course. I read only part of the patch, it seems. Sorry ...
Best regards,
Wolfgang Denk
On Wed, 2008-10-15 at 12:57 +0200, Wolfgang Denk wrote:
Dear Stefano,
In message 1224065384-24171-1-git-send-email-sbabic@denx.de you wrote:
The command interpreter checks always if an environment variable for that name exists and in this case the content of the variable is executed. It becomes possible to redefine all U-Boot commands. A new "builtin" command is added to be able to run builtin U-boot commands even if they are redefined.
Cool. That's just great.
If I had known it's suffcient to put ideas on a web page to have people put them into code I would have started doing this much earlier :-)
/* Check if exists a variable with that name */if ((p = getenv (child->argv[i])) != NULL ) {int rcode;rcode = (parse_string_outer(p,FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0);return rcode;}I think this covers only the hush shell version. We need something like this instead:
#ifdef CFG_HUSH_PARSER rcode = parse_string_outer(p, FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP); #else rcode =run_command (p, 0); #endif
Move the parse_string_outer() into run_command() instead.
Jocke
participants (6)
-
Joakim Tjernlund -
Petri Lehtinen -
Stefano Babic -
stefano babic
-
stefano babic -
Wolfgang Denk