All argv types should be char * const * not const char **

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@5599 42af7a65-404d-4744-a932-0658087f49c3

ChangeLog.txt

@@ -507,3 +507,8 @@
 	  occur (it would return an empty string which is not very useful).
 
 6.26 2013-xx-xx Gregory Nutt <gnutt@nuttx.org>
+
+	* apps/builtin/exec_builtin.c:  Now uses task_spawn() to start
+	  builtin applications.
+	* Type of argv has changed from const char ** to char * const *
+

builtin/exec_builtin.c

@@ -46,44 +46,30 @@
 
 #include <nuttx/config.h>
 
-#include <sys/wait.h>
+#include <spawn.h>
-#include <sched.h>
-#include <string.h>
 #include <fcntl.h>
-#include <semaphore.h>
 #include <errno.h>
 #include <debug.h>
 
+#if 0
+#include <sys/wait.h>
+#include <sched.h>
+#include <string.h>
+#include <semaphore.h>
+
 #include <nuttx/binfmt/builtin.h>
+#endif
+
 #include <apps/builtin.h>
 
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
-#ifndef CONFIG_BUILTIN_PROXY_STACKSIZE
-#  define CONFIG_BUILTIN_PROXY_STACKSIZE 1024
-#endif
-
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
-struct builtin_parms_s
-{
-  /* Input values */
-
-  FAR const char *redirfile;
-  FAR const char **argv;
-  int oflags;
-  int index;
-
-  /* Returned values */
-
-  pid_t result;
-  int errcode;
-};
-
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
@@ -92,319 +78,10 @@ struct builtin_parms_s
  * Private Data
  ****************************************************************************/
 
-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;
-
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: bultin_semtake and builtin_semgive
- *
- * Description:
- *   Give and take semaphores
- *
- * Input Parameters:
- *
- *   sem - The semaphore to act on.
- *
- * Returned Value:
- *   None
- *
- ****************************************************************************/
-
-static void bultin_semtake(FAR sem_t *sem)
-{
-  int ret;
-
-  do
-    {
-      ret = sem_wait(sem);
-      ASSERT(ret == 0 || get_errno() == EINTR);
-    }
-  while (ret != 0);
-}
-
-#define builtin_semgive(sem) sem_post(sem)
-
-/****************************************************************************
- * Name: builtin_taskcreate
- *
- * Description:
- *   Execute the builtin task
- *
- * Returned Value:
- *   On success, the task ID of the builtin task is returned; On failure, -1
- *  (ERROR) is returned and the errno is set appropriately.
- *
- ****************************************************************************/
-
-static int builtin_taskcreate(int index, FAR const char **argv)
-{
-  FAR const struct builtin_s *b;
-  int ret;
-
-  b = builtin_for_index(index);
-
-  if (b == NULL)
-    { 
-      set_errno(ENOENT);
-      return ERROR;
-    }
-
-  /* Disable pre-emption.  This means that although we start the builtin
-   * application here, it will not actually run until pre-emption is
-   * re-enabled below.
-   */
-
-  sched_lock();
-
-  /* Start the builtin application task */
-
-  ret = TASK_CREATE(b->name, b->priority, b->stacksize, b->main,
-                    (argv) ? &argv[1] : (FAR const char **)NULL);
-
-  /* If robin robin scheduling is enabled, then set the scheduling policy
-   * of the new task to SCHED_RR before it has a chance to run.
-   */
-
-#if CONFIG_RR_INTERVAL > 0
-  if (ret > 0)
-    {
-      struct sched_param param;
-
-      /* Pre-emption is disabled so the task creation and the
-       * following operation will be atomic.  The priority of the
-       * new task cannot yet have changed from its initial value.
-       */
-
-      param.sched_priority = b->priority;
-      (void)sched_setscheduler(ret, SCHED_RR, &param);
-    }
-#endif
-
-  /* Now let the builtin application run */
-
-  sched_unlock();
-
-  /* Return the task ID of the new task if the task was sucessfully
-   * started.  Otherwise, ret will be ERROR (and the errno value will
-   * be set appropriately).
-   */
-
-  return ret;
-}
-
-/****************************************************************************
- * Name: builtin_proxy
- *
- * Description:
- *   Perform output redirection, then execute the builtin task.
- *
- * Input Parameters:
- *   Standard task start-up parameters
- *
- * Returned Value:
- *   Standard task return value.
- *
- ****************************************************************************/
-
-static int builtin_proxy(int argc, char *argv[])
-{
-  int fd;
-  int ret = ERROR;
-
-  /* Open the output file for redirection */
-
-  svdbg("Open'ing redirfile=%s oflags=%04x mode=0644\n",
-        g_builtin_parms.redirfile, g_builtin_parms.oflags);
-
-  fd = open(g_builtin_parms.redirfile, g_builtin_parms.oflags, 0644);
-  if (fd < 0)
-    {
-      /* Remember the errno value.  ret is already set to ERROR */
-
-      g_builtin_parms.errcode = get_errno();
-      sdbg("ERROR: open of %s failed: %d\n",
-           g_builtin_parms.redirfile, g_builtin_parms.errcode);
-    }
-
-  /* Does the return file descriptor happen to match the required file
-   * desciptor number?
-   */
-
-  else if (fd != 1)
-    {
-      /* No.. dup2 to get the correct file number */
-
-      svdbg("Dup'ing %d->1\n", fd);
-
-      ret = dup2(fd, 1);
-      if (ret < 0)
-        {
-          g_builtin_parms.errcode = get_errno();
-          sdbg("ERROR: dup2 failed: %d\n", g_builtin_parms.errcode);
-        }
-
-      svdbg("Closing fd=%d\n", fd);
-      close(fd);
-    }
-
-  /* Was the setup successful? */
-
-  if (ret == OK)
-    {
-      /* Yes.. Start the task.  On success, the task ID of the builtin task
-       * is returned; On failure, -1 (ERROR) is returned and the errno
-       * is set appropriately.
-       */
-
-      ret = builtin_taskcreate(g_builtin_parms.index, g_builtin_parms.argv);
-      if (ret < 0)
-        {
-          g_builtin_parms.errcode = get_errno();
-          sdbg("ERROR: builtin_taskcreate failed: %d\n",
-               g_builtin_parms.errcode);
-        }
-    }
-
-  /* NOTE:  There is a logical error here if CONFIG_SCHED_HAVE_PARENT is
-   * defined:  The new task is the child of this proxy task, not the
-   * original caller.  As a consequence, operations like waitpid() will
-   * fail on the caller's thread.
-   */
-
-  /* Post the semaphore to inform the parent task that we have completed
-   * what we need to do.
-   */
-
-  g_builtin_parms.result = ret;
-#ifndef CONFIG_SCHED_WAITPID
-  builtin_semgive(&g_builtin_execsem);
-#endif
-  return 0;
-}
-
-/****************************************************************************
- * Name: builtin_startproxy
- *
- * Description:
- *   Perform output redirection, then execute the builtin task.
- *
- * Input Parameters:
- *   Standard task start-up parameters
- *
- * Returned Value:
- *   On success, the task ID of the builtin task is returned; On failure, -1
- *  (ERROR) is returned and the errno is set appropriately.
- *
- ****************************************************************************/
-
-static inline int builtin_startproxy(int index, FAR const char **argv,
-                                     FAR const char *redirfile, int oflags)
-{
-  struct sched_param param;
-  pid_t proxy;
-  int errcode = OK;
-#ifdef CONFIG_SCHED_WAITPID
-  int status;
-#endif
-  int ret;
-
-  svdbg("index=%d argv=%p redirfile=%s oflags=%04x\n",
-        index, argv, redirfile, oflags);
-
-  /* We will have to go through an intermediary/proxy task in order to
-   * perform the I/O redirection.  This would be a natural place to fork().
-   * However, true fork() behavior requires an MMU and most implementations
-   * of vfork() are not capable of these operations.
-   *
-   * Even without fork(), we can still do the job, but parameter passing is
-   * messier.  Unfortunately, there is no (clean) way to pass binary values
-   * as a task parameter, so we will use a semaphore-protected global
-   * structure.
-   */
-
-  /* Get exclusive access to the global parameter structure */
-
-  bultin_semtake(&g_builtin_parmsem);
-
-  /* Populate the parameter structure */
-
-  g_builtin_parms.redirfile = redirfile;
-  g_builtin_parms.argv      = argv;
-  g_builtin_parms.result    = ERROR;
-  g_builtin_parms.oflags    = oflags;
-  g_builtin_parms.index     = index;
-
-  /* Get the priority of this (parent) task */
-
-  ret = sched_getparam(0, &param);
-  if (ret < 0)
-    {
-      errcode = get_errno();
-      sdbg("ERROR: sched_getparam failed: %d\n", errcode);
-      goto errout_with_sem;
-    }
-
-  /* Disable pre-emption so that the proxy does not run until we waitpid
-   * is called.  This is probably unnecessary since the builtin_proxy has
-   * the same priority as this thread; it should be schedule behind this
-   * task in the ready-to-run list.
-   */
-
-#ifdef CONFIG_SCHED_WAITPID
-  sched_lock();
-#endif
-
-  /* Start the intermediary/proxy task at the same priority as the parent task. */
-
-  proxy = TASK_CREATE("builtin_proxy", param.sched_priority,
-                      CONFIG_BUILTIN_PROXY_STACKSIZE, (main_t)builtin_proxy,
-                      (FAR const char **)NULL);
-  if (proxy < 0)
-    {
-      errcode = get_errno();
-      sdbg("ERROR: Failed to start builtin_proxy: %d\n", errcode);
-      goto errout_with_lock;
-    }
-
-   /* Wait for the proxy to complete its job.  We could use waitpid()
-    * for this.
-    */
-
-#ifdef CONFIG_SCHED_WAITPID
-   ret = waitpid(proxy, &status, 0);
-   if (ret < 0)
-     {
-       sdbg("ERROR: waitpid() failed: %d\n", get_errno());
-       goto errout_with_lock;
-     }
-#else
-   bultin_semtake(&g_builtin_execsem);
-#endif
-
-   /* Get the result and relinquish our access to the parameter structure */
-
-   set_errno(g_builtin_parms.errcode);
-   builtin_semgive(&g_builtin_parmsem);
-   return g_builtin_parms.result;
-
-errout_with_lock:
-#ifdef CONFIG_SCHED_WAITPID
-  sched_unlock();
-#endif
-
-errout_with_sem:
-  set_errno(errcode);
-  builtin_semgive(&g_builtin_parmsem);
-  return ERROR;
-}
-
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -432,36 +109,124 @@ errout_with_sem:
  *
  ****************************************************************************/
 
-int exec_builtin(FAR const char *appname, FAR const char **argv,
+int exec_builtin(FAR const char *appname, FAR char * const *argv,
                  FAR const char *redirfile, int oflags)
 {
+  FAR const struct builtin_s *builtin;
+  posix_spawnattr_t attr;
+  posix_spawn_file_actions_t file_actions;
+  struct sched_param param;
+  pid_t pid;
   int index;
-  int ret = ERROR;
+  int ret;
 
   /* Verify that an application with this name exists */
 
   index = builtin_isavail(appname);
-  if (index >= 0)
+  if (index < 0)
     {
+      ret = ENOENT;
+      goto errout_with_errno;
+    }
+
+  /* Get information about the builtin */
+
+  builtin = builtin_for_index(index);
+  if (builtin == NULL)
+    { 
+      ret = ENOENT;
+      goto errout_with_errno;
+    }
+
+  /* Initialize attributes for task_spawn(). */
+
+  ret = posix_spawn_file_actions_init(&file_actions);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+
+  ret = posix_spawnattr_init(&attr);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+
+  /* Set the correct task size and priority */
+
+  param.sched_priority = builtin->priority;
+  ret = posix_spawnattr_setschedparam(&attr, &param);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+
+  ret = task_spawnattr_setstacksize(&attr, builtin->stacksize);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+
+   /* If robin robin scheduling is enabled, then set the scheduling policy
+    * of the new task to SCHED_RR before it has a chance to run.
+    */
+
+#if CONFIG_RR_INTERVAL > 0
+  ret = posix_spawnattr_setschedpolicy(&attr, SCHED_RR);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+
+  ret = posix_spawnattr_setflags(&attr,
+                                 POSIX_SPAWN_SETSCHEDPARAM |
+                                 POSIX_SPAWN_SETSCHEDULER);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+#else
+  ret = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSCHEDPARAM);
+  if (ret != 0)
+    {
+      goto errout_with_errno;
+    }
+#endif
+
   /* Is output being redirected? */
 
   if (redirfile)
     {
-          ret = builtin_startproxy(index, argv, redirfile, oflags);
+      /* Set up to close open redirfile and set to stdout (1) */
-        }
+
-      else
+      ret = posix_spawn_file_actions_addopen(&file_actions, 1,
+                                             redirfile, O_WRONLY, 0644);
+      if (ret != 0)
         {
-          /* Start the builtin application task */
+          sdbg("ERROR: posix_spawn_file_actions_addopen failed: %d\n", ret);
-
+          goto errout_with_errno;
-          ret = builtin_taskcreate(index, argv);
         }
     }
 
+  /* Start the built-in */
+
+  ret = task_spawn(&pid, builtin->name, builtin->main, &file_actions,
+                   &attr, (argv) ? &argv[1] : (FAR char * const *)NULL,
+                   (FAR char * const *)NULL);
+  if (ret != 0)
+    {
+      sdbg("ERROR: task_spawn failed: %d\n", ret);
+      goto errout_with_errno;
+    }
 
   /* Return the task ID of the new task if the task was sucessfully
    * started.  Otherwise, ret will be ERROR (and the errno value will
    * be set appropriately).
    */
 
-  return ret;
+  return pid;
+
+errout_with_errno:
+  set_errno(ret);
+  return ERROR;
 }

examples/elf/tests/task/task.c

@@ -128,7 +128,7 @@ int main(int argc, char **argv)
 
   child_argv[0] = child_arg;
   child_argv[1] = 0;
-  child_pid = task_create(child_name, 50, 512, child_task, (const char**)child_argv);
+  child_pid = task_create(child_name, 50, 512, child_task, (FAR char * const *)child_argv);
   if (child_pid < 0)
     {
       printf("Parent: task_create failed: %d\n", errno);

examples/hidkbd/hidkbd_main.c

@@ -290,10 +290,10 @@ int hidkbd_main(int argc, char *argv[])
 #ifndef CONFIG_CUSTOM_STACK
       pid = task_create("usbhost", CONFIG_EXAMPLES_HIDKBD_DEFPRIO,
                         CONFIG_EXAMPLES_HIDKBD_STACKSIZE,
-                        (main_t)hidkbd_waiter, (const char **)NULL);
+                        (main_t)hidkbd_waiter, (FAR char * const *)NULL);
 #else
       pid = task_create("usbhost", CONFIG_EXAMPLES_HIDKBD_DEFPRIO,
-                        (main_t)hidkbd_waiter, (const char **)NULL);
+                        (main_t)hidkbd_waiter, (FAR char * const *)NULL);
 #endif
 
       /* Now just sleep.  Eventually logic here will open the kbd device and

examples/nxflat/tests/task/task.c

@@ -128,7 +128,7 @@ int main(int argc, char **argv)
 
   child_argv[0] = child_arg;
   child_argv[1] = 0;
-  child_pid = task_create(child_name, 50, 512, child_task, (const char**)child_argv);
+  child_pid = task_create(child_name, 50, 512, child_task, (FAR char * const *)child_argv);
   if (child_pid < 0)
     {
       printf("Parent: task_create failed: %d\n", errno);

include/builtin.h

@@ -100,7 +100,7 @@ extern "C" {
  *
  ****************************************************************************/
 
-EXTERN int exec_builtin(FAR const char *appname, FAR const char **argv,
+EXTERN int exec_builtin(FAR const char *appname, FAR char * const *argv,
                         FAR const char *redirfile, int oflags);
 
 #undef EXTERN

netutils/thttpd/libhttpd.c

@@ -1863,10 +1863,10 @@ static int ls(httpd_conn *hc)
 #ifndef CONFIG_CUSTOM_STACK
       child = task_create("CGI child", CONFIG_THTTPD_CGI_PRIORITY,
                           CONFIG_THTTPD_CGI_STACKSIZE,
-                          (main_t)ls_child, (const char **)argv);
+                          (main_t)ls_child, (FAR char * const *)argv);
 #else
       child = task_create("CGI child", CONFIG_THTTPD_CGI_PRIORITY,
-                          (main_t)ls_child, (const char **)argv);
+                          (main_t)ls_child, (FAR char * const *)argv);
 #endif
       if (child < 0)
         {

netutils/thttpd/thttpd_cgi.c

@@ -1024,10 +1024,10 @@ int cgi(httpd_conn *hc)
 #ifndef CONFIG_CUSTOM_STACK
       child = task_create("CGI child", CONFIG_THTTPD_CGI_PRIORITY,
                           CONFIG_THTTPD_CGI_STACKSIZE,
-                          (main_t)cgi_child, (const char **)argv);
+                          (main_t)cgi_child, (FAR char * const *)argv);
 #else
       child = task_create("CGI child", CONFIG_THTTPD_CGI_PRIORITY,
-                          (main_t)cgi_child, (const char **)argv);
+                          (main_t)cgi_child, (FAR char * const *)argv);
 #endif
       if (child < 0)
         {

nshlib/nsh_builtin.c

@@ -126,7 +126,7 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd,
    * applications.
    */
 
-  ret = exec_builtin(cmd, (FAR const char **)argv, redirfile, oflags);
+  ret = exec_builtin(cmd, (FAR char * const *)argv, redirfile, oflags);
   if (ret >= 0)
     {
       /* The application was successfully started with pre-emption disabled.

system/usbmonitor/usbmonitor.c

@@ -185,7 +185,7 @@ int usbmonitor_start(int argc, char **argv)
 
       ret = TASK_CREATE("USB Monitor", CONFIG_SYSTEM_USBMONITOR_PRIORITY,
                         CONFIG_SYSTEM_USBMONITOR_STACKSIZE,
-                        (main_t)usbmonitor_daemon, (const char **)NULL);
+                        (main_t)usbmonitor_daemon, (FAR char * const *)NULL);
       if (ret < 0)
         {
           int errcode = errno;