Re: [PATCH 2/3] mingw: replace MSVCRT's fstat() with a Win32-based implementation
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Oct 24, 2018, 22:40 UTC
- Message-ID
- <20181024224047.GF6119@genre.crustytoothpaste.net>
- In-Reply-To
- <nycvar.QRO.7.76.6.1810240927520.4546@tvgsbejvaqbjf.bet>
On Wed, Oct 24, 2018 at 09:37:43AM +0200, Johannes Schindelin wrote:
Show 14 quoted lines
> Hi brian, > > On Wed, 24 Oct 2018, brian m. carlson wrote: > > 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).
Yeah, that behavior is quite old. I'm surprised that Linux ever did that.
Show 9 quoted lines
> 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.)
I don't feel strongly either way. I feel confident the rest of Git doesn't use that field, so I don't see any downsides to keeping it other than the slight overhead of populating it. I just thought I'd ask in case there was something important I was missing.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204