Re: [PATCH 4/4] Add 'filter' attribute and external filter driver definition.
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Apr 22, 2007, 00:39 UTC
- Message-ID
- <20070422003929.GD17480@spearce.org>
- In-Reply-To
- <11771520591703-git-send-email-junkio@cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> The interface is similar to the custom low-level merge drivers.
...
Show 6 quoted lines
> +static int filter_buffer(const char *path, const char *src,
> + unsigned long size, const char *cmd)
> +{
> + /*
> + * Spawn cmd and feed the buffer contents through its stdin.
> + */...
ick. What about something like this on top? I moved the extra child process for the input pipe down into the start_command routine, where we can do something a little smarter on some systems, like using a thread rather than a full process. Its also a shorter patch and uses more of the run-command API.
Its not documented very well, but if you set child_process.{in,out} to <0 we open the pipe for you, and close your side of the pipe for you. That really simplifies the logic in the callers.
I did consider rewriting this as a select() based loop to feed the input and read the output, but that might not port well onto a more native Win32 based set of APIs.
---
diff --git a/convert.c b/convert.c index 62d8cee..2ba7ea3 100644 --- a/convert.c +++ b/convert.c @@ -201,99 +201,37 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long * return buffer; } -static int filter_buffer(const char *path, const char *src, - unsigned long size, const char *cmd) -{ - /* - * Spawn cmd and feed the buffer contents through its stdin. - */ - struct child_process child_process; - int pipe_feed[2]; - int write_err, status; - - memset(&child_process, 0, sizeof(child_process)); - - if (pipe(pipe_feed) < 0) { - error("cannot create pipe to run external filter %s", cmd); - return 1; - } - - child_process.pid = fork(); - if (child_process.pid < 0) { - error("cannot fork to run external filter %s", cmd); - close(pipe_feed[0]); - close(pipe_feed[1]); - return 1; - } - if (!child_process.pid) { - dup2(pipe_feed[0], 0); - close(pipe_feed[0]); - close(pipe_feed[1]); - execlp(cmd, cmd, NULL); - return 1; - } - close(pipe_feed[0]); - - write_err = (write_in_full(pipe_feed[1], src, size) < 0); - if (close(pipe_feed[1])) - write_err = 1; - if (write_err) - error("cannot feed the input to external filter %s", cmd); - - status = finish_command(&child_process); - if (status) - error("external filter %s failed %d", cmd, -status); - return (write_err || status); -} - static char *apply_filter(const char *path, const char *src, unsigned long *sizep, const char *cmd) { - /* - * Create a pipeline to have the command filter the buffer's - * contents. - * - * (child --> cmd) --> us - */ const int SLOP = 4096; - int pipe_feed[2]; + struct child_process filter_process; + const char *filter_argv[] = {cmd, NULL}; int status; char *dst; unsigned long dstsize, dstalloc; - struct child_process child_process; if (!cmd) return NULL; - memset(&child_process, 0, sizeof(child_process)); - - if (pipe(pipe_feed) < 0) { - error("cannot create pipe to run external filter %s", cmd); - return NULL; - } - - child_process.pid = fork(); - if (child_process.pid < 0) { - error("cannot fork to run external filter %s", cmd); - close(pipe_feed[0]); - close(pipe_feed[1]); + memset(&filter_process, 0, sizeof(filter_process)); + filter_process.in_bufptr = src; + filter_process.in_buflen = *sizep; + filter_process.out = -1; + filter_process.argv = filter_argv; + status = start_command(&filter_process); + if (status) { + error("external filter %s failed %d", cmd, -status); return NULL; } - if (!child_process.pid) { - dup2(pipe_feed[1], 1); - close(pipe_feed[0]); - close(pipe_feed[1]); - exit(filter_buffer(path, src, *sizep, cmd)); - } - close(pipe_feed[1]); dstalloc = *sizep; dst = xmalloc(dstalloc); dstsize = 0; while (1) { - ssize_t numread = xread(pipe_feed[0], dst + dstsize, - dstalloc - dstsize); + ssize_t numread = xread(filter_process.out, + dst + dstsize, dstalloc - dstsize); if (numread <= 0) { if (!numread) @@ -310,7 +248,7 @@ static char *apply_filter(const char *path, const char *src, } } - status = finish_command(&child_process); + status = finish_command(&filter_process); if (status) { error("external filter %s failed %d", cmd, -status); free(dst); diff --git a/run-command.c b/run-command.c index eff523e..72887f8 100644 --- a/run-command.c +++ b/run-command.c @@ -20,12 +20,29 @@ int start_command(struct child_process *cmd) int need_in, need_out; int fdin[2], fdout[2]; - need_in = !cmd->no_stdin && cmd->in < 0; + need_in = !cmd->no_stdin && (cmd->in_bufptr || cmd->in < 0); if (need_in) { if (pipe(fdin) < 0) return -ERR_RUN_COMMAND_PIPE; - cmd->in = fdin[1]; - cmd->close_in = 1; + if (cmd->in_bufptr) { + pid_t in_feeder = fork(); + if (in_feeder < 0) { + close_pair(fdin); + return -ERR_RUN_COMMAND_PIPE; + } + if (!in_feeder) { + close(fdin[0]); + if (write_in_full(fdin[1], + cmd->in_bufptr, + cmd->in_buflen) != cmd->in_buflen) + die("'%s' did not read all input", cmd->argv[0]); + exit(0); + } + close(fdin[1]); + } else { + cmd->in = fdin[1]; + cmd->close_in = 1; + } } need_out = !cmd->no_stdout diff --git a/run-command.h b/run-command.h index 3680ef9..7632843 100644 --- a/run-command.h +++ b/run-command.h @@ -13,6 +13,8 @@ enum { struct child_process { const char **argv; + const char *in_bufptr; + size_t in_buflen; pid_t pid; int in; int out;
-- 1.5.1.1.135.gf948 -- Shawn.