[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Peter Tyser]

Previously, passing readline() or readline_into_buffer() a NULL 'prompt'
parameter would result in puts() printing garbage when
CONFIG_CMDLINE_EDITING was enabled.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 common/main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/common/main.c b/common/main.c
index 026edd1..298982a 100644
--- a/common/main.c
+++ b/common/main.c
@@ -964,7 +964,8 @@ int readline_into_buffer (const char *const prompt, char * buffer)
 			initted = 1;
 		}
 
-		puts (prompt);
+		if (prompt)
+			puts (prompt);
 
 		rc = cread_line(prompt, p, &len);
 		return rc < 0 ? rc : len;
-- 
1.6.2.1

[U-Boot] [PATCH 2/5] cread_line(): Remove unused variables

[Peter Tyser]

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 common/main.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/common/main.c b/common/main.c
index 298982a..b47e443 100644
--- a/common/main.c
+++ b/common/main.c
@@ -715,16 +715,13 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len)
 {
 	unsigned long num = 0;
 	unsigned long eol_num = 0;
-	unsigned long rlen;
 	unsigned long wlen;
 	char ichar;
 	int insert = 1;
 	int esc_len = 0;
-	int rc = 0;
 	char esc_save[8];
 
 	while (1) {
-		rlen = 1;
 #ifdef CONFIG_BOOT_RETRY_TIME
 		while (!tstc()) {	/* while no incoming data */
 			if (retry_time >= 0 && get_ticks() > endtime)
@@ -923,7 +920,7 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len)
 		cread_add_to_hist(buf);
 	hist_cur = hist_add_idx;
 
-	return (rc);
+	return 0;
 }
 
 #endif /* CONFIG_CMDLINE_EDITING */
-- 
1.6.2.1

[U-Boot] [PATCH 3/5] readline(): Add ability to modify a string buffer

[Peter Tyser]

If the 'buf' parameter is a non-0-length string, its contents will be
edited.  Previously, the initial value of 'buf' was ignored and the
user entered its contents from scratch.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 common/main.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/common/main.c b/common/main.c
index b47e443..10d8904 100644
--- a/common/main.c
+++ b/common/main.c
@@ -720,6 +720,10 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len)
 	int insert = 1;
 	int esc_len = 0;
 	char esc_save[8];
+	int init_len = strlen(buf);
+
+	if (init_len)
+		cread_add_str(buf, init_len, 1, &num, &eol_num, buf, *len);
 
 	while (1) {
 #ifdef CONFIG_BOOT_RETRY_TIME
@@ -937,6 +941,12 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len)
  */
 int readline (const char *const prompt)
 {
+	/*
+	 * If console_buffer isn't 0-length the user will be prompted to modify
+	 * it instead of entering it from scratch as desired.
+	 */
+	console_buffer[0] = '\0';
+
 	return readline_into_buffer(prompt, console_buffer);
 }
 
-- 
1.6.2.1

[U-Boot] [PATCH 3/5] readline(): Add ability to modify a string buffer

[Wolfgang Denk]

Dear Peter Tyser,

In message <1256259563-32725-3-git-send-email-ptyser@xes-inc.com> you wrote:
> If the 'buf' parameter is a non-0-length string, its contents will be
> edited.  Previously, the initial value of 'buf' was ignored and the
> user entered its contents from scratch.

What is the purpose of this modification? Are you fixing a bug (which
one?) or implementing an extension (which one) or what?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I came home the other night and tried to open the door  with  my  car
keys...and  the  building started up. So I took it out for a drive. A
cop pulled me over for speeding. He asked me where I  live...  "Right
here".                                                - Steven Wright

[U-Boot] [PATCH 3/5] readline(): Add ability to modify a string buffer

[Peter Tyser]

Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <1256259563-32725-3-git-send-email-ptyser@xes-inc.com> you wrote:
>> If the 'buf' parameter is a non-0-length string, its contents will be
>> edited.  Previously, the initial value of 'buf' was ignored and the
>> user entered its contents from scratch.
> 
> What is the purpose of this modification? Are you fixing a bug (which
> one?) or implementing an extension (which one) or what?

This change was necessary to add a new "editenv" command.  This change 
allows the readline() function to edit a string instead of just enter 
one from scratch.  In theory this feature could be used for other areas 
of U-Boot in the future, but at this point the only user of it would be 
the "editenv" command which I submitted a patch for.  If the 'editenv' 
patch is rejected, I don't mind this one being rejected too.

Best,
Peter

[U-Boot] [PATCH 3/5] readline(): Add ability to modify a string buffer

[Mike Frysinger]

On Saturday 24 October 2009 17:47:27 Peter Tyser wrote:
> Wolfgang Denk wrote:
> > Peter Tyser wrote:
> >> If the 'buf' parameter is a non-0-length string, its contents will be
> >> edited.  Previously, the initial value of 'buf' was ignored and the
> >> user entered its contents from scratch.
> >
> > What is the purpose of this modification? Are you fixing a bug (which
> > one?) or implementing an extension (which one) or what?
> 
> This change was necessary to add a new "editenv" command.  This change
> allows the readline() function to edit a string instead of just enter
> one from scratch.  In theory this feature could be used for other areas
> of U-Boot in the future, but at this point the only user of it would be
> the "editenv" command which I submitted a patch for.  If the 'editenv'
> patch is rejected, I don't mind this one being rejected too.

+1 from me for new "editenv" :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091025/9679bbec/attachment.pgp 

[U-Boot] [PATCH 4/5] _do_setenv(): Delete 0-length environment variables

[Peter Tyser]

Previously _do_setenv() would only delete an environment variable if it
was passed a NULL string pointer as a value.  It should also delete an
environment variable when it encounters a valid string pointer of
0-length.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 common/cmd_nvedit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..725e573 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -400,7 +400,7 @@ int _do_setenv (int flag, int argc, char *argv[])
 int setenv (char *varname, char *varvalue)
 {
 	char *argv[4] = { "setenv", varname, varvalue, NULL };
-	if (varvalue == NULL)
+	if ((varvalue == NULL) || (varvalue[0] == '\0'))
 		return _do_setenv (0, 2, argv);
 	else
 		return _do_setenv (0, 3, argv);
-- 
1.6.2.1

[U-Boot] [PATCH 5/5] Add 'editenv' command

[Peter Tyser]

The editenv command can be used to edit an environment variable.
Editing an environment variable is useful when one wants to tweak an
existing variable, for example fix a typo or change the baudrate in the
'bootargs' environment variable.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
The footprint of editenv is relatively small and I personally find it very
useful so I added it to the config_cmd_default.h.  Let me know if others
would prefer not to include it by default.

 README                       |    1 +
 common/cmd_nvedit.c          |   40 ++++++++++++++++++++++++++++++++++++++++
 include/config_cmd_all.h     |    1 +
 include/config_cmd_default.h |    1 +
 4 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 744f6bf..c300443 100644
--- a/README
+++ b/README
@@ -620,6 +620,7 @@ The following options need to be configured:
 		CONFIG_CMD_DS4510_RST	* ds4510 I2C rst command
 		CONFIG_CMD_DTT		* Digital Therm and Thermostat
 		CONFIG_CMD_ECHO		  echo arguments
+		CONFIG_CMD_EDITENV	  edit env variable
 		CONFIG_CMD_EEPROM	* EEPROM read/write support
 		CONFIG_CMD_ELF		* bootelf, bootvx
 		CONFIG_CMD_SAVEENV	  saveenv
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 725e573..eb89e9e 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -42,6 +42,9 @@
 #include <common.h>
 #include <command.h>
 #include <environment.h>
+#if defined(CONFIG_CMD_EDITENV)
+#include <malloc.h>
+#endif
 #include <watchdog.h>
 #include <serial.h>
 #include <linux/stddef.h>
@@ -503,6 +506,34 @@ int do_askenv ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 #endif
 
 /************************************************************************
+ * Interactively edit an environment variable
+ */
+#if defined(CONFIG_CMD_EDITENV)
+int do_editenv(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	char buffer[CONFIG_SYS_CBSIZE];
+	char *init_val;
+	int len;
+
+	if (argc < 2) {
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	/* Set read buffer to initial value or empty sting */
+	init_val = getenv(argv[1]);
+	if (init_val)
+		len = sprintf(buffer, "%s", init_val);
+	else
+		buffer[0] = '\0';
+
+	readline_into_buffer("edit: ", buffer);
+
+	return setenv(argv[1], buffer);
+}
+#endif /* CONFIG_CMD_EDITENV */
+
+/************************************************************************
  * Look up variable from environment,
  * return address of storage for that variable,
  * or NULL if not found
@@ -597,6 +628,15 @@ int envmatch (uchar *s1, int i2)
 
 /**************************************************/
 
+#if defined(CONFIG_CMD_EDITENV)
+U_BOOT_CMD(
+	editenv, 2, 0,	do_editenv,
+	"edit environment variable",
+	"name\n"
+	"    - edit environment variable 'name'"
+);
+#endif
+
 U_BOOT_CMD(
 	printenv, CONFIG_SYS_MAXARGS, 1,	do_printenv,
 	"print environment variables",
diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
index c747b4b..058fdf1 100644
--- a/include/config_cmd_all.h
+++ b/include/config_cmd_all.h
@@ -30,6 +30,7 @@
 #define CONFIG_CMD_DOC		/* Disk-On-Chip Support		*/
 #define CONFIG_CMD_DTT		/* Digital Therm and Thermostat */
 #define CONFIG_CMD_ECHO		/* echo arguments		*/
+#define CONFIG_CMD_EDITENV	/* editenv			*/
 #define CONFIG_CMD_EEPROM	/* EEPROM read/write support	*/
 #define CONFIG_CMD_ELF		/* ELF (VxWorks) load/boot cmd	*/
 #define CONFIG_CMD_SAVEENV	/* saveenv			*/
diff --git a/include/config_cmd_default.h b/include/config_cmd_default.h
index a5d87a6..6e3903c 100644
--- a/include/config_cmd_default.h
+++ b/include/config_cmd_default.h
@@ -20,6 +20,7 @@
 #define CONFIG_CMD_BOOTD	/* bootd			*/
 #define CONFIG_CMD_CONSOLE	/* coninfo			*/
 #define CONFIG_CMD_ECHO		/* echo arguments		*/
+#define CONFIG_CMD_EDITENV	/* editenv			*/
 #define CONFIG_CMD_FPGA		/* FPGA configuration Support	*/
 #define CONFIG_CMD_IMI		/* iminfo			*/
 #define CONFIG_CMD_ITEST	/* Integer (and string) test	*/
-- 
1.6.2.1

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Wolfgang Denk]

Dear Peter Tyser,

In message <1256259563-32725-1-git-send-email-ptyser@xes-inc.com> you wrote:
> Previously, passing readline() or readline_into_buffer() a NULL 'prompt'
> parameter would result in puts() printing garbage when
> CONFIG_CMDLINE_EDITING was enabled.

What would be the situation when this happens? I cannot see easily how
to trigger this bug?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
An age is called Dark not because  the  light  fails  to  shine,  but
because people refuse to see it.           -- James Michener, "Space"

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Peter Tyser]

Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <1256259563-32725-1-git-send-email-ptyser@xes-inc.com> you wrote:
>> Previously, passing readline() or readline_into_buffer() a NULL 'prompt'
>> parameter would result in puts() printing garbage when
>> CONFIG_CMDLINE_EDITING was enabled.
> 
> What would be the situation when this happens? I cannot see easily how
> to trigger this bug?

I don't think anything in current code would trigger this.  If some 
boards enabled CONFIG_CMDLINE_EDITING, they would have issues, eg 
board/eltec/bab7xx/misc.c.

So its not fixing a bug, just preventing a future one:)  It was also 
necessary to implement the 5/5 patch "Add 'editenv' command" which is 
how I triggered the "future bug".

In this series, patches 1/2/4 are non-critical cleanup/fixes.  I didn't 
plan on them get merged for the upcoming release, but I think they 
should get picked up at some point.  Patches 3/5 add new features - 
basically support for a new 'editenv' command.  These changes are more 
debatable, so we can discuss if you'd like to include these or not.

Best,
Peter

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Wolfgang Denk]

Dear Peter Tyser,

In message <4AE373AB.6040705@xes-inc.com> you wrote:
>
> In this series, patches 1/2/4 are non-critical cleanup/fixes.  I didn't 
> plan on them get merged for the upcoming release, but I think they 
> should get picked up at some point.  Patches 3/5 add new features - 
> basically support for a new 'editenv' command.  These changes are more 
> debatable, so we can discuss if you'd like to include these or not.

I don't want to debate these. Actually I like the code. And if you
could be so kind and write a little more about this background
information in the commit messages, I'm really tempted to pull this
in ASAP :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Maintain an awareness for contribution --  to  your  schedule,  your
project, our company."                         - A Group of Employees

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Peter Tyser]

Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <4AE373AB.6040705@xes-inc.com> you wrote:
>> In this series, patches 1/2/4 are non-critical cleanup/fixes.  I didn't 
>> plan on them get merged for the upcoming release, but I think they 
>> should get picked up at some point.  Patches 3/5 add new features - 
>> basically support for a new 'editenv' command.  These changes are more 
>> debatable, so we can discuss if you'd like to include these or not.
> 
> I don't want to debate these. Actually I like the code.

Fantastic!

> And if you
> could be so kind and write a little more about this background
> information in the commit messages, I'm really tempted to pull this
> in ASAP :-)

OK.  Let me know which patch commit messages should be improved and I'll 
resubmit.

Best,
Peter

[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()

[Wolfgang Denk]

Dear Peter Tyser,

In message <4AE37708.7050704@xes-inc.com> you wrote:
>
> > I don't want to debate these. Actually I like the code.
> 
> Fantastic!

Indeed :-)

> > And if you
> > could be so kind and write a little more about this background
> > information in the commit messages, I'm really tempted to pull this
> > in ASAP :-)
> 
> OK.  Let me know which patch commit messages should be improved and I'll 
> resubmit.

Well, just look at my questions. Add the explanations you  just  sent
in  the  mails  (prepare the code for adding the editenv command...),
and everybody understands the context.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
... Jesus cried with a loud voice: Lazarus, come forth; the bug  hath
been  found  and  thy  program  runneth.  And  he  that was dead came
forth...                              -- John 11:43-44 [version 2.0?]