sched/task: Simplify the syscall handling of task_spawn

It's better to save one argument by returning pid directly.
This change also follow the convention of task_create.
BTW, it is reasonable to adjust the function prototype a
little bit from both implementation and consistency since
task_spawn is NuttX specific API.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>

Documentation/reference/user/01_task_control.rst

@@ -789,7 +789,7 @@ Functions
   :return: On success, this function returns 0; on failure it
     will return an error number from ``<errno.h>``
 
-.. c:function:: int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry, \
+.. c:function:: int task_spawn(FAR const char *name, main_t entry, \
       FAR const posix_spawn_file_actions_t *file_actions, \
       FAR const posix_spawnattr_t *attr, \
       FAR char * const argv[], FAR char * const envp[])
@@ -797,11 +797,6 @@ Functions
   The ``task_spawn()`` function will create a new, child
   task, where the entry point to the task is an address in memory.
 
-  :param pid: Upon successful completion, ``task_spawn()`` will return the
-     task ID of the child task to the parent task, in the variable pointed
-     to by a non-NULL ``pid`` argument. If the ``pid`` argument is a null
-     pointer, the process ID of the child is not returned to the caller.
-
   :param name: The name to assign to the child task.
 
   :param entry: The child task's entry point (an address in memory).
@@ -844,9 +839,9 @@ Functions
   :param envp: The ``envp[]`` argument is not used by NuttX and may be
      ``NULL``.
 
-  :return: ``task_spawn()`` will return zero on success.
+  :return: ``task_spawn()`` will return process ID of new task on success.
-    Otherwise, an error number will be returned as the function return value
+    Otherwise, a negative number will be returned as the function return
-    to indicate the error:
+    value to indicate the error:
 
   **POSIX Compatibility:** This is a non-standard interface inspired by
   ``posix_spawn()``.

include/nuttx/spawn.h

@@ -88,27 +88,6 @@ struct spawn_open_file_action_s
 #define SIZEOF_OPEN_FILE_ACTION_S(n) \
   (sizeof(struct spawn_open_file_action_s) + (n))
 
-#ifdef CONFIG_LIB_SYSCALL
-/* task_spawn() and posix_spawn() are NuttX OS interfaces.  In PROTECTED and
- * KERNEL build modes, then can be reached from applications only via a
- * system call.  Currently, the number of parameters in a system call is
- * limited to six; these spawn function have seven parameters.  Rather than
- * extend the maximum number of parameters across all architectures, I opted
- * instead to marshal the seven parameters into the following structure:
- */
-
-struct spawn_syscall_parms_s
-{
-  FAR pid_t *pid;
-  FAR const char *name;
-  main_t entry;
-  FAR const posix_spawn_file_actions_t *file_actions;
-  FAR const posix_spawnattr_t *attr;
-  FAR char * const *argv;
-  FAR char * const *envp;
-};
-#endif
-
 /****************************************************************************
  * Public Function Prototypes
  ****************************************************************************/
@@ -120,9 +99,6 @@ extern "C"
 
 void add_file_action(FAR posix_spawn_file_actions_t *file_action,
                      FAR struct spawn_general_file_action_s *entry);
-#if defined(CONFIG_LIB_SYSCALL) && !defined(CONFIG_BUILD_KERNEL)
-int nx_task_spawn(FAR const struct spawn_syscall_parms_s *parms);
-#endif
 
 #ifdef __cplusplus
 }

include/spawn.h

@@ -147,7 +147,7 @@ int posix_spawn(FAR pid_t *pid, FAR const char *path,
  * 'name'.
  */
 
-int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
+int task_spawn(FAR const char *name, main_t entry,
       FAR const posix_spawn_file_actions_t *file_actions,
       FAR const posix_spawnattr_t *attr,
       FAR char * const argv[], FAR char * const envp[]);

include/sys/syscall_lookup.h

@@ -85,9 +85,7 @@ SYSCALL_LOOKUP(sem_wait,                   1)
 
 #ifndef CONFIG_BUILD_KERNEL
   SYSCALL_LOOKUP(task_create,              5)
-#ifdef CONFIG_LIB_SYSCALL
+  SYSCALL_LOOKUP(task_spawn,               6)
-  SYSCALL_LOOKUP(nx_task_spawn,            1)
-#endif
 #else
   SYSCALL_LOOKUP(pgalloc,                  2)
 #endif

libs/libc/spawn/Make.defs

@@ -34,9 +34,6 @@ CSRCS += lib_psa_setschedpolicy.c lib_psa_getsigmask.c lib_psa_setsigmask.c
 ifneq ($(CONFIG_BUILD_KERNEL),y)
 CSRCS += lib_psa_getstackaddr.c lib_psa_setstackaddr.c
 CSRCS += lib_psa_getstacksize.c lib_psa_setstacksize.c
-ifeq ($(CONFIG_LIB_SYSCALL),y)
-CSRCS += lib_task_spawn.c
-endif
 endif
 
 ifeq ($(CONFIG_DEBUG_FEATURES),y)

libs/libc/spawn/lib_task_spawn.c

@@ -1,79 +0,0 @@
-/****************************************************************************
- * libs/libc/spawn/lib_task_spawn.c
- *
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.  The
- * ASF licenses this file to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the
- * License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
- * License for the specific language governing permissions and limitations
- * under the License.
- *
- ****************************************************************************/
-
-/****************************************************************************
- * Included Files
- ****************************************************************************/
-
-#include <nuttx/config.h>
-
-#include <spawn.h>
-#include <nuttx/spawn.h>
-
-#if defined(CONFIG_LIB_SYSCALL) && !defined(CONFIG_BUILD_KERNEL)
-
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: task_spawn
- *
- * Description:
- *   This function marshals task_spwan() parameters and invokes the
- *   nx_task_spawn() system call.
- *
- *   task_spawn() and posix_spawn() are NuttX OS interfaces.  In PROTECTED
- *   and KERNEL build modes, then can be reached from applications only via
- *   a system call.  Currently, the number of parameters in a system call
- *   is limited to six; these spawn function have seven parameters.  Rather
- *   than extend the maximum number of parameters across all architectures,
- *   I opted instead to marshal the seven parameters into a structure.
- *
- *
- * Input Parameters:
- *   file_actions - The address of the posix_spawn_file_actions_t to be
- *   initialized.
- *
- * Returned Value:
- *   On success, these functions return 0; on failure they return an error
- *   number from <errno.h>.
- *
- ****************************************************************************/
-
-int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
-               FAR const posix_spawn_file_actions_t *file_actions,
-               FAR const posix_spawnattr_t *attr,
-               FAR char * const argv[], FAR char * const envp[])
-{
-  struct spawn_syscall_parms_s parms;
-
-  parms.pid          = pid;
-  parms.name         = name;
-  parms.entry        = entry;
-  parms.file_actions = file_actions;
-  parms.attr         = attr;
-  parms.argv         = (FAR char * const *)argv;
-  parms.envp         = (FAR char * const *)envp;
-
-  return nx_task_spawn(&parms);
-}
-
-#endif /* CONFIG_LIB_SYSCALL && !CONFIG_BUILD_KERNEL */

sched/task/task_spawn.c

@@ -116,7 +116,6 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
       ret = nxsched_get_param(0, &param);
       if (ret < 0)
         {
-          ret = -ret;
           goto errout;
         }
 
@@ -129,7 +128,7 @@ static int nxtask_spawn_exec(FAR pid_t *pidp, FAR const char *name,
   pid = nxtask_create(name, priority, stacksize, entry, argv);
   if (pid < 0)
     {
-      ret = -pid;
+      ret = pid;
       serr("ERROR: nxtask_create failed: %d\n", ret);
       goto errout;
     }
@@ -245,7 +244,7 @@ static int nxtask_spawn_proxy(int argc, FAR char *argv[])
  ****************************************************************************/
 
 /****************************************************************************
- * Name: task_spawn/_task_spawn
+ * Name: task_spawn
  *
  * Description:
  *   The task_spawn() function will create a new, child task, where the
@@ -253,11 +252,6 @@ static int nxtask_spawn_proxy(int argc, FAR char *argv[])
  *
  * Input Parameters:
  *
- *   pid - Upon successful completion, task_spawn() will return the task ID
- *     of the child task to the parent task, in the variable pointed to by
- *     a non-NULL 'pid' argument.  If the 'pid' argument is a null pointer,
- *     the process ID of the child is not returned to the caller.
- *
  *   name - The name to assign to the child task.
  *
  *   entry - The child task's entry point (an address in memory)
@@ -300,8 +294,9 @@ static int nxtask_spawn_proxy(int argc, FAR char *argv[])
  *   envp - The envp[] argument is not used by NuttX and may be NULL.
  *
  * Returned Value:
- *   task_spawn() will return zero on success. Otherwise, an error number
+ *   task_spawn() will return process ID of new task on success.
- *   will be returned as the function return value to indicate the error:
+ *   Otherwise, a negative number will be returned as the function return
+ *   value to indicate the error:
  *
  *   - EINVAL: The value specified by 'file_actions' or 'attr' is invalid.
  *   - Any errors that might have been return if vfork() and excec[l|v]()
@@ -309,27 +304,21 @@ static int nxtask_spawn_proxy(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
-#ifdef CONFIG_LIB_SYSCALL
+int task_spawn(FAR const char *name, main_t entry,
-static int _task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
-                       FAR const posix_spawn_file_actions_t *file_actions,
-                       FAR const posix_spawnattr_t *attr,
-                       FAR char * const argv[], FAR char * const envp[])
-#else
-int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
                FAR const posix_spawn_file_actions_t *file_actions,
                FAR const posix_spawnattr_t *attr,
                FAR char * const argv[], FAR char * const envp[])
-#endif
 {
   struct sched_param param;
   pid_t proxy;
+  pid_t pid;
 #ifdef CONFIG_SCHED_WAITPID
   int status;
 #endif
   int ret;
 
-  sinfo("pid=%p name=%s entry=%p file_actions=%p attr=%p argv=%p\n",
+  sinfo("name=%s entry=%p file_actions=%p attr=%p argv=%p\n",
-        pid, name, entry, file_actions, attr, argv);
+        name, entry, file_actions, attr, argv);
 
   /* If there are no file actions to be performed and there is no change to
    * the signal mask, then start the new child task directly from the parent
@@ -339,7 +328,13 @@ int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
   if ((file_actions == NULL || *file_actions == NULL) &&
       (attr == NULL || (attr->flags & POSIX_SPAWN_SETSIGMASK) == 0))
     {
-      return nxtask_spawn_exec(pid, name, entry, attr, argv);
+      ret = nxtask_spawn_exec(&pid, name, entry, attr, argv);
+      if (ret < 0)
+        {
+          return ret;
+        }
+
+      return pid;
     }
 
   /* Otherwise, we will have to go through an intermediary/proxy task in
@@ -359,13 +354,13 @@ int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
   if (ret < 0)
     {
       serr("ERROR: spawn_semtake failed: %d\n", ret);
-      return -ret;
+      return ret;
     }
 
   /* Populate the parameter structure */
 
   g_spawn_parms.result       = ENOSYS;
-  g_spawn_parms.pid          = pid;
+  g_spawn_parms.pid          = &pid;
   g_spawn_parms.file_actions = file_actions ? *file_actions : NULL;
   g_spawn_parms.attr         = attr;
   g_spawn_parms.argv         = argv;
@@ -379,7 +374,7 @@ int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
     {
       serr("ERROR: nxsched_get_param failed: %d\n", ret);
       spawn_semgive(&g_spawn_parmsem);
-      return -ret;
+      return ret;
     }
 
 #ifdef CONFIG_SCHED_WAITPID
@@ -404,7 +399,7 @@ int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
                         (FAR char * const *)NULL);
   if (proxy < 0)
     {
-      ret = -proxy;
+      ret = proxy;
       serr("ERROR: Failed to start nxtask_spawn_proxy: %d\n", ret);
       goto errout_with_lock;
     }
@@ -434,7 +429,13 @@ int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
 
   /* Get the result and relinquish our access to the parameter structure */
 
-  ret = g_spawn_parms.result;
+  ret = -g_spawn_parms.result;
+  if (ret < 0)
+    {
+      goto errout_with_lock;
+    }
+
+  return (int)pid;
 
 errout_with_lock:
 #ifdef CONFIG_SCHED_WAITPID
@@ -444,36 +445,4 @@ errout_with_lock:
   return ret;
 }
 
-/****************************************************************************
- * Name: nx_task_spawn
- *
- * Description:
- *   This function de-marshals parameters and invokes _task_spawn().
- *
- *   task_spawn() and posix_spawn() are NuttX OS interfaces.  In PROTECTED
- *   and KERNEL build modes, then can be reached from applications only via
- *   a system call.  Currently, the number of parameters in a system call
- *   is limited to six; these spawn function have seven parameters.  Rather
- *   than extend the maximum number of parameters across all architectures,
- *   I opted instead to marshal the seven parameters into a structure.
- *
- * Input Parameters:
- *   parms - The marshaled task_spawn() parameters.
- *
- * Returned Value:
- *   On success, these functions return 0; on failure they return an error
- *   number from <errno.h> (see the comments associated with task_spawn()).
- *
- ****************************************************************************/
-
-#ifdef CONFIG_LIB_SYSCALL
-int nx_task_spawn(FAR const struct spawn_syscall_parms_s *parms)
-{
-  DEBUGASSERT(parms != NULL);
-  return _task_spawn(parms->pid, parms->name, parms->entry,
-                     parms->file_actions, parms->attr,
-                     parms->argv, parms->envp);
-}
-#endif
-
 #endif /* CONFIG_BUILD_KERNEL */

syscall/syscall.csv

@@ -68,7 +68,6 @@
 "nx_pipe","nuttx/fs/fs.h","defined(CONFIG_PIPES) && CONFIG_DEV_PIPE_SIZE > 0","int","int [2]|FAR int *","size_t","int"
 "nx_pthread_create","nuttx/pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","int","pthread_trampoline_t","FAR pthread_t *","FAR const pthread_attr_t *","pthread_startroutine_t","pthread_addr_t","pthread_exitroutine_t"
 "nx_pthread_exit","nuttx/pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","noreturn","pthread_addr_t"
-"nx_task_spawn","nuttx/spawn.h","defined(CONFIG_LIB_SYSCALL) && !defined(CONFIG_BUILD_KERNEL)","int","FAR const struct spawn_syscall_parms_s *"
 "nx_vsyslog","nuttx/syslog/syslog.h","","int","int","FAR const IPTR char *","FAR va_list *"
 "nxsched_get_stackinfo","nuttx/sched.h","","int","pid_t","FAR struct stackinfo_s *"
 "nxsched_get_streams","nuttx/sched.h","defined(CONFIG_FILE_STREAM)","FAR struct streamlist *"
@@ -169,6 +168,7 @@
 "task_restart","sched.h","","int","pid_t"
 "task_setcancelstate","sched.h","","int","int","FAR int *"
 "task_setcanceltype","sched.h","defined(CONFIG_CANCELLATION_POINTS)","int","int","FAR int *"
+"task_spawn","nuttx/spawn.h","!defined(CONFIG_BUILD_KERNEL)","int","FAR const char *","main_t","FAR const posix_spawn_file_actions_t *","FAR const posix_spawnattr_t *","FAR char * const []|FAR char * const *","FAR char * const []|FAR char * const *"
 "task_testcancel","pthread.h","defined(CONFIG_CANCELLATION_POINTS)","void"
 "tcdrain","termios.h","defined(CONFIG_SERIAL_TERMIOS)","int","int"
 "telldir","dirent.h","","off_t","FAR DIR *"