Re: [PATCH v3 2/3] cli: allow users to determine history buffer allocation method

2024-03-13 Thread Tom Rini
On Tue, Mar 05, 2024 at 03:37:35PM +0800, Hanyuan Zhao wrote:

> This commit allows users to choose the appropriate memory
> allocation method between static allocated and dynamically
> calloc. The previous static-array way will not obviously
> contribute to the final binary size since it is uninitialized,
> and might have better performance than the dynamical one.
> Now we provide the users with both the two options.
> 
> Signed-off-by: Hanyuan Zhao 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH v3 2/3] cli: allow users to determine history buffer allocation method

2024-03-04 Thread Hanyuan Zhao
This commit allows users to choose the appropriate memory
allocation method between static allocated and dynamically
calloc. The previous static-array way will not obviously
contribute to the final binary size since it is uninitialized,
and might have better performance than the dynamical one.
Now we provide the users with both the two options.

Signed-off-by: Hanyuan Zhao 
---
This is v3 of patch series cli: allow users to disable history
if unused at all. Please ignore the v2 version.
---
Changes v1 -> v3:
  - Add more detailed information about the CMD_HISTORY_USE_CALLOC
option both in the Kconfig and the code.
  - Update the comments on global history array and flash running
problems.
---
 cmd/Kconfig   | 11 +++
 common/cli_readline.c | 36 +---
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index a86b570517..7d2c050e08 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -189,6 +189,17 @@ config CMD_HISTORY
  Show the command-line history, i.e. a list of commands that are in
  the history buffer.
 
+config CMD_HISTORY_USE_CALLOC
+   bool "dynamically allocate memory"
+   default y
+   depends on CMD_HISTORY
+   help
+ Saying Y to this will use calloc to get the space for history
+ storing. Otherwise the history buffer will be an uninitialized
+ static array directly, without the memory allocation, and it is
+ writable after relocation to RAM. If u-boot is running from ROM
+ all the time or unsure, say Y to this.
+
 config CMD_LICENSE
bool "license"
select BUILD_BIN2C
diff --git a/common/cli_readline.c b/common/cli_readline.c
index 99e7efdfe5..cf4339d0e5 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -86,6 +86,9 @@ static int hist_add_idx;
 static int hist_cur = -1;
 static unsigned hist_num;
 
+#ifndef CONFIG_CMD_HISTORY_USE_CALLOC
+static char hist_data[HIST_MAX][HIST_SIZE + 1];
+#endif
 static char *hist_list[HIST_MAX];
 
 #define add_idx_minus_one() ((hist_add_idx == 0) ? hist_max : hist_add_idx-1)
@@ -100,20 +103,26 @@ static void getcmd_putchars(int count, int ch)
 
 static int hist_init(void)
 {
-   unsigned char *hist;
int i;
 
-   hist_max = 0;
-   hist_add_idx = 0;
-   hist_cur = -1;
-   hist_num = 0;
-
-   hist = calloc(HIST_MAX, HIST_SIZE + 1);
+#ifndef CONFIG_CMD_HISTORY_USE_CALLOC
+   for (i = 0; i < HIST_MAX; i++) {
+   hist_list[i] = hist_data[i];
+   hist_list[i][0] = '\0';
+   }
+#else
+   unsigned char *hist = calloc(HIST_MAX, HIST_SIZE + 1);
if (!hist)
panic("%s: calloc: out of memory!\n", __func__);
 
for (i = 0; i < HIST_MAX; i++)
hist_list[i] = hist + (i * (HIST_SIZE + 1));
+#endif
+
+   hist_max = 0;
+   hist_add_idx = 0;
+   hist_cur = -1;
+   hist_num = 0;
 
return 0;
 }
@@ -643,10 +652,15 @@ int cli_readline_into_buffer(const char *const prompt, 
char *buffer,
static int initted;
 
/*
-* History uses a global array which is not
-* writable until after relocation to RAM.
-* Revert to non-history version if still
-* running from flash.
+* Say N to CMD_HISTORY_USE_CALLOC will skip runtime
+* allocation for the history buffer and directly
+* use an uninitialized static array as the buffer.
+* Doing this might have better performance and not
+* increase the binary file's size, as it only marks
+* the size. However, the array is only writable after
+* relocation to RAM. If u-boot is running from ROM
+* all the time, consider say Y to CMD_HISTORY_USE_CALLOC
+* or disable CMD_HISTORY.
 */
if (IS_ENABLED(CONFIG_CMDLINE_EDITING) && (gd->flags & GD_FLG_RELOC)) {
if (!initted) {
-- 
2.34.1