Re: [PATCH v5 3/8] convert: Split start_multi_file_filter into two separate functions
flat view
From: Lars Schneider <hidden>
Date: 2017-04-09 19:56:13
quoted hunk ↗ jump to hunk
On 07 Apr 2017, at 14:03, Ben Peart [off-list ref] wrote: To enable future reuse of the filter.<driver>.process infrastructure, split start_multi_file_filter into two separate parts. start_multi_file_filter will now only contain the generic logic to manage the creation and tracking of the child process in a hashmap. start_multi_file_filter_fn is a protocol specific initialization function that will negotiate the multi-file-filter interface version and capabilities. Signed-off-by: Ben Peart <redacted> --- convert.c | 63 ++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 25 deletions(-)diff --git a/convert.c b/convert.c index 793c29ebfd..404757eac9 100644 --- a/convert.c +++ b/convert.c@@ -565,35 +565,14 @@ static void stop_multi_file_filter(struct child_process *process)finish_command(process); } -static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd) +static int start_multi_file_filter_fn(struct cmd2process *entry) { int err; - struct cmd2process *entry; - struct child_process *process; - const char *argv[] = { cmd, NULL }; struct string_list cap_list = STRING_LIST_INIT_NODUP; char *cap_buf; const char *cap_name; - - entry = xmalloc(sizeof(*entry)); - entry->cmd = cmd; - entry->supported_capabilities = 0; - process = &entry->process; - - child_process_init(process); - process->argv = argv; - process->use_shell = 1; - process->in = -1; - process->out = -1; - process->clean_on_exit = 1; - process->clean_on_exit_handler = stop_multi_file_filter; - - if (start_command(process)) { - error("cannot fork to run external filter '%s'", cmd); - return NULL; - } - - hashmap_entry_init(entry, strhash(cmd)); + struct child_process *process = &entry->process; + const char *cmd = entry->cmd; sigchain_push(SIGPIPE, SIG_IGN);@@ -642,7 +621,41 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, consdone: sigchain_pop(SIGPIPE); - if (err || errno == EPIPE) { + if (err || errno == EPIPE) + err = err ? err : errno;
Nice! I should have done this here, too: https://github.com/git/git/blob/b14f27f91770e0f99f64135348977a0ce1c7993a/convert.c#L755 This is clearly a bug in my code. I'll send a patch shortly.
+
+ return err;
+}
+
+static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)
+{
+ int err;
+ struct cmd2process *entry;
+ struct child_process *process;
+ const char *argv[] = { cmd, NULL };
+
+ entry = xmalloc(sizeof(*entry));
+ entry->cmd = cmd;
+ entry->supported_capabilities = 0;
+ process = &entry->process;
+
+ child_process_init(process);
+ process->argv = argv;
+ process->use_shell = 1;
+ process->in = -1;
+ process->out = -1;
+ process->clean_on_exit = 1;
+ process->clean_on_exit_handler = stop_multi_file_filter;
+
+ if (start_command(process)) {
+ error("cannot fork to run external filter '%s'", cmd);
+ return NULL;
+ }
+
+ hashmap_entry_init(entry, strhash(cmd));
+
+ err = start_multi_file_filter_fn(entry);
+ if (err) {
error("initialization for external filter '%s' failed", cmd);
kill_multi_file_filter(hashmap, entry);
return NULL;
--
2.12.0.windows.1.31.g1548525701.dirtyLooks good to me. Thanks, Lars