Thread (41 messages) 41 messages, 4 authors, 2022-08-20

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

From: Jeff King <hidden>
Date: 2022-08-18 05:39:04
Subsystem: the rest · Maintainer: Linus Torvalds

On Wed, Aug 17, 2022 at 11:57:01AM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
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 [off-list ref]
Signed-off-by: Jeff King <redacted>
---
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).
Yeah, you may be right. I had originally written a quite long comment
here (before I split things up so much), but found in the splitting that
it made more sense to put most of the details into the commit message.

But maybe it's worth squashing this in?
diff --git a/run-command.c b/run-command.c
index 5fbaa8b5ac..065883672b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1372,6 +1372,10 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)
 			 *
 			 * Note that we lose xwrite()'s handling of MAX_IO_SIZE
 			 * and EINTR, so we have to implement those ourselves.
+			 *
+			 * We also check for ENOSPC to handle a quirk of
+			 * mingw_write(), which uses that for a full pipe
+			 * instead of EAGAIN.
 			 */
 			len = write(io->fd, io->u.out.buf,
 				    io->u.out.len <= MAX_IO_SIZE ?

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help