Changeset 97cdbaf5

Timestamp:

Mar 11, 2012, 10:57:35 PM (13 years ago)

Author:

David Benjamin <davidben@mit.edu>

Branches:

master, release-1.10, release-1.9

Children:

a03a409

Parents:

1f39ded

git-author:

David Benjamin <davidben@mit.edu> (01/23/12 00:38:29)

git-committer:

David Benjamin <davidben@mit.edu> (03/11/12 22:57:35)

Message:

Rewrite call_filter to use g_spawn_async_with_pipes

This simplifies the error-handling code. Also fixes a bug where file
descriptors get double-closed in call_filter. Also adds a unit test. The
separate prog argument is removed to avoid having to deal with
G_SPAWN_FILE_AND_ARGV_ZERO, and since we don't really use it anyway.

Files:

• filterproc.c (modified) (4 diffs)
• filterproc.h (modified) (1 diff)
• functions.c (modified) (2 diffs)
• message.c (modified) (2 diffs)
• tester.c (modified) (4 diffs)
• zcrypt.c (modified) (2 diffs)

filterproc.c

@@ -3 +3 @@
 #include <poll.h>
 
-int send_receive(int rfd, int wfd, const char *out, char **in)
+/* Even in case of error, send_receive is responsible for closing wfd
+ * (to EOF the child) and rfd (for consistency). */
+static int send_receive(int rfd, int wfd, const char *out, char **in)
 {
   GString *str = g_string_new("");
@@ -21 +23 @@
   if(!out || !*out) {
     /* Nothing to write. Close our end so the child doesn't hang waiting. */
-    close(wfd);
+    close(wfd); wfd = -1;
     out = NULL;
   }
@@ -46 +48 @@
       }
       if(!out || !*out || fds[1].revents & (POLLERR | POLLHUP)) {
-        close(wfd);
+        close(wfd); wfd = -1;
         out = NULL;
       }
@@ -62 +64 @@
   }
 
+  if (wfd >= 0) close(wfd);
+  close(rfd);
   *in = g_string_free(str, err < 0);
   return err;
 }
 
-int call_filter(const char *prog, const char *const *argv, const char *in, char **out, int *status)
+int call_filter(const char *const *argv, const char *in, char **out, int *status)
 {
-  int err = 0;
-  pid_t pid;
-  int rfd[2];
-  int wfd[2];
+  int err;
+  GPid child_pid;
+  int child_stdin, child_stdout;
 
-  if((err = pipe(rfd))) goto out;
-  if((err = pipe(wfd))) goto out_close_rfd;
-
-  pid = fork();
-  if(pid < 0) {
-    err = pid;
-    goto out_close_all;
-  }
-  if(pid) {
-    /* parent */
-    close(rfd[1]);
-    close(wfd[0]);
-    err = send_receive(rfd[0], wfd[1], in, out);
-    if(err == 0) {
-      waitpid(pid, status, 0);
-    }
-  } else {
-    /* child */
-    close(rfd[0]);
-    close(wfd[1]);
-    dup2(rfd[1], 1);
-    dup2(wfd[0], 0);
-    close(rfd[1]);
-    close(wfd[0]);
-
-    if(execvp(prog, (char *const *)argv)) {
-      _exit(-1);
-    }
+  if (!g_spawn_async_with_pipes(NULL, (char**)argv, NULL,
+                                G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD,
+                                NULL, NULL,
+                                &child_pid, &child_stdin, &child_stdout, NULL,
+                                NULL)) {
+    return 1;
   }
 
- out_close_all:
-  close(wfd[0]);
-  close(wfd[1]);
- out_close_rfd:
-  close(rfd[0]);
-  close(rfd[1]);
- out:
+  err = send_receive(child_stdout, child_stdin, in, out);
+  if (err == 0) {
+    waitpid(child_pid, status, 0);
+  }
   return err;
 }

filterproc.h

@@ -2 +2 @@
 #define INC_BARNOWL_FILTER_PROC_H
 
-int call_filter(const char *prog,
-                const char *const *argv,
+int call_filter(const char *const *argv,
                 const char *in,
                 char **out, int *status);

functions.c

@@ -405 +405 @@
 
   zcrypt = g_build_filename(owl_get_bindir(), "zcrypt", NULL);
-  argv[0] = "zcrypt";
+  argv[0] = zcrypt;
   argv[1] = "-E";
   argv[2] = "-c"; argv[3] = owl_zwrite_get_class(z);
@@ -411 +411 @@
   argv[6] = NULL;
 
-  rv = call_filter(zcrypt, argv, owl_zwrite_get_message(z), &cryptmsg, &status);
+  rv = call_filter(argv, owl_zwrite_get_message(z), &cryptmsg, &status);
 
   g_free(zcrypt);

message.c

@@ -875 +875 @@
   if (owl_global_is_zcrypt(&g) && !strcasecmp(n->z_opcode, "crypt")) {
     const char *argv[] = {
-      "zcrypt",
+      NULL,
       "-D",
       "-c", owl_message_get_class(m),
@@ -887 +887 @@
 
     zcrypt = g_build_filename(owl_get_bindir(), "zcrypt", NULL);
-
-    rv = call_filter(zcrypt, argv, owl_message_get_body(m), &out, &status);
+    argv[0] = zcrypt;
+
+    rv = call_filter(argv, owl_message_get_body(m), &out, &status);
     g_free(zcrypt);
 

tester.c

@@ -3 +3 @@
 #include "owl.h"
 #undef WINDOW
+#include "filterproc.h"
 
 #include <stdio.h>
@@ -23 +24 @@
 int owl_smartfilter_regtest(void);
 int owl_history_regtest(void);
+int call_filter_regtest(void);
 
 extern void owl_perl_xs_init(pTHX);
@@ -113 +115 @@
   numfailures += owl_smartfilter_regtest();
   numfailures += owl_history_regtest();
+  numfailures += call_filter_regtest();
   if (numfailures) {
       fprintf(stderr, "# *** WARNING: %d failures total\n", numfailures);
@@ -981 +984 @@
   return numfailed;
 }
+
+int call_filter_regtest(void)
+{
+  int numfailed = 0;
+  int ret;
+  char *out = NULL;
+  int status;
+
+  printf("# BEGIN testing call_filter\n");
+
+  const char *cat_argv[] = { "cat", NULL };
+  ret = call_filter(cat_argv, "Mangos!", &out, &status);
+  FAIL_UNLESS("call_filter cat", (ret == 0 &&
+                                  status == 0 &&
+                                  strcmp(out, "Mangos!") == 0));
+  g_free(out); out = NULL;
+
+  ret = call_filter(cat_argv, "", &out, &status);
+  FAIL_UNLESS("call_filter cat", (ret == 0 &&
+                                  status == 0 &&
+                                  strcmp(out, "") == 0));
+  g_free(out); out = NULL;
+
+  ret = call_filter(cat_argv, NULL, &out, &status);
+  FAIL_UNLESS("call_filter cat", (ret == 0 &&
+                                  status == 0 &&
+                                  strcmp(out, "") == 0));
+  g_free(out); out = NULL;
+
+  printf("# END testing call_filter (%d failures)\n", numfailed);
+  return numfailed;
+}

zcrypt.c

@@ -774 +774 @@
     NULL
   };
-  err = call_filter("gpg", argv, in, &out, &status);
+  err = call_filter(argv, in, &out, &status);
   if(err || status) {
     g_free(out);
@@ -857 +857 @@
   if(!in) return FALSE;
 
-  err = call_filter("gpg", argv, in, &out, &status);
+  err = call_filter(argv, in, &out, &status);
   if(err || status) {
     g_free(out);