On Tue, Sep 27, 2022 at 2:19 PM Andres Freund <andres@anarazel.de> wrote:
> Subject: [PATCH v17 15/23] windows: Set UMDF_USING_NTSTATUS globally, include ntstatus.h
No Windows expertise here, but this looks reasonable. I originally
tried to contain UMDF_USING_NTSTATUS to small translation units for
fear of unintended consequences, but PCH requires you not to mess with
macros that affect the compilation of a header as seen by different
translation units, which is an incompatible goal. If this is passing
on MSVC and MingGW then +1 from me.
You mentioned WIN32_NO_STATUS in the commit message -- a mistake?
Digging out my old emails/notes... that's another way to be allowed to
include both ntstatus.h and windows.h/etc in the same translation
unit, but not the one we're using. I assume it's worse because you
have to define it and then undefine it, which sounds more antithetical
to the PCH dream. Admittedly UMDF_USING_NTSTATUS -- from the
"User-Mode Driver Framework" -- is a weird thing to be getting tangled
up with because we aren't writing a driver here, but it seems to be a
well known and widely used alternative, and is nicer because you only
have to define it.
> Subject: [PATCH v17 16/23] windows: adjust FD_SETSIZE via commandline define
Right, we have to fix that across translation units for the same
reason. But why as -D and not in win32_port.h? I followed the
discussion from 9acda73118 to try to find the answer to that and saw
that Michael wanted to put it there, but wanted to minimise the blast
radius at the time:
https://www.postgresql.org/message-id/20190826054000.GE7005%40paquier.xyz
