Re: [PATCH] Add git-bundle: move objects and references by archive

[Johannes Sixt <J.Sixt@eudaptics.com>]

Johannes Schindelin wrote:
> +/* if in && *in >= 0, take that as input file descriptor instead */
> +static int fork_with_pipe(const char **argv, int *in, int *out)
> +{
> +       int needs_in, needs_out;
> +       int fdin[2], fdout[2], pid;
> +
> +       needs_in = in && *in < 0;
> +       if (needs_in) {
> +               if (pipe(fdin) < 0)
> +                       return error("could not setup pipe");
> +               *in = fdin[1];
> +       }
> +
> +       needs_out = out && *out < 0;
> +       if (needs_out) {
> +               if (pipe(fdout) < 0)
> +                       return error("could not setup pipe");
> +               *out = fdout[0];
> +       }
> +
> +       if ((pid = fork()) < 0) {
> +               if (needs_in) {
> +                       close(fdin[0]);
> +                       close(fdin[1]);
> +               }
> +               if (needs_out) {
> +                       close(fdout[0]);
> +                       close(fdout[1]);
> +               }
> +               return error("could not fork");
> +       }
> +       if (!pid) {
> +               if (needs_in) {
> +                       dup2(fdin[0], 0);
> +                       close(fdin[0]);
> +                       close(fdin[1]);
> +               } else if (in)
> +                       dup2(*in, 0);
> +               if (needs_out) {
> +                       dup2(fdout[1], 1);
> +                       close(fdout[0]);
> +                       close(fdout[1]);
> +               } else if (out)
> +                       dup2(*out, 1);
> +               exit(execv_git_cmd(argv));
> +       }
> +       if (needs_in)
> +               close(fdin[0]);
> +       if (needs_out)
> +               close(fdout[1]);
> +       return pid;
> +}

This function looks very similar to spawnvppe, which I use in the MinGW
port for a number of fork/exec pairs. Do you see a chance to make this
into a helper that can be used in more places (so that it reduces the
differences to the MinGW code)?

> +       in = out = -1;
> +       pid = fork_with_pipe(argv, &in, &out);
> +       if (pid < 0)
> +               return error("Could not fork rev-list");
> +       if (!fork()) {
> +               for (i = 0; i < p->nr; i++) {
> +                       write(in, sha1_to_hex(p->list[i].sha1), 40);
> +                       write(in, "\n", 1);
> +               }
> +               close(in);
> +               exit(0);
> +       }
> +       close(in);

Oops, fork error check missing?

-- Hannes