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
