Hi brian, On Wed, 24 Oct 2018, brian m. carlson wrote:
> On Tue, Oct 23, 2018 at 03:23:21AM -0700, Karsten Blees via GitGitGadget > wrote: > > - if (!get_file_info_by_handle(fh, buf)) > > + case FILE_TYPE_CHAR: > > + case FILE_TYPE_PIPE: > > + /* initialize stat fields */ > > + memset(buf, 0, sizeof(*buf)); > > + buf->st_nlink = 1; > > + > > + if (type == FILE_TYPE_CHAR) { > > + buf->st_mode = _S_IFCHR; > > + } else { > > + buf->st_mode = _S_IFIFO; > > + if (PeekNamedPipe(fh, NULL, 0, NULL, &avail, NULL)) > > + buf->st_size = avail; > > These lines strike me as a bit odd. As far as I'm aware, Unix systems > don't return anything useful in this field when calling fstat on a pipe. > Is there a reason we fill this in on Windows? If so, could the commit > message explain what that is? AFAICT the idea was to imitate MSVCRT's fstat() in these cases. But a quick web search suggests that you are right: https://bugzilla.redhat.com/show_bug.cgi?id=58768#c4 (I could not find any official documentation talking about fstat() and pipes, but I trust Alan to know their stuff). Do note, please, that according to the issue described in that link, at least *some* glibc/Linux combinations behave in exactly the way this patch implements it. At this point, I am wary of changing this, too, as the code in question has been in production (read: tested thoroughly) in the current form for *years*, and I am really loathe to introduce a bug where even Windows-specific code in compat/ might rely on this behavior. (And no, I do not trust our test suite to find all of those use cases.) Ciao, Dscho