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

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

From: Junio C Hamano <hidden>
Date: 2022-08-17 20:23:42

Jeff King [off-list ref] writes:
We'd like to be able to make some of our pipes nonblocking so that
poll() can be used effectively, but O_NONBLOCK isn't portable. Let's
introduce a compat wrapper so this can be abstracted for each platform.

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

The next commit will add Windows support, at which point we should be
covering all platforms in practice. But if we do find some other
platform without O_NONBLOCK, we'll return ENOSYS. Arguably we could just
trigger a build-time #error in this case, which would catch the problem
earlier. But since we're not planning to use this compat wrapper in many
code paths, a seldom-seen runtime error may be friendlier for such a
platform than blocking compilation completely. Our test suite would
still notice it.
Very well reasoned.

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.

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