build system: Include signing of tarballs in release checklist

[secnet.git] / process.c

diff --git a/process.c b/process.c
index b40801b17a593fc5ce5875687c64220af6eab0b9..48a82eb86c1ab88d14677af4c41470df86aec9a8 100644 (file)
--- a/process.c
+++ b/process.c
@@ -5,6 +5,23 @@
 #include <sys/wait.h>
 #include "process.h"
 
+/* Advice about children from Peter:
+Better way: before the fork, make a pipe. In the child close the
++reading end. Make the writing end close-on-exec. If the dup2 or exec fails,
++write the errno value. In the parent, close the writing end. Now you can read
++from it. If you get an errno value from the pipe, the process failed and you
++know why. If you get EOF, the exec succeeded.
+
+<Senji> So, close on exec only closes if exec isn't going to return then?
+<Diziet> qu: I wouldn't bother with all that with pipes.  Remember that the
++runtime system can still make exec fail when it's `too late'.
+<Senji> Diz - I would rather have a coherant error message than 'child failed'
+<Diziet> The child, if it fails to exec, should print a message to stderr
++(giving errno and what it was trying to execute, most likely), and exit
++nonzero.
+<Diziet> It should exit calling _exit.
+*/
+
 /* Process handling - subprocesses, signals, etc. */
 
 static bool_t signal_handling=False;
@@ -13,7 +30,7 @@ static sigset_t registered,pending;
 
 struct child {
     pid_t pid;
-    string_t desc;
+    cstring_t desc;
     process_callback_fn *cb;
     void *cst;
     bool_t finished;
@@ -39,11 +56,10 @@ static void set_default_signals(void);
    signal processing so that we can catch SIGCHLD for them and report
    their exit status using the callback function.  We block SIGCHLD
    until signal processing has begun. */
-extern void makesubproc(process_entry_fn *entry, process_callback_fn *cb,
-                       void *est, void *cst, string_t desc)
+pid_t makesubproc(process_entry_fn *entry, process_callback_fn *cb,
+                void *est, void *cst, cstring_t desc)
 {
     struct child *c;
-    sigset_t sigchld;
     pid_t p;
 
     c=safe_malloc(sizeof(*c),"makesubproc");
@@ -52,9 +68,7 @@ extern void makesubproc(process_entry_fn *entry, process_callback_fn *cb,
     c->cst=cst;
 
     if (!signal_handling) {
-       sigemptyset(&sigchld);
-       sigaddset(&sigchld,SIGCHLD);
-       sigprocmask(SIG_BLOCK,&sigchld,NULL);
+       fatal("makesubproc called before signal handling started");
     }
     p=fork();
     if (p==0) {
@@ -70,6 +84,7 @@ extern void makesubproc(process_entry_fn *entry, process_callback_fn *cb,
     c->finished=False;
     c->next=children;
     children=c;
+    return p;
 }
 
 static signal_notify_fn sigchld_handler;
@@ -125,7 +140,7 @@ static void sigchld_handler(void *st, int signum)
     }
 }
 
-int sys_cmd(const char *path, char *arg, ...)
+int sys_cmd(const char *path, const char *arg, ...)
 {
     va_list ap;
     int rv;
@@ -140,7 +155,12 @@ int sys_cmd(const char *path, char *arg, ...)
        char *args[100];
        int i;
        /* Child -> exec command */
-       args[0]=arg;
+       /* Really we ought to strcpy() the arguments into the args array,
+          since the arguments are const char *.  Since we'll exit anyway
+          if the execvp() fails this seems somewhat pointless, and
+          increases the chance of the child process failing before it
+          gets to exec(). */
+       args[0]=(char *)arg;
        i=1;
        while ((args[i++]=va_arg(ap,char *)));
        execvp(path,args);
@@ -169,6 +189,9 @@ static int signal_beforepoll(void *st, struct pollfd *fds, int *nfds_io,
     return 0;
 }
 
+/* Bodge to work around Ubuntu's strict header files */
+static void discard(int anything) {}
+
 static afterpoll_fn signal_afterpoll;
 static void signal_afterpoll(void *st, struct pollfd *fds, int nfds,
                             const struct timeval *tv, uint64_t *now)
@@ -178,7 +201,8 @@ static void signal_afterpoll(void *st, struct pollfd *fds, int nfds,
     sigset_t todo,old;
 
     if (nfds && (fds->revents & POLLIN)) {
-       read(spr,buf,16); /* We don't actually care what we read; as
+       discard(read(spr,buf,16));
+                         /* We don't actually care what we read; as
                             long as there was at least one byte
                             (which there was) we'll pick up the
                             signals in the pending set */
@@ -219,11 +243,22 @@ static void set_default_signals(void)
 
 static void signal_handler(int signum)
 {
+    int saved_errno;
     uint8_t thing=0;
     sigaddset(&pending,signum);
-    write(spw,&thing,1); /* We don't care if this fails (i.e. the pipe
+    /* XXX the write() may set errno, which can make the main program fail.
+       However, signal handlers aren't allowed to modify anything which
+       is not of type sig_atomic_t.  The world is broken. */
+    /* I have decided to save and restore errno anyway; on most
+       architectures on which secnet can run modifications to errno
+       will be atomic, and it seems to be the lesser of the two
+       evils. */
+    saved_errno=errno;
+    discard(write(spw,&thing,1));
+                         /* We don't care if this fails (i.e. the pipe
                            is full) because the service routine will
                            spot the pending signal anyway */
+    errno=saved_errno;
 }
 
 static void register_signal_handler(struct signotify *s)