X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?p=secnet.git;a=blobdiff_plain;f=process.c;h=b75b9c0d6e70159ec60d56738e4e6bfe05a551f7;hp=f2a2e4271671c9dd60b31be0a4bc152762f632b4;hb=4ac7fd3ff73c98100d8a8546668d0212e4af3ffa;hpb=4f5e39ecfaa49376b0a5c3a4c384e91a828c1105

diff --git a/process.c b/process.c
index f2a2e42..b75b9c0 100644
--- a/process.c
+++ b/process.c
@@ -1,27 +1,12 @@
+#define _GNU_SOURCE
 #include "secnet.h"
 #include <unistd.h>
 #include <fcntl.h>
 #include <errno.h>
 #include <sys/wait.h>
+#include <string.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;
@@ -30,7 +15,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;
@@ -57,7 +42,7 @@ static void set_default_signals(void);
    their exit status using the callback function.  We block SIGCHLD
    until signal processing has begun. */
 pid_t makesubproc(process_entry_fn *entry, process_callback_fn *cb,
-		 void *est, void *cst, string_t desc)
+		 void *est, void *cst, cstring_t desc)
 {
     struct child *c;
     pid_t p;
@@ -140,60 +125,74 @@ 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;
+    int rv, rc;
     pid_t c;
 
-    va_start(ap,arg);
     c=fork();
     if (c) {
 	/* Parent -> wait for child */
-	waitpid(c,&rv,0);
+	do {
+	    rc = waitpid(c,&rv,0);
+	} while(rc < 0 && errno == EINTR);
+	if (rc < 0)
+	    fatal_perror("sys_cmd: waitpid for %s", path);
+	if (rc != c) /* OS has gone mad */
+	    fatal("sys_cmd: waitpid for %s returned wrong process ID!",
+		  path);
+	if (rv) {
+	    /* If the command failed report its exit status */
+	    lg_exitstatus(0,"sys_cmd",0,M_ERR,rv,path);
+	}
     } else if (c==0) {
 	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(). */
+	va_start(ap,arg);
+	args[0]=(char *)arg; /* program name */
 	i=1;
 	while ((args[i++]=va_arg(ap,char *)));
 	execvp(path,args);
-	exit(1);
+	fprintf(stderr, "sys_cmd(%s,%s,...): %s\n", path, arg, strerror(errno));
+	_exit(1);
     } else {
 	/* Error */
-	fatal_perror("sys_cmd(%s,%s,...)");
+	fatal_perror("sys_cmd(%s,%s,...)", path, arg);
     }
 
-    va_end(ap);
     return rv;
 }
 
 static beforepoll_fn signal_beforepoll;
 static int signal_beforepoll(void *st, struct pollfd *fds, int *nfds_io,
-			     int *timeout_io, const struct timeval *tv_now,
-			     uint64_t *now)
+			     int *timeout_io)
 {
-    if (*nfds_io<1) {
-	*nfds_io=1;
-	return ERANGE;
-    }
-    *nfds_io=1;
+    BEFOREPOLL_WANT_FDS(1);
     fds[0].fd=spr;
     fds[0].events=POLLIN;
     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)
+static void signal_afterpoll(void *st, struct pollfd *fds, int nfds)
 {
     uint8_t buf[16];
     struct signotify *n;
     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 */
@@ -245,7 +244,8 @@ static void signal_handler(int signum)
        will be atomic, and it seems to be the lesser of the two
        evils. */
     saved_errno=errno;
-    write(spw,&thing,1); /* We don't care if this fails (i.e. the pipe
+    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;
@@ -297,16 +297,14 @@ void start_signal_handling(void)
     sigemptyset(&registered);
     sigemptyset(&pending);
 
-    if (pipe(p)!=0) {
-	fatal_perror("start_signal_handling: pipe");
-    }
+    pipe_cloexec(p);
     spw=p[1];
     spr=p[0];
     if (fcntl(spw, F_SETFL, fcntl(spw, F_GETFL)|O_NONBLOCK)==-1) {
 	fatal_perror("start_signal_handling: fcntl(O_NONBLOCK)");
     }
 
-    register_for_poll(NULL,signal_beforepoll,signal_afterpoll,1,"signal");
+    register_for_poll(NULL,signal_beforepoll,signal_afterpoll,"signal");
     signal_handling=True;
 
     /* Register signal handlers for all the signals we're interested in */