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

5 files changed, 123 insertions, 49 deletions

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 <nuttx/config.h>
 
+#include <sys/wait.h>
 #include <sched.h>
 #include <string.h>
 #include <fcntl.h>
@@ -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 <stdbool.h>
-#include <signal.h>
 #include <errno.h>
 #include <string.h>
 
@@ -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 <stdbool.h>
-#include <signal.h>
 #include <spawn.h>
 #include <errno.h>
 #include <string.h>
@@ -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 
diff --git a/nuttx/configs/sim/README.txt b/nuttx/configs/sim/README.txt
index 67ff73019..963a46e08 100644
--- a/nuttx/configs/sim/README.txt
+++ b/nuttx/configs/sim/README.txt
@@ -291,14 +291,31 @@ nsh
      apps/examples/hello.
 
   3. This configuration has BINFS enabled so that the builtin applications
-     can be made visible in the file system.  For example:
-
-     NuttShell (NSH) NuttX-6.24
-     nsh> mount -t binfs /bin
-     nsh> ls /bin
-     /bin:
-       hello
-     nsh> 
+     can be made visible in the file system.  Because of that, the
+     build in applications do not work as other examples.
+
+     For example trying to execute the hello builtin application will
+     fail:
+
+       nsh> hello   
+       nsh: hello: command not found
+       nsh>
+
+     Unless you first mount the BINFS file system:
+
+       nsh> mount -t binfs /bin
+       nsh> ls /bin
+       /bin:
+         hello
+       nsh> echo $PATH
+       /bin
+       nsh> hello
+       Hello, World!!
+       nsh> 
+
+     Notice that the executable 'hello' is found using the value in the PATH
+     variable (which was preset to "/bin").  If the PATH variable were not set
+     then you would have to use /bin/hello on the command line.
 
 nsh2
 
diff --git a/nuttx/libc/spawn/lib_ps.c b/nuttx/libc/spawn/lib_ps.c
index 000f711a3..1bf7c2113 100644
--- a/nuttx/libc/spawn/lib_ps.c
+++ b/nuttx/libc/spawn/lib_ps.c
@@ -39,6 +39,7 @@
 
 #include <nuttx/config.h>
 
+#include <sys/wait.h>
 #include <semaphore.h>
 #include <signal.h>
 #include <sched.h>
@@ -75,7 +76,9 @@ struct spawn_parms_s
  ****************************************************************************/
 
 static sem_t g_ps_parmsem = SEM_INITIALIZER(1);
+#ifndef CONFIG_SCHED_WAITPID
 static sem_t g_ps_execsem = SEM_INITIALIZER(0);
+#endif
 static struct spawn_parms_s g_ps_parms;
 
 /****************************************************************************
@@ -425,7 +428,9 @@ static int spawn_proxy(int argc, char *argv[])
    */
 
   g_ps_parms.result = ret;
+#ifndef CONFIG_SCHED_WAITPID
   ps_semgive(&g_ps_execsem);
+#endif
   return 0;
 }
 
@@ -540,6 +545,9 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path,
 {
   struct sched_param param;
   pid_t proxy;
+#ifdef CONFIG_SCHED_WAITPID
+  int status;
+#endif
   int ret;
 
   DEBUGASSERT(path);
@@ -613,7 +621,15 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path,
 
    /* Wait for the proxy to complete its job */
 
+#ifdef CONFIG_SCHED_WAITPID
+   ret = waitpid(proxy, &status, 0);
+   if (ret < 0)
+     {
+      sdbg("ERROR: waitpid() failed: %d\n", errno);
+     }
+#else
    ps_semtake(&g_ps_execsem);
+#endif
 
    /* Get the result and relinquish our access to the parameter structure */