Johannes Sixt <j6t@xxxxxxxx> writes: >> +#define IS_SBS(ch) (((ch) == '/') || ((ch) == '\\')) I think you already have mingw_is_dir_sep() and its shorter alias is_dir_sep() available to you. >> +/* >> + * Does the pathname map to the local named pipe filesystem? >> + * That is, does it have a "//./pipe/" prefix? >> + */ >> +static int mingw_is_local_named_pipe_path(const char *filename) There is no need to prefix mingw_ to this function that is file local static. Isn't is_local_named_pipe() descriptive and unique enough? >> +{ >> + return (IS_SBS(filename[0]) && >> + IS_SBS(filename[1]) && >> + filename[2] == '.' && >> + IS_SBS(filename[3]) && >> + !strncasecmp(filename+4, "pipe", 4) && >> + IS_SBS(filename[8]) && >> + filename[9]); >> +} >> +#undef IS_SBS It is kind-of surprising that there hasn't been any existing need for a helper function that would allow us to write this function like so: static int is_local_named_pipe(const char *path) { return path_is_in_directory(path, "//./pipe/"); } Not a suggestion to add such a thing; as long as we know there is no other codepath that would benefit from having one, a generalization like that can and should wait. >> int mingw_open (const char *filename, int oflags, ...) >> { >> typedef int (*open_fn_t)(wchar_t const *wfilename, int oflags, ...); >> @@ -387,7 +419,7 @@ int mingw_open (const char *filename, int oflags, ...) >> if (filename && !strcmp(filename, "/dev/null")) >> filename = "nul"; >> - if (oflags & O_APPEND) >> + if ((oflags & O_APPEND) && !mingw_is_local_named_pipe_path(filename)) >> open_fn = mingw_open_append; >> else >> open_fn = _wopen; > > This looks reasonable. > > I wonder which part of the code uses local named pipes. Is it > downstream in Git for Windows or one of the topics in flight? > > -- Hannes