From 5f76ed9108c5c48850cf339096527c4d53f0bc12 Mon Sep 17 00:00:00 2001
From: Laurent Bercot <ska-skaware@skarnet.org>
Date: Thu, 18 Dec 2014 19:06:06 +0000
Subject:  Separate socket from pipe in child_spawn. Less code in skaclient!

---
 src/include/skalibs/djbunix.h              |   8 +-
 src/libstddjb/child_spawn.c                |  73 +++++------------
 src/libstddjb/child_spawn1.c               | 115 --------------------------
 src/libstddjb/child_spawn1_internal.c      | 125 +++++++++++++++++++++++++++++
 src/libstddjb/child_spawn1_pipe.c          |  16 ++++
 src/libstddjb/child_spawn1_socket.c        |  16 ++++
 src/libstddjb/djbunix-internal.h           |  10 +++
 src/libunixonacid/skaclient_startf_async.c |   2 +-
 8 files changed, 193 insertions(+), 172 deletions(-)
 delete mode 100644 src/libstddjb/child_spawn1.c
 create mode 100644 src/libstddjb/child_spawn1_internal.c
 create mode 100644 src/libstddjb/child_spawn1_pipe.c
 create mode 100644 src/libstddjb/child_spawn1_socket.c
 create mode 100644 src/libstddjb/djbunix-internal.h

(limited to 'src')

diff --git a/src/include/skalibs/djbunix.h b/src/include/skalibs/djbunix.h
index b5f5db4..1a54db6 100644
--- a/src/include/skalibs/djbunix.h
+++ b/src/include/skalibs/djbunix.h
@@ -129,18 +129,18 @@ extern int rmstar (char const *) ;
 extern int rmstar_tmp (char const *, stralloc *) ;
 
 
- /* Simple spawn functions. 0 for no communication, 1 for a child/parent pipe. */
+ /* Simple spawn functions with 0 or 1 communicating fds. */
 
 extern pid_t child_spawn0 (char const *, char const *const *, char const *const *) ;
-extern pid_t child_spawn1 (char const *, char const *const *, char const *const *, int *, int) ;
+extern pid_t child_spawn1_pipe (char const *, char const *const *, char const *const *, int *, int) ;
+extern pid_t child_spawn1_socket (char const *, char const *const *, char const *const *, int *) ;
 
 
  /*
     Unified function to fork a child with communication canals.
     * uses posix_spawn() if available, else uses fork+exec
     * requests n (the last arg) communication fds between parent and child
-    * if n=1, the fd is a Unix socket. If more canals are needed, you can
-      pass them through that socket.
+    * if n=1, equivalent to child_spawn1_pipe; child writes, parent reads.
     * if n>=2, the fds are pipes, parent reads on even and writes on odd.
  */
 
diff --git a/src/libstddjb/child_spawn.c b/src/libstddjb/child_spawn.c
index 5d696e4..a9003e5 100644
--- a/src/libstddjb/child_spawn.c
+++ b/src/libstddjb/child_spawn.c
@@ -15,7 +15,6 @@
 #include <skalibs/env.h>
 #include <skalibs/djbunix.h>
 #include <skalibs/uint.h>
-#include <skalibs/webipc.h>
 
 #ifdef SKALIBS_HASPOSIXSPAWN
 
@@ -35,9 +34,7 @@
 
  /*
     If n = 0 : child's stdin and stdout are the same as the parent's
-    If n = 1 : Unix socket between parent and child.
-               Additional canals, if needed, may be fd-passed through it.
-    If n >= 2 : pipes between parent and child.
+    If n >= 1 : pipes between parent and child.
                 Parent reads on even ones, writes on odd ones.
  */
 
@@ -53,17 +50,10 @@ pid_t child_spawn (char const *prog, char const *const *argv, char const *const
   pid_t pid ;
   int e ;
   unsigned int m = sizeof(NOFDVAR) ;
-  unsigned int i ;
+  unsigned int i = 0 ;
   char modifs[m + 1 + n * UINT_FMT] ;
   byte_copy(modifs, sizeof(NOFDVAR), NOFDVAR "=") ;
-  if (n == 1)
-  {
-    if (ipc_pair_b(p[0]) < 0) return 0 ;
-  }
-  else if (n >= 2)
-  {
-    for (i = 0 ; i < n ; i++) if (pipe(p[i]) < 0) { e = errno ; goto errp ; }
-  }
+  for (; i < n ; i++) if (pipe(p[i]) < 0) { e = errno ; goto errp ; }
   for (i = 0 ; i < n ; i++)
     if ((ndelay_on(p[i][i & 1]) < 0) || (coe(p[i][i & 1]) < 0))
     {
@@ -88,28 +78,19 @@ pid_t child_spawn (char const *prog, char const *const *argv, char const *const
   if (e) goto errattr ;
   e = posix_spawn_file_actions_init(&actions) ;
   if (e) goto errattr ;
-  switch (n)
+  if (n >= 2)
+  {
+    e = posix_spawn_file_actions_adddup2(&actions, p[1][0], 0) ;
+    if (e) goto erractions ;
+    e = posix_spawn_file_actions_addclose(&actions, p[1][0]) ;
+    if (e) goto erractions ;
+  }
+  if (n)
   {
-    case 0 :
-       break ;
-    case 1 :
-      e = posix_spawn_file_actions_adddup2(&actions, p[0][1], 1) ;
-      if (e) goto erractions ;
-      e = posix_spawn_file_actions_addclose(&actions, p[0][1]) ;
-      if (e) goto erractions ;
-      e = posix_spawn_file_actions_adddup2(&actions, 1, 0) ;
-      if (e) goto erractions ;
-      break ;
-    default : 
-      e = posix_spawn_file_actions_adddup2(&actions, p[1][0], 0) ;
-      if (e) goto erractions ;
-      e = posix_spawn_file_actions_addclose(&actions, p[1][0]) ;
-      if (e) goto erractions ;
-      e = posix_spawn_file_actions_adddup2(&actions, p[0][1], 1) ;
-      if (e) goto erractions ;
-      e = posix_spawn_file_actions_addclose(&actions, p[0][1]) ;
-      if (e) goto erractions ;
-      break ;
+    e = posix_spawn_file_actions_adddup2(&actions, p[0][1], 1) ;
+    if (e) goto erractions ;
+    e = posix_spawn_file_actions_addclose(&actions, p[0][1]) ;
+    if (e) goto erractions ;
   }
   {
     int haspath = !!env_get("PATH") ;
@@ -142,25 +123,13 @@ pid_t child_spawn (char const *prog, char const *const *argv, char const *const
     byte_copy(name + len, 9, " (child)") ;
     PROG = name ;
     fd_close(syncpipe[0]) ;
-    switch (n)
+    if (n >= 2)
     {
-      case 0 :
-      {
-        int fd = open2("/dev/null", O_RDONLY) ;
-        if (fd < 0) goto syncdie ;
-        if (fd_move(0, fd) < 0) goto syncdie ;
-        fd = open2("/dev/null", O_WRONLY) ;
-        if (fd < 0) goto syncdie ;
-        if (fd_move(1, fd) < 0) goto syncdie ;
-        break ;
-      }
-      case 1 :
-        if (fd_move(1, p[0][1]) < 0) goto syncdie ;
-        if (fd_copy(0, 1) < 0) goto syncdie ;
-        break ;
-      default :
-        if (fd_move2(0, p[1][0], 1, p[0][1]) < 0) goto syncdie ;
-        break ;
+      if (fd_move2(0, p[1][0], 1, p[0][1]) < 0) goto syncdie ;
+    }
+    else if (n)
+    {
+      if (fd_move(1, p[0][1]) < 0) goto syncdie ;
     }
     sig_blocknone() ;
     pathexec_r_name(prog, argv, envp, env_len(envp), modifs, m) ;
diff --git a/src/libstddjb/child_spawn1.c b/src/libstddjb/child_spawn1.c
deleted file mode 100644
index bb51ab9..0000000
--- a/src/libstddjb/child_spawn1.c
+++ /dev/null
@@ -1,115 +0,0 @@
-/* ISC license. */
-
-#include <skalibs/sysdeps.h>
-#include <sys/types.h>
-#include <unistd.h>
-#include <errno.h>
-#include <skalibs/djbunix.h>
-
-#ifdef SKALIBS_HASPOSIXSPAWN
-
-#include <spawn.h>
-#include <stdlib.h>
-#include <skalibs/config.h>
-#include <skalibs/env.h>
-
-pid_t child_spawn1 (char const *prog, char const *const *argv, char const *const *envp, int *fd, int to)
-{
-  posix_spawn_file_actions_t actions ;
-  int e ;
-  int p[2] ;
-  pid_t pid ;
-  int haspath = !!env_get("PATH") ;
-  if (pipe(p) < 0) return 0 ;
-  to = !!to ;
-  e = posix_spawn_file_actions_init(&actions) ;
-  if (e) goto err ;
-  e = posix_spawn_file_actions_addclose(&actions, p[!to]) ;
-  if (e) goto erractions ;
-  e = posix_spawn_file_actions_adddup2(&actions, p[to], to) ;
-  if (e) goto erractions ;
-  if (!haspath && (setenv("PATH", SKALIBS_DEFAULTPATH, 0) < 0)) { e = errno ; goto erractions ; }
-  e = posix_spawnp(&pid, prog, &actions, 0, (char *const *)argv, (char *const *)envp) ;
-  if (!haspath) unsetenv("PATH") ;
-  posix_spawn_file_actions_destroy(&actions) ;
-  fd_close(p[to]) ;
-  if (e) goto errp ;
-  *fd = p[!to] ;
-  return pid ;
-
- erractions:
-  posix_spawn_file_actions_destroy(&actions) ;
- err:
-  fd_close(p[to]) ;
- errp:
-  fd_close(p[!to]) ;
-  errno = e ;
-  return 0 ;
-}
-
-#else
-
-#include <skalibs/allreadwrite.h>
-
-pid_t child_spawn1 (char const *prog, char const *const *argv, char const *const *envp, int *fd, int to)
-{
-  int e ;
-  int syncp[2] ;
-  int p[2] ;
-  pid_t pid ;
-  if (pipe(p) < 0) return 0 ;
-  if (pipecoe(syncp) < 0)
-  {
-    e = errno ;
-    fd_close(p[1]) ;
-    fd_close(p[0]) ;
-    errno = e ;
-    return 0 ;
-  }
-  to = !!to ;
-  pid = fork() ;
-  if (pid < 0)
-  {
-    e = errno ;
-    fd_close(syncp[1]) ;
-    fd_close(syncp[0]) ;
-    fd_close(p[1]) ;
-    fd_close(p[0]) ;
-    errno = e ;
-    return 0 ;
-  }
-  if (pid)
-  {
-    fd_close(syncp[0]) ;
-    fd_close(p[!to]) ;
-    if (fd_move(to, p[to]) < 0) goto err ;
-    pathexec_run(prog, argv, envp) ;
-err:
-    e = errno ;
-    fd_write(syncp[1], (char *)&e, sizeof(e)) ;
-    _exit(127) ;
-  }
-  fd_close(syncp[1]) ;
-  fd_close(p[to]) ;
-  syncp[1] = fd_read(syncp[0], (char *)&e, sizeof(e)) ;
-  if (syncp[1] < 0)
-  {
-    e = errno ;
-    fd_close(syncp[0]) ;
-    fd_close(p[!to]) ;
-    errno = e ;
-    return 0 ;
-  }
-  fd_close(syncp[0]) ;
-  if (syncp[1] == sizeof(e))
-  {
-    fd_close(p[!to]) ;
-    wait_pid(pid, &syncp[1]) ;
-    errno = e ;
-    return 0 ;
-  }
-  *fd = p[!to] ;
-  return pid ;
-}
-
-#endif
diff --git a/src/libstddjb/child_spawn1_internal.c b/src/libstddjb/child_spawn1_internal.c
new file mode 100644
index 0000000..68b0242
--- /dev/null
+++ b/src/libstddjb/child_spawn1_internal.c
@@ -0,0 +1,125 @@
+/* ISC license. */
+
+#include <skalibs/sysdeps.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <skalibs/djbunix.h>
+
+#ifdef SKALIBS_HASPOSIXSPAWN
+
+#include <signal.h>
+#include <spawn.h>
+#include <stdlib.h>
+#include <skalibs/config.h>
+#include <skalibs/env.h>
+
+pid_t child_spawn1_internal (char const *prog, char const *const *argv, char const *const *envp, int *p, int to)
+{
+  posix_spawn_file_actions_t actions ;
+  posix_spawnattr_t attr ;
+  int e ;
+  pid_t pid ;
+  int haspath = !!env_get("PATH") ;
+  to = !!to ;
+  if (coe(p[!to]) < 0) { e = errno ; goto err ; }
+  e = posix_spawnattr_init(&attr) ;
+  if (e) goto err ;
+  {
+    sigset_t set ;
+    sigemptyset(&set) ;
+    e = posix_spawnattr_setsigmask(&attr, &set) ;
+  }
+  if (e) goto errattr ;
+  e = posix_spawn_file_actions_init(&actions) ;
+  if (e) goto errattr ;
+  e = posix_spawn_file_actions_addclose(&actions, p[!to]) ;
+  if (e) goto erractions ;
+  e = posix_spawn_file_actions_adddup2(&actions, p[to], to) ;
+  if (e) goto erractions ;
+  if (!haspath && (setenv("PATH", SKALIBS_DEFAULTPATH, 0) < 0)) { e = errno ; goto erractions ; }
+  e = posix_spawnp(&pid, prog, &actions, &attr, (char *const *)argv, (char *const *)envp) ;
+  if (!haspath) unsetenv("PATH") ;
+  posix_spawn_file_actions_destroy(&actions) ;
+  posix_spawnattr_destroy(&attr) ;
+  fd_close(p[to]) ;
+  if (e) goto errp ;
+  return pid ;
+
+ erractions:
+  posix_spawn_file_actions_destroy(&actions) ;
+ errattr:
+  posix_spawnattr_destroy(&attr) ;
+ err:
+  fd_close(p[to]) ;
+ errp:
+  fd_close(p[!to]) ;
+  errno = e ;
+  return 0 ;
+}
+
+#else
+
+#include <skalibs/allreadwrite.h>
+#include <skalibs/sig.h>
+
+pid_t child_spawn1_internal (char const *prog, char const *const *argv, char const *const *envp, int *p, int to)
+{
+  int e ;
+  int syncp[2] ;
+  pid_t pid ;
+  if (coe(p[0]) < 0 || pipecoe(syncp) < 0)
+  {
+    e = errno ;
+    fd_close(p[1]) ;
+    fd_close(p[0]) ;
+    errno = e ;
+    return 0 ;
+  }
+  to = !!to ;
+  pid = fork() ;
+  if (pid < 0)
+  {
+    e = errno ;
+    fd_close(syncp[1]) ;
+    fd_close(syncp[0]) ;
+    fd_close(p[1]) ;
+    fd_close(p[0]) ;
+    errno = e ;
+    return 0 ;
+  }
+  if (pid)
+  {
+    fd_close(syncp[0]) ;
+    fd_close(p[!to]) ;
+    if (fd_move(to, p[to]) < 0) goto err ;
+    sig_blocknone() ;
+    pathexec_run(prog, argv, envp) ;
+err:
+    e = errno ;
+    fd_write(syncp[1], (char *)&e, sizeof(e)) ;
+    _exit(127) ;
+  }
+  fd_close(syncp[1]) ;
+  fd_close(p[to]) ;
+  syncp[1] = fd_read(syncp[0], (char *)&e, sizeof(e)) ;
+  if (syncp[1] < 0)
+  {
+    e = errno ;
+    fd_close(syncp[0]) ;
+    fd_close(p[!to]) ;
+    errno = e ;
+    return 0 ;
+  }
+  fd_close(syncp[0]) ;
+  if (syncp[1] == sizeof(e))
+  {
+    fd_close(p[!to]) ;
+    wait_pid(pid, &syncp[1]) ;
+    errno = e ;
+    return 0 ;
+  }
+  return pid ;
+}
+
+#endif
diff --git a/src/libstddjb/child_spawn1_pipe.c b/src/libstddjb/child_spawn1_pipe.c
new file mode 100644
index 0000000..a436f6e
--- /dev/null
+++ b/src/libstddjb/child_spawn1_pipe.c
@@ -0,0 +1,16 @@
+/* ISC license. */
+
+#include <unistd.h>
+#include <skalibs/djbunix.h>
+#include "djbunix-internal.h"
+
+pid_t child_spawn1_pipe (char const *prog, char const *const *argv, char const *const *envp, int *fd, int to)
+{
+  pid_t pid ;
+  int p[2] ;
+  if (pipe(p) < 0) return 0 ;
+  pid = child_spawn1_internal(prog, argv, envp, p, !!to) ;
+  if (!pid) return 0 ;
+  *fd = p[!to] ;
+  return pid ;
+}
diff --git a/src/libstddjb/child_spawn1_socket.c b/src/libstddjb/child_spawn1_socket.c
new file mode 100644
index 0000000..46c900f
--- /dev/null
+++ b/src/libstddjb/child_spawn1_socket.c
@@ -0,0 +1,16 @@
+/* ISC license. */
+
+#include <skalibs/webipc.h>
+#include <skalibs/djbunix.h>
+#include "djbunix-internal.h"
+
+pid_t child_spawn1_socket (char const *prog, char const *const *argv, char const *const *envp, int *fd)
+{
+  pid_t pid ;
+  int p[2] ;
+  if (ipc_pair_b(p) < 0) return 0 ;
+  pid = child_spawn1_internal(prog, argv, envp, p, 1) ;
+  if (!pid) return 0 ;
+  *fd = p[0] ;
+  return pid ;
+}
diff --git a/src/libstddjb/djbunix-internal.h b/src/libstddjb/djbunix-internal.h
new file mode 100644
index 0000000..775cb80
--- /dev/null
+++ b/src/libstddjb/djbunix-internal.h
@@ -0,0 +1,10 @@
+/* ISC license. */
+
+#ifndef DJBUNIX_INTERNAL_H
+#define DJBUNIX_INTERNAL_H
+
+#include <sys/types.h>
+
+extern pid_t child_spawn1_internal (char const *, char const *const *, char const *const *, int *, int) ;
+
+#endif
diff --git a/src/libunixonacid/skaclient_startf_async.c b/src/libunixonacid/skaclient_startf_async.c
index 4077c1e..14fa29a 100644
--- a/src/libunixonacid/skaclient_startf_async.c
+++ b/src/libunixonacid/skaclient_startf_async.c
@@ -32,7 +32,7 @@ int skaclient_startf_async (
   skaclient_cbdata_t *blah)
 {
   int fd ;
-  pid_t pid = child_spawn(prog, argv, envp, &fd, 1) ;
+  pid_t pid = child_spawn1_socket(prog, argv, envp, &fd) ;
   if (!pid) return 0 ;
   if (!skaclient_init(a, fd, bufss, bufsn, auxbufss, auxbufsn, bufas, bufan, auxbufas, auxbufan, q, qlen, before, beforelen))
   {
-- 
cgit v1.2.3