When we read from a trailer.*.command sub-program, the
current code uses run_command followed by a pipe read, which
can result in deadlock (though in practice you would have to
have a large trailer for this to be a problem). The current
code also leaks the file descriptor for the pipe to the
sub-command.

Instead, let's use capture_command, which makes this simpler
(and we can get rid of our custom helper).

Signed-off-by: Jeff King <p...@peff.net>
---
 trailer.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/trailer.c b/trailer.c
index 05b3859..4b14a56 100644
--- a/trailer.c
+++ b/trailer.c
@@ -214,16 +214,6 @@ static struct trailer_item *remove_first(struct 
trailer_item **first)
        return item;
 }
 
-static int read_from_command(struct child_process *cp, struct strbuf *buf)
-{
-       if (run_command(cp))
-               return error("running trailer command '%s' failed", 
cp->argv[0]);
-       if (strbuf_read(buf, cp->out, 1024) < 1)
-               return error("reading from trailer command '%s' failed", 
cp->argv[0]);
-       strbuf_trim(buf);
-       return 0;
-}
-
 static const char *apply_command(const char *command, const char *arg)
 {
        struct strbuf cmd = STRBUF_INIT;
@@ -240,14 +230,16 @@ static const char *apply_command(const char *command, 
const char *arg)
        cp.argv = argv;
        cp.env = local_repo_env;
        cp.no_stdin = 1;
-       cp.out = -1;
        cp.use_shell = 1;
 
-       if (read_from_command(&cp, &buf)) {
+       if (capture_command(&cp, &buf, 1024)) {
+               error("running trailer command '%s' failed", cmd.buf);
                strbuf_release(&buf);
                result = xstrdup("");
-       } else
+       } else {
+               strbuf_trim(&buf);
                result = strbuf_detach(&buf, NULL);
+       }
 
        strbuf_release(&cmd);
        return result;
-- 
2.3.3.618.ga041503

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to