| Message ID | tencent_411A9B7DA959D26CCAFCE7961870C400E80A@qq.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/2] cli: allow users to disable history if unused at all | expand |
On Sat, Feb 17, 2024 at 09:54:43PM +0800, Hanyuan Zhao wrote: > This commit allows user to determine whether to have history recording > in command-line. Previously the history data as uninitialized static > array would not directly take much space in binary file since it only > marks size in the binary. However now it asks to allocate space. By > connecting the original CMD_HISTORY flag in Kconfig, users could unset > the whole history function, and the memory usage could be eased, if the > history function is not used at all. > > Signed-off-by: Hanyuan Zhao <hanyuan-z@qq.com> > --- > cmd/Kconfig | 1 + > common/cli_readline.c | 44 ++++++++++++++++++++++++------------------- > 2 files changed, 26 insertions(+), 19 deletions(-) This breaks current tests, please re-test, thanks.
Hi Tom, Thanks for reviewing! I am not quite sure about the tests which you refer to. Is it the CI in the gitlab? I think this patch is simple and it doesn’t occur any errors during my work these days, thus I tested it manually which didn’t throw any problems, and sent it. Could you please give me some hints about the broken tests? If it is a CI, could you please give me the link to the error information? Thanks! Regards, Hanyuan > 2024年3月2日 05:37，Tom Rini <trini@konsulko.com> 写道： > > On Sat, Feb 17, 2024 at 09:54:43PM +0800, Hanyuan Zhao wrote: > >> This commit allows user to determine whether to have history recording >> in command-line. Previously the history data as uninitialized static >> array would not directly take much space in binary file since it only >> marks size in the binary. However now it asks to allocate space. By >> connecting the original CMD_HISTORY flag in Kconfig, users could unset >> the whole history function, and the memory usage could be eased, if the >> history function is not used at all. >> >> Signed-off-by: Hanyuan Zhao <hanyuan-z@qq.com> >> --- >> cmd/Kconfig | 1 + >> common/cli_readline.c | 44 ++++++++++++++++++++++++------------------- >> 2 files changed, 26 insertions(+), 19 deletions(-) > > This breaks current tests, please re-test, thanks. > > -- > Tom
On Sat, Mar 02, 2024 at 12:35:12PM +0800, hanyuan wrote: > Hi Tom, > > Thanks for reviewing! I am not quite sure about the tests which > you refer to. Is it the CI in the gitlab? I think this patch is simple > and it doesn’t occur any errors during my work these days, thus I tested > it manually which didn’t throw any problems, and sent it. Could you > please give me some hints about the broken tests? If it is a CI, could > you please give me the link to the error information? Yes, our pytests for sandbox failed, see https://docs.u-boot.org/en/latest/develop/ci_testing.html and https://docs.u-boot.org/en/latest/develop/tests_sandbox.html
diff --git a/cmd/Kconfig b/cmd/Kconfig index a86b570517..af4dbc95fc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -184,6 +184,7 @@ config CMD_FWU_METADATA config CMD_HISTORY bool "history" + default y depends on CMDLINE_EDITING help Show the command-line history, i.e. a list of commands that are in diff --git a/common/cli_readline.c b/common/cli_readline.c index 2507be2295..eec6d8b0a2 100644 --- a/common/cli_readline.c +++ b/common/cli_readline.c @@ -67,12 +67,13 @@ static char *delete_char (char *buffer, char *p, int *colp, int *np, int plen) #define CTL_BACKSPACE ('\b') #define DEL ((char)255) #define DEL7 ((char)127) -#define CREAD_HIST_CHAR ('!') #define getcmd_putch(ch) putc(ch) #define getcmd_getch() getchar() #define getcmd_cbeep() getcmd_putch('\a') +#ifdef CONFIG_CMD_HISTORY +#define CREAD_HIST_CHAR ('!') #ifdef CONFIG_SPL_BUILD #define HIST_MAX 3 #define HIST_SIZE 32 @@ -90,14 +91,6 @@ static char *hist_list[HIST_MAX]; #define add_idx_minus_one() ((hist_add_idx == 0) ? hist_max : hist_add_idx-1) -static void getcmd_putchars(int count, int ch) -{ - int i; - - for (i = 0; i < count; i++) - getcmd_putch(ch); -} - static int hist_init(void) { unsigned char *hist; @@ -110,7 +103,7 @@ static int hist_init(void) hist = calloc(HIST_MAX, HIST_SIZE + 1); if (!hist) - return -ENOMEM; + panic("%s: calloc: out of memory!\n", __func__); for (i = 0; i < HIST_MAX; i++) hist_list[i] = hist + (i * (HIST_SIZE + 1)); @@ -192,6 +185,15 @@ void cread_print_hist_list(void) i++; } } +#endif + +static void getcmd_putchars(int count, int ch) +{ + int i; + + for (i = 0; i < count; i++) + getcmd_putch(ch); +} #define BEGINNING_OF_LINE() { \ while (cls->num) { \ @@ -365,6 +367,7 @@ int cread_line_process_ch(struct cli_line_state *cls, char ichar) cls->eol_num--; } break; +#ifdef CONFIG_CMD_HISTORY case CTL_CH('p'): case CTL_CH('n'): if (cls->history) { @@ -394,6 +397,7 @@ int cread_line_process_ch(struct cli_line_state *cls, char ichar) break; } break; +#endif case '\t': if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && cls->cmd_complete) { int num2, col; @@ -490,19 +494,23 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len, } *len = cls->eol_num; +#ifdef CONFIG_CMD_HISTORY if (buf[0] && buf[0] != CREAD_HIST_CHAR) cread_add_to_hist(buf); hist_cur = hist_add_idx; +#endif return 0; } #else /* !CONFIG_CMDLINE_EDITING */ +#ifdef CONFIG_CMD_HISTORY static inline int hist_init(void) { return 0; } +#endif static int cread_line(const char *const prompt, char *buf, unsigned int *len, int timeout) @@ -640,20 +648,18 @@ int cli_readline_into_buffer(const char *const prompt, char *buffer, char *p = buffer; uint len = CONFIG_SYS_CBSIZE; int rc; - static int initted; +#ifdef CONFIG_CMD_HISTORY + static int hist_initted; +#endif - /* - * History uses a global array which is not - * writable until after relocation to RAM. - * Revert to non-history version if still - * running from flash. - */ if (IS_ENABLED(CONFIG_CMDLINE_EDITING) && (gd->flags & GD_FLG_RELOC)) { - if (!initted) { +#ifdef CONFIG_CMD_HISTORY + if (!hist_initted) { rc = hist_init(); if (rc == 0) - initted = 1; + hist_initted = 1; } +#endif if (prompt) puts(prompt);
This commit allows user to determine whether to have history recording in command-line. Previously the history data as uninitialized static array would not directly take much space in binary file since it only marks size in the binary. However now it asks to allocate space. By connecting the original CMD_HISTORY flag in Kconfig, users could unset the whole history function, and the memory usage could be eased, if the history function is not used at all. Signed-off-by: Hanyuan Zhao <hanyuan-z@qq.com> --- cmd/Kconfig | 1 + common/cli_readline.c | 44 ++++++++++++++++++++++++------------------- 2 files changed, 26 insertions(+), 19 deletions(-)