[PATCH 1/2] command: Allocate history buffer using calloc()
The history buffer is currently a static array which can be some 10-40 kiB depending on configuration, and so adds considerably to the U-Boot binary size. Allocate it dynamically instead to reduce the U-Boot binary size.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Simon Glass sjg@chromium.org --- common/cli_readline.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/common/cli_readline.c b/common/cli_readline.c index 06b8d465044..85453beed76 100644 --- a/common/cli_readline.c +++ b/common/cli_readline.c @@ -12,6 +12,8 @@ #include <bootretry.h> #include <cli.h> #include <command.h> +#include <hang.h> +#include <malloc.h> #include <time.h> #include <watchdog.h> #include <asm/global_data.h> @@ -85,7 +87,6 @@ static int hist_cur = -1; static unsigned hist_num;
static char *hist_list[HIST_MAX]; -static char hist_lines[HIST_MAX][HIST_SIZE + 1]; /* Save room for NULL */
#define add_idx_minus_one() ((hist_add_idx == 0) ? hist_max : hist_add_idx-1)
@@ -97,8 +98,9 @@ static void getcmd_putchars(int count, int ch) getcmd_putch(ch); }
-static void hist_init(void) +static int hist_init(void) { + unsigned char *hist; int i;
hist_max = 0; @@ -106,10 +108,14 @@ static void hist_init(void) hist_cur = -1; hist_num = 0;
- for (i = 0; i < HIST_MAX; i++) { - hist_list[i] = hist_lines[i]; - hist_list[i][0] = '\0'; - } + hist = calloc(HIST_MAX, HIST_SIZE + 1); + if (!hist) + return -ENOMEM; + + for (i = 0; i < HIST_MAX; i++) + hist_list[i] = hist + (i * (HIST_SIZE + 1)); + + return 0; }
static void cread_add_to_hist(char *line) @@ -493,8 +499,9 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
#else /* !CONFIG_CMDLINE_EDITING */
-static inline void hist_init(void) +static inline int hist_init(void) { + return 0; }
static int cread_line(const char *const prompt, char *buf, unsigned int *len, @@ -643,8 +650,9 @@ int cli_readline_into_buffer(const char *const prompt, char *buffer, */ if (IS_ENABLED(CONFIG_CMDLINE_EDITING) && (gd->flags & GD_FLG_RELOC)) { if (!initted) { - hist_init(); - initted = 1; + rc = hist_init(); + if (rc == 0) + initted = 1; }
if (prompt)
The command completion temporary buffer seems to be only used by the argv tokenizer, move it to stack. This saves 2 kiB from the binary size (depends on configuration) per: $ aarch64-linux-gnu-readelf -s u-boot | sort -n -k 3
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Simon Glass sjg@chromium.org --- common/command.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c index 846e16e2ada..7821c273dae 100644 --- a/common/command.c +++ b/common/command.c @@ -355,10 +355,9 @@ static int find_common_prefix(char *const argv[]) return len; }
-static char tmp_buf[CONFIG_SYS_CBSIZE + 1]; /* copy of console I/O buffer */ - int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp) { + char tmp_buf[CONFIG_SYS_CBSIZE + 1]; /* copy of console I/O buffer */ int n = *np, col = *colp; char *argv[CONFIG_SYS_MAXARGS + 1]; /* NULL terminated */ char *cmdv[20];
On Sat, 2 Dec 2023 at 13:53, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The command completion temporary buffer seems to be only used by the argv tokenizer, move it to stack. This saves 2 kiB from the binary size (depends on configuration) per: $ aarch64-linux-gnu-readelf -s u-boot | sort -n -k 3
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Simon Glass sjg@chromium.org
common/command.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Sat, Dec 02, 2023 at 09:52:31PM +0100, Marek Vasut wrote:
The command completion temporary buffer seems to be only used by the argv tokenizer, move it to stack. This saves 2 kiB from the binary size (depends on configuration) per: $ aarch64-linux-gnu-readelf -s u-boot | sort -n -k 3
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!
Hi Marek,
On Sat, 2 Dec 2023 at 13:53, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The history buffer is currently a static array which can be some 10-40 kiB depending on configuration, and so adds considerably to the U-Boot binary size. Allocate it dynamically instead to reduce the U-Boot binary size.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Simon Glass sjg@chromium.org
common/cli_readline.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
This is very intriguing...
I would expect this to end up in BSS, so not part of the image, but allocated when U-Boot starts.
What do you mean by U-Boot binary size? Which architecture is this?
Regards, Simon
On 12/3/23 18:44, Simon Glass wrote:
Hi Marek,
On Sat, 2 Dec 2023 at 13:53, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The history buffer is currently a static array which can be some 10-40 kiB depending on configuration, and so adds considerably to the U-Boot binary size. Allocate it dynamically instead to reduce the U-Boot binary size.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Simon Glass sjg@chromium.org
common/cli_readline.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
This is very intriguing...
I would expect this to end up in BSS, so not part of the image, but allocated when U-Boot starts.
What do you mean by U-Boot binary size? Which architecture is this?
ARM64 , I spotted it at rcar3_salvator-x_defconfig but it has been a while since I created those patches , they were on hold for a while .
Hi Marek,
On Sun, 3 Dec 2023 at 13:44, Marek Vasut marek.vasut@mailbox.org wrote:
On 12/3/23 18:44, Simon Glass wrote:
Hi Marek,
On Sat, 2 Dec 2023 at 13:53, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The history buffer is currently a static array which can be some 10-40 kiB depending on configuration, and so adds considerably to the U-Boot binary size. Allocate it dynamically instead to reduce the U-Boot binary size.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Simon Glass sjg@chromium.org
common/cli_readline.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
This is very intriguing...
I would expect this to end up in BSS, so not part of the image, but allocated when U-Boot starts.
What do you mean by U-Boot binary size? Which architecture is this?
ARM64 , I spotted it at rcar3_salvator-x_defconfig but it has been a while since I created those patches , they were on hold for a while .
Yes, then really we should fix that bug.
E.g. __rel_dyn_start (OVERLAY) is missing for arm64. This affects all boards:
For firefly-rk3288 (ARMv7): 11 .bss_start 00000000 0106c578 0106c578 0007d0a4 2**0 CONTENTS 12 .bss 0000cc44 0106c578 0106c578 0007d0a8 2**3 CONTENTS 13 .bss_end 00000000 010791bc 010791bc 00089cec 2**0 CONTENTS
For rcar3_salvator-x (ARMv8):
10 .bss_start 00000000 00000000000f1a18 00000000000f1a18 00101a18 2**0 CONTENTS, ALLOC, LOAD, DATA 11 .bss 00011108 00000000000f1a40 00000000000f1a40 00101a18 2**6 ALLOC 12 .bss_end 00000000 0000000000102b48 0000000000102b48 00112b48 2**0 CONTENTS, ALLOC, LOAD, DATA
The 'LOAD' attributes are a bug.
Regards, Simon
On Sat, Dec 02, 2023 at 09:52:30PM +0100, Marek Vasut wrote:
The history buffer is currently a static array which can be some 10-40 kiB depending on configuration, and so adds considerably to the U-Boot binary size. Allocate it dynamically instead to reduce the U-Boot binary size.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Applied to u-boot/next, thanks!
participants (4)
-
Marek Vasut -
Marek Vasut -
Simon Glass -
Tom Rini