This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
commit 3da204c23ede587924fd6577e3cfb59f12b18626 Author: wangjianyu3 <[email protected]> AuthorDate: Mon Oct 14 23:27:14 2024 +0800 Pack parameters of nsh_execute() as struct nsh_exec_param_s 1. Input redirect flags currently is hardcode, passing by arguments maybe better. 2. Only support redirect to path_name, redirect to fd is needed for pipeline. Test 1. Redirect in cat < /etc/init.d/rc.sysinit 2. Redirect with FIFO mkfifo /dev/testfifo cat /dev/testfifo & ls > /dev/testfifo 3. NSH Params set name `uname` echo $name Signed-off-by: wangjianyu3 <[email protected]> --- builtin/exec_builtin.c | 64 +++++++++++----------- include/builtin/builtin.h | 12 ++--- include/nshlib/nshlib.h | 19 +++++++ nshlib/nsh.h | 7 ++- nshlib/nsh_builtin.c | 12 ++--- nshlib/nsh_fileapps.c | 72 +++++++++++++------------ nshlib/nsh_parse.c | 126 ++++++++++++++++++++++++++----------------- testing/cmocka/cmocka_main.c | 2 +- 8 files changed, 181 insertions(+), 133 deletions(-) diff --git a/builtin/exec_builtin.c b/builtin/exec_builtin.c index 4c2920b09..6287cf07c 100644 --- a/builtin/exec_builtin.c +++ b/builtin/exec_builtin.c @@ -45,13 +45,7 @@ * Input Parameter: * filename - Name of the linked-in binary to be started. * argv - Argument list - * redirfile_in - If input is redirected, this parameter will be non-NULL - * and will provide the full path to the file. - * redirfile_out - If output is redirected, this parameter will be non-NULL - * and will provide the full path to the file. - * oflags - If output is redirected, this parameter will provide the - * open flags to use. This will support file replacement - * of appending to an existing file. + * param - Parameters for execute. * * Returned Value: * This is an end-user function, so it follows the normal convention: @@ -61,13 +55,12 @@ ****************************************************************************/ int exec_builtin(FAR const char *appname, FAR char * const *argv, - FAR const char *redirfile_in, FAR const char *redirfile_out, - int oflags) + FAR const struct nsh_param_s *param) { FAR const struct builtin_s *builtin; posix_spawnattr_t attr; posix_spawn_file_actions_t file_actions; - struct sched_param param; + struct sched_param sched; pid_t pid; int index; int ret; @@ -106,8 +99,8 @@ int exec_builtin(FAR const char *appname, FAR char * const *argv, /* Set the correct task size and priority */ - param.sched_priority = builtin->priority; - ret = posix_spawnattr_setschedparam(&attr, ¶m); + sched.sched_priority = builtin->priority; + ret = posix_spawnattr_setschedparam(&attr, &sched); if (ret != 0) { goto errout_with_actions; @@ -147,33 +140,40 @@ int exec_builtin(FAR const char *appname, FAR char * const *argv, #endif - /* Is input being redirected? */ - - if (redirfile_in) + if (param) { - /* Set up to close open redirfile and set to stdin (0) */ + /* Is input being redirected? */ - ret = posix_spawn_file_actions_addopen(&file_actions, 0, - redirfile_in, O_RDONLY, 0); - if (ret != 0) + if (param->file_in) { - serr("ERROR: posix_spawn_file_actions_addopen failed: %d\n", ret); - goto errout_with_actions; + /* Set up to close open redirfile and set to stdin (0) */ + + ret = posix_spawn_file_actions_addopen(&file_actions, 0, + param->file_in, + param->oflags_in, 0); + if (ret != 0) + { + serr("ERROR: posix_spawn_file_actions_addopen failed: %d\n", + ret); + goto errout_with_actions; + } } - } - /* Is output being redirected? */ - - if (redirfile_out) - { - /* Set up to close open redirfile and set to stdout (1) */ + /* Is output being redirected? */ - ret = posix_spawn_file_actions_addopen(&file_actions, 1, - redirfile_out, oflags, 0644); - if (ret != 0) + if (param->file_out) { - serr("ERROR: posix_spawn_file_actions_addopen failed: %d\n", ret); - goto errout_with_actions; + /* Set up to close open redirfile and set to stdout (1) */ + + ret = posix_spawn_file_actions_addopen(&file_actions, 1, + param->file_out, + param->oflags_out, 0644); + if (ret != 0) + { + serr("ERROR: posix_spawn_file_actions_addopen failed: %d\n", + ret); + goto errout_with_actions; + } } } diff --git a/include/builtin/builtin.h b/include/builtin/builtin.h index cc589db2f..8b4ae4925 100644 --- a/include/builtin/builtin.h +++ b/include/builtin/builtin.h @@ -30,6 +30,7 @@ #include <sys/types.h> #include <nuttx/lib/builtin.h> +#include <nshlib/nshlib.h> /**************************************************************************** * Pre-processor Definitions @@ -66,13 +67,7 @@ extern "C" * Input Parameter: * filename - Name of the linked-in binary to be started. * argv - Argument list - * redirfile_in - If input is redirected, this parameter will be non-NULL - * and will provide the full path to the file. - * redirfile_out - If output is redirected, this parameter will be non-NULL - * and will provide the full path to the file. - * oflags - If output is redirected, this parameter will provide the - * open flags to use. This will support file replacement - * of appending to an existing file. + * param - Parameters for execute. * * Returned Value: * This is an end-user function, so it follows the normal convention: @@ -82,8 +77,7 @@ extern "C" ****************************************************************************/ int exec_builtin(FAR const char *appname, FAR char * const *argv, - FAR const char *redirfile_in, FAR const char *redirfile_out, - int oflags); + FAR const struct nsh_param_s *param); #undef EXTERN #if defined(__cplusplus) diff --git a/include/nshlib/nshlib.h b/include/nshlib/nshlib.h index 68ab44810..b8f8bbbe4 100644 --- a/include/nshlib/nshlib.h +++ b/include/nshlib/nshlib.h @@ -65,6 +65,25 @@ # define SCHED_NSH SCHED_FIFO #endif +struct nsh_param_s +{ + /* Redirect input/output through `fd` OR `path_name` + * + * Select one: + * 1. Using fd_in/fd_out as oldfd for dup2() if greater than -1. + * 2. Using file_in/file_out as full path to the file if it is + * not NULL, and oflags_in/oflags_out as flags for open(). + */ + + int fd_in; + int fd_out; + + int oflags_in; + int oflags_out; + FAR const char *file_in; + FAR const char *file_out; +}; + /**************************************************************************** * Public Data ****************************************************************************/ diff --git a/nshlib/nsh.h b/nshlib/nsh.h index 5347ef015..20d8f82fa 100644 --- a/nshlib/nsh.h +++ b/nshlib/nsh.h @@ -39,6 +39,7 @@ #endif #include <nuttx/usb/usbdev_trace.h> +#include <nshlib/nshlib.h> /**************************************************************************** * Pre-processor Definitions @@ -858,14 +859,12 @@ int nsh_command(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char *argv[]); #ifdef CONFIG_NSH_BUILTIN_APPS int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, - FAR char **argv, FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags); + FAR char **argv, FAR const struct nsh_param_s *param); #endif #ifdef CONFIG_NSH_FILE_APPS int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, - FAR char **argv, FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags); + FAR char **argv, FAR const struct nsh_param_s *param); #endif #ifndef CONFIG_DISABLE_ENVIRON diff --git a/nshlib/nsh_builtin.c b/nshlib/nsh_builtin.c index 7cdfd9b61..cfd951207 100644 --- a/nshlib/nsh_builtin.c +++ b/nshlib/nsh_builtin.c @@ -69,8 +69,8 @@ ****************************************************************************/ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, - FAR char **argv, FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags) + FAR char **argv, + FAR const struct nsh_param_s *param) { #if !defined(CONFIG_NSH_DISABLEBG) && defined(CONFIG_SCHED_CHILD_STATUS) struct sigaction act; @@ -102,7 +102,7 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, * applications. */ - ret = exec_builtin(cmd, argv, redirfile_in, redirfile_out, oflags); + ret = exec_builtin(cmd, argv, param); if (ret >= 0) { /* The application was successfully started with pre-emption disabled. @@ -234,9 +234,9 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, sigaction(SIGCHLD, &old, NULL); # endif - struct sched_param param; - sched_getparam(ret, ¶m); - nsh_output(vtbl, "%s [%d:%d]\n", cmd, ret, param.sched_priority); + struct sched_param sched; + sched_getparam(ret, &sched); + nsh_output(vtbl, "%s [%d:%d]\n", cmd, ret, sched.sched_priority); /* Backgrounded commands always 'succeed' as long as we can start * them. diff --git a/nshlib/nsh_fileapps.c b/nshlib/nsh_fileapps.c index a5ae19dd5..6aa4f5030 100644 --- a/nshlib/nsh_fileapps.c +++ b/nshlib/nsh_fileapps.c @@ -70,8 +70,7 @@ ****************************************************************************/ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, - FAR char **argv, FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags) + FAR char **argv, FAR const struct nsh_param_s *param) { posix_spawn_file_actions_t file_actions; posix_spawnattr_t attr; @@ -107,39 +106,46 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, goto errout_with_actions; } - /* Handle redirection of input */ - - if (redirfile_in) + if (param) { - /* Set up to close open redirfile and set to stdin (0) */ + /* Handle redirection of input */ - ret = posix_spawn_file_actions_addopen(&file_actions, 0, - redirfile_in, O_RDONLY, 0); - if (ret != 0) + if (param->file_in) { - nsh_error(vtbl, g_fmtcmdfailed, cmd, - "posix_spawn_file_actions_addopen", - NSH_ERRNO); - goto errout_with_actions; + /* Set up to close open redirfile and set to stdin (0) */ + + ret = posix_spawn_file_actions_addopen(&file_actions, 0, + param->file_in, + param->oflags_in, + 0); + if (ret != 0) + { + nsh_error(vtbl, g_fmtcmdfailed, cmd, + "posix_spawn_file_actions_addopen", + NSH_ERRNO); + goto errout_with_actions; + } } - } - /* Handle re-direction of output */ + /* Handle re-direction of output */ - if (redirfile_out) - { - ret = posix_spawn_file_actions_addopen(&file_actions, 1, redirfile_out, - oflags, 0644); - if (ret != 0) + if (param->file_out) { - /* posix_spawn_file_actions_addopen returns a positive errno - * value on failure. - */ + ret = posix_spawn_file_actions_addopen(&file_actions, 1, + param->file_out, + param->oflags_out, + 0644); + if (ret != 0) + { + /* posix_spawn_file_actions_addopen returns a positive errno + * value on failure. + */ - nsh_error(vtbl, g_fmtcmdfailed, cmd, - "posix_spawn_file_actions_addopen", - NSH_ERRNO); - goto errout_with_attrs; + nsh_error(vtbl, g_fmtcmdfailed, cmd, + "posix_spawn_file_actions_addopen", + NSH_ERRNO); + goto errout_with_attrs; + } } } @@ -151,7 +157,7 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, if (index >= 0) { FAR const struct builtin_s *builtin; - struct sched_param param; + struct sched_param sched; /* Get information about the builtin */ @@ -164,8 +170,8 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, /* Set the correct task size and priority */ - param.sched_priority = builtin->priority; - ret = posix_spawnattr_setschedparam(&attr, ¶m); + sched.sched_priority = builtin->priority; + ret = posix_spawnattr_setschedparam(&attr, &sched); if (ret != 0) { goto errout_with_actions; @@ -298,9 +304,9 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, #if !defined(CONFIG_SCHED_WAITPID) || !defined(CONFIG_NSH_DISABLEBG) { - struct sched_param param; - sched_getparam(ret, ¶m); - nsh_output(vtbl, "%s [%d:%d]\n", cmd, ret, param.sched_priority); + struct sched_param sched; + sched_getparam(ret, &sched); + nsh_output(vtbl, "%s [%d:%d]\n", cmd, ret, sched.sched_priority); /* Backgrounded commands always 'succeed' as long as we can start * them. diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c index dd1a9aa97..ac99fbf53 100644 --- a/nshlib/nsh_parse.c +++ b/nshlib/nsh_parse.c @@ -162,8 +162,8 @@ static void nsh_alist_free(FAR struct nsh_vtbl_s *vtbl, static int nsh_saveresult(FAR struct nsh_vtbl_s *vtbl, bool result); static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, - int argc, FAR char *argv[], FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags); + int argc, FAR char *argv[], + FAR const struct nsh_param_s *param); #ifdef CONFIG_NSH_CMDPARMS static FAR char *nsh_filecat(FAR struct nsh_vtbl_s *vtbl, FAR char *s1, @@ -239,7 +239,7 @@ static int nsh_nice(FAR struct nsh_vtbl_s *vtbl, FAR char **ppcmd, #ifdef CONFIG_NSH_CMDPARMS static int nsh_parse_cmdparm(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline, - FAR const char *redirfile_out); + FAR const struct nsh_param_s *param); #endif static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline); @@ -491,8 +491,7 @@ static int nsh_saveresult(FAR struct nsh_vtbl_s *vtbl, bool result) static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char *argv[], - FAR const char *redirfile_in, - FAR const char *redirfile_out, int oflags) + FAR const struct nsh_param_s *param) { int ret; @@ -531,8 +530,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, */ #ifdef CONFIG_NSH_BUILTIN_APPS - ret = nsh_builtin(vtbl, argv[0], argv, redirfile_in, redirfile_out, - oflags); + ret = nsh_builtin(vtbl, argv[0], argv, param); if (ret >= 0) { /* nsh_builtin() returned 0 or 1. This means that the built-in @@ -569,8 +567,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, */ #ifdef CONFIG_NSH_FILE_APPS - ret = nsh_fileapp(vtbl, argv[0], argv, redirfile_in, - redirfile_out, oflags); + ret = nsh_fileapp(vtbl, argv[0], argv, param); if (ret >= 0) { /* nsh_fileapp() returned 0 or 1. This means that the built-in @@ -623,8 +620,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, * dispatch the backgroud by sh -c "" */ - return nsh_execute(vtbl, 4, sh_argv, - redirfile_in, redirfile_out, oflags); + return nsh_execute(vtbl, 4, sh_argv, param); } else #endif @@ -638,17 +634,25 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, if (vtbl->np.np_redir_out) { - /* Open the redirection file. This file will eventually - * be closed by a call to either nsh_release (if the command - * is executed in the background) or by nsh_undirect if the - * command is executed in the foreground. - */ + if (param->file_out) + { + /* Open the redirection file. This file will eventually + * be closed by a call to either nsh_release (if the command + * is executed in the background) or by nsh_undirect if the + * command is executed in the foreground. + */ - fd_out = open(redirfile_out, oflags, 0666); - if (fd_out < 0) + fd_out = open(param->file_out, param->oflags_out, 0666); + if (fd_out < 0) + { + nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", + NSH_ERRNO); + return nsh_saveresult(vtbl, true); + } + } + else { - nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO); - return nsh_saveresult(vtbl, true); + fd_out = param->fd_out; } } @@ -656,17 +660,25 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl, if (vtbl->np.np_redir_in) { - /* Open the redirection file. This file will eventually - * be closed by a call to either nsh_release (if the command - * is executed in the background) or by nsh_undirect if the - * command is executed in the foreground. - */ + if (param->file_in) + { + /* Open the redirection file. This file will eventually + * be closed by a call to either nsh_release (if the command + * is executed in the background) or by nsh_undirect if the + * command is executed in the foreground. + */ - fd_in = open(redirfile_in, O_RDONLY, 0); - if (fd_in < 0) + fd_in = open(param->file_in, param->oflags_in, 0); + if (fd_in < 0) + { + nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", + NSH_ERRNO); + return nsh_saveresult(vtbl, true); + } + } + else { - nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO); - return nsh_saveresult(vtbl, true); + fd_in = param->fd_in; } } @@ -841,6 +853,16 @@ errout_with_alloc: static FAR char *nsh_cmdparm(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline, FAR char **allocation) { + struct nsh_param_s param = + { + .fd_in = -1, + .fd_out = -1, + .oflags_in = 0, + .oflags_out = O_WRONLY | O_CREAT | O_TRUNC, + .file_in = NULL, + .file_out = NULL + }; + FAR char *tmpfile; FAR char *argument; int ret; @@ -869,7 +891,8 @@ static FAR char *nsh_cmdparm(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline, * options. */ - ret = nsh_parse_cmdparm(vtbl, cmdline, tmpfile); + param.file_out = tmpfile; + ret = nsh_parse_cmdparm(vtbl, cmdline, ¶m); if (ret != OK) { /* Report the failure */ @@ -2261,7 +2284,7 @@ static int nsh_nice(FAR struct nsh_vtbl_s *vtbl, FAR char **ppcmd, #ifdef CONFIG_NSH_CMDPARMS static int nsh_parse_cmdparm(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline, - FAR const char *redirfile_out) + FAR const struct nsh_param_s *param) { NSH_MEMLIST_TYPE memlist; NSH_ALIASLIST_TYPE alist; @@ -2356,8 +2379,7 @@ static int nsh_parse_cmdparm(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline, /* Then execute the command */ - ret = nsh_execute(vtbl, argc, argv, NULL, redirfile_out, - O_WRONLY | O_CREAT | O_TRUNC); + ret = nsh_execute(vtbl, argc, argv, param); /* Restore the backgrounding and redirection state */ @@ -2383,14 +2405,21 @@ exit: static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) { + struct nsh_param_s param = + { + .fd_in = -1, + .fd_out = -1, + .oflags_in = 0, + .oflags_out = 0, + .file_in = NULL, + .file_out = NULL + }; + NSH_MEMLIST_TYPE memlist; NSH_ALIASLIST_TYPE alist; FAR char *argv[MAX_ARGV_ENTRIES]; FAR char *saveptr; FAR char *cmd; - FAR char *redirfile_out = NULL; - FAR char *redirfile_in = NULL; - int oflags = 0; int argc; int ret; bool redirect_out_save = false; @@ -2567,8 +2596,8 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) redirect_out_save = vtbl->np.np_redir_out; vtbl->np.np_redir_out = true; - oflags = O_WRONLY | O_CREAT | O_APPEND; - redirfile_out = nsh_getfullpath(vtbl, arg); + param.oflags_out = O_WRONLY | O_CREAT | O_APPEND; + param.file_out = nsh_getfullpath(vtbl, arg); } else if (!strncmp(argv[argc], g_redirect_out1, redirect_out1_len)) { @@ -2591,8 +2620,8 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) redirect_out_save = vtbl->np.np_redir_out; vtbl->np.np_redir_out = true; - oflags = O_WRONLY | O_CREAT | O_TRUNC; - redirfile_out = nsh_getfullpath(vtbl, arg); + param.oflags_out = O_WRONLY | O_CREAT | O_TRUNC; + param.file_out = nsh_getfullpath(vtbl, arg); } else if (!strncmp(argv[argc], g_redirect_in1, redirect_in1_len)) { @@ -2613,9 +2642,10 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) goto dynlist_free; } - redirect_in_save = vtbl->np.np_redir_in; - vtbl->np.np_redir_in = true; - redirfile_in = nsh_getfullpath(vtbl, arg); + redirect_in_save = vtbl->np.np_redir_in; + vtbl->np.np_redir_in = true; + param.oflags_in = O_RDONLY; + param.file_in = nsh_getfullpath(vtbl, arg); } else { @@ -2646,23 +2676,23 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline) /* Then execute the command */ - ret = nsh_execute(vtbl, argc, argv, redirfile_in, redirfile_out, oflags); + ret = nsh_execute(vtbl, argc, argv, ¶m); /* Free any allocated resources */ /* Free the redirected output file path */ - if (redirfile_out) + if (param.file_out) { - nsh_freefullpath(redirfile_out); + nsh_freefullpath((char *)param.file_out); vtbl->np.np_redir_out = redirect_out_save; } /* Free the redirected input file path */ - if (redirfile_in) + if (param.file_in) { - nsh_freefullpath(redirfile_in); + nsh_freefullpath((char *)param.file_in); vtbl->np.np_redir_in = redirect_in_save; } diff --git a/testing/cmocka/cmocka_main.c b/testing/cmocka/cmocka_main.c index f3220931f..299550954 100644 --- a/testing/cmocka/cmocka_main.c +++ b/testing/cmocka/cmocka_main.c @@ -198,7 +198,7 @@ int main(int argc, FAR char *argv[]) } bypass[0] = (FAR char *)builtin->name; - ret = exec_builtin(builtin->name, bypass, NULL, NULL, 0); + ret = exec_builtin(builtin->name, bypass, NULL); if (ret >= 0) { waitpid(ret, &ret, WUNTRACED);
