On Wed, Mar 9, 2022 at 5:26 PM brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > On 2022-03-09 at 23:03:14, Neeraj Singh via GitGitGadget wrote: > > From: Neeraj Singh <neerajsi@xxxxxxxxxxxxx> > > > > Including NTSecAPI.h in git-compat-util.h causes build errors in any > > other file that includes winternl.h. That file was included in order to > > get access to the RtlGenRandom cryptographically secure PRNG. This > > change scopes the inclusion of all PRNG headers to just the wrapper.c > > file, which is the only place it is really needed. > > We generally prefer to do system includes in git-compat-util.h because > it allows us to paper over platform incompatibilities in one place and > to deal with the various ordering problems that can happen on certain > systems. > > It may be that Windows needs additional help here; I don't know, because > I don't use Windows. I personally find it unsavoury that Windows ships > with multiple incompatible header files like this, since such problems > are typically avoided by suitable include guards, whose utility has been > well known for several decades. However, if that's the case, let's move > only the Windows changes there, and leave the Unix systems, which lack > this problem, alone. > > It would also be helpful to explain the problem that Windows has in more > detail here, including any references to documentation that explains > this incompatibility, so those of us who are not Windows users can more > accurately reason about why we need to be so careful when including > header files there and why this is the best solution (and not, say, > providing our own include guards in a compat file). > -- > brian m. carlson (he/him or they/them) > Toronto, Ontario, CA I wasn't able to find any documentation from other people who hit this problem. The root cause is that NtSecAPI.h has a typedef like this: ``` #ifndef _NTDEF_ typedef LSA_UNICODE_STRING UNICODE_STRING, *PUNICODE_STRING; typedef LSA_STRING STRING, *PSTRING ; #endif ``` That's not really appropriate since NtSecAPI.h isn't the correct place to define this core primitive NT type. It should be including winternl.h or a similar header. I'll update the change to only move the Windows definition to the .c file.
