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

Re: [PATCH v2 1/6] compat: add function to enable nonblocking pipes

From: Jeff King <hidden>
Date: 2022-08-18 05:41:45

On Wed, Aug 17, 2022 at 01:23:25PM -0700, Junio C Hamano wrote:
The only small "huh?" factor was that the name of the helper is not
enable_nonblock(), but singles out pipe as valid thing to work on.
I think it is perfectly fine, given that the current plan we have is
to use this on the pipe with the command we spawn with
pipe_command(), and it probably is even preferrable to explicitly
declare that this is designed to only work with pipes and future
developers who try to abuse it on other kinds of file descriptors
are on their own.  And "pipe" in the name of this helper may be such
a declaration that is strong enough.
My goal was to explain that "huh" with this bit in the commit message:
quoted
The interface is as narrow as possible to let platforms do what's
natural there (rather than having to implement fcntl() and a fake
O_NONBLOCK for example, or having to handle other types of descriptors).
without going into too many details. I do think it would be easier to
explain if the Windows implementation was added in the same commit
(since it is explicitly just about pipes), but I wanted to credit René
as the author there. I'm not sure if it's worth folding them together
and adding a co-author credit instead. Or maybe I should state outright
in this commit message that we're doing it for Windows. ;)

-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