Fix compilation using mingw.org's MinGW

Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

windows-nat.c uses CONSOLE_FONT_INFO unconditionally, but the
corresponding declaration in the Windows API headers is conditional,
because the APIs using CONSOLE_FONT_INFO are only available since
Windows XP.  When that condition doesn't hold, windows-nat.c won't
compile:

     windows-nat.c:114:10: error: 'CONSOLE_FONT_INFO' has not been declared
	       CONSOLE_FONT_INFO *);
	       ^~~~~~~~~~~~~~~~~
     windows-nat.c: In function 'void windows_set_console_info(STARTUPINFOA*, DWORD*)':
     windows-nat.c:2174:7: error: 'CONSOLE_FONT_INFO' was not declared in this scope
	    CONSOLE_FONT_INFO cfi;
	    ^~~~~~~~~~~~~~~~~
     windows-nat.c:2174:7: note: suggested alternative: 'CONSOLE_CURSOR_INFO'
	    CONSOLE_FONT_INFO cfi;
	    ^~~~~~~~~~~~~~~~~
	    CONSOLE_CURSOR_INFO
     windows-nat.c:2176:48: error: 'cfi' was not declared in this scope
	    GetCurrentConsoleFont (hconsole, FALSE, &cfi);
						     ^~~
     windows-nat.c: At global scope:
     windows-nat.c:3302:55: error: 'CONSOLE_FONT_INFO' has not been declared
      bad_GetCurrentConsoleFont (HANDLE w, BOOL bMaxWindow, CONSOLE_FONT_INFO *f)
							    ^~~~~~~~~~~~~~~~~
     windows-nat.c: In function 'BOOL bad_GetCurrentConsoleFont(HANDLE, BOOL, int*)':

     windows-nat.c:3304:6: error: request for member 'nFont' in '* f', which is of non-class type 'int'
	f->nFont = 0;
	   ^~~~~

MinGW64 no longer supports versions of Windows older than XP, but
mingw.org's MinGW does.  Since we load GetCurrentConsoleFont
dynamically at run time, and have code for when the load fails, it
makes sense to not assume XP at compile time.  The patch below does
that.  OK to push (with the pertinent ChangeLog entry)?

--- gdb/windows-nat.c~0	2019-03-27 00:52:05.000000000 +0200
+++ gdb/windows-nat.c	2019-04-18 11:37:02.061945600 +0300
@@ -81,6 +81,16 @@
 #define GetConsoleFontSize		dyn_GetConsoleFontSize
 #define GetCurrentConsoleFont		dyn_GetCurrentConsoleFont
 
+#if _WIN32_WINNT < 0x0501
+
+typedef
+struct _CONSOLE_FONT_INFO
+{ DWORD 			nFont;
+  COORD 			dwFontSize;
+} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO;
+
+#endif
+
 typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
 						   PTOKEN_PRIVILEGES,
 						   DWORD, PTOKEN_PRIVILEGES,

Re: Fix compilation using mingw.org's MinGW

[Kevin Buettner]

On Thu, 18 Apr 2019 18:36:11 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> --- gdb/windows-nat.c~0	2019-03-27 00:52:05.000000000 +0200
> +++ gdb/windows-nat.c	2019-04-18 11:37:02.061945600 +0300
> @@ -81,6 +81,16 @@
>  #define GetConsoleFontSize		dyn_GetConsoleFontSize
>  #define GetCurrentConsoleFont		dyn_GetCurrentConsoleFont
>  
> +#if _WIN32_WINNT < 0x0501
> +
> +typedef
> +struct _CONSOLE_FONT_INFO
> +{ DWORD 			nFont;
> +  COORD 			dwFontSize;
> +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO;
> +
> +#endif
> +
>  typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
>  						   PTOKEN_PRIVILEGES,
>  						   DWORD, PTOKEN_PRIVILEGES,

I had expected to see a typedef for struct CONSOLE_FONT_INFO, but
instead see one for _CONSOLE_FONT_INFO (where the difference is
the leading underscore.)

I'm guessing that there's some magic in Windows specific header files
which makes this work, but I just want to check to make sure that this
wasn't a typo on your part...

Kevin

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> Date: Thu, 18 Apr 2019 10:00:58 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > +#if _WIN32_WINNT < 0x0501
> > +
> > +typedef
> > +struct _CONSOLE_FONT_INFO
> > +{ DWORD 			nFont;
> > +  COORD 			dwFontSize;
> > +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO;
> > +
> > +#endif
> > +
> >  typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
> >  						   PTOKEN_PRIVILEGES,
> >  						   DWORD, PTOKEN_PRIVILEGES,
> 
> I had expected to see a typedef for struct CONSOLE_FONT_INFO, but
> instead see one for _CONSOLE_FONT_INFO (where the difference is
> the leading underscore.)

The struct is with the leading underscore, the typedef isn't.

> I'm guessing that there's some magic in Windows specific header files
> which makes this work, but I just want to check to make sure that this
> wasn't a typo on your part...

It wasn't a typo, and there is no magic ;-)

Re: Fix compilation using mingw.org's MinGW

[Kevin Buettner]

On Thu, 18 Apr 2019 21:54:05 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Thu, 18 Apr 2019 10:00:58 -0700
> > From: Kevin Buettner <kevinb@redhat.com>
> > Cc: gdb-patches@sourceware.org
> >   
> > > +#if _WIN32_WINNT < 0x0501
> > > +
> > > +typedef
> > > +struct _CONSOLE_FONT_INFO
> > > +{ DWORD 			nFont;
> > > +  COORD 			dwFontSize;
> > > +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO;
> > > +
> > > +#endif
> > > +
> > >  typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
> > >  						   PTOKEN_PRIVILEGES,
> > >  						   DWORD, PTOKEN_PRIVILEGES,  
> > 
> > I had expected to see a typedef for struct CONSOLE_FONT_INFO, but
> > instead see one for _CONSOLE_FONT_INFO (where the difference is
> > the leading underscore.)  
> 
> The struct is with the leading underscore, the typedef isn't.
> 
> > I'm guessing that there's some magic in Windows specific header files
> > which makes this work, but I just want to check to make sure that this
> > wasn't a typo on your part...  
> 
> It wasn't a typo, and there is no magic ;-)

Apologies - I clearly wasn't awake enough when I looked at your patch.

So... no objections from me regarding this patch.

Kevin

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> Date: Thu, 18 Apr 2019 13:38:46 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > It wasn't a typo, and there is no magic ;-)
> 
> Apologies - I clearly wasn't awake enough when I looked at your patch.

No sweat.  Thanks for reviewing the patch.

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

On 4/18/19 4:36 PM, Eli Zaretskii wrote:

> MinGW64 no longer supports versions of Windows older than XP, but
> mingw.org's MinGW does.  Since we load GetCurrentConsoleFont
> dynamically at run time, and have code for when the load fails, it
> makes sense to not assume XP at compile time.  The patch below does
> that.  OK to push (with the pertinent ChangeLog entry)?

Didn't we drop support for < XP some time ago?

Thanks,
Pedro Alves

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

On 4/18/19 6:20 PM, Pedro Alves wrote:
> On 4/18/19 4:36 PM, Eli Zaretskii wrote:
> 
>> MinGW64 no longer supports versions of Windows older than XP, but
>> mingw.org's MinGW does.  Since we load GetCurrentConsoleFont
>> dynamically at run time, and have code for when the load fails, it
>> makes sense to not assume XP at compile time.  The patch below does
>> that.  OK to push (with the pertinent ChangeLog entry)?
> 
> Didn't we drop support for < XP some time ago?
> 

From gdb/NEWS:

~~~~~~~~~~~~
* Removed targets

GDB no longer supports native debugging on versions of MS-Windows
before Windows XP.
~~~~~~~~~~~~

So shouldn't we instead be setting _WIN32_WINNT to some
appropriate number?

Thanks,
Pedro Alves

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 18 Apr 2019 18:22:14 +0100
> 
> * Removed targets
> 
> GDB no longer supports native debugging on versions of MS-Windows
> before Windows XP.

I was talking about compile time, not run time.

In any case, if we don't support systems older than XP, then why do we
load those functions dynamically at run time and call them via a
function pointer?

> So shouldn't we instead be setting _WIN32_WINNT to some
> appropriate number?

I don't mind, but where?  And also: should we make such changes on the
8.3 branch at this time?

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

On 4/18/19 7:56 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 18 Apr 2019 18:22:14 +0100
>>
>> * Removed targets
>>
>> GDB no longer supports native debugging on versions of MS-Windows
>> before Windows XP.
> 
> I was talking about compile time, not run time.

Not sure it makes any practical difference.  From the text above, you could
claim that we wanted to support hosting GDB on older versions of Windows,
but not support native debugging, say, so you could still remote debug
from such older Windows boxes.  But was that the intention?  I'd say
keep it simple -- if we don't support native debugging, then we don't support
building at all either.  The subject for the commit that added that NEWS entry
sound like that was the intention:

 commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d
 Author:     Eli Zaretskii <eliz@gnu.org>
 AuthorDate: Sat Mar 2 15:18:32 2019 +0200

     GDB no longer supports Windows before XP.

But surprisingly, I can't find the discussion behind this commit
in the archives to see the context and what was decided.

> 
> In any case, if we don't support systems older than XP, then why do we
> load those functions dynamically at run time and call them via a
> function pointer?

Because that code is older than the decision to stop supporting older
Windows versions, surely? :

 ChangeLog-2010: * windows-nat.c (GetConsoleFontSize, GetCurrentConsoleFont):
 ChangeLog-2010: (bad_GetCurrentConsoleFont, bad_GetConsoleFontSize): New functions.
 ChangeLog-2010: GetCurrentConsoleFont.
 ChangeLog-2015: (GetCurrentConsoleFont_ftype, GetModuleInformation_ftype)

> 
>> So shouldn't we instead be setting _WIN32_WINNT to some
>> appropriate number?
> 
> I don't mind, but where?

I'd do it in common/common-defs.h, before any #include, where we define
other macros that must be defined before any include, like 
__STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc.

> And also: should we make such changes on the
> 8.3 branch at this time?

Not sure.  Off hand I'd think it's pretty safe, but maybe for 8.3
your patch is safer.

Thanks,
Pedro Alves

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 Apr 2019 11:51:46 +0100
> 
> building at all either.  The subject for the commit that added that NEWS entry
> sound like that was the intention:
> 
>  commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d
>  Author:     Eli Zaretskii <eliz@gnu.org>
>  AuthorDate: Sat Mar 2 15:18:32 2019 +0200
> 
>      GDB no longer supports Windows before XP.
> 
> But surprisingly, I can't find the discussion behind this commit
> in the archives to see the context and what was decided.

The discussion which led to that change is here:

  https://www.sourceware.org/ml/gdb-patches/2019-02/msg00574.html

> >> So shouldn't we instead be setting _WIN32_WINNT to some
> >> appropriate number?
> > 
> > I don't mind, but where?
> 
> I'd do it in common/common-defs.h, before any #include, where we define
> other macros that must be defined before any include, like 
> __STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc.
> 
> > And also: should we make such changes on the
> > 8.3 branch at this time?
> 
> Not sure.  Off hand I'd think it's pretty safe, but maybe for 8.3
> your patch is safer.

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?

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

On 4/19/19 12:23 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 19 Apr 2019 11:51:46 +0100
>>
>> building at all either.  The subject for the commit that added that NEWS entry
>> sound like that was the intention:
>>
>>  commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d
>>  Author:     Eli Zaretskii <eliz@gnu.org>
>>  AuthorDate: Sat Mar 2 15:18:32 2019 +0200
>>
>>      GDB no longer supports Windows before XP.
>>
>> But surprisingly, I can't find the discussion behind this commit
>> in the archives to see the context and what was decided.
> 
> The discussion which led to that change is here:
> 
>   https://www.sourceware.org/ml/gdb-patches/2019-02/msg00574.html
> 

So we're already assuming XP at build time in common code, in
common/netstuff.c.

>>>> So shouldn't we instead be setting _WIN32_WINNT to some
>>>> appropriate number?
>>>
>>> I don't mind, but where?
>>
>> I'd do it in common/common-defs.h, before any #include, where we define
>> other macros that must be defined before any include, like 
>> __STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc.
>>
>>> And also: should we make such changes on the
>>> 8.3 branch at this time?
>>
>> Not sure.  Off hand I'd think it's pretty safe, but maybe for 8.3
>> your patch is safer.
> 
> 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.

Thanks,
Pedro Alves

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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)?

--- gdb/windows-nat.c~0	2019-03-27 00:52:05.000000000 +0200
+++ gdb/windows-nat.c	2019-04-28 11:00:38.415049400 +0300
@@ -34,6 +34,15 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <fcntl.h>
+/* We need at least the level of XP for CONSOLE_FONT_INFO.  */
+#ifdef _WIN32_WINNT
+# if _WIN32_WINNT < 0x0501
+#  undef _WIN32_WINNT
+#  define _WIN32_WINNT 0x0501
+# endif
+#else
+# define _WIN32_WINNT 0x0501
+#endif
 #include <windows.h>
 #include <imagehlp.h>
 #include <psapi.h>


--- gdb/common/common-defs.h~0	2019-03-27 00:52:05.000000000 +0200
+++ gdb/common/common-defs.h	2019-04-28 11:26:11.785455800 +0300
@@ -72,6 +72,20 @@
 #define _FORTIFY_SOURCE 2
 #endif
 
+/* We don't support Windows versions before XP, so we define
+   _WIN32_WINNT correspondingly to ensure the Windows API headers
+   expose the required symbols.  */
+#ifdef __MINGW32__
+# ifdef _WIN32_WINNT
+#  if _WIN32_WINNT < 0x0501
+#   undef _WIN32_WINNT
+#   define _WIN32_WINNT 0x0501
+#  endif
+# else
+#  define _WIN32_WINNT 0x0501
+# endif
+#endif	/* __MINGW32__ */
+
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[LRN]

[-- Attachment #1.1: Type: text/plain, Size: 275 bytes --]

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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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?

Re: Fix compilation using mingw.org's MinGW

[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

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[LRN]

[-- Attachment #1.1: Type: text/plain, Size: 765 bytes --]

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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.

Re: Fix compilation using mingw.org's MinGW

[Pedro Alves]

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

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.

Re: Fix compilation using mingw.org's MinGW

[Eli Zaretskii]

> 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.