Fix compilation using mingw.org's MinGW

  > Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 Apr 2019 12:33:37 +0100
> 
> > I will try doing this in common-defs.h, but for the branch, we could
> > set _WIN32_WINNT only in windows-nat.c, as that's the only file that
> > currently cares, which should be safer.  WDYT?
> 
> Sounds good to me.

Below please find 2 patches: the first one for the 8.3 branch, the
second one for master.  OK to commit (after writing ChangeLog
entries)?
  

On 4/28/19 3:33 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 19 Apr 2019 12:33:37 +0100
>>
>>> I will try doing this in common-defs.h, but for the branch, we could
>>> set _WIN32_WINNT only in windows-nat.c, as that's the only file that
>>> currently cares, which should be safer.  WDYT?
>>
>> Sounds good to me.
> 
> Below please find 2 patches: the first one for the 8.3 branch, the
> second one for master.  OK to commit (after writing ChangeLog
> entries)?

Sure, LGTM.

A few remarks:

 - The override in _WIN32_WINNT is no longer necessary.  It could
   be removed at the same time.

 - IIRC, Cygwin uses the same w32api headers as mingw.org.  I don't know
   whether Cygwin sets _WIN32_WINNT to a higher number already; we
   haven't heard any complaints, so I guess it does.

 - We could remove the dynamic loading of GetConsoleFontSize & friends
   now, right?  (for a separate patch/rainy day, not in this patch.)

Thanks,
Pedro Alves
  

On 30.04.2019 15:56, Pedro Alves wrote:
>  - IIRC, Cygwin uses the same w32api headers as mingw.org.  I don't know
>    whether Cygwin sets _WIN32_WINNT to a higher number already; we
>    haven't heard any complaints, so I guess it does.
> 

Cygwin uses mingw-w64.
  

On 4/30/19 1:56 PM, Pedro Alves wrote:
> Sure, LGTM.
> 
> A few remarks:
> 
>  - The override in _WIN32_WINNT is no longer necessary.  It could
>    be removed at the same time.

I meant the _WIN32_WINNT override in common/netstuff.c.

I see now that there are more of those in other files too.

Thanks,
Pedro Alves
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 14:10:53 +0100
> 
> >  - The override in _WIN32_WINNT is no longer necessary.  It could
> >    be removed at the same time.
> 
> I meant the _WIN32_WINNT override in common/netstuff.c.
> 
> I see now that there are more of those in other files too.

I will review them for the changeset on master, but I don't think we
should change them on the 8.3 branch.  OK?

Thanks for the review.
  

> From: LRN <lrn1986@gmail.com>
> Date: Tue, 30 Apr 2019 16:05:55 +0300
> 
> >  - IIRC, Cygwin uses the same w32api headers as mingw.org.  I don't know
> >    whether Cygwin sets _WIN32_WINNT to a higher number already; we
> >    haven't heard any complaints, so I guess it does.
> > 
> 
> Cygwin uses mingw-w64.

Maybe I'm missing something, but I don't think this is relevant.
_WIN32_WINNT is not a MinGW invention, it's an official symbol in
Windows headers, so all Windows compilers need to support it in the
same way and with the same semantics.  The only difference is the
default value.

Am I missing something?
  

On 4/30/19 4:23 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 30 Apr 2019 14:10:53 +0100
>>
>>>  - The override in _WIN32_WINNT is no longer necessary.  It could
>>>    be removed at the same time.
>>
>> I meant the _WIN32_WINNT override in common/netstuff.c.
>>
>> I see now that there are more of those in other files too.
> 
> I will review them for the changeset on master, but I don't think we
> should change them on the 8.3 branch.  OK?

Yes, my comments were for master, since changing removing the overrides
in the other files only makes sense with the _WIN32_WINNT definition
in common-defs.h.

Thanks,
Pedro Alves
  

On 4/30/19 4:25 PM, Eli Zaretskii wrote:
>> From: LRN <lrn1986@gmail.com>
>> Date: Tue, 30 Apr 2019 16:05:55 +0300
>>
>>>  - IIRC, Cygwin uses the same w32api headers as mingw.org.  I don't know
>>>    whether Cygwin sets _WIN32_WINNT to a higher number already; we
>>>    haven't heard any complaints, so I guess it does.
>>>
>>
>> Cygwin uses mingw-w64.
> 
> Maybe I'm missing something, but I don't think this is relevant.
> _WIN32_WINNT is not a MinGW invention, it's an official symbol in
> Windows headers, so all Windows compilers need to support it in the
> same way and with the same semantics.  The only difference is the
> default value.
> 
> Am I missing something?
> 

The issue is where is that default set?

On my Fedora mingw-w64 cross, it is not set by default by the
compiler:

 $ x86_64-w64-mingw32-gcc -x c /dev/null -dM -E | grep _WIN32_WINNT
 $ (empty)

It seems to be set instead in the headers, in include/sdkddkver.h.

I guess that if Cygwin indeed uses mingw-w64 headers nowadays, it won't
need the fix as long as the headers default to a higher version than
gdb requires.  Pedantically, I think we should tweak
the "#ifdef __MINGW32__" to consider Cygwin too, since the Cygwin ports
also pulls in w32api headers, but if it doesn't matter in practice, and
it's not convenient to test, then we can simply forget about it until
a time when someone notices something odd.

Thanks,
Pedro Alves
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 18:03:57 +0100
> 
> The issue is where is that default set?

On some internal header file (it differs between various flavors of
MinGW).  But no matter where it is set, it must be defined after _any_
standard header is included, so in practice I think it's defined at
the place where the patch tests for it.

In any case, the only platform which really needs this is mingw.org's
MinGW, where I actually tested this assumption.  The other two,
MinGW64 and Cygwin, don't support older platforms (they actually don't
support XP anymore, only Vista and onward), so their default values
are higher than 0x0501 anyway.
  

On 4/30/19 6:17 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 30 Apr 2019 18:03:57 +0100
>>
>> The issue is where is that default set?
> 
> On some internal header file (it differs between various flavors of
> MinGW).  

Right, which is what I said.

> But no matter where it is set, it must be defined after _any_
> standard header is included, so in practice I think it's defined at
> the place where the patch tests for it.

I think you mean "before".  But I did not say that this was the wrong
place (since I was the one that suggested the place).  Only that
pedantically the new code could/should be tweaked like this:

- #ifdef __MINGW32__
+ #if defined (__MINGW32__) || defined (__CYGWIN__)
 # ifdef _WIN32_WINNT
 #  if _WIN32_WINNT < 0x0501
 #   undef _WIN32_WINNT
 #   define _WIN32_WINNT 0x0501
 #  endif
 # else
 #  define _WIN32_WINNT 0x0501
 # endif
 #endif	/* __MINGW32__ */

> 
> In any case, the only platform which really needs this is mingw.org's
> MinGW, where I actually tested this assumption.  The other two,
> MinGW64 and Cygwin, don't support older platforms (they actually don't
> support XP anymore, only Vista and onward), so their default values
> are higher than 0x0501 anyway.
> 

Right, like I said.

Thanks,
Pedro Alves
  

> Cc: lrn1986@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 18:26:19 +0100
> 
> On 4/30/19 6:17 PM, Eli Zaretskii wrote:
> >> Cc: gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Tue, 30 Apr 2019 18:03:57 +0100
> >>
> >> The issue is where is that default set?
> > 
> > On some internal header file (it differs between various flavors of
> > MinGW).  
> 
> Right, which is what I said.

Yes.

> > But no matter where it is set, it must be defined after _any_
> > standard header is included, so in practice I think it's defined at
> > the place where the patch tests for it.
> 
> I think you mean "before".

No, I meant "after".  The default value is set once you included at
least on standard header.  Hence you can at that place test for
whether it is defined and what is its default value.  Overriding that
default is generally important only before including w32api headers,
such as windows.h.

> But I did not say that this was the wrong
> place (since I was the one that suggested the place).  Only that
> pedantically the new code could/should be tweaked like this:
> 
> - #ifdef __MINGW32__
> + #if defined (__MINGW32__) || defined (__CYGWIN__)

If you want me to add __CYGWIN__, I'm okay with that.

> > In any case, the only platform which really needs this is mingw.org's
> > MinGW, where I actually tested this assumption.  The other two,
> > MinGW64 and Cygwin, don't support older platforms (they actually don't
> > support XP anymore, only Vista and onward), so their default values
> > are higher than 0x0501 anyway.
> > 
> 
> Right, like I said.

Sure, we agree.  I didn't mean to contradict what you were saying, I
wanted to back that up.
  

On 30.04.2019 18:25, Eli Zaretskii wrote:
>> From: LRN
>> Date: Tue, 30 Apr 2019 16:05:55 +0300
>>
>>>  - IIRC, Cygwin uses the same w32api headers as mingw.org.  I don't know
>>>    whether Cygwin sets _WIN32_WINNT to a higher number already; we
>>>    haven't heard any complaints, so I guess it does.
>>>
>>
>> Cygwin uses mingw-w64.
> 
> Maybe I'm missing something, but I don't think this is relevant.
> _WIN32_WINNT is not a MinGW invention, it's an official symbol in
> Windows headers, so all Windows compilers need to support it in the
> same way and with the same semantics.  The only difference is the
> default value.
> 
> Am I missing something?
> 

I replied to the "Cygwin uses the same w32api headers as mingw.org" part only.
  

On 4/30/19 6:40 PM, Eli Zaretskii wrote:

>>> But no matter where it is set, it must be defined after _any_
>>> standard header is included, so in practice I think it's defined at
>>> the place where the patch tests for it.
>> I think you mean "before".
> No, I meant "after".  The default value is set once you included at
> least on standard header.  Hence you can at that place test for
> whether it is defined and what is its default value.  

Sorry, but no, there's no need to wait until there's a default to
override it.

In fact, your patch is doing exactly that.  common-defs.h is garanteed
to be included before any started header.  That's exactly why I suggested
that spot.

That's because the default is only defined if not already defined:

 /* Select Default WIN32_WINNT Value */
 #if !defined(_WIN32_WINNT) && !defined(_CHICAGO_)
 #define _WIN32_WINNT    _WIN32_WINNT_WS03
 #endif

This family of macros is usually defined on the command line / Makefile
on Windows-only projects, even:

  $CC -D_WIN32_WINNT=xxx

That's what MSVC/Visual Studio does.  Or at least, used to last I used it,
years ago.

I did not suggest doing that because it's not as convenient on portable
projects that don't just target Windows.

> Overriding that
> default is generally important only before including w32api headers,
> such as windows.h.

Agreed.  But defining it really early just makes sure it's consistently
set.

Thanks,
Pedro Alves
  

> Cc: lrn1986@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 18:58:42 +0100
> 
> In fact, your patch is doing exactly that.  common-defs.h is garanteed
> to be included before any started header.  That's exactly why I suggested
> that spot.

You were talking about the patch for master, whereas I was talking
about the patch for the release branch.
  

On 4/30/19 7:34 PM, Eli Zaretskii wrote:
>> Cc: lrn1986@gmail.com, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 30 Apr 2019 18:58:42 +0100
>>
>> In fact, your patch is doing exactly that.  common-defs.h is garanteed
>> to be included before any started header.  That's exactly why I suggested
>> that spot.
> 
> You were talking about the patch for master, whereas I was talking
> about the patch for the release branch.

Ah, indeed it seemed like we were kind of talking past each other.

Still, my remarks apply likewise (we don't _have_ to define after
the standard headers, before is fine and is the standard practice).
But what I had in the 8.3 patch is fine as is, TBC.

Thanks,
Pedro Alves
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 17:31:22 +0100
> 
> On 4/30/19 4:23 PM, Eli Zaretskii wrote:
> >> Cc: gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Tue, 30 Apr 2019 14:10:53 +0100
> >>
> >>>  - The override in _WIN32_WINNT is no longer necessary.  It could
> >>>    be removed at the same time.
> >>
> >> I meant the _WIN32_WINNT override in common/netstuff.c.
> >>
> >> I see now that there are more of those in other files too.
> > 
> > I will review them for the changeset on master, but I don't think we
> > should change them on the 8.3 branch.  OK?
> 
> Yes, my comments were for master, since changing removing the overrides
> in the other files only makes sense with the _WIN32_WINNT definition
> in common-defs.h.

OK, done.  I think I removed all the overrides that were there.

Thanks.
  

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 30 Apr 2019 13:56:52 +0100
> 
>  - We could remove the dynamic loading of GetConsoleFontSize & friends
>    now, right?  (for a separate patch/rainy day, not in this patch.)

I will look at all the functions we load dynamically and see which
could now be assumed available statically.