From c0ae392704e4e19f7c09fae1e753f777133afe8e Mon Sep 17 00:00:00 2001 From: patacongo Date: Thu, 17 Jan 2013 20:25:32 +0000 Subject: Misc bug fixes related to NSH file execution git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@5530 42af7a65-404d-4744-a932-0658087f49c3 --- apps/builtin/exec_builtin.c | 16 ++++++++++++++ apps/nshlib/nsh_builtin.c | 54 +++++++++++++++++++++++++++------------------ apps/nshlib/nsh_fileapps.c | 53 +++++++++++++++++++++++++++----------------- 3 files changed, 82 insertions(+), 41 deletions(-) (limited to 'apps') diff --git a/apps/builtin/exec_builtin.c b/apps/builtin/exec_builtin.c index d4431164c..803d1ef34 100644 --- a/apps/builtin/exec_builtin.c +++ b/apps/builtin/exec_builtin.c @@ -46,6 +46,7 @@ #include +#include #include #include #include @@ -92,7 +93,9 @@ struct builtin_parms_s ****************************************************************************/ static sem_t g_builtin_parmsem = SEM_INITIALIZER(1); +#ifndef CONFIG_SCHED_WAITPID static sem_t g_builtin_execsem = SEM_INITIALIZER(0); +#endif static struct builtin_parms_s g_builtin_parms; /**************************************************************************** @@ -274,7 +277,9 @@ static int builtin_proxy(int argc, char *argv[]) */ g_builtin_parms.result = ret; +#ifndef CONFIG_SCHED_WAITPID builtin_semgive(&g_builtin_execsem); +#endif return 0; } @@ -299,6 +304,9 @@ static inline int builtin_startproxy(int index, FAR const char **argv, struct sched_param param; pid_t proxy; int errcode; +#ifdef CONFIG_SCHED_WAITPID + int status; +#endif int ret; svdbg("index=%d argv=%p redirfile=%s oflags=%04x\n", @@ -353,7 +361,15 @@ static inline int builtin_startproxy(int index, FAR const char **argv, * for this. */ +#ifdef CONFIG_SCHED_WAITPID + ret = waitpid(proxy, &status, 0); + if (ret < 0) + { + sdbg("ERROR: waitpid() failed: %d\n", errno); + } +#else bultin_semtake(&g_builtin_execsem); +#endif /* Get the result and relinquish our access to the parameter structure */ diff --git a/apps/nshlib/nsh_builtin.c b/apps/nshlib/nsh_builtin.c index 2d23ca8d8..343a0824b 100644 --- a/apps/nshlib/nsh_builtin.c +++ b/apps/nshlib/nsh_builtin.c @@ -51,7 +51,6 @@ #endif #include -#include #include #include @@ -117,8 +116,8 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { int ret = OK; - /* Lock the scheduler to prevent the application from running until the - * waitpid() has been called. + /* Lock the scheduler in an attempt to prevent the application from + * running until waitpid() has been called. */ sched_lock(); @@ -146,18 +145,6 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, #ifdef CONFIG_SCHED_WAITPID - /* Check if the application is still running */ - - if (kill(ret, 0) < 0) - { - /* It is not running. In this case, we have no idea if the - * application ran successfully or not. Let's assume that is - * did. - */ - - return 0; - } - /* CONFIG_SCHED_WAITPID is selected, so we may run the command in * foreground unless we were specifically requested to run the command * in background (and running commands in background is enabled). @@ -169,15 +156,40 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { int rc = 0; - /* Wait for the application to exit. Since we have locked the - * scheduler above, we know that the application has not yet - * started and there is no possibility that it has already exited. - * The scheduler will be unlocked while waitpid is waiting and the - * application will be able to run. + /* Wait for the application to exit. We did locked the scheduler + * above, but that does not guarantee that the application did not + * run in the case where I/O was redirected. The scheduler will + * be unlocked while waitpid is waiting and if the application has + * not yet run, it will be able to to do so. */ ret = waitpid(ret, &rc, 0); - if (ret >= 0) + if (ret < 0) + { + /* If the child thread does not exist, waitpid() will return + * the error ECHLD. Since we know that the task was successfully + * started, this must be one of the cases described above; we + * have to assume that the task already exit'ed. In this case, + * we have no idea if the application ran successfully or not + * (because NuttX does not retain exit status of child tasks). + * Let's assume that is did run successfully. + */ + + int errcode = errno; + if (errcode == ECHILD) + { + ret = OK; + } + else + { + nsh_output(vtbl, g_fmtcmdfailed, cmd, "waitpid", + NSH_ERRNO_OF(errcode)); + } + } + + /* Waitpid completed the wait successfully */ + + else { /* We can't return the exact status (nsh has nowhere to put it) * so just pass back zero/nonzero in a fashion that doesn't look diff --git a/apps/nshlib/nsh_fileapps.c b/apps/nshlib/nsh_fileapps.c index 7f9f58e53..92146a68d 100644 --- a/apps/nshlib/nsh_fileapps.c +++ b/apps/nshlib/nsh_fileapps.c @@ -44,7 +44,6 @@ #endif #include -#include #include #include #include @@ -154,8 +153,8 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, } } - /* Lock the scheduler to prevent the application from running until the - * waitpid() has been called. + /* Lock the scheduler in an attempt to prevent the application from + * running until waitpid() has been called. */ sched_lock(); @@ -182,17 +181,6 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, */ #ifdef CONFIG_SCHED_WAITPID - /* Check if the application is still running */ - - if (kill(ret, 0) < 0) - { - /* It is not running. In this case, we have no idea if the - * application ran successfully or not. Let's assume that is - * did. - */ - - return 0; - } /* CONFIG_SCHED_WAITPID is selected, so we may run the command in * foreground unless we were specifically requested to run the command @@ -205,15 +193,40 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { int rc = 0; - /* Wait for the application to exit. Since we have locked the - * scheduler above, we know that the application has not yet - * started and there is no possibility that it has already exited. - * The scheduler will be unlocked while waitpid is waiting and the - * application will be able to run. + /* Wait for the application to exit. We did locked the scheduler + * above, but that does not guarantee that the application did not + * run in the case where I/O was redirected. The scheduler will + * be unlocked while waitpid is waiting and if the application has + * not yet run, it will be able to to do so. */ ret = waitpid(pid, &rc, 0); - if (ret >= 0) + if (ret < 0) + { + /* If the child thread does not exist, waitpid() will return + * the error ECHLD. Since we know that the task was successfully + * started, this must be one of the cases described above; we + * have to assume that the task already exit'ed. In this case, + * we have no idea if the application ran successfully or not + * (because NuttX does not retain exit status of child tasks). + * Let's assume that is did run successfully. + */ + + int errcode = errno; + if (errcode == ECHILD) + { + ret = OK; + } + else + { + nsh_output(vtbl, g_fmtcmdfailed, cmd, "waitpid", + NSH_ERRNO_OF(errcode)); + } + } + + /* Waitpid completed the wait successfully */ + + else { /* We can't return the exact status (nsh has nowhere to put it) * so just pass back zero/nonzero in a fashion that doesn't look -- cgit v1.2.3