mingw: fix mingw_open_append to work with named pipes

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff Hostetler committed Sep 11, 2018 at 13:06 UTC eeaf7ddac7594636b517497fff3ab181966edbcc
2 files changed +34 -4
compat/mingw.c
+33 -3
@@ -341,6 +341,19 @@ int mingw_mkdir(const char *path, int mode)
341 return ret;
342 }
343
344 +/*
345 + * Calling CreateFile() using FILE_APPEND_DATA and without FILE_WRITE_DATA
346 + * is documented in [1] as opening a writable file handle in append mode.
347 + * (It is believed that) this is atomic since it is maintained by the
348 + * kernel unlike the O_APPEND flag which is racily maintained by the CRT.
349 + *
350 + * [1] https://docs.microsoft.com/en-us/windows/desktop/fileio/file-access-rights-constants
351 + *
352 + * This trick does not appear to work for named pipes. Instead it creates
353 + * a named pipe client handle that cannot be written to. Callers should
354 + * just use the regular _wopen() for them. (And since client handle gets
355 + * bound to a unique server handle, it isn't really an issue.)
356 + */
357 static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
358 {
359 HANDLE handle;
@@ -360,10 +373,12 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
373 NULL, create, FILE_ATTRIBUTE_NORMAL, NULL);
374 if (handle == INVALID_HANDLE_VALUE)
375 return errno = err_win_to_posix(GetLastError()), -1;
376 +
377 /*
378 * No O_APPEND here, because the CRT uses it only to reset the
365 - * file pointer to EOF on write(); but that is not necessary
366 - * for a file created with FILE_APPEND_DATA.
379 + * file pointer to EOF before each write(); but that is not
380 + * necessary (and may lead to races) for a file created with
381 + * FILE_APPEND_DATA.
382 */
383 fd = _open_osfhandle((intptr_t)handle, O_BINARY);
384 if (fd < 0)
@@ -371,6 +386,21 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
386 return fd;
387 }
388
389 +/*
390 + * Does the pathname map to the local named pipe filesystem?
391 + * That is, does it have a "//./pipe/" prefix?
392 + */
393 +static int is_local_named_pipe_path(const char *filename)
394 +{
395 + return (is_dir_sep(filename[0]) &&
396 + is_dir_sep(filename[1]) &&
397 + filename[2] == '.' &&
398 + is_dir_sep(filename[3]) &&
399 + !strncasecmp(filename+4, "pipe", 4) &&
400 + is_dir_sep(filename[8]) &&
401 + filename[9]);
402 +}
403 +
404 int mingw_open (const char *filename, int oflags, ...)
405 {
406 typedef int (*open_fn_t)(wchar_t const *wfilename, int oflags, ...);
@@ -387,7 +417,7 @@ int mingw_open (const char *filename, int oflags, ...)
417 if (filename && !strcmp(filename, "/dev/null"))
418 filename = "nul";
419
390 - if (oflags & O_APPEND)
420 + if ((oflags & O_APPEND) && !is_local_named_pipe_path(filename))
421 open_fn = mingw_open_append;
422 else
423 open_fn = _wopen;
t/t0051-windows-named-pipe.sh
+1 -1
@@ -4,7 +4,7 @@ test_description='Windows named pipes'
4
5 . ./test-lib.sh
6
7 -test_expect_failure MINGW 'o_append write to named pipe' '
7 +test_expect_success MINGW 'o_append write to named pipe' '
8 GIT_TRACE="$(pwd)/expect" git status >/dev/null 2>&1 &&
9 { test-tool windows-named-pipe t0051 >actual 2>&1 & } &&
10 pid=$! &&