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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2022, 18:57 UTC
Message-ID: <xmqqo7wivgua.fsf@gitster.g>
In-Reply-To: <YvyGJiz+CFPcgpML@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

> 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).

Show 17 quoted lines

>  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;