[PATCH] winsock include fixes

[PATCH] winsock include fixes

[Ozkan Sezer]

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

Hi: Here is a patch fixing the winsock include problem
which appeared after the recent winsock header work in
mingw-w64-headers. Problem is this:  GDB does windows.h
and winsock2.h includes in its own headers and then it
includes them in its sources in an arbitrary order. If
winsock2.h or a header including winsock2.h (such as
gdb_select.h) is included after windows.h or a header
including windows.h (such as serial.h), then conflicting
definitions occur and they result in error, because windows.h
already includes winsock.h and including winsock2.h after
that is an error. The patch fixes that in a quick and
dirty way mostly by tweaking the include order. Tested
by compiling gdb for x86_64-w64-mingw32, i686-w64-mingw32
and x86_64-pc-linux-gnu. Please consider for applying.

--
Ozkan

PS:
A possibly better solution is defining WIN32_LEAN_AND_MEAN
before including windows.h and then manually including
the necessary additional headers in the sources, but that
may require some more work.

[-- Attachment #2: winsock_includes.patch --]
[-- Type: application/octet-stream, Size: 3749 bytes --]

Index: gdb/inflow.c
===================================================================
RCS file: /cvs/src/src/gdb/inflow.c,v
retrieving revision 1.58
diff -u -p -r1.58 inflow.c
--- gdb/inflow.c	24 Feb 2010 07:51:44 -0000	1.58
+++ gdb/inflow.c	26 Mar 2010 10:48:03 -0000
@@ -19,6 +19,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdb_select.h"
 #include "frame.h"
 #include "inferior.h"
 #include "command.h"
@@ -31,7 +32,6 @@
 #include "gdb_string.h"
 #include <signal.h>
 #include <fcntl.h>
-#include "gdb_select.h"
 
 #include "inflow.h"
 
Index: gdb/mingw-hdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mingw-hdep.c,v
retrieving revision 1.11
diff -u -p -r1.11 mingw-hdep.c
--- gdb/mingw-hdep.c	1 Jan 2010 07:31:37 -0000	1.11
+++ gdb/mingw-hdep.c	26 Mar 2010 10:48:03 -0000
@@ -18,11 +18,11 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdb_select.h"
 #include "serial.h"
 #include "event-loop.h"
 
 #include "gdb_assert.h"
-#include "gdb_select.h"
 #include "gdb_string.h"
 #include "readline/readline.h"
 
Index: gdb/ser-base.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-base.c,v
retrieving revision 1.15
diff -u -p -r1.15 ser-base.c
--- gdb/ser-base.c	1 Jan 2010 07:31:41 -0000	1.15
+++ gdb/ser-base.c	26 Mar 2010 10:48:03 -0000
@@ -19,6 +19,9 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#ifdef USE_WIN32API
+#include <winsock2.h>
+#endif
 #include "serial.h"
 #include "ser-base.h"
 #include "event-loop.h"
@@ -26,9 +29,6 @@
 #include "gdb_select.h"
 #include "gdb_string.h"
 #include <sys/time.h>
-#ifdef USE_WIN32API
-#include <winsock2.h>
-#endif
 
 
 static timer_handler_func push_event;
Index: gdb/ser-mingw.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-mingw.c,v
retrieving revision 1.20
diff -u -p -r1.20 ser-mingw.c
--- gdb/ser-mingw.c	1 Jan 2010 07:31:41 -0000	1.20
+++ gdb/ser-mingw.c	26 Mar 2010 10:48:03 -0000
@@ -18,6 +18,9 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#ifdef USE_WIN32API
+#include <winsock2.h>
+#endif
 #include "serial.h"
 #include "ser-base.h"
 #include "ser-tcp.h"
Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.33
diff -u -p -r1.33 ser-tcp.c
--- gdb/ser-tcp.c	1 Jan 2010 07:31:41 -0000	1.33
+++ gdb/ser-tcp.c	26 Mar 2010 10:48:03 -0000
@@ -19,6 +19,9 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#ifdef USE_WIN32API
+#include <winsock2.h>
+#endif
 #include "serial.h"
 #include "ser-base.h"
 #include "ser-tcp.h"
@@ -38,7 +41,6 @@
 #include <sys/time.h>
 
 #ifdef USE_WIN32API
-#include <winsock2.h>
 #define ETIMEDOUT WSAETIMEDOUT
 #define close(fd) closesocket (fd)
 #define ioctl ioctlsocket
Index: gdb/ser-unix.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-unix.c,v
retrieving revision 1.34
diff -u -p -r1.34 ser-unix.c
--- gdb/ser-unix.c	1 Jan 2010 07:31:41 -0000	1.34
+++ gdb/ser-unix.c	26 Mar 2010 10:48:03 -0000
@@ -19,6 +19,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdb_select.h"
 #include "serial.h"
 #include "ser-base.h"
 #include "ser-unix.h"
@@ -29,7 +30,6 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 
-#include "gdb_select.h"
 #include "gdb_string.h"
 #include "gdbcmd.h"
 

Re: [PATCH] winsock include fixes

[Kai Tietz]

2010/3/26 Ozkan Sezer <sezeroz@gmail.com>:
> Hi: Here is a patch fixing the winsock include problem
> which appeared after the recent winsock header work in
> mingw-w64-headers. Problem is this:  GDB does windows.h
> and winsock2.h includes in its own headers and then it
> includes them in its sources in an arbitrary order. If
> winsock2.h or a header including winsock2.h (such as
> gdb_select.h) is included after windows.h or a header
> including windows.h (such as serial.h), then conflicting
> definitions occur and they result in error, because windows.h
> already includes winsock.h and including winsock2.h after
> that is an error. The patch fixes that in a quick and
> dirty way mostly by tweaking the include order. Tested
> by compiling gdb for x86_64-w64-mingw32, i686-w64-mingw32
> and x86_64-pc-linux-gnu. Please consider for applying.
>
> --
> Ozkan

Ok, patch is ok. Ugly, but ok.

> PS:
> A possibly better solution is defining WIN32_LEAN_AND_MEAN
> before including windows.h and then manually including
> the necessary additional headers in the sources, but that
> may require some more work.

Long term we should clean it up in gdb. But I am pretty sure this
wrong order appears in other projects, too. So I think we should make
winsock2.h able to override definitions of winsock.h to simply allow
this. I dislike to priest and have malfunctional projects using our
runtime. It is correct that this behavior is default for VC, but it
hard to argument here, as we will always hear that things are working
by using w32api.

Kai


-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [PATCH] winsock include fixes

[Kai Tietz]

> Ok, patch is ok. Ugly, but ok.

Not to get misunderstood here. This was no approval. On gdb I can't
approve patches.

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Fri, Mar 26, 2010 at 1:15 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2010/3/26 Ozkan Sezer <sezeroz@gmail.com>:
>> Hi: Here is a patch fixing the winsock include problem
>> which appeared after the recent winsock header work in
>> mingw-w64-headers. Problem is this:  GDB does windows.h
>> and winsock2.h includes in its own headers and then it
>> includes them in its sources in an arbitrary order. If
>> winsock2.h or a header including winsock2.h (such as
>> gdb_select.h) is included after windows.h or a header
>> including windows.h (such as serial.h), then conflicting
>> definitions occur and they result in error, because windows.h
>> already includes winsock.h and including winsock2.h after
>> that is an error. The patch fixes that in a quick and
>> dirty way mostly by tweaking the include order. Tested
>> by compiling gdb for x86_64-w64-mingw32, i686-w64-mingw32
>> and x86_64-pc-linux-gnu. Please consider for applying.
>>
>> --
>> Ozkan
>
> Ok, patch is ok. Ugly, but ok.
>
>> PS:
>> A possibly better solution is defining WIN32_LEAN_AND_MEAN
>> before including windows.h and then manually including
>> the necessary additional headers in the sources, but that
>> may require some more work.
>
> Long term we should clean it up in gdb. But I am pretty sure this
> wrong order appears in other projects, too. So I think we should make
> winsock2.h able to override definitions of winsock.h to simply allow
> this. I dislike to priest and have malfunctional projects using our
> runtime. It is correct that this behavior is default for VC, but it
> hard to argument here, as we will always hear that things are working
> by using w32api.

It is the fault of w32api (cygwin/mingw.org) that winsock2.h
is included from within windows.h and not winsock.h. It
is the reason that many projects are doing this include
order wrongly, and we (mingw-w64) must not try to be
compatible with an incorrect implementation. W32api is
not windows itself and it is doing this winsock inclusion
wrong.

>
> Kai

Re: [PATCH] winsock include fixes

[Daniel Jacobowitz]

On Fri, Mar 26, 2010 at 12:54:10PM +0200, Ozkan Sezer wrote:
> Hi: Here is a patch fixing the winsock include problem
> which appeared after the recent winsock header work in
> mingw-w64-headers.

This patch isn't OK, because it fixes an inclusion order problem by
moving things around without a comment.  Everywhere else the
"gdb_XXX.h" headers are included in a group, last.  So someone's going
to clean up these files and reintroduce the problem.

It sounds like part of the problem is the inclusion of winsock2.h in
gdb_select.h, and windows.h in serial.h.  Should we include both
headers in both files to avoid the problem?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Fri, Mar 26, 2010 at 4:17 PM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Fri, Mar 26, 2010 at 12:54:10PM +0200, Ozkan Sezer wrote:
>> Hi: Here is a patch fixing the winsock include problem
>> which appeared after the recent winsock header work in
>> mingw-w64-headers.
>
> This patch isn't OK, because it fixes an inclusion order problem by
> moving things around without a comment.  Everywhere else the
> "gdb_XXX.h" headers are included in a group, last.  So someone's going
> to clean up these files and reintroduce the problem.
>
> It sounds like part of the problem is the inclusion of winsock2.h in
> gdb_select.h, and windows.h in serial.h.  Should we include both
> headers in both files to avoid the problem?
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

That may be a solution, too: Inclusion of winsock2.h
should be harmless in all cases when USE_WIN32API
is defined.

--
Ozkan

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Fri, Mar 26, 2010 at 4:47 PM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> On Fri, Mar 26, 2010 at 4:17 PM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
>> On Fri, Mar 26, 2010 at 12:54:10PM +0200, Ozkan Sezer wrote:
>>> Hi: Here is a patch fixing the winsock include problem
>>> which appeared after the recent winsock header work in
>>> mingw-w64-headers.
>>
>> This patch isn't OK, because it fixes an inclusion order problem by
>> moving things around without a comment.  Everywhere else the
>> "gdb_XXX.h" headers are included in a group, last.  So someone's going
>> to clean up these files and reintroduce the problem.
>>
>> It sounds like part of the problem is the inclusion of winsock2.h in
>> gdb_select.h, and windows.h in serial.h.  Should we include both
>> headers in both files to avoid the problem?
>>
>> --
>> Daniel Jacobowitz
>> CodeSourcery
>>
>
> That may be a solution, too: Inclusion of winsock2.h
> should be harmless in all cases when USE_WIN32API
> is defined.
>
> --
> Ozkan
>

Indeed, adding winsock2.h include to serial.h does fix the
problems for mingw-w64, like the following:

Index: gdb/serial.h
===================================================================
RCS file: /cvs/src/src/gdb/serial.h,v
retrieving revision 1.21
diff -u -p -r1.21 serial.h
--- gdb/serial.h	1 Jan 2010 07:31:41 -0000	1.21
+++ gdb/serial.h	26 Mar 2010 15:21:45 -0000
@@ -21,6 +21,7 @@
 #define SERIAL_H

 #ifdef USE_WIN32API
+#include <winsock2.h>
 #include <windows.h>
 #endif

Compile tested for x86_64-w64-mingw32 and i686-w64-mingw32.

--
Ozkan

Re: [PATCH] winsock include fixes

[Daniel Jacobowitz]

On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
> Indeed, adding winsock2.h include to serial.h does fix the
> problems for mingw-w64, like the following:

This change is OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Fri, Mar 26, 2010 at 5:33 PM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
>> Indeed, adding winsock2.h include to serial.h does fix the
>> problems for mingw-w64, like the following:
>
> This change is OK.
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

I don't have write access, so please apply as you will.
Thanks.

--
Ozkan

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Fri, Mar 26, 2010 at 5:33 PM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
>> Indeed, adding winsock2.h include to serial.h does fix the
>> problems for mingw-w64, like the following:
>
> This change is OK.
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

If someone can please apply it, I'd be grateful (I
don't have cvs write access). Thanks.

--
Ozkan

Re: [PATCH] winsock include fixes

[Christopher Faylor]

On Fri, Mar 26, 2010 at 11:33:02AM -0400, Daniel Jacobowitz wrote:
>On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
>> Indeed, adding winsock2.h include to serial.h does fix the
>> problems for mingw-w64, like the following:
>
>This change is OK.

Isn't this really a problem with mingw-w64 rather than a gdb
problem?  It seems like this should be fixed there.

cgf

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Sun, Mar 28, 2010 at 9:43 PM, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Fri, Mar 26, 2010 at 11:33:02AM -0400, Daniel Jacobowitz wrote:
>>On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
>>> Indeed, adding winsock2.h include to serial.h does fix the
>>> problems for mingw-w64, like the following:
>>
>>This change is OK.
>
> Isn't this really a problem with mingw-w64 rather than a gdb
> problem?  It seems like this should be fixed there.
>
> cgf
>

I posted the fix to gdb, because I believe that it is a gdb problem.
Can you explain how is it really supposed to be a mingw-w64
problem?

--
Ozkan

Re: [PATCH] winsock include fixes

[Christopher Faylor]

On Sun, Mar 28, 2010 at 09:48:04PM +0300, Ozkan Sezer wrote:
>On Sun, Mar 28, 2010 at 9:43 PM, Christopher Faylor wrote:
>> On Fri, Mar 26, 2010 at 11:33:02AM -0400, Daniel Jacobowitz wrote:
>>>On Fri, Mar 26, 2010 at 05:24:23PM +0200, Ozkan Sezer wrote:
>>>> Indeed, adding winsock2.h include to serial.h does fix the
>>>> problems for mingw-w64, like the following:
>>>
>>>This change is OK.
>>
>> Isn't this really a problem with mingw-w64 rather than a gdb
>> problem? ?It seems like this should be fixed there.
>
>I posted the fix to gdb, because I believe that it is a gdb problem.
>Can you explain how is it really supposed to be a mingw-w64 problem?

Actually, nevermind.  I've just googled this and it seems like including
winsock2.h first is the accepted solution even for regular Visual C.

cgf

Re: [PATCH] winsock include fixes

[Tom Tromey]

>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:

Ozkan> If someone can please apply it, I'd be grateful (I
Ozkan> don't have cvs write access). Thanks.

I've checked this in.

I wrote a ChangeLog entry; but in the future please include one in the
patch.  Thanks.

Tom

Re: [PATCH] winsock include fixes

[Ozkan Sezer]

On Tue, Mar 30, 2010 at 9:08 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> If someone can please apply it, I'd be grateful (I
> Ozkan> don't have cvs write access). Thanks.
>
> I've checked this in.
>
> I wrote a ChangeLog entry; but in the future please include one in the
> patch.  Thanks.
>
> Tom
>

Sorry that I forgot to add a ChangeLog. Thanks for applying it.

--
Ozkan