I've gone through this entire series and am quite happy with it.
Except one bug in this particular patch.
Johannes Sixt <johannes.sixt@xxxxxxxxxx> wrote:
> We run the sideband demultiplexer in an asynchronous function.
...
> diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
> index 871b704..51d8a32 100644
> --- a/builtin-fetch-pack.c
> +++ b/builtin-fetch-pack.c
> @@ -457,42 +457,37 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
> return retval;
> }
>
> -static pid_t setup_sideband(int fd[2], int xd[2])
> +static int sideband_demux(int fd, void *data)
> {
> - pid_t side_pid;
> + int *xd = data;
>
> + close(xd[1]);
If this is a threaded start_async() system this close is going
to impact the caller.
> close(xd[0]);
> - close(fd[1]);
> + fd[0] = demux->out;
> fd[1] = xd[1];
Which is relying on xd[1] right here in the caller. Therefore you
cannot actually use this code with a start_async() implementation
that isn't fork() based. Isn't that going to cause you trouble in
the msysGit tree?
--
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
