Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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.
  • Loading branch information...
commit 97cdbaf5d7b6609c5ab134fee3b9c400ae069425 1 parent 1f39ded
@davidben davidben authored
View
62 filterproc.c
@@ -2,7 +2,9 @@
#include <sys/wait.h>
#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("");
char buf[1024];
@@ -20,7 +22,7 @@ int send_receive(int rfd, int wfd, const char *out, char **in)
if(!out || !*out) {
/* Nothing to write. Close our end so the child doesn't hang waiting. */
- close(wfd);
+ close(wfd); wfd = -1;
out = NULL;
}
@@ -45,7 +47,7 @@ int send_receive(int rfd, int wfd, const char *out, char **in)
}
}
if(!out || !*out || fds[1].revents & (POLLERR | POLLHUP)) {
- close(wfd);
+ close(wfd); wfd = -1;
out = NULL;
}
}
@@ -61,53 +63,29 @@ int send_receive(int rfd, int wfd, const char *out, char **in)
}
}
+ 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];
-
- if((err = pipe(rfd))) goto out;
- if((err = pipe(wfd))) goto out_close_rfd;
+ int err;
+ GPid child_pid;
+ int child_stdin, child_stdout;
- pid = fork();
- if(pid < 0) {
- err = pid;
- goto out_close_all;
+ 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;
}
- 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);
- }
+ err = send_receive(child_stdout, child_stdin, in, out);
+ if (err == 0) {
+ waitpid(child_pid, status, 0);
}
-
- out_close_all:
- close(wfd[0]);
- close(wfd[1]);
- out_close_rfd:
- close(rfd[0]);
- close(rfd[1]);
- out:
return err;
}
View
3  filterproc.h
@@ -1,8 +1,7 @@
#ifndef INC_BARNOWL_FILTER_PROC_H
#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);
View
4 functions.c
@@ -404,13 +404,13 @@ void owl_function_zcrypt(owl_zwrite *z, const char *msg)
old_msg = g_strdup(owl_zwrite_get_message(z));
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);
argv[4] = "-i"; argv[5] = owl_zwrite_get_instance(z);
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);
View
5 message.c
@@ -874,7 +874,7 @@ void owl_message_create_from_znotice(owl_message *m, const ZNotice_t *n)
/* if zcrypt is enabled try to decrypt the message */
if (owl_global_is_zcrypt(&g) && !strcasecmp(n->z_opcode, "crypt")) {
const char *argv[] = {
- "zcrypt",
+ NULL,
"-D",
"-c", owl_message_get_class(m),
"-i", owl_message_get_instance(m),
@@ -886,8 +886,9 @@ void owl_message_create_from_znotice(owl_message *m, const ZNotice_t *n)
char *zcrypt;
zcrypt = g_build_filename(owl_get_bindir(), "zcrypt", NULL);
+ argv[0] = zcrypt;
- rv = call_filter(zcrypt, argv, owl_message_get_body(m), &out, &status);
+ rv = call_filter(argv, owl_message_get_body(m), &out, &status);
g_free(zcrypt);
if(!rv && !status) {
View
35 tester.c
@@ -2,6 +2,7 @@
#define WINDOW FAKE_WINDOW
#include "owl.h"
#undef WINDOW
+#include "filterproc.h"
#include <stdio.h>
@@ -22,6 +23,7 @@ int owl_editwin_regtest(void);
int owl_fmtext_regtest(void);
int owl_smartfilter_regtest(void);
int owl_history_regtest(void);
+int call_filter_regtest(void);
extern void owl_perl_xs_init(pTHX);
@@ -112,6 +114,7 @@ int owl_regtest(void) {
numfailures += owl_fmtext_regtest();
numfailures += owl_smartfilter_regtest();
numfailures += owl_history_regtest();
+ numfailures += call_filter_regtest();
if (numfailures) {
fprintf(stderr, "# *** WARNING: %d failures total\n", numfailures);
}
@@ -980,3 +983,35 @@ int owl_history_regtest(void)
printf("# END testing owl_history (%d failures)\n", numfailed);
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;
+}
View
4 zcrypt.c
@@ -773,7 +773,7 @@ int do_encrypt_aes(const char *keyfile, const char *in, int length, FILE *outfil
"--passphrase-file", keyfile,
NULL
};
- err = call_filter("gpg", argv, in, &out, &status);
+ err = call_filter(argv, in, &out, &status);
if(err || status) {
g_free(out);
return FALSE;
@@ -856,7 +856,7 @@ int do_decrypt_aes(const char *keyfile) {
in = slurp_stdin(TRUE, &length);
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);
return FALSE;
Please sign in to comment.
Something went wrong with that request. Please try again.