run-command: add pipe_command helper

We already have capture_command(), which captures the stdout of a command in a way that avoids deadlocks. But sometimes we need to do more I/O, like capturing stderr as well, or sending data to stdin. It's easy to write code that deadlocks racily in these situations depending on how fast the command reads its input, or in which order it writes its output. Let's give callers an easy interface for doing this the right way, similar to what capture_command() did for the simple case. The whole thing is backed by a generic poll() loop that can feed an arbitrary number of buffers to descriptors, and fill an arbitrary number of strbufs from other descriptors. This seems like overkill, but the resulting code is actually a bit cleaner than just handling the three descriptors (because the output code for stdout/stderr is effectively duplicated, so being able to loop is a benefit). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 17, 2016 at 19:38 UTC 96335bcf4d64c29add3692fb41671190123cf44e
2 files changed +171 -12
run-command.c
+147 -5
@@ -864,19 +864,161 @@ int run_hook_le(const char *const *env, const char *name, ...)
864 return ret;
865 }
866
867 -int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
867 +struct io_pump {
868 + /* initialized by caller */
869 + int fd;
870 + int type; /* POLLOUT or POLLIN */
871 + union {
872 + struct {
873 + const char *buf;
874 + size_t len;
875 + } out;
876 + struct {
877 + struct strbuf *buf;
878 + size_t hint;
879 + } in;
880 + } u;
881 +
882 + /* returned by pump_io */
883 + int error; /* 0 for success, otherwise errno */
884 +
885 + /* internal use */
886 + struct pollfd *pfd;
887 +};
888 +
889 +static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)
890 +{
891 + int pollsize = 0;
892 + int i;
893 +
894 + for (i = 0; i < nr; i++) {
895 + struct io_pump *io = &slots[i];
896 + if (io->fd < 0)
897 + continue;
898 + pfd[pollsize].fd = io->fd;
899 + pfd[pollsize].events = io->type;
900 + io->pfd = &pfd[pollsize++];
901 + }
902 +
903 + if (!pollsize)
904 + return 0;
905 +
906 + if (poll(pfd, pollsize, -1) < 0) {
907 + if (errno == EINTR)
908 + return 1;
909 + die_errno("poll failed");
910 + }
911 +
912 + for (i = 0; i < nr; i++) {
913 + struct io_pump *io = &slots[i];
914 +
915 + if (io->fd < 0)
916 + continue;
917 +
918 + if (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLERR|POLLNVAL)))
919 + continue;
920 +
921 + if (io->type == POLLOUT) {
922 + ssize_t len = xwrite(io->fd,
923 + io->u.out.buf, io->u.out.len);
924 + if (len < 0) {
925 + io->error = errno;
926 + close(io->fd);
927 + io->fd = -1;
928 + } else {
929 + io->u.out.buf += len;
930 + io->u.out.len -= len;
931 + if (!io->u.out.len) {
932 + close(io->fd);
933 + io->fd = -1;
934 + }
935 + }
936 + }
937 +
938 + if (io->type == POLLIN) {
939 + ssize_t len = strbuf_read_once(io->u.in.buf,
940 + io->fd, io->u.in.hint);
941 + if (len < 0)
942 + io->error = errno;
943 + if (len <= 0) {
944 + close(io->fd);
945 + io->fd = -1;
946 + }
947 + }
948 + }
949 +
950 + return 1;
951 +}
952 +
953 +static int pump_io(struct io_pump *slots, int nr)
954 +{
955 + struct pollfd *pfd;
956 + int i;
957 +
958 + for (i = 0; i < nr; i++)
959 + slots[i].error = 0;
960 +
961 + ALLOC_ARRAY(pfd, nr);
962 + while (pump_io_round(slots, nr, pfd))
963 + ; /* nothing */
964 + free(pfd);
965 +
966 + /* There may be multiple errno values, so just pick the first. */
967 + for (i = 0; i < nr; i++) {
968 + if (slots[i].error) {
969 + errno = slots[i].error;
970 + return -1;
971 + }
972 + }
973 + return 0;
974 +}
975 +
976 +
977 +int pipe_command(struct child_process *cmd,
978 + const char *in, size_t in_len,
979 + struct strbuf *out, size_t out_hint,
980 + struct strbuf *err, size_t err_hint)
981 {
869 - cmd->out = -1;
982 + struct io_pump io[3];
983 + int nr = 0;
984 +
985 + if (in)
986 + cmd->in = -1;
987 + if (out)
988 + cmd->out = -1;
989 + if (err)
990 + cmd->err = -1;
991 +
992 if (start_command(cmd) < 0)
993 return -1;
994
873 - if (strbuf_read(buf, cmd->out, hint) < 0) {
874 - close(cmd->out);
995 + if (in) {
996 + io[nr].fd = cmd->in;
997 + io[nr].type = POLLOUT;
998 + io[nr].u.out.buf = in;
999 + io[nr].u.out.len = in_len;
1000 + nr++;
1001 + }
1002 + if (out) {
1003 + io[nr].fd = cmd->out;
1004 + io[nr].type = POLLIN;
1005 + io[nr].u.in.buf = out;
1006 + io[nr].u.in.hint = out_hint;
1007 + nr++;
1008 + }
1009 + if (err) {
1010 + io[nr].fd = cmd->err;
1011 + io[nr].type = POLLIN;
1012 + io[nr].u.in.buf = err;
1013 + io[nr].u.in.hint = err_hint;
1014 + nr++;
1015 + }
1016 +
1017 + if (pump_io(io, nr) < 0) {
1018 finish_command(cmd); /* throw away exit code */
1019 return -1;
1020 }
1021
879 - close(cmd->out);
1022 return finish_command(cmd);
1023 }
1024
run-command.h
+24 -7
@@ -79,17 +79,34 @@ int run_command_v_opt(const char **argv, int opt);
79 int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);
80
81 /**
82 - * Execute the given command, capturing its stdout in the given strbuf.
82 + * Execute the given command, sending "in" to its stdin, and capturing its
83 + * stdout and stderr in the "out" and "err" strbufs. Any of the three may
84 + * be NULL to skip processing.
85 + *
86 * Returns -1 if starting the command fails or reading fails, and otherwise
84 - * returns the exit code of the command. The output collected in the
85 - * buffer is kept even if the command returns a non-zero exit. The hint field
86 - * gives a starting size for the strbuf allocation.
87 + * returns the exit code of the command. Any output collected in the
88 + * buffers is kept even if the command returns a non-zero exit. The hint fields
89 + * gives starting sizes for the strbuf allocations.
90 *
91 * The fields of "cmd" should be set up as they would for a normal run_command
89 - * invocation. But note that there is no need to set cmd->out; the function
90 - * sets it up for the caller.
92 + * invocation. But note that there is no need to set the in, out, or err
93 + * fields; pipe_command handles that automatically.
94 + */
95 +int pipe_command(struct child_process *cmd,
96 + const char *in, size_t in_len,
97 + struct strbuf *out, size_t out_hint,
98 + struct strbuf *err, size_t err_hint);
99 +
100 +/**
101 + * Convenience wrapper around pipe_command for the common case
102 + * of capturing only stdout.
103 */
92 -int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint);
104 +static inline int capture_command(struct child_process *cmd,
105 + struct strbuf *out,
106 + size_t hint)
107 +{
108 + return pipe_command(cmd, NULL, 0, out, hint, NULL, 0);
109 +}
110
111 /*
112 * The purpose of the following functions is to feed a pipe by running