On Wed, Feb 24, 2021 at 01:23:51PM -0500, Jeff King wrote:
> > I'm not so much opposed to "enable it all the time" in general, but when
> > we already have an override of open(), like for the Windows case in
> > compat/mingw.h, I find it a bit rough to put another wrapper around it,
> > even more so since we won't have the EINTR problem on Windows due to the
> > absence of signals.
>
> That's fair. I don't think my new wrapper would interact well with
> mingw_open(). They are both trying to #define open. I think since mine
> comes later in git-compat-util.h, it is probably overriding mingw_open()
> completely on Windows, which is quite bad (we call into my function in
> wrapper.c, but then its "#undef open" means we get the original open(),
> not the previously defined wrapper).
Thanks again for pointing out mingw_open(). The v2 I posted should avoid
any bad interactions there, even if we do end up enabling it all the
time.
By the way, I did notice something funny when looking at mingw_open().
It unconditionally does:
va_start(args, oflags);
mode = va_arg(args, int);
va_end(args);
no matter what we see in oflags. But callers who are not passing O_CREAT
will not provide a third argument at all. So probably it is loading
random trash from the stack and passing it around in the "mode"
variable. This doesn't hurt anything, because ultimately we pass the
trash along to the real open function, which will ignore it without
O_CREAT. But it is definitely undefined behavior to call va_arg() at all
in this instance.
-Peff
