From 827bfdd5050bda8ad8954d8231e7836dd7aac867 Mon Sep 17 00:00:00 2001
From: John Denker <jsd@av8n.com>
Date: Sun, 15 Jul 2012 12:11:15 -0700
Subject: allow filters to terminate in arbitrary order; report best blame (not
 first blame)

---
 tools/hi-q.c        | 143 ++++++++++++++++++++++++++++++++--------------------
 tools/hi-test.c     |  13 +++--
 tools/hi-test.conf  |   2 +-
 tools/hi-test2.conf |   3 ++
 4 files changed, 101 insertions(+), 60 deletions(-)
 create mode 100644 tools/hi-test2.conf

diff --git a/tools/hi-q.c b/tools/hi-q.c
index 914bb57..1896bf4 100644
--- a/tools/hi-q.c
+++ b/tools/hi-q.c
@@ -133,7 +133,6 @@ extern char** environ;
 int main(int argc, char** argv) {
   int verbose(1);
   int kidstatus;
-  pid_t somekid;
 
   int rslt;
   int loose_end = 0;
@@ -207,6 +206,12 @@ int main(int argc, char** argv) {
     if (job.size()) filter.push_back(job);
   }
   unsigned int nkids = filter.size();
+
+// Check for nothing to do.
+// This is important, because the "last kid" is a special case.
+// This makes it safe to assume that nkids-1 is non-negative.
+  if (nkids == 0) exit(0);              // nothing to do
+
   if (0 && verbose) for (unsigned int ii = 0; ii < nkids; ii++) {
     cerr << "hi-q filter[" << ii << "] :; ";
     for (VS::const_iterator token = filter[ii].begin();
@@ -407,51 +412,80 @@ int main(int argc, char** argv) {
     cerr << endl;
   }
 
-// loop over N-1 kids ... _not_ including last kid
-  for (unsigned int ii=0; ii<nkids-1; ii++){
-#ifdef testing
-    blurb(ii, kidpid);
-#else
-    for (;;){
-      somekid = waitpid(kidpid[ii], &kidstatus, WUNTRACED);
-      if (somekid) {}             // avoid silly compiler warning
+  pid_t special_kid = kidpid[nkids-1];
+  int alive(nkids-1);           // not counting the special kid
+  int best_blame(0);            // best reason, even if not a great reason
+  pid_t argbest_blame(-1);      // kid# associated with best blame
+
+  for (;;) {
+    if (alive == 0) break;
+    pid_t somekid = waitpid(-1, &kidstatus, WUNTRACED);
+    if (somekid == special_kid){
+      // do not decrement the "alive" counter
+      // since that only applies to non-special kids
       if (WIFEXITED(kidstatus)) {
-        int sts = WEXITSTATUS(kidstatus);
-        if (sts == 1) {
-          cerr << "hi-q says: kid " << ii
-               << " i.e. '" << filter[ii][0] << "' reports spam."
-               << endl;
-          panic(ex_spam);
-        }
-        if (sts != 0) {
-          cerr << "hi-q says: kid " << ii
-               << " i.e. '" << filter[ii][0] << "'"
-               << " exited with bad status: " << sts
-               << endl;
-          panic(ex_syserr);
-        }
-        break;                 // kidstatus==0 means clean exit;
-                               // go check other kids
-      } else if (WIFSIGNALED(kidstatus)) {
-        int sig = WTERMSIG(kidstatus);
-        cerr << "hi-q says: kid " << ii
-               << " == "  << kidpid[ii]
-               << " == '" << filter[ii][0] << "'"
-               << " was killed by signal " << sig
-               << endl;
-        if (sig == SIGUSR1) exit(ex_spam);
-        panic(ex_syserr);      // any kill, not a normal exit
+        cerr << "hi-q: special kid exited early" << endl;
+        return(ex_syserr);
+      } else if (WIFSIGNALED(kidstatus) && WTERMSIG(kidstatus) != SIGUSR1) {
+        cerr << "hi-q: special kid exited early" << endl;
+        return(ex_syserr);
       } else {
-        // some status change other than exit or kill
-        // perhaps stopped for terminal input
+        /* paused, not dead */
       }
+      continue;
+    }
+// here if somekid is not the special kid
+    if (WIFEXITED(kidstatus)) {
+      alive--;
+      if (WEXITSTATUS(kidstatus)) {
+        argbest_blame = somekid;
+        best_blame = kidstatus;
+        break;
+      }
+    } else if (WIFSIGNALED(kidstatus)) {
+      alive--;
+      argbest_blame = somekid;
+      best_blame = kidstatus;
+      if (WTERMSIG(kidstatus) != SIGUSR1) break;
+    } else {
+      /* kid is paused, not dead */
+      /* not a problem */
+    }
+  }
+// here if all kids have exited normally
+// *or* if there is a great reason for quitting early
+
+///////////////////
+// decode the best reason why the filter-chain terminated
+  if (best_blame) {
+    if (WIFEXITED(best_blame)) {
+      int sts = WEXITSTATUS(best_blame);
+      if (sts == 1) {
+        cerr << "hi-q says: kid " << argbest_blame
+                << " reports spam." << endl;
+        panic(ex_spam);
+      }
+      if (sts != 0) {
+        cerr << "hi-q says: kid " << argbest_blame
+             << " exited with bad status: " << sts
+             << endl;
+        panic(ex_syserr);
+      } else {
+        // should never get here unless exit status was nonzero
+        cerr << "hi-q: should never happen" << endl;
+        panic(ex_syserr);
+      }
+    } else if (WIFSIGNALED(best_blame)) {
+      int sig = WTERMSIG(best_blame);
+      cerr << "hi-q says: kid " << argbest_blame
+             << " was killed by signal " << sig
+             << endl;
+      // if the *best* blame is a kill, that's not normal
+      panic(ex_syserr);
     }
-#endif
   }
 
-//xx fprintf(stderr, "slurping %d %d\n", 1, loose_end);
-
-// All filters agree this is not spam.
+// Here if all filters agree this is not spam.
 // Now it is safe to transfer the envelope information:
   slurp(1, loose_end);
   close(1);
@@ -460,25 +494,24 @@ int main(int argc, char** argv) {
 // now that the envelope information has been transfered,
 // wait for the last kid in the usual way
   {
-    int ii = nkids-1;
 
 #ifdef moretesting
 fprintf(stderr, "About to wait for kid #%d (%d)\n",
-                ii, kidpid[ii]);
-    blurb(nkids-1, kidpid);
+                special_kid, kidpid[special_kid]);
 #endif
-    somekid = waitpid(kidpid[ii], &kidstatus, WUNTRACED);
-    if (WIFEXITED(kidstatus)) {
-      int sts = WEXITSTATUS(kidstatus);
-      cerr << "hi-q ends with status: " << sts << endl;
-      return sts;
+    for(;;) {
+      waitpid(special_kid, &kidstatus, WUNTRACED);
+      if (WIFEXITED(kidstatus)) {
+        int sts = WEXITSTATUS(kidstatus);
+        cerr << "hi-q ends with status: " << sts << endl;
+        return sts;
+      } else if (WIFSIGNALED(kidstatus)) {
+        cerr << "hi-q: special kid was killed by signal "
+                << WTERMSIG(kidstatus) << endl;
+        return ex_syserr;
+      } else {
+        /* paused, not dead */
+      }
     }
   }
-
-#ifdef testing
-  sleep(1);
-#endif
-// the last kid did not exit but was killed:
-  return ex_syserr;
-
 }
diff --git a/tools/hi-test.c b/tools/hi-test.c
index 50e4a76..0c9a35f 100644
--- a/tools/hi-test.c
+++ b/tools/hi-test.c
@@ -44,6 +44,7 @@ using namespace std;
 int main(int _argc, const char** _argv){
   int snooze(0);
   int status(0);
+  int killmode(0);
   int argc(_argc);
   const char **argv(_argv);
   string progname(*argv); argv++; argc--;
@@ -62,14 +63,18 @@ int main(int _argc, const char** _argv){
       snooze = atoi(*argv); argv++; argc--;
       continue;
     }
-    if (prefix(arg, "-kill")) {
+    if (prefix(arg, "-exit")) {
       if (!argc) {
-        cerr << "Option -kill requires an argument" << endl;
+        cerr << "Option -exit requires an argument" << endl;
         exit(sa_usage);
       }
       status = atoi(*argv); argv++; argc--;
       continue;
     }
+    if (prefix(arg, "-kill")) {
+      killmode++;
+      continue;
+    }
     if (arg.substr(0,1) == "x") {
       continue;
     }
@@ -83,12 +88,12 @@ int main(int _argc, const char** _argv){
       exit(sa_usage);
     }
   }
-
   
   cerr << "++++ hi-test pid: " << getpid() << "  group: " << getpgid(0);
   char* foo = getenv("HI_Q_GROUP");
   if (foo) cerr << " HI_Q_GROUP: " << foo;
   cerr << endl;
   sleep(snooze);
-  exeunt(status);
+  if (killmode) exeunt(status);
+  exit(status);
 }
diff --git a/tools/hi-test.conf b/tools/hi-test.conf
index 8addf21..df3f8bb 100644
--- a/tools/hi-test.conf
+++ b/tools/hi-test.conf
@@ -1,3 +1,3 @@
 hi-test x0 -snooze 10
-hi-test x1 -snooze 1 -kill 10
+hi-test x1 -snooze 1 -exit 3 -kill
 hi-test x2 -snooze 10
diff --git a/tools/hi-test2.conf b/tools/hi-test2.conf
new file mode 100644
index 0000000..2dbad3b
--- /dev/null
+++ b/tools/hi-test2.conf
@@ -0,0 +1,3 @@
+hi-test x0 -snooze 10
+hi-test x2 -snooze 10
+hi-test x1 -snooze 1 -exit 3
-- 
cgit v1.2.3