[U-Boot] [PATCH 1/5] Check for NULL prompt in readline_into_buffer()
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;
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 */
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); }
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
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
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
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);
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 */
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
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
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
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
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
participants (3)
-
Mike Frysinger -
Peter Tyser -
Wolfgang Denk