svn commit: r215615 - head/usr.bin/xargs

[Stephen McKay]

Author: mckay
Date: Sun Nov 21 10:55:16 2010
New Revision: 215615
URL: http://svn.freebsd.org/changeset/base/215615

Log:
  xargs can be fooled by exiting children that it did not start, causing
  it to kick off a new command before the previous has finished, resulting
  in corrupted (interleaved) output.  It is also fooled by non-exiting
  children it did not start, failing to exit until all extraneous children
  have exited.
  
  This patch makes xargs keep track of children it starts, ignoring
  pre-existing ones.

Modified:
  head/usr.bin/xargs/xargs.c

Modified: head/usr.bin/xargs/xargs.c
==============================================================================
--- head/usr.bin/xargs/xargs.c	Sun Nov 21 10:52:47 2010	(r215614)
+++ head/usr.bin/xargs/xargs.c	Sun Nov 21 10:55:16 2010	(r215615)
@@ -73,7 +73,16 @@ static int	prompt(void);
 static void	run(char **);
 static void	usage(void);
 void		strnsubst(char **, const char *, const char *, size_t);
+static pid_t	xwait(int block, int *status);
 static void	waitchildren(const char *, int);
+static void	pids_init(void);
+static int	pids_empty(void);
+static int	pids_full(void);
+static void	pids_add(pid_t pid);
+static int	pids_remove(pid_t pid);
+static int	findslot(pid_t pid);
+static int	findfreeslot(void);
+static void	clearslot(int slot);
 
 static char echo[] = _PATH_ECHO;
 static char **av, **bxp, **ep, **endxp, **xp;
@@ -82,6 +91,7 @@ static const char *eofstr;
 static int count, insingle, indouble, oflag, pflag, tflag, Rflag, rval, zflag;
 static int cnt, Iflag, jfound, Lflag, Sflag, wasquoted, xflag;
 static int curprocs, maxprocs;
+static pid_t *childpids;
 
 static volatile int childerr;
 
@@ -205,6 +215,8 @@ main(int argc, char *argv[])
 	if (replstr != NULL && *replstr == '\0')
 		errx(1, "replstr may not be empty");
 
+	pids_init();
+
 	/*
 	 * Allocate pointers for the utility name, the utility arguments,
 	 * the maximum arguments to be read from stdin and the trailing
@@ -556,19 +568,41 @@ exec:
 		childerr = errno;
 		_exit(1);
 	}
-	curprocs++;
+	pids_add(pid);
 	waitchildren(*argv, 0);
 }
 
+/*
+ * Wait for a tracked child to exit and return its pid and exit status.
+ *
+ * Ignores (discards) all untracked child processes.
+ * Returns -1 and sets errno to ECHILD if no tracked children exist.
+ * If block is set, waits indefinitely for a child process to exit.
+ * If block is not set and no children have exited, returns 0 immediately.
+ */
+static pid_t
+xwait(int block, int *status) {
+	pid_t pid;
+
+	if (pids_empty()) {
+		errno = ECHILD;
+		return -1;
+	}
+
+	while ((pid = waitpid(-1, status, block ? 0 : WNOHANG)) > 0)
+		if (pids_remove(pid))
+			break;
+
+	return pid;
+}
+
 static void
 waitchildren(const char *name, int waitall)
 {
 	pid_t pid;
 	int status;
 
-	while ((pid = waitpid(-1, &status, !waitall && curprocs < maxprocs ?
-	    WNOHANG : 0)) > 0) {
-		curprocs--;
+	while ((pid = xwait(waitall || pids_full(), &status)) > 0) {
 		/* If we couldn't invoke the utility, exit. */
 		if (childerr != 0) {
 			errno = childerr;
@@ -583,8 +617,87 @@ waitchildren(const char *name, int waita
 		if (WEXITSTATUS(status))
 			rval = 1;
 	}
+
 	if (pid == -1 && errno != ECHILD)
-		err(1, "wait3");
+		err(1, "waitpid");
+}
+
+#define	NOPID	(0)
+
+static void
+pids_init()
+{
+	int i;
+
+	if ((childpids = malloc(maxprocs * sizeof(*childpids))) == NULL)
+	    errx(1, "malloc failed");
+
+	for (i = 0; i < maxprocs; i++)
+		clearslot(i);
+}
+
+static int
+pids_empty()
+{
+	return curprocs == 0;
+}
+
+static int
+pids_full()
+{
+	return curprocs >= maxprocs;
+}
+
+static void
+pids_add(pid_t pid)
+{
+	int slot;
+
+	slot = findfreeslot();
+	childpids[slot] = pid;
+	curprocs++;
+}
+
+static int
+pids_remove(pid_t pid)
+{
+	int slot;
+
+	if ((slot = findslot(pid)) < 0)
+	    return 0;
+
+	clearslot(slot);
+	curprocs--;
+	return 1;
+}
+
+static int
+findfreeslot()
+{
+	int slot;
+
+	if ((slot = findslot(NOPID)) < 0)
+		errx(1, "internal error: no free pid slot");
+
+	return slot;
+}
+
+static int
+findslot(pid_t pid)
+{
+	int slot;
+
+	for (slot = 0; slot < maxprocs; slot++)
+		if (childpids[slot] == pid)
+			return slot;
+
+	return -1;
+}
+
+static void
+clearslot(int slot)
+{
+	childpids[slot] = NOPID;
 }
 
 /*