sub-process: separate process lifecycle from hashmap management

subprocess_start() and subprocess_stop() couple two concerns:
managing a child process (setup, handshake, teardown) and
managing a hashmap that indexes running processes by command
string.  The hashmap suits callers like convert.c where many
files may share one filter process looked up by name, but
callers that manage process lifetime through their own data
structures do not need it.

Extract subprocess_start_command() and subprocess_stop_command()
so callers can reuse the child process setup and handshake
machinery without maintaining a hashmap.  subprocess_start()
and subprocess_stop() become thin wrappers that add hashmap
operations on top.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sub-process.c b/sub-process.c
index 2d5c965169727b77a1e15c879d1dc9d859bddb0d..3cef42b0880e3f7e78b73de234fa8f90abffc1ef 100644 (file)
--- a/sub-process.c
+++ b/sub-process.c
@@ -49,7 +49,7 @@ int subprocess_read_status(int fd, struct strbuf *status)
        return (len < 0) ? len : 0;
 }
 
-void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
+void subprocess_stop_command(struct subprocess_entry *entry)
 {
        if (!entry)
                return;
@@ -57,7 +57,14 @@ void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
        entry->process.clean_on_exit = 0;
        kill(entry->process.pid, SIGTERM);
        finish_command(&entry->process);
+}
 
+void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry)
+{
+       if (!entry)
+               return;
+
+       subprocess_stop_command(entry);
        hashmap_remove(hashmap, &entry->ent, NULL);
 }
 
@@ -72,7 +79,7 @@ static void subprocess_exit_handler(struct child_process *process)
        finish_command(process);
 }
 
-int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
+int subprocess_start_command(struct subprocess_entry *entry, const char *cmd,
        subprocess_start_fn startfn)
 {
        int err;
@@ -96,15 +103,26 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
                return err;
        }
 
-       hashmap_entry_init(&entry->ent, strhash(cmd));
-
        err = startfn(entry);
        if (err) {
                error("initialization for subprocess '%s' failed", cmd);
-               subprocess_stop(hashmap, entry);
+               subprocess_stop_command(entry);
                return err;
        }
 
+       return 0;
+}
+
+int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
+       subprocess_start_fn startfn)
+{
+       int err;
+
+       err = subprocess_start_command(entry, cmd, startfn);
+       if (err)
+               return err;
+
+       hashmap_entry_init(&entry->ent, strhash(cmd));
        hashmap_add(hashmap, &entry->ent);
        return 0;
 }

diff --git a/sub-process.h b/sub-process.h
index bfc3959a1b4894b736584884c848be9a5f871452..45f1b8e5e3212f9481bdcbda15105761280daff3 100644 (file)
--- a/sub-process.h
+++ b/sub-process.h
@@ -52,10 +52,17 @@ int cmd2process_cmp(const void *unused_cmp_data,
  */
 typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);
 
-/* Start a subprocess and add it to the subprocess hashmap. */
+/* Start a subprocess and run the startfn (typically handshake). */
+int subprocess_start_command(struct subprocess_entry *entry, const char *cmd,
+               subprocess_start_fn startfn);
+
+/* Start a subprocess, run startfn, and add it to the subprocess hashmap. */
 int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
                subprocess_start_fn startfn);
 
+/* Kill a subprocess. */
+void subprocess_stop_command(struct subprocess_entry *entry);
+
 /* Kill a subprocess and remove it from the subprocess hashmap. */
 void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry);