Skip to contentSkip to conversation

Handle environment variables like commands

PATCH New Stefano Babic 5 participants 9 messages Oct 15, 2008

lore
Share & tools
lore

Stefano Babic

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 common/hush.c |    9 ++++++++-
 common/main.c |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletions(-)
common/hush.c +8 -1
index 093c428..68f8a6a 100644
@@ -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 */
common/main.c +37 -0
index c06ea07..b101b4f 100644
@@ -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",
+);
Signature · 1 line
1.5.4.3
#1 lore raw

PATCH 1/1 Handle environment variables like commands

Quoted 8 lines · Stefano Babic

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 <[email protected]>

---

It seems to me that the "builtin" command itself can still be redefined. Is this an issue?

Signature · 5 lines
Petri Lehtinen
Software Specialist
Inoi Oy
Tel. : +358 40 758 0229
Email: petri.lehtinen at inoi.fi
#2 lore raw

PATCH 1/1 Handle environment variables like commands

Dear Petri Lehtinen,

Quoted 6 lines · In message <20081015104550.GB6881@colossus> you wrote:
Quoted 6 lines

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.

...

Quoted 2 lines

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

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
Objects in mirror are closer than they appear.
#3 lore raw

PATCH 1/1 Handle environment variables like commands

Quoted 5 lines · Wolfgang Denk

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

Signature · 3 lines
stefano <[email protected]>
GPG Key: 0x55814DDE
Fingerprint 4E85 2A66 4CBA 497A 2A7B D3BF 5973 F216 5581 4DDE
#4 lore raw

PATCH 1/1 Handle environment variables like commands

Dear Stefano,

Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 5 lines · Wolfgang Denk

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).

Quoted 5 lines

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

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
You said you  didn't  want  to  use  CGI.pm,  but  methinks  you  are
needlessly reinventing the wheel, one spoke at a time. Either you are
masochistic,  or  you  just haven't seen enough of what CGI.pm can do
for you. -- Randal L. Schwartz in <[email protected]>
#5 lore raw

PATCH 1/1 Handle environment variables like commands

In reply to Stefano Babic

Dear Stefano,

Quoted 5 lines · In message <[email protected]> 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 :-)

common/hush.c static int run_pipe_real(struct pipe *pi) · line 1687

Quoted 7 lines
common/hush.c +7 -0
+			/* 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

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
For every complex problem, there is a solution that is simple,  neat,
and wrong.                                               - Mark Twain
author
#6 lore raw

PATCH 1/1 Handle environment variables like commands

Quoted 3 lines · Wolfgang Denk

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 ;).

Quoted 2 lines

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

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-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#7 lore raw

PATCH 1/1 Handle environment variables like commands

Dear Stefano,

Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 3 lines

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

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
"Faith: not *wanting* to know what is true."    - Friedrich Nietzsche
#8 lore raw

PATCH 1/1 Handle environment variables like commands

In reply to Wolfgang Denk
Quoted 27 lines · Wolfgang Denk

Dear Stefano,

Quoted 5 lines · In message <[email protected]> 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 :-)

common/hush.c static int run_pipe_real(struct pipe *pi) · line 1687

Quoted 7 lines
common/hush.c +7 -0
+			/* 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:

Quoted 6 lines

#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