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

Re: [PATCH v2] pipe_command(): mark stdin descriptor as non-blocking

From: Johannes Schindelin <hidden>
Date: 2022-08-08 12:55:51

Hi René,

On Sun, 7 Aug 2022, René Scharfe wrote:
quoted hunk ↗ jump to hunk
Am 05.08.2022 um 17:36 schrieb Jeff King:
quoted
On Wed, Aug 03, 2022 at 11:56:13PM +0200, René Scharfe wrote:
quoted
quoted
 int enable_nonblock(int fd)
 {
+	DWORD mode;
+	HANDLE handle = winansi_get_osfhandle(fd);
+	if (!handle)
+		return -1;
+	if (!GetNamedPipeHandleState(handle, &mode, NULL, NULL, NULL, NULL, 0))
+		return -1;
+	if (mode & PIPE_NOWAIT)
+		return 0;
+	mode |= PIPE_NOWAIT;
+	if (!SetNamedPipeHandleState(handle, &mode, NULL, NULL))
+		return -1;
 	return 0;
 }
This looks plausibly correct to me. ;) We might want to change the name
of the compat layer to enable_pipe_nonblock(), since one assumes from
the function names this only works for pipes.
Or how about this?  Squashable.  Needs testing.
--- >8 ---
Subject: [PATCH] nonblock: support Windows

Implement enable_nonblock() using the Windows API.  Supports only named
and anonymous pipes for now, which suffices for the current caller.

Signed-off-by: René Scharfe <redacted>
---
 compat/nonblock.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
diff --git a/compat/nonblock.c b/compat/nonblock.c
index 897c099010..78923cd2c3 100644
--- a/compat/nonblock.c
+++ b/compat/nonblock.c
@@ -14,9 +14,40 @@ int enable_nonblock(int fd)

 #else

+#ifdef GIT_WINDOWS_NATIVE
Maybe use an `#elif defined(GIT_WINDOWS_NATIVE)` here? That would make the
code structures clearer, methinks.
+
+#include "win32.h"
+
+int enable_nonblock(int fd)
+{
+	HANDLE h = (HANDLE)_get_osfhandle(fd);
+	DWORD mode;
+	DWORD type = GetFileType(h);
+	if (type == FILE_TYPE_UNKNOWN && GetLastError() != NO_ERROR) {
+		errno = EBADF;
+		return -1;
+	}
+	if (type != FILE_TYPE_PIPE)
+		BUG("unsupported file type: %lu", type);
+	if (!GetNamedPipeHandleState(h, &mode, NULL, NULL, NULL, NULL, 0)) {
+		errno = err_win_to_posix(GetLastError());
+		return -1;
+	}
+	mode |= PIPE_NOWAIT;
+	if (!SetNamedPipeHandleState(h, &mode, NULL, NULL)) {
Nice.

FWIW the documentation of `PIPE_NOWAIT` says:

	Note that nonblocking mode is supported for compatibility with
	Microsoft LAN Manager version 2.0 and should not be used to
	achieve asynchronous input and output (I/O) with named pipes.

(see
https://docs.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-setnamedpipehandlestate
for full details)

There is little more information to be found on the interwebs, the closest
to an in-depth explanation is here:
https://devblogs.microsoft.com/oldnewthing/20110114-00/?p=11753

Even if that comment suggests that this mode is deprecated, I think it is
safe to rely on it in Git's source code.

Thanks,
Dscho
+		errno = err_win_to_posix(GetLastError());
+		return -1;
+	}
+	return 0;
+}
+
+#else
+
 int enable_nonblock(int fd)
 {
 	return 0;
 }

 #endif
+
+#endif
--
2.37.1.windows.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help