Re: [PATCH v2 5/6] pipe_command(): handle ENOSPC when writing to a pipe

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> When write() to a non-blocking pipe fails because the buffer is full,
> POSIX says we should see EAGAIN. But our mingw_write() compat layer on
> Windows actually returns ENOSPC for this case. This is probably
> something we want to correct, but given that we don't plan to use
> non-blocking descriptors in a lot of places, we can work around it by
> just catching ENOSPC alongside EAGAIN. If we ever do fix mingw_write(),
> then this patch can be reverted.
>
> We don't actually use a non-blocking pipe yet, so this is still just
> preparation.
>
> Helped-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Ironically, this ENOSPC bug means that switching away from xwrite() in
> the previous patch wasn't necessary (because it's not clever enough to
> know that ENOSPC on a pipe means EAGAIN!). But I think handling both
> shows the intent, and sets us up better for fixing mingw_write().

Yeah, I am impressed by the attention of small details by you two
shown here to split the steps 4/6 and 5/6.  If we consider that this
step is a band-aid we'd be happier if we can remove, perhaps in-code
comment to explain why we deal with ENOSPC here, instead of burying
it only in the log message, would help remind people of the issue
(but of course the patch is good with or without such a tweak, which
is only relevant in the longer term).

>  run-command.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index e078c3046f..5fbaa8b5ac 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1377,7 +1377,8 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)
>  				    io->u.out.len <= MAX_IO_SIZE ?
>  				    io->u.out.len : MAX_IO_SIZE);
>  			if (len < 0) {
> -				if (errno != EINTR && errno != EAGAIN) {
> +				if (errno != EINTR && errno != EAGAIN &&
> +				    errno != ENOSPC) {
>  					io->error = errno;
>  					close(io->fd);
>  					io->fd = -1;