From cc7b560efd46c5aff1809b512f135bcf16b6962e Mon Sep 17 00:00:00 2001 From: chao an Date: Fri, 27 Dec 2024 14:20:37 +0800 Subject: [PATCH 1/5] apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX Applications should not depend on any properties of nshlib Signed-off-by: chao an --- examples/lp503x/lp503x_main.c | 2 +- netutils/rexec/rexec.c | 2 +- nshlib/nsh.h | 2 +- system/nxcamera/nxcamera_main.c | 2 +- system/nxlooper/nxlooper_main.c | 2 +- system/nxplayer/nxplayer_main.c | 2 +- system/nxrecorder/nxrecorder_main.c | 2 +- system/readline/readline.c | 14 ++------------ system/taskset/taskset.c | 4 ++-- system/trace/trace.c | 2 +- 10 files changed, 12 insertions(+), 22 deletions(-) diff --git a/examples/lp503x/lp503x_main.c b/examples/lp503x/lp503x_main.c index 644305248..7a2136d29 100644 --- a/examples/lp503x/lp503x_main.c +++ b/examples/lp503x/lp503x_main.c @@ -624,7 +624,7 @@ static int lp503x_cmd_help(FAR char *parg) int main(int argc, FAR char *argv[]) { bool running; - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int len; int x; char *cmd; diff --git a/netutils/rexec/rexec.c b/netutils/rexec/rexec.c index 69346d872..e52869d8f 100644 --- a/netutils/rexec/rexec.c +++ b/netutils/rexec/rexec.c @@ -153,7 +153,7 @@ static int do_rexec(FAR struct rexec_arg_s *arg) int main(int argc, FAR char **argv) { - char cmd[CONFIG_NSH_LINELEN]; + char cmd[LINE_MAX]; struct rexec_arg_s arg; int option; int i; diff --git a/nshlib/nsh.h b/nshlib/nsh.h index 75c84f856..1edf9ca1c 100644 --- a/nshlib/nsh.h +++ b/nshlib/nsh.h @@ -380,7 +380,7 @@ /* Maximum size of one command line (telnet or serial) */ #ifndef CONFIG_NSH_LINELEN -# define CONFIG_NSH_LINELEN 80 +# define CONFIG_NSH_LINELEN LINE_MAX #endif /* The maximum number of nested if-then[-else]-fi sequences that diff --git a/system/nxcamera/nxcamera_main.c b/system/nxcamera/nxcamera_main.c index e834932f4..8f55df3e3 100644 --- a/system/nxcamera/nxcamera_main.c +++ b/system/nxcamera/nxcamera_main.c @@ -396,7 +396,7 @@ static int nxcamera_cmd_help(FAR struct nxcamera_s *pcam, FAR char *parg) int main(int argc, FAR char *argv[]) { - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int len; int x; bool running = true; diff --git a/system/nxlooper/nxlooper_main.c b/system/nxlooper/nxlooper_main.c index 39be125ed..c4f79e683 100644 --- a/system/nxlooper/nxlooper_main.c +++ b/system/nxlooper/nxlooper_main.c @@ -500,7 +500,7 @@ static int nxlooper_cmd_help(FAR struct nxlooper_s *plooper, char *parg) int main(int argc, FAR char *argv[]) { - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int len; int x; int running; diff --git a/system/nxplayer/nxplayer_main.c b/system/nxplayer/nxplayer_main.c index dc9d60152..4ac350724 100644 --- a/system/nxplayer/nxplayer_main.c +++ b/system/nxplayer/nxplayer_main.c @@ -737,7 +737,7 @@ static int nxplayer_cmd_help(FAR struct nxplayer_s *pplayer, char *parg) int main(int argc, FAR char *argv[]) { - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int len; int x; int running; diff --git a/system/nxrecorder/nxrecorder_main.c b/system/nxrecorder/nxrecorder_main.c index bdfc2c3eb..9bda8e1b2 100644 --- a/system/nxrecorder/nxrecorder_main.c +++ b/system/nxrecorder/nxrecorder_main.c @@ -529,7 +529,7 @@ static int nxrecorder_cmd_help(FAR struct nxrecorder_s *precorder, int main(int argc, FAR char *argv[]) { - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int len; int x; int running; diff --git a/system/readline/readline.c b/system/readline/readline.c index b077b4e86..8f4b0579c 100644 --- a/system/readline/readline.c +++ b/system/readline/readline.c @@ -30,16 +30,6 @@ #include "system/readline.h" -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/* Maximum size of one command line (telnet or serial) */ - -#ifndef CONFIG_NSH_LINELEN -# define CONFIG_NSH_LINELEN 80 -#endif - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -57,14 +47,14 @@ FAR char *readline(FAR const char *prompt) { - FAR char *line = malloc(CONFIG_NSH_LINELEN); + FAR char *line = malloc(LINE_MAX); if (line != NULL) { #ifdef CONFIG_READLINE_TABCOMPLETION FAR const char *orig = readline_prompt(prompt); #endif - if (readline_fd(line, CONFIG_NSH_LINELEN, + if (readline_fd(line, LINE_MAX, STDIN_FILENO, STDOUT_FILENO) == 0) { free(line); diff --git a/system/taskset/taskset.c b/system/taskset/taskset.c index 3cffd2e28..30fea9fcb 100644 --- a/system/taskset/taskset.c +++ b/system/taskset/taskset.c @@ -80,7 +80,7 @@ static bool get_cpuset(const char *arg, cpu_set_t *cpu_set) int main(int argc, FAR char *argv[]) { - char command[CONFIG_NSH_LINELEN]; + char command[LINE_MAX]; int exitcode; int option; int pid = -1; @@ -151,7 +151,7 @@ int main(int argc, FAR char *argv[]) } /* Construct actual command with args - * NOTE: total length does not exceed CONFIG_NSH_LINELEN + * NOTE: total length does not exceed LINE_MAX */ for (i = 0; i < argc - 2; i++) diff --git a/system/trace/trace.c b/system/trace/trace.c index e41bf8e8b..abb57783d 100644 --- a/system/trace/trace.c +++ b/system/trace/trace.c @@ -227,7 +227,7 @@ static int trace_cmd_dump(FAR const char *name, int index, int argc, static int trace_cmd_cmd(FAR const char *name, int index, int argc, FAR char **argv, int notectlfd) { - char command[CONFIG_NSH_LINELEN]; + char command[LINE_MAX]; bool changed; bool cont = false; From c7ee2a0394361f8d7486a5f05549f47812332c75 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Tue, 14 Jan 2025 10:55:27 +0800 Subject: [PATCH 2/5] apps/nshlib: replace CONFIG_NSH_LINELEN to LINE_MAX LINE_MAX: https://github.com/apache/nuttx/pull/15344 Signed-off-by: wangjianyu3 --- nshlib/nsh.h | 6 ------ nshlib/nsh_console.h | 2 +- nshlib/nsh_login.c | 5 ++--- nshlib/nsh_mmcmds.c | 8 ++++---- nshlib/nsh_parse.c | 6 +++--- nshlib/nsh_script.c | 2 +- nshlib/nsh_session.c | 4 ++-- nshlib/nsh_telnetlogin.c | 4 ++-- nshlib/nsh_timcmds.c | 4 ++-- 9 files changed, 17 insertions(+), 24 deletions(-) diff --git a/nshlib/nsh.h b/nshlib/nsh.h index 1edf9ca1c..06b7c3356 100644 --- a/nshlib/nsh.h +++ b/nshlib/nsh.h @@ -377,12 +377,6 @@ # define NSH_HERRNO_OF(err) (err) #endif -/* Maximum size of one command line (telnet or serial) */ - -#ifndef CONFIG_NSH_LINELEN -# define CONFIG_NSH_LINELEN LINE_MAX -#endif - /* The maximum number of nested if-then[-else]-fi sequences that * are permissible. */ diff --git a/nshlib/nsh_console.h b/nshlib/nsh_console.h index 31edf27e6..f22176d46 100644 --- a/nshlib/nsh_console.h +++ b/nshlib/nsh_console.h @@ -180,7 +180,7 @@ struct console_stdio_s /* Line input buffer */ - char cn_line[CONFIG_NSH_LINELEN]; + char cn_line[LINE_MAX]; }; /**************************************************************************** diff --git a/nshlib/nsh_login.c b/nshlib/nsh_login.c index 0b5996367..4dabf89d4 100644 --- a/nshlib/nsh_login.c +++ b/nshlib/nsh_login.c @@ -171,7 +171,7 @@ int nsh_login(FAR struct console_stdio_s *pstate) /* readline() returns EOF on failure */ - ret = readline_fd(pstate->cn_line, CONFIG_NSH_LINELEN, + ret = readline_fd(pstate->cn_line, LINE_MAX, INFD(pstate), OUTFD(pstate)); if (ret != EOF) { @@ -207,8 +207,7 @@ int nsh_login(FAR struct console_stdio_s *pstate) } password[0] = '\0'; - ret = readline_fd(pstate->cn_line, CONFIG_NSH_LINELEN, - INFD(pstate), -1); + ret = readline_fd(pstate->cn_line, LINE_MAX, INFD(pstate), -1); /* Enable echo again after password */ diff --git a/nshlib/nsh_mmcmds.c b/nshlib/nsh_mmcmds.c index e84f6f599..fcbb92260 100644 --- a/nshlib/nsh_mmcmds.c +++ b/nshlib/nsh_mmcmds.c @@ -56,12 +56,12 @@ int cmd_free(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) int cmd_memdump(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) { - char arg[CONFIG_NSH_LINELEN] = ""; + char arg[LINE_MAX] = ""; int i; if (argc == 1) { - strlcpy(arg, "used", CONFIG_NSH_LINELEN); + strlcpy(arg, "used", LINE_MAX); } else if (argc >= 2 && (strcmp(argv[1], "-h") == 0 || strcmp(argv[1], "help") == 0)) @@ -73,10 +73,10 @@ int cmd_memdump(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) { for (i = 1; i < argc; i++) { - strlcat(arg, argv[i], CONFIG_NSH_LINELEN); + strlcat(arg, argv[i], LINE_MAX); if (i < argc - 1) { - strlcat(arg, " ", CONFIG_NSH_LINELEN); + strlcat(arg, " ", LINE_MAX); } } } diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c index 96de181e3..26517b77e 100644 --- a/nshlib/nsh_parse.c +++ b/nshlib/nsh_parse.c @@ -603,7 +603,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { FAR char *sh_argv[4]; FAR char *sh_cmd = "sh"; - char sh_arg2[CONFIG_NSH_LINELEN]; + char sh_arg2[LINE_MAX]; DEBUGASSERT(strncmp(argv[0], sh_cmd, 3) != 0); @@ -2454,7 +2454,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) #endif #ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP - char tracebuf[CONFIG_NSH_LINELEN + 1]; + char tracebuf[LINE_MAX + 1]; strlcpy(tracebuf, cmdline, sizeof(tracebuf)); sched_note_beginex(NOTE_TAG_APP, tracebuf); @@ -2677,7 +2677,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) { FAR char *arg; FAR char *sh_argv[4]; - char sh_arg2[CONFIG_NSH_LINELEN]; + char sh_arg2[LINE_MAX]; if (argv[argc][g_pipeline1_len]) { diff --git a/nshlib/nsh_script.c b/nshlib/nsh_script.c index b9a210dff..63cd93be7 100644 --- a/nshlib/nsh_script.c +++ b/nshlib/nsh_script.c @@ -161,7 +161,7 @@ int nsh_script(FAR struct nsh_vtbl_s *vtbl, FAR const FAR char *cmd, /* Now read the next line from the script file */ - ret = readline_fd(buffer, CONFIG_NSH_LINELEN, vtbl->np.np_fd, -1); + ret = readline_fd(buffer, LINE_MAX, vtbl->np.np_fd, -1); if (ret >= 0) { /* Parse process the command. NOTE: this is recursive... diff --git a/nshlib/nsh_session.c b/nshlib/nsh_session.c index 19442791f..92ef1afc2 100644 --- a/nshlib/nsh_session.c +++ b/nshlib/nsh_session.c @@ -207,7 +207,7 @@ int nsh_session(FAR struct console_stdio_s *pstate, * occurs. Either will cause the session to terminate. */ - ret = cle_fd(pstate->cn_line, nsh_prompt(), CONFIG_NSH_LINELEN, + ret = cle_fd(pstate->cn_line, nsh_prompt(), LINE_MAX, INFD(pstate), OUTFD(pstate)); if (ret == EOF) { @@ -226,7 +226,7 @@ int nsh_session(FAR struct console_stdio_s *pstate, * will cause the session to terminate. */ - ret = readline_fd(pstate->cn_line, CONFIG_NSH_LINELEN, + ret = readline_fd(pstate->cn_line, LINE_MAX, INFD(pstate), OUTFD(pstate)); if (ret == EOF) { diff --git a/nshlib/nsh_telnetlogin.c b/nshlib/nsh_telnetlogin.c index 4fc2a4af0..bea66060f 100644 --- a/nshlib/nsh_telnetlogin.c +++ b/nshlib/nsh_telnetlogin.c @@ -176,7 +176,7 @@ int nsh_telnetlogin(FAR struct console_stdio_s *pstate) write(OUTFD(pstate), g_userprompt, strlen(g_userprompt)); username[0] = '\0'; - if (readline_fd(pstate->cn_line, CONFIG_NSH_LINELEN, + if (readline_fd(pstate->cn_line, LINE_MAX, INFD(pstate), OUTFD(pstate)) >= 0) { @@ -212,7 +212,7 @@ int nsh_telnetlogin(FAR struct console_stdio_s *pstate) } password[0] = '\0'; - ret = readline_fd(pstate->cn_line, CONFIG_NSH_LINELEN, + ret = readline_fd(pstate->cn_line, LINE_MAX, INFD(pstate), OUTFD(pstate)); /* Enable echo again after password */ diff --git a/nshlib/nsh_timcmds.c b/nshlib/nsh_timcmds.c index 3c54ca538..c85a92a86 100644 --- a/nshlib/nsh_timcmds.c +++ b/nshlib/nsh_timcmds.c @@ -550,7 +550,7 @@ int cmd_timedatectl(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) #ifndef CONFIG_NSH_DISABLE_WATCH int cmd_watch(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) { - char buffer[CONFIG_NSH_LINELEN]; + char buffer[LINE_MAX]; int interval = 2; int count = -1; FAR char *cmd; @@ -593,7 +593,7 @@ int cmd_watch(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv) for (i = 0; i < count; i++) { - strlcpy(buffer, cmd, CONFIG_NSH_LINELEN); + strlcpy(buffer, cmd, LINE_MAX); ret = nsh_parse(vtbl, buffer); if (ret < 0) { From 6dfab987e053ac52aa52ad6fe40a21803637d394 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Tue, 14 Jan 2025 11:39:07 +0800 Subject: [PATCH 3/5] apps/nshlib: Using `lib_get_tempbuffer()` to save stack space Comparison Config: "esp32s3-devkit:adb" with `CONFIG_LINE_MAX=512` Test CMD: `ls | cat | cat | cat` Without this patch | | Before Test CMD | After Test CMD | |---------------:|----------------:|----------------:| | nsh_main.STACK | 0002624/0008096 | 0002624/0008096 | | sh.STACK | 0003360/0004008 | 0003360/0004008 | | Free/Total | 355288/403116 | 355288/403116 | With this patch | | Before Test CMD | After Test CMD | |---------------:|----------------:|----------------:| | nsh_main.STACK | 0001616/0008096 | 0001616/0008096 | | sh.STACK | 0002352/0004008 | 0002352/0004008 | | Free/Total | 355288/403116 | 354760/403116 | Signed-off-by: wangjianyu3 --- nshlib/nsh_parse.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c index 26517b77e..7100a23ce 100644 --- a/nshlib/nsh_parse.c +++ b/nshlib/nsh_parse.c @@ -603,15 +603,22 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { FAR char *sh_argv[4]; FAR char *sh_cmd = "sh"; - char sh_arg2[LINE_MAX]; + FAR char *sh_arg2; DEBUGASSERT(strncmp(argv[0], sh_cmd, 3) != 0); + sh_arg2 = lib_get_tempbuffer(LINE_MAX); + if (sh_arg2 == NULL) + { + nsh_error(vtbl, g_fmtcmdoutofmemory, sh_cmd); + ret = -errno; + } + sh_arg2[0] = '\0'; for (ret = 0; ret < argc; ret++) { - strlcat(sh_arg2, argv[ret], sizeof(sh_arg2)); + strlcat(sh_arg2, argv[ret], LINE_MAX); if (ret < argc - 1) { @@ -628,7 +635,9 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, * dispatch the backgroud by sh -c "" */ - return nsh_execute(vtbl, 4, sh_argv, param); + ret = nsh_execute(vtbl, 4, sh_argv, param); + lib_put_tempbuffer(sh_arg2); + return ret; } else #endif @@ -2677,7 +2686,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) { FAR char *arg; FAR char *sh_argv[4]; - char sh_arg2[LINE_MAX]; + FAR char *sh_arg2; if (argv[argc][g_pipeline1_len]) { @@ -2695,11 +2704,19 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) goto dynlist_free; } + sh_arg2 = lib_get_tempbuffer(LINE_MAX); + if (sh_arg2 == NULL) + { + nsh_error(vtbl, g_fmtcmdoutofmemory, cmd); + ret = -errno; + goto dynlist_free; + } + sh_arg2[0] = '\0'; for (ret = 0; ret < argc; ret++) { - strlcat(sh_arg2, argv[ret], sizeof(sh_arg2)); + strlcat(sh_arg2, argv[ret], LINE_MAX); if (ret < argc - 1) { @@ -2715,6 +2732,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) ret = pipe2(pipefd, 0); if (ret < 0) { + lib_put_tempbuffer(sh_arg2); ret = -errno; goto dynlist_free; } @@ -2727,6 +2745,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) vtbl->np.np_bg = true; ret = nsh_execute(vtbl, 4, sh_argv, ¶m); + lib_put_tempbuffer(sh_arg2); vtbl->np.np_bg = bg_save; From c27607ec2284d475a6c47095fd29be3993f72f95 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Thu, 16 Jan 2025 12:12:17 +0800 Subject: [PATCH 4/5] apps/nshlib: Save result and return ERROR if `lib_get_tempbuffer()` fails Signed-off-by: wangjianyu3 --- nshlib/nsh_parse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c index 7100a23ce..b9b1dd101 100644 --- a/nshlib/nsh_parse.c +++ b/nshlib/nsh_parse.c @@ -612,6 +612,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { nsh_error(vtbl, g_fmtcmdoutofmemory, sh_cmd); ret = -errno; + goto close_redir; } sh_arg2[0] = '\0'; From c75dc2d25f0aaef376a208fb4c8b50bfc6222ded Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Wed, 18 Dec 2024 17:26:03 +0800 Subject: [PATCH 5/5] nshlib/nsh_parse: Closing fds opened for redirection if necessary Coverity Log CID 1612743: (#1 of 1): Resource leak (RESOURCE_LEAK) 12. leaked_handle: The handle variable fd_out goes out of scope and leaks the handle. Signed-off-by: wangjianyu3 --- nshlib/nsh_parse.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c index b9b1dd101..7362a6cd7 100644 --- a/nshlib/nsh_parse.c +++ b/nshlib/nsh_parse.c @@ -500,6 +500,8 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char *argv[], FAR const struct nsh_param_s *param) { + int fd_out = STDOUT_FILENO; + int fd_in = STDIN_FILENO; int ret; /* DO NOT CHANGE THE ORDERING OF THE FOLLOWING STEPS @@ -645,9 +647,6 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { uint8_t save[SAVE_SIZE]; - int fd_in = STDIN_FILENO; - int fd_out = STDOUT_FILENO; - /* Redirected output? */ if (vtbl->np.np_redir_out) @@ -665,7 +664,8 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO); - return nsh_saveresult(vtbl, true); + ret = errno; + goto close_redir; } } else @@ -691,7 +691,8 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO); - return nsh_saveresult(vtbl, true); + ret = errno; + goto close_redir; } } else @@ -724,22 +725,27 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, { nsh_undirect(vtbl, save); } + } - /* Mark errors so that it is possible to test for non-zero return - * values in nsh scripts. - */ +close_redir: - if (ret < 0) - { - return nsh_saveresult(vtbl, true); - } + /* Closing fds opened for redirection if necessary */ + + if (fd_out > STDOUT_FILENO) + { + close(fd_out); + } + + if (fd_in > STDIN_FILENO) + { + close(fd_in); } /* Return success if the command succeeded (or at least, starting of the * command task succeeded). */ - return nsh_saveresult(vtbl, false); + return nsh_saveresult(vtbl, ret != OK); } /****************************************************************************