{"thread":{"id":"45549","subject":"[PATCH v4 0/8] refactor the filter process code into a reusable module","startedAt":"2017-03-30T15:54:34Z","lastAt":"2017-03-30T20:12:00Z","messageCount":11,"participants":["Ben Peart","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":8},"messages":[{"id":"315778","messageId":"20170330155417.8928-1-benpeart@microsoft.com","threadId":"45549","inReplyTo":null,"subject":"[PATCH v4 0/8] refactor the filter process code into a reusable module","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:08Z","receivedAt":"2017-03-30T15:54:34Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Refactor the filter.<driver>.process code into a separate sub-process\nmodule that can be used to reduce the cost of starting up a sub-process\nfor multiple commands.  It does this by keeping the external process\nrunning and processing all commands by communicating over standard input\nand standard output using the packet format (pkt-line) based protocol.\nFull documentation is in Documentation/technical/api-sub-process.txt.\n\nThis code is refactored from:\n\n\tCommit edcc85814c (\"convert: add filter.<driver>.process option\", 2016-10-16)\n\tkeeps the external process running and processes all commands\n\nBen Peart (8):\n  pkt-line: add packet_read_line_gently()\n  convert: move packet_write_list() into pkt-line as packet_writel()\n  convert: Split start_multi_file_filter into two separate functions\n  convert: Separate generic structures and variables from the filter\n    specific ones\n  convert: Update generic functions to only use generic data structures\n  convert: rename reusable sub-process functions\n  sub-process: move sub-process functions into separate files\n  convert: Update subprocess_read_status to not die on EOF\n\n Documentation/technical/api-sub-process.txt |  54 ++++++++++\n Makefile                                    |   1 +\n convert.c                                   | 159 +++++-----------------------\n pkt-line.c                                  |  31 ++++++\n pkt-line.h                                  |  11 ++\n sub-process.c                               | 120 +++++++++++++++++++++\n sub-process.h                               |  46 ++++++++\n 7 files changed, 292 insertions(+), 130 deletions(-)\n create mode 100644 Documentation/technical/api-sub-process.txt\n create mode 100644 sub-process.c\n create mode 100644 sub-process.h\n\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315779","messageId":"20170330155417.8928-3-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 2/8] convert: move packet_write_list() into pkt-line as packet_writel()","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:10Z","receivedAt":"2017-03-30T15:54:36Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add packet_writel() which writes multiple lines in a single call and\nthen calls packet_flush_gently(). Update convert.c to use the new\npacket_writel() function from pkt-line.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c  | 23 ++---------------------\n pkt-line.c | 19 +++++++++++++++++++\n pkt-line.h |  1 +\n 3 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 8d652bf27c..793c29ebfd 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -521,25 +521,6 @@ static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap,\n \treturn hashmap_get(hashmap, &key, NULL);\n }\n \n-static int packet_write_list(int fd, const char *line, ...)\n-{\n-\tva_list args;\n-\tint err;\n-\tva_start(args, line);\n-\tfor (;;) {\n-\t\tif (!line)\n-\t\t\tbreak;\n-\t\tif (strlen(line) > LARGE_PACKET_DATA_MAX)\n-\t\t\treturn -1;\n-\t\terr = packet_write_fmt_gently(fd, \"%s\\n\", line);\n-\t\tif (err)\n-\t\t\treturn err;\n-\t\tline = va_arg(args, const char*);\n-\t}\n-\tva_end(args);\n-\treturn packet_flush_gently(fd);\n-}\n-\n static void read_multi_file_filter_status(int fd, struct strbuf *status)\n {\n \tstruct strbuf **pair;\n@@ -616,7 +597,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n-\terr = packet_write_list(process->in, \"git-filter-client\", \"version=2\", NULL);\n+\terr = packet_writel(process->in, \"git-filter-client\", \"version=2\", NULL);\n \tif (err)\n \t\tgoto done;\n \n@@ -632,7 +613,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \tif (err)\n \t\tgoto done;\n \n-\terr = packet_write_list(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n+\terr = packet_writel(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n \n \tfor (;;) {\n \t\tcap_buf = packet_read_line(process->out, NULL);\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 58842544b4..2788aa1af6 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -171,6 +171,25 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n \treturn status;\n }\n \n+int packet_writel(int fd, const char *line, ...)\n+{\n+\tva_list args;\n+\tint err;\n+\tva_start(args, line);\n+\tfor (;;) {\n+\t\tif (!line)\n+\t\t\tbreak;\n+\t\tif (strlen(line) > LARGE_PACKET_DATA_MAX)\n+\t\t\treturn -1;\n+\t\terr = packet_write_fmt_gently(fd, \"%s\\n\", line);\n+\t\tif (err)\n+\t\t\treturn err;\n+\t\tline = va_arg(args, const char*);\n+\t}\n+\tva_end(args);\n+\treturn packet_flush_gently(fd);\n+}\n+\n static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n {\n \tstatic char packet_write_buffer[LARGE_PACKET_MAX];\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 12b18991f6..cb3eda9695 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -25,6 +25,7 @@ void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n int packet_flush_gently(int fd);\n int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+int packet_writel(int fd, const char *line, ...);\n int write_packetized_from_fd(int fd_in, int fd_out);\n int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);\n \n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315780","messageId":"20170330155417.8928-5-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 3/8] convert: Split start_multi_file_filter into two separate functions","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:12Z","receivedAt":"2017-03-30T15:54:37Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"To enable future reuse of the filter.<driver>.process infrastructure,\nsplit start_multi_file_filter into two separate parts.\n\nstart_multi_file_filter will now only contain the generic logic to\nmanage the creation and tracking of the child process in a hashmap.\n\nstart_multi_file_filter_fn is a protocol specific initialization\nfunction that will negotiate the multi-file-filter interface version\nand capabilities.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c | 63 ++++++++++++++++++++++++++++++++++++++-------------------------\n 1 file changed, 38 insertions(+), 25 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 793c29ebfd..404757eac9 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -565,35 +565,14 @@ static void stop_multi_file_filter(struct child_process *process)\n \tfinish_command(process);\n }\n \n-static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)\n+static int start_multi_file_filter_fn(struct cmd2process *entry)\n {\n \tint err;\n-\tstruct cmd2process *entry;\n-\tstruct child_process *process;\n-\tconst char *argv[] = { cmd, NULL };\n \tstruct string_list cap_list = STRING_LIST_INIT_NODUP;\n \tchar *cap_buf;\n \tconst char *cap_name;\n-\n-\tentry = xmalloc(sizeof(*entry));\n-\tentry->cmd = cmd;\n-\tentry->supported_capabilities = 0;\n-\tprocess = &entry->process;\n-\n-\tchild_process_init(process);\n-\tprocess->argv = argv;\n-\tprocess->use_shell = 1;\n-\tprocess->in = -1;\n-\tprocess->out = -1;\n-\tprocess->clean_on_exit = 1;\n-\tprocess->clean_on_exit_handler = stop_multi_file_filter;\n-\n-\tif (start_command(process)) {\n-\t\terror(\"cannot fork to run external filter '%s'\", cmd);\n-\t\treturn NULL;\n-\t}\n-\n-\thashmap_entry_init(entry, strhash(cmd));\n+\tstruct child_process *process = &entry->process;\n+\tconst char *cmd = entry->cmd;\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -642,7 +621,41 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n done:\n \tsigchain_pop(SIGPIPE);\n \n-\tif (err || errno == EPIPE) {\n+\tif (err || errno == EPIPE)\n+\t\terr = err ? err : errno;\n+\n+\treturn err;\n+}\n+\n+static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)\n+{\n+\tint err;\n+\tstruct cmd2process *entry;\n+\tstruct child_process *process;\n+\tconst char *argv[] = { cmd, NULL };\n+\n+\tentry = xmalloc(sizeof(*entry));\n+\tentry->cmd = cmd;\n+\tentry->supported_capabilities = 0;\n+\tprocess = &entry->process;\n+\n+\tchild_process_init(process);\n+\tprocess->argv = argv;\n+\tprocess->use_shell = 1;\n+\tprocess->in = -1;\n+\tprocess->out = -1;\n+\tprocess->clean_on_exit = 1;\n+\tprocess->clean_on_exit_handler = stop_multi_file_filter;\n+\n+\tif (start_command(process)) {\n+\t\terror(\"cannot fork to run external filter '%s'\", cmd);\n+\t\treturn NULL;\n+\t}\n+\n+\thashmap_entry_init(entry, strhash(cmd));\n+\n+\terr = start_multi_file_filter_fn(entry);\n+\tif (err) {\n \t\terror(\"initialization for external filter '%s' failed\", cmd);\n \t\tkill_multi_file_filter(hashmap, entry);\n \t\treturn NULL;\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315781","messageId":"20170330155417.8928-6-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 4/8] convert: Separate generic structures and variables from the filter specific ones","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:13Z","receivedAt":"2017-03-30T15:54:40Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"To enable future reuse of the filter.<driver>.process infrastructure,\nsplit the cmd2process structure into two separate parts.\n\nsubprocess_entry will now contain the generic data required to manage\nthe creation and tracking of the child process in a hashmap. Also move\nall knowledge of the hashmap into the generic functions.\n\ncmd2process is a filter protocol specific structure that is used to\ntrack the negotiated capabilities of the filter.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c | 57 +++++++++++++++++++++++++++++++--------------------------\n 1 file changed, 31 insertions(+), 26 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 404757eac9..f569026511 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -496,29 +496,40 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le\n #define CAP_CLEAN    (1u<<0)\n #define CAP_SMUDGE   (1u<<1)\n \n-struct cmd2process {\n+struct subprocess_entry {\n \tstruct hashmap_entry ent; /* must be the first member! */\n-\tunsigned int supported_capabilities;\n \tconst char *cmd;\n \tstruct child_process process;\n };\n \n+struct cmd2process {\n+\tstruct subprocess_entry subprocess; /* must be the first member! */\n+\tunsigned int supported_capabilities;\n+};\n+\n static int cmd_process_map_initialized;\n static struct hashmap cmd_process_map;\n \n-static int cmd2process_cmp(const struct cmd2process *e1,\n-\t\t\t   const struct cmd2process *e2,\n+static int cmd2process_cmp(const struct subprocess_entry *e1,\n+\t\t\t   const struct subprocess_entry *e2,\n \t\t\t   const void *unused)\n {\n \treturn strcmp(e1->cmd, e2->cmd);\n }\n \n-static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap, const char *cmd)\n+static struct subprocess_entry *find_multi_file_filter_entry(const char *cmd)\n {\n-\tstruct cmd2process key;\n+\tstruct subprocess_entry key;\n+\n+\tif (!cmd_process_map_initialized) {\n+\t\tcmd_process_map_initialized = 1;\n+\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\t\treturn NULL;\n+\t}\n+\n \thashmap_entry_init(&key, strhash(cmd));\n \tkey.cmd = cmd;\n-\treturn hashmap_get(hashmap, &key, NULL);\n+\treturn hashmap_get(&cmd_process_map, &key, NULL);\n }\n \n static void read_multi_file_filter_status(int fd, struct strbuf *status)\n@@ -541,7 +552,7 @@ static void read_multi_file_filter_status(int fd, struct strbuf *status)\n \t}\n }\n \n-static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *entry)\n+static void kill_multi_file_filter(struct subprocess_entry *entry)\n {\n \tif (!entry)\n \t\treturn;\n@@ -550,7 +561,7 @@ static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *\n \tkill(entry->process.pid, SIGTERM);\n \tfinish_command(&entry->process);\n \n-\thashmap_remove(hashmap, entry, NULL);\n+\thashmap_remove(&cmd_process_map, entry, NULL);\n \tfree(entry);\n }\n \n@@ -571,8 +582,8 @@ static int start_multi_file_filter_fn(struct cmd2process *entry)\n \tstruct string_list cap_list = STRING_LIST_INIT_NODUP;\n \tchar *cap_buf;\n \tconst char *cap_name;\n-\tstruct child_process *process = &entry->process;\n-\tconst char *cmd = entry->cmd;\n+\tstruct child_process *process = &entry->subprocess.process;\n+\tconst char *cmd = entry->subprocess.cmd;\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -627,7 +638,7 @@ static int start_multi_file_filter_fn(struct cmd2process *entry)\n \treturn err;\n }\n \n-static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)\n+static struct cmd2process *start_multi_file_filter(const char *cmd)\n {\n \tint err;\n \tstruct cmd2process *entry;\n@@ -635,9 +646,9 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \tconst char *argv[] = { cmd, NULL };\n \n \tentry = xmalloc(sizeof(*entry));\n-\tentry->cmd = cmd;\n+\tentry->subprocess.cmd = cmd;\n \tentry->supported_capabilities = 0;\n-\tprocess = &entry->process;\n+\tprocess = &entry->subprocess.process;\n \n \tchild_process_init(process);\n \tprocess->argv = argv;\n@@ -657,11 +668,11 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \terr = start_multi_file_filter_fn(entry);\n \tif (err) {\n \t\terror(\"initialization for external filter '%s' failed\", cmd);\n-\t\tkill_multi_file_filter(hashmap, entry);\n+\t\tkill_multi_file_filter(&entry->subprocess);\n \t\treturn NULL;\n \t}\n \n-\thashmap_add(hashmap, entry);\n+\thashmap_add(&cmd_process_map, entry);\n \treturn entry;\n }\n \n@@ -676,22 +687,16 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tstruct strbuf filter_status = STRBUF_INIT;\n \tconst char *filter_type;\n \n-\tif (!cmd_process_map_initialized) {\n-\t\tcmd_process_map_initialized = 1;\n-\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn) cmd2process_cmp, 0);\n-\t\tentry = NULL;\n-\t} else {\n-\t\tentry = find_multi_file_filter_entry(&cmd_process_map, cmd);\n-\t}\n+\tentry = (struct cmd2process *)find_multi_file_filter_entry(cmd);\n \n \tfflush(NULL);\n \n \tif (!entry) {\n-\t\tentry = start_multi_file_filter(&cmd_process_map, cmd);\n+\t\tentry = start_multi_file_filter(cmd);\n \t\tif (!entry)\n \t\t\treturn 0;\n \t}\n-\tprocess = &entry->process;\n+\tprocess = &entry->subprocess.process;\n \n \tif (!(wanted_capability & entry->supported_capabilities))\n \t\treturn 0;\n@@ -762,7 +767,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \t\t\t * Force shutdown and restart if another blob requires filtering.\n \t\t\t */\n \t\t\terror(\"external filter '%s' failed\", cmd);\n-\t\t\tkill_multi_file_filter(&cmd_process_map, entry);\n+\t\t\tkill_multi_file_filter(&entry->subprocess);\n \t\t}\n \t} else {\n \t\tstrbuf_swap(dst, &nbuf);\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315782","messageId":"20170330155417.8928-10-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 8/8] convert: Update subprocess_read_status to not die on EOF","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:17Z","receivedAt":"2017-03-30T15:54:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Enable sub-processes to gracefully handle when the process dies by\nupdating subprocess_read_status to return an error on EOF instead of\ndying.\n\nUpdate apply_multi_file_filter to take advantage of the revised\nsubprocess_read_status.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c     | 10 ++++++++--\n sub-process.c | 10 +++++++---\n sub-process.h |  2 +-\n 3 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex baa41da760..9e181e27ad 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -629,7 +629,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tif (err)\n \t\tgoto done;\n \n-\tsubprocess_read_status(process->out, &filter_status);\n+\terr = subprocess_read_status(process->out, &filter_status);\n+\tif (err)\n+\t\tgoto done;\n+\n \terr = strcmp(filter_status.buf, \"success\");\n \tif (err)\n \t\tgoto done;\n@@ -638,7 +641,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tif (err)\n \t\tgoto done;\n \n-\tsubprocess_read_status(process->out, &filter_status);\n+\terr = subprocess_read_status(process->out, &filter_status);\n+\tif (err)\n+\t\tgoto done;\n+\n \terr = strcmp(filter_status.buf, \"success\");\n \n done:\ndiff --git a/sub-process.c b/sub-process.c\nindex a9e998cd75..ce919383fa 100644\n--- a/sub-process.c\n+++ b/sub-process.c\n@@ -30,13 +30,15 @@ struct subprocess_entry *subprocess_find_entry(const char *cmd)\n \treturn hashmap_get(&subprocess_map, &key, NULL);\n }\n \n-void subprocess_read_status(int fd, struct strbuf *status)\n+int subprocess_read_status(int fd, struct strbuf *status)\n {\n \tstruct strbuf **pair;\n \tchar *line;\n+\tint len;\n+\n \tfor (;;) {\n-\t\tline = packet_read_line(fd, NULL);\n-\t\tif (!line)\n+\t\tlen = packet_read_line_gently(fd, NULL, &line);\n+\t\tif ((len == -1) || !line)\n \t\t\tbreak;\n \t\tpair = strbuf_split_str(line, '=', 2);\n \t\tif (pair[0] && pair[0]->len && pair[1]) {\n@@ -48,6 +50,8 @@ void subprocess_read_status(int fd, struct strbuf *status)\n \t\t}\n \t\tstrbuf_list_free(pair);\n \t}\n+\n+\treturn len == -1 ? len : 0;\n }\n \n void subprocess_stop(struct subprocess_entry *entry)\ndiff --git a/sub-process.h b/sub-process.h\nindex 0cf1760a0a..5a1eeeece0 100644\n--- a/sub-process.h\n+++ b/sub-process.h\n@@ -41,6 +41,6 @@ static inline struct child_process *subprocess_get_child_process(\n  * key/value pairs and return the value from the last \"status\" packet\n  */\n \n-void subprocess_read_status(int fd, struct strbuf *status);\n+int subprocess_read_status(int fd, struct strbuf *status);\n \n #endif\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315783","messageId":"20170330155417.8928-2-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 1/8] pkt-line: add packet_read_line_gently()","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:09Z","receivedAt":"2017-03-30T15:54:46Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add packet_read_line_gently() to enable reading a line without dying on\nEOF.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n pkt-line.c | 12 ++++++++++++\n pkt-line.h | 10 ++++++++++\n 2 files changed, 22 insertions(+)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex d4b6bfe076..58842544b4 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -323,6 +323,18 @@ char *packet_read_line(int fd, int *len_p)\n \treturn packet_read_line_generic(fd, NULL, NULL, len_p);\n }\n \n+int packet_read_line_gently(int fd, int *dst_len, char** dst_line)\n+{\n+\tint len = packet_read(fd, NULL, NULL,\n+\t\tpacket_buffer, sizeof(packet_buffer),\n+\t\tPACKET_READ_CHOMP_NEWLINE|PACKET_READ_GENTLE_ON_EOF);\n+\tif (dst_len)\n+\t\t*dst_len = len;\n+\tif (dst_line)\n+\t\t*dst_line = (len > 0) ? packet_buffer : NULL;\n+\treturn len;\n+}\n+\n char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)\n {\n \treturn packet_read_line_generic(-1, src, src_len, dst_len);\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 18eac64830..12b18991f6 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -74,6 +74,16 @@ int packet_read(int fd, char **src_buffer, size_t *src_len, char\n char *packet_read_line(int fd, int *size);\n \n /*\n+ * Convenience wrapper for packet_read that sets the PACKET_READ_GENTLE_ON_EOF\n+ * and CHOMP_NEWLINE options. The return value specifies the number of bytes\n+ * read into the buffer or -1 on truncated input. if the *dst_line parameter\n+ * is not NULL it will return NULL for a flush packet and otherwise points to\n+ * a static buffer (that may be overwritten by subsequent calls). If the size\n+ * parameter is not NULL, the length of the packet is written to it.\n+ */\n+int packet_read_line_gently(int fd, int *size, char** dst_line);\n+\n+/*\n  * Same as packet_read_line, but read from a buf rather than a descriptor;\n  * see packet_read for details on how src_* is used.\n  */\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315784","messageId":"20170330155417.8928-9-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 7/8] sub-process: move sub-process functions into separate files","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:16Z","receivedAt":"2017-03-30T15:54:48Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Move the sub-proces functions into sub-process.h/c.  Add documentation\nfor the new module in Documentation/technical/api-sub-process.txt\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n Documentation/technical/api-sub-process.txt |  54 +++++++++++++\n Makefile                                    |   1 +\n convert.c                                   | 119 +---------------------------\n sub-process.c                               | 116 +++++++++++++++++++++++++++\n sub-process.h                               |  46 +++++++++++\n 5 files changed, 218 insertions(+), 118 deletions(-)\n create mode 100644 Documentation/technical/api-sub-process.txt\n create mode 100644 sub-process.c\n create mode 100644 sub-process.h\n\ndiff --git a/Documentation/technical/api-sub-process.txt b/Documentation/technical/api-sub-process.txt\nnew file mode 100644\nindex 0000000000..eb5005aa72\n--- /dev/null\n+++ b/Documentation/technical/api-sub-process.txt\n@@ -0,0 +1,54 @@\n+sub-process API\n+===============\n+\n+The sub-process API makes it possible to run background sub-processes\n+that should run until the git command exits and communicate with it\n+through stdin and stdout.  This reduces the overhead of having to fork\n+a new process each time it needs to be communicated with.\n+\n+The sub-processes are kept in a hashmap by command name and looked up\n+via the subprocess_find_entry function.  If an existing instance can not\n+be found then a new process should be created and started.  When the\n+parent git command terminates, all sub-processes are also terminated.\n+\n+This API is based on the run-command API.\n+\n+Data structures\n+---------------\n+\n+* `struct subprocess_entry`\n+\n+The sub-process structure.  Members should not be accessed directly.\n+\n+Types\n+-----\n+\n+'int(*subprocess_start_fn)(struct subprocess_entry *entry)'::\n+\n+\tUser-supplied function to initialize the sub-process.  This is\n+\ttypically used to negoiate the interface version and capabilities.\n+\n+\n+Functions\n+---------\n+\n+`subprocess_start`::\n+\n+\tStart a subprocess and add it to the subprocess hashmap.\n+\n+`subprocess_stop`::\n+\n+\tKill a subprocess and remove it from the subprocess hashmap.\n+\n+`subprocess_find_entry`::\n+\n+\tFind a subprocess in the subprocess hashmap.\n+\n+`subprocess_get_child_process`::\n+\n+\tGet the underlying `struct child_process` from a subprocess.\n+\n+`subprocess_read_status`::\n+\n+\tHelper function to read packets looking for the last \"status=<foo>\"\n+\tkey/value pair.\ndiff --git a/Makefile b/Makefile\nindex 9f8b35ad41..add945b560 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -838,6 +838,7 @@ LIB_OBJS += streaming.o\n LIB_OBJS += string-list.o\n LIB_OBJS += submodule.o\n LIB_OBJS += submodule-config.o\n+LIB_OBJS += sub-process.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += tempfile.o\ndiff --git a/convert.c b/convert.c\nindex f68a7be622..baa41da760 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -4,6 +4,7 @@\n #include \"quote.h\"\n #include \"sigchain.h\"\n #include \"pkt-line.h\"\n+#include \"sub-process.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -496,86 +497,11 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le\n #define CAP_CLEAN    (1u<<0)\n #define CAP_SMUDGE   (1u<<1)\n \n-struct subprocess_entry {\n-\tstruct hashmap_entry ent; /* must be the first member! */\n-\tconst char *cmd;\n-\tstruct child_process process;\n-};\n-\n struct cmd2process {\n \tstruct subprocess_entry subprocess; /* must be the first member! */\n \tunsigned int supported_capabilities;\n };\n \n-static int subprocess_map_initialized;\n-static struct hashmap subprocess_map;\n-\n-static int cmd2process_cmp(const struct subprocess_entry *e1,\n-\t\t\t   const struct subprocess_entry *e2,\n-\t\t\t   const void *unused)\n-{\n-\treturn strcmp(e1->cmd, e2->cmd);\n-}\n-\n-static struct subprocess_entry *subprocess_find_entry(const char *cmd)\n-{\n-\tstruct subprocess_entry key;\n-\n-\tif (!subprocess_map_initialized) {\n-\t\tsubprocess_map_initialized = 1;\n-\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n-\t\treturn NULL;\n-\t}\n-\n-\thashmap_entry_init(&key, strhash(cmd));\n-\tkey.cmd = cmd;\n-\treturn hashmap_get(&subprocess_map, &key, NULL);\n-}\n-\n-static void subprocess_read_status(int fd, struct strbuf *status)\n-{\n-\tstruct strbuf **pair;\n-\tchar *line;\n-\tfor (;;) {\n-\t\tline = packet_read_line(fd, NULL);\n-\t\tif (!line)\n-\t\t\tbreak;\n-\t\tpair = strbuf_split_str(line, '=', 2);\n-\t\tif (pair[0] && pair[0]->len && pair[1]) {\n-\t\t\t/* the last \"status=<foo>\" line wins */\n-\t\t\tif (!strcmp(pair[0]->buf, \"status=\")) {\n-\t\t\t\tstrbuf_reset(status);\n-\t\t\t\tstrbuf_addbuf(status, pair[1]);\n-\t\t\t}\n-\t\t}\n-\t\tstrbuf_list_free(pair);\n-\t}\n-}\n-\n-static void subprocess_stop(struct subprocess_entry *entry)\n-{\n-\tif (!entry)\n-\t\treturn;\n-\n-\tentry->process.clean_on_exit = 0;\n-\tkill(entry->process.pid, SIGTERM);\n-\tfinish_command(&entry->process);\n-\n-\thashmap_remove(&subprocess_map, entry, NULL);\n-\tfree(entry);\n-}\n-\n-static void subprocess_exit_handler(struct child_process *process)\n-{\n-\tsigchain_push(SIGPIPE, SIG_IGN);\n-\t/* Closing the pipe signals the filter to initiate a shutdown. */\n-\tclose(process->in);\n-\tclose(process->out);\n-\tsigchain_pop(SIGPIPE);\n-\t/* Finish command will wait until the shutdown is complete. */\n-\tfinish_command(process);\n-}\n-\n static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)\n {\n \tint err;\n@@ -639,49 +565,6 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)\n \treturn err;\n }\n \n-typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);\n-int subprocess_start(struct subprocess_entry *entry, const char *cmd,\n-\tsubprocess_start_fn startfn)\n-{\n-\tint err;\n-\tstruct child_process *process;\n-\tconst char *argv[] = { cmd, NULL };\n-\n-\tif (!subprocess_map_initialized) {\n-\t\tsubprocess_map_initialized = 1;\n-\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n-\t}\n-\n-\tentry->cmd = cmd;\n-\tprocess = &entry->process;\n-\n-\tchild_process_init(process);\n-\tprocess->argv = argv;\n-\tprocess->use_shell = 1;\n-\tprocess->in = -1;\n-\tprocess->out = -1;\n-\tprocess->clean_on_exit = 1;\n-\tprocess->clean_on_exit_handler = subprocess_exit_handler;\n-\n-\terr = start_command(process);\n-\tif (err) {\n-\t\terror(\"cannot fork to run external filter '%s'\", cmd);\n-\t\treturn err;\n-\t}\n-\n-\thashmap_entry_init(entry, strhash(cmd));\n-\n-\terr = startfn(entry);\n-\tif (err) {\n-\t\terror(\"initialization for external filter '%s' failed\", cmd);\n-\t\tsubprocess_stop(entry);\n-\t\treturn err;\n-\t}\n-\n-\thashmap_add(&subprocess_map, entry);\n-\treturn 0;\n-}\n-\n static int apply_multi_file_filter(const char *path, const char *src, size_t len,\n \t\t\t\t   int fd, struct strbuf *dst, const char *cmd,\n \t\t\t\t   const unsigned int wanted_capability)\ndiff --git a/sub-process.c b/sub-process.c\nnew file mode 100644\nindex 0000000000..a9e998cd75\n--- /dev/null\n+++ b/sub-process.c\n@@ -0,0 +1,116 @@\n+/*\n+ * Generic implementation of background process infrastructure.\n+ */\n+#include \"sub-process.h\"\n+#include \"sigchain.h\"\n+#include \"pkt-line.h\"\n+\n+static int subprocess_map_initialized;\n+static struct hashmap subprocess_map;\n+\n+static int cmd2process_cmp(const struct subprocess_entry *e1,\n+\t\t\t   const struct subprocess_entry *e2,\n+\t\t\t   const void *unused)\n+{\n+\treturn strcmp(e1->cmd, e2->cmd);\n+}\n+\n+struct subprocess_entry *subprocess_find_entry(const char *cmd)\n+{\n+\tstruct subprocess_entry key;\n+\n+\tif (!subprocess_map_initialized) {\n+\t\tsubprocess_map_initialized = 1;\n+\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\t\treturn NULL;\n+\t}\n+\n+\thashmap_entry_init(&key, strhash(cmd));\n+\tkey.cmd = cmd;\n+\treturn hashmap_get(&subprocess_map, &key, NULL);\n+}\n+\n+void subprocess_read_status(int fd, struct strbuf *status)\n+{\n+\tstruct strbuf **pair;\n+\tchar *line;\n+\tfor (;;) {\n+\t\tline = packet_read_line(fd, NULL);\n+\t\tif (!line)\n+\t\t\tbreak;\n+\t\tpair = strbuf_split_str(line, '=', 2);\n+\t\tif (pair[0] && pair[0]->len && pair[1]) {\n+\t\t\t/* the last \"status=<foo>\" line wins */\n+\t\t\tif (!strcmp(pair[0]->buf, \"status=\")) {\n+\t\t\t\tstrbuf_reset(status);\n+\t\t\t\tstrbuf_addbuf(status, pair[1]);\n+\t\t\t}\n+\t\t}\n+\t\tstrbuf_list_free(pair);\n+\t}\n+}\n+\n+void subprocess_stop(struct subprocess_entry *entry)\n+{\n+\tif (!entry)\n+\t\treturn;\n+\n+\tentry->process.clean_on_exit = 0;\n+\tkill(entry->process.pid, SIGTERM);\n+\tfinish_command(&entry->process);\n+\n+\thashmap_remove(&subprocess_map, entry, NULL);\n+}\n+\n+static void subprocess_exit_handler(struct child_process *process)\n+{\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\t/* Closing the pipe signals the filter to initiate a shutdown. */\n+\tclose(process->in);\n+\tclose(process->out);\n+\tsigchain_pop(SIGPIPE);\n+\t/* Finish command will wait until the shutdown is complete. */\n+\tfinish_command(process);\n+}\n+\n+int subprocess_start(struct subprocess_entry *entry, const char *cmd,\n+\tsubprocess_start_fn startfn)\n+{\n+\tint err;\n+\tstruct child_process *process;\n+\tconst char *argv[] = { cmd, NULL };\n+\n+\tif (!subprocess_map_initialized) {\n+\t\tsubprocess_map_initialized = 1;\n+\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\t}\n+\n+\tentry->cmd = cmd;\n+\tprocess = &entry->process;\n+\n+\tchild_process_init(process);\n+\tprocess->argv = argv;\n+\tprocess->use_shell = 1;\n+\tprocess->in = -1;\n+\tprocess->out = -1;\n+\tprocess->clean_on_exit = 1;\n+\tprocess->clean_on_exit_handler = subprocess_exit_handler;\n+\n+\terr = start_command(process);\n+\tif (err) {\n+\t\terror(\"cannot fork to run external filter '%s'\", cmd);\n+\t\treturn err;\n+\t}\n+\n+\thashmap_entry_init(entry, strhash(cmd));\n+\n+\terr = startfn(entry);\n+\tif (err) {\n+\t\terror(\"initialization for external filter '%s' failed\", cmd);\n+\t\tsubprocess_stop(entry);\n+\t\treturn err;\n+\t}\n+\n+\thashmap_add(&subprocess_map, entry);\n+\treturn 0;\n+}\ndiff --git a/sub-process.h b/sub-process.h\nnew file mode 100644\nindex 0000000000..0cf1760a0a\n--- /dev/null\n+++ b/sub-process.h\n@@ -0,0 +1,46 @@\n+#ifndef SUBPROCESS_H\n+#define SUBPROCESS_H\n+\n+#include \"git-compat-util.h\"\n+#include \"hashmap.h\"\n+#include \"run-command.h\"\n+\n+/*\n+ * Generic implementation of background process infrastructure.\n+ * See Documentation/technical/api-background-process.txt.\n+ */\n+\n+ /* data structures */\n+\n+struct subprocess_entry {\n+\tstruct hashmap_entry ent; /* must be the first member! */\n+\tconst char *cmd;\n+\tstruct child_process process;\n+};\n+\n+/* subprocess functions */\n+\n+typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);\n+int subprocess_start(struct subprocess_entry *entry, const char *cmd,\n+\t\tsubprocess_start_fn startfn);\n+\n+void subprocess_stop(struct subprocess_entry *entry);\n+\n+struct subprocess_entry *subprocess_find_entry(const char *cmd);\n+\n+/* subprocess helper functions */\n+\n+static inline struct child_process *subprocess_get_child_process(\n+\t\tstruct subprocess_entry *entry)\n+{\n+\treturn &entry->process;\n+}\n+\n+/*\n+ * Helper function that will read packets looking for \"status=<foo>\"\n+ * key/value pairs and return the value from the last \"status\" packet\n+ */\n+\n+void subprocess_read_status(int fd, struct strbuf *status);\n+\n+#endif\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315785","messageId":"20170330155417.8928-4-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 2/8] convert: move packet_write_list() into pkt-line as packet_writel()","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:11Z","receivedAt":"2017-03-30T15:54:50Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add packet_writel() which writes multiple lines in a single call and\nthen calls packet_flush_gently(). Update convert.c to use the new\npacket_writel() function from pkt-line.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c  | 23 ++---------------------\n pkt-line.c | 19 +++++++++++++++++++\n pkt-line.h |  1 +\n 3 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 8d652bf27c..793c29ebfd 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -521,25 +521,6 @@ static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap,\n \treturn hashmap_get(hashmap, &key, NULL);\n }\n \n-static int packet_write_list(int fd, const char *line, ...)\n-{\n-\tva_list args;\n-\tint err;\n-\tva_start(args, line);\n-\tfor (;;) {\n-\t\tif (!line)\n-\t\t\tbreak;\n-\t\tif (strlen(line) > LARGE_PACKET_DATA_MAX)\n-\t\t\treturn -1;\n-\t\terr = packet_write_fmt_gently(fd, \"%s\\n\", line);\n-\t\tif (err)\n-\t\t\treturn err;\n-\t\tline = va_arg(args, const char*);\n-\t}\n-\tva_end(args);\n-\treturn packet_flush_gently(fd);\n-}\n-\n static void read_multi_file_filter_status(int fd, struct strbuf *status)\n {\n \tstruct strbuf **pair;\n@@ -616,7 +597,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n-\terr = packet_write_list(process->in, \"git-filter-client\", \"version=2\", NULL);\n+\terr = packet_writel(process->in, \"git-filter-client\", \"version=2\", NULL);\n \tif (err)\n \t\tgoto done;\n \n@@ -632,7 +613,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \tif (err)\n \t\tgoto done;\n \n-\terr = packet_write_list(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n+\terr = packet_writel(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n \n \tfor (;;) {\n \t\tcap_buf = packet_read_line(process->out, NULL);\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 58842544b4..2788aa1af6 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -171,6 +171,25 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n \treturn status;\n }\n \n+int packet_writel(int fd, const char *line, ...)\n+{\n+\tva_list args;\n+\tint err;\n+\tva_start(args, line);\n+\tfor (;;) {\n+\t\tif (!line)\n+\t\t\tbreak;\n+\t\tif (strlen(line) > LARGE_PACKET_DATA_MAX)\n+\t\t\treturn -1;\n+\t\terr = packet_write_fmt_gently(fd, \"%s\\n\", line);\n+\t\tif (err)\n+\t\t\treturn err;\n+\t\tline = va_arg(args, const char*);\n+\t}\n+\tva_end(args);\n+\treturn packet_flush_gently(fd);\n+}\n+\n static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n {\n \tstatic char packet_write_buffer[LARGE_PACKET_MAX];\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 12b18991f6..cb3eda9695 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -25,6 +25,7 @@ void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n int packet_flush_gently(int fd);\n int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+int packet_writel(int fd, const char *line, ...);\n int write_packetized_from_fd(int fd_in, int fd_out);\n int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);\n \n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315786","messageId":"20170330155417.8928-8-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 6/8] convert: rename reusable sub-process functions","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:15Z","receivedAt":"2017-03-30T15:54:52Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Do a mechanical rename of the functions that will become the reusable\nsub-process module.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c | 47 ++++++++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 747c0c363b..f68a7be622 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -507,8 +507,8 @@ struct cmd2process {\n \tunsigned int supported_capabilities;\n };\n \n-static int cmd_process_map_initialized;\n-static struct hashmap cmd_process_map;\n+static int subprocess_map_initialized;\n+static struct hashmap subprocess_map;\n \n static int cmd2process_cmp(const struct subprocess_entry *e1,\n \t\t\t   const struct subprocess_entry *e2,\n@@ -517,22 +517,22 @@ static int cmd2process_cmp(const struct subprocess_entry *e1,\n \treturn strcmp(e1->cmd, e2->cmd);\n }\n \n-static struct subprocess_entry *find_multi_file_filter_entry(const char *cmd)\n+static struct subprocess_entry *subprocess_find_entry(const char *cmd)\n {\n \tstruct subprocess_entry key;\n \n-\tif (!cmd_process_map_initialized) {\n-\t\tcmd_process_map_initialized = 1;\n-\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\tif (!subprocess_map_initialized) {\n+\t\tsubprocess_map_initialized = 1;\n+\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n \t\treturn NULL;\n \t}\n \n \thashmap_entry_init(&key, strhash(cmd));\n \tkey.cmd = cmd;\n-\treturn hashmap_get(&cmd_process_map, &key, NULL);\n+\treturn hashmap_get(&subprocess_map, &key, NULL);\n }\n \n-static void read_multi_file_filter_status(int fd, struct strbuf *status)\n+static void subprocess_read_status(int fd, struct strbuf *status)\n {\n \tstruct strbuf **pair;\n \tchar *line;\n@@ -552,7 +552,7 @@ static void read_multi_file_filter_status(int fd, struct strbuf *status)\n \t}\n }\n \n-static void kill_multi_file_filter(struct subprocess_entry *entry)\n+static void subprocess_stop(struct subprocess_entry *entry)\n {\n \tif (!entry)\n \t\treturn;\n@@ -561,11 +561,11 @@ static void kill_multi_file_filter(struct subprocess_entry *entry)\n \tkill(entry->process.pid, SIGTERM);\n \tfinish_command(&entry->process);\n \n-\thashmap_remove(&cmd_process_map, entry, NULL);\n+\thashmap_remove(&subprocess_map, entry, NULL);\n \tfree(entry);\n }\n \n-static void stop_multi_file_filter(struct child_process *process)\n+static void subprocess_exit_handler(struct child_process *process)\n {\n \tsigchain_push(SIGPIPE, SIG_IGN);\n \t/* Closing the pipe signals the filter to initiate a shutdown. */\n@@ -640,16 +640,16 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)\n }\n \n typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);\n-int start_multi_file_filter(struct subprocess_entry *entry, const char *cmd,\n+int subprocess_start(struct subprocess_entry *entry, const char *cmd,\n \tsubprocess_start_fn startfn)\n {\n \tint err;\n \tstruct child_process *process;\n \tconst char *argv[] = { cmd, NULL };\n \n-\tif (!cmd_process_map_initialized) {\n-\t\tcmd_process_map_initialized = 1;\n-\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\tif (!subprocess_map_initialized) {\n+\t\tsubprocess_map_initialized = 1;\n+\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n \t}\n \n \tentry->cmd = cmd;\n@@ -661,7 +661,7 @@ int start_multi_file_filter(struct subprocess_entry *entry, const char *cmd,\n \tprocess->in = -1;\n \tprocess->out = -1;\n \tprocess->clean_on_exit = 1;\n-\tprocess->clean_on_exit_handler = stop_multi_file_filter;\n+\tprocess->clean_on_exit_handler = subprocess_exit_handler;\n \n \terr = start_command(process);\n \tif (err) {\n@@ -674,11 +674,11 @@ int start_multi_file_filter(struct subprocess_entry *entry, const char *cmd,\n \terr = startfn(entry);\n \tif (err) {\n \t\terror(\"initialization for external filter '%s' failed\", cmd);\n-\t\tkill_multi_file_filter(entry);\n+\t\tsubprocess_stop(entry);\n \t\treturn err;\n \t}\n \n-\thashmap_add(&cmd_process_map, entry);\n+\thashmap_add(&subprocess_map, entry);\n \treturn 0;\n }\n \n@@ -693,7 +693,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tstruct strbuf filter_status = STRBUF_INIT;\n \tconst char *filter_type;\n \n-\tentry = (struct cmd2process *)find_multi_file_filter_entry(cmd);\n+\tentry = (struct cmd2process *)subprocess_find_entry(cmd);\n \n \tfflush(NULL);\n \n@@ -701,7 +701,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \t\tentry = xmalloc(sizeof(*entry));\n \t\tentry->supported_capabilities = 0;\n \n-\t\tif (start_multi_file_filter(&entry->subprocess, cmd, start_multi_file_filter_fn)) {\n+\t\tif (subprocess_start(&entry->subprocess, cmd, start_multi_file_filter_fn)) {\n \t\t\tfree(entry);\n \t\t\treturn 0;\n \t\t}\n@@ -746,7 +746,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tif (err)\n \t\tgoto done;\n \n-\tread_multi_file_filter_status(process->out, &filter_status);\n+\tsubprocess_read_status(process->out, &filter_status);\n \terr = strcmp(filter_status.buf, \"success\");\n \tif (err)\n \t\tgoto done;\n@@ -755,7 +755,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tif (err)\n \t\tgoto done;\n \n-\tread_multi_file_filter_status(process->out, &filter_status);\n+\tsubprocess_read_status(process->out, &filter_status);\n \terr = strcmp(filter_status.buf, \"success\");\n \n done:\n@@ -777,7 +777,8 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \t\t\t * Force shutdown and restart if another blob requires filtering.\n \t\t\t */\n \t\t\terror(\"external filter '%s' failed\", cmd);\n-\t\t\tkill_multi_file_filter((struct subprocess_entry *)entry);\n+\t\t\tsubprocess_stop((struct subprocess_entry *)entry);\n+\t\t\tfree(entry);\n \t\t}\n \t} else {\n \t\tstrbuf_swap(dst, &nbuf);\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315787","messageId":"20170330155417.8928-7-benpeart@microsoft.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"[PATCH v4 5/8] convert: Update generic functions to only use generic data structures","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-03-30T15:54:14Z","receivedAt":"2017-03-30T15:54:55Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Update all functions that are going to be moved into a reusable module\nso that they only work with the reusable data structures.  Move code\nthat is specific to the filter out into the filter specific functions.\n\nSigned-off-by: Ben Peart <benpeart@microsoft.com>\n---\n convert.c | 46 ++++++++++++++++++++++++++++------------------\n 1 file changed, 28 insertions(+), 18 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex f569026511..747c0c363b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -576,14 +576,15 @@ static void stop_multi_file_filter(struct child_process *process)\n \tfinish_command(process);\n }\n \n-static int start_multi_file_filter_fn(struct cmd2process *entry)\n+static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)\n {\n \tint err;\n+\tstruct cmd2process *entry = (struct cmd2process *)subprocess;\n \tstruct string_list cap_list = STRING_LIST_INIT_NODUP;\n \tchar *cap_buf;\n \tconst char *cap_name;\n-\tstruct child_process *process = &entry->subprocess.process;\n-\tconst char *cmd = entry->subprocess.cmd;\n+\tstruct child_process *process = &subprocess->process;\n+\tconst char *cmd = subprocess->cmd;\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -638,17 +639,21 @@ static int start_multi_file_filter_fn(struct cmd2process *entry)\n \treturn err;\n }\n \n-static struct cmd2process *start_multi_file_filter(const char *cmd)\n+typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);\n+int start_multi_file_filter(struct subprocess_entry *entry, const char *cmd,\n+\tsubprocess_start_fn startfn)\n {\n \tint err;\n-\tstruct cmd2process *entry;\n \tstruct child_process *process;\n \tconst char *argv[] = { cmd, NULL };\n \n-\tentry = xmalloc(sizeof(*entry));\n-\tentry->subprocess.cmd = cmd;\n-\tentry->supported_capabilities = 0;\n-\tprocess = &entry->subprocess.process;\n+\tif (!cmd_process_map_initialized) {\n+\t\tcmd_process_map_initialized = 1;\n+\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn)cmd2process_cmp, 0);\n+\t}\n+\n+\tentry->cmd = cmd;\n+\tprocess = &entry->process;\n \n \tchild_process_init(process);\n \tprocess->argv = argv;\n@@ -658,22 +663,23 @@ static struct cmd2process *start_multi_file_filter(const char *cmd)\n \tprocess->clean_on_exit = 1;\n \tprocess->clean_on_exit_handler = stop_multi_file_filter;\n \n-\tif (start_command(process)) {\n+\terr = start_command(process);\n+\tif (err) {\n \t\terror(\"cannot fork to run external filter '%s'\", cmd);\n-\t\treturn NULL;\n+\t\treturn err;\n \t}\n \n \thashmap_entry_init(entry, strhash(cmd));\n \n-\terr = start_multi_file_filter_fn(entry);\n+\terr = startfn(entry);\n \tif (err) {\n \t\terror(\"initialization for external filter '%s' failed\", cmd);\n-\t\tkill_multi_file_filter(&entry->subprocess);\n-\t\treturn NULL;\n+\t\tkill_multi_file_filter(entry);\n+\t\treturn err;\n \t}\n \n \thashmap_add(&cmd_process_map, entry);\n-\treturn entry;\n+\treturn 0;\n }\n \n static int apply_multi_file_filter(const char *path, const char *src, size_t len,\n@@ -692,9 +698,13 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \tfflush(NULL);\n \n \tif (!entry) {\n-\t\tentry = start_multi_file_filter(cmd);\n-\t\tif (!entry)\n+\t\tentry = xmalloc(sizeof(*entry));\n+\t\tentry->supported_capabilities = 0;\n+\n+\t\tif (start_multi_file_filter(&entry->subprocess, cmd, start_multi_file_filter_fn)) {\n+\t\t\tfree(entry);\n \t\t\treturn 0;\n+\t\t}\n \t}\n \tprocess = &entry->subprocess.process;\n \n@@ -767,7 +777,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \t\t\t * Force shutdown and restart if another blob requires filtering.\n \t\t\t */\n \t\t\terror(\"external filter '%s' failed\", cmd);\n-\t\t\tkill_multi_file_filter(&entry->subprocess);\n+\t\t\tkill_multi_file_filter((struct subprocess_entry *)entry);\n \t\t}\n \t} else {\n \t\tstrbuf_swap(dst, &nbuf);\n-- \n2.12.1.gvfs.1.18.ge47db72\n\n"},{"id":"315819","messageId":"xmqqpogy34om.fsf@gitster.mtv.corp.google.com","threadId":"45549","inReplyTo":"20170330155417.8928-1-benpeart@microsoft.com","subject":"Re: [PATCH v4 0/8] refactor the filter process code into a reusable module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T20:11:53Z","receivedAt":"2017-03-30T20:12:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Now 1 & 2 are as equally pleasant to read as others ;-).\n\nLet's wait for a few days and then merge to 'next'.  I didn't see\nanything questionable.\n\nThanks.\n"}]}