#34 Failure building libada, conflicting types in winsock2.h

closed
5
2010-04-18
2010-04-06
No

For the current 1.0 branch rev. 2146 I get failures while building libada, build log attached. This used to work in the past.
Crossbuild x86_64_unknown-linux-gnu to i686-w64-mingw32, x86_64-pc-mingw32 and x86_64-w64-mingw32 all fail l the same way for gcc-4.4.3 and gcc-4.4.4.
All failures are of the same type, redefinition of something in winsock2.h previously declared in winsock.h

Rainer

Discussion

1 2 > >> (Page 1 of 2)
  • Rainer Emrich

    Rainer Emrich - 2010-04-06
     
  • Kai Tietz

    Kai Tietz - 2010-04-06

    Hello Rainer,

    this issue is reasoned by libada's wrong include order of <windows.h> and <winsock2.h>. As you can find easily by google'ing for this issue, <winsock2.h> has to be include before <winsock2.h>, as windows.h inherits by default winsock.h header. The w32api has here a incompatibility to msdn, as they are defaulting by default to winsock2.h header.

    Ozkan, this is something for you. I still think we shouldn't enforce here VC behavior. It leads to more pain with less gain. I would suggest that we choose one of the following solutions for this:
    a) Making winsock2.h able to override prior included winsock.h (this is my favorite)
    b) Introducing an macro to enforce msdn compatibility and by default including in windows.h the winsock2.h header.

    Kai

     
  • Kai Tietz

    Kai Tietz - 2010-04-06
    • assigned_to: nobody --> sezero
     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-06

    https://sourceforge.net/apps/trac/mingw-w64/wiki/FAQ

    I'd say applying the following trivial patch would fix this thing instead of trying to support broken code ina more broken way.

    --- gsocket.h.orig
    +++ gsocket.h
    @@ -72,9 +72,10 @@

    #elif defined (WINNT)
    #define FD_SETSIZE 1024
    +#ifndef __MINGW32__
    #include <windows.h>
    -
    -#ifdef __MINGW32__
    +#else
    +/* winsock[2].h already includes windows.h */
    #include <winsock2.h>
    #include <ws2tcpip.h>

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-06

    patch fixing winsock include order in ada

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-06

    Rainer: can you please try with the attached patch applied?
    Kai: If Rainer reports success, feel free to apply it to gcc, to all active branches if possible.

     
  • Rainer Emrich

    Rainer Emrich - 2010-04-06

    Ozkan,

    I will try your patch tomorrow in the afternoon.

    Rainer

     
  • NightStrike

    NightStrike - 2010-04-07

    Moving to Support Requests, since the real issue is in libada, not our headers.

     
  • NightStrike

    NightStrike - 2010-04-07
    • labels: 1008198 -->
     
  • NightStrike

    NightStrike - 2010-04-07
    • labels: --> Build 3rd party app
     
  • NightStrike

    NightStrike - 2010-04-07

    Reminder - we have to verify how the patch affects mingw.org. That shouldn't be hard to deal with.

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    patch (better one) fixing winsock include order in ada

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    A cleaner + better lookinig patch is attached. Rainer, Kai: I'd prefer this one, although the effect is the same.

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    Nightstrike:

    > Reminder - we have to verify how the patch affects mingw.org.
    > That shouldn't be hard to deal with.

    The suggested patching doesn't affect mingw.org.

     
  • Kai Tietz

    Kai Tietz - 2010-04-07

    Hi Ozakan,

    your patch to libada is of course correct but has in my opinion low-priority.
    I agree that our current behavior about winsock/winsock2 is compatible to VC, but we have to admit also that in fact a lot of projects are assuming that winsock2.h gets included by windows.h (as this failure (or extention) is present in w32api). I see no good reason to follow here VC as it produces just failures on existing code and well we are no priests to heal the world.
    So I still strongly see this issue to be solved better on our side, too. We should allow that winsock2.h can override a prior winsock.h include. So we keep origin behavior that windows.h defaults to winsock.h and we allow people to override this by including <winsock2.h> directly without having troubles. Alos we have to admit that winsock.h API gets more and more replaced by winsock2.h API, and so even less this forced behavior looks sane to me.

    Cheers,
    Kai

    PS: Use of __MINGW32__ is no adequate check for environment.

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    > your patch to libada is of course correct but has in my opinion
    > low-priority.
    > I agree that our current behavior about winsock/winsock2 is compatible to
    > VC, but we have to admit also that in fact a lot of projects are assuming
    > that winsock2.h gets included by windows.h (as this failure (or extention)
    > is present in w32api). I see no good reason to follow here VC as it
    > produces just failures on existing code and well we are no priests to heal
    > the world.
    > So I still strongly see this issue to be solved better on our side, too.

    Well, I don't see this as an issue on our side.

    > We should allow that winsock2.h can override a prior winsock.h include. So
    > we keep origin behavior that windows.h defaults to winsock.h and we allow
    > people to override this by including <winsock2.h> directly without having
    > troubles. Alos we have to admit that winsock.h API gets more and more
    > replaced by winsock2.h API, and so even less this forced behavior looks
    > sane to me.
    >

    You already know that I disagree with that when trivial
    solutions are present for existing problematic software.
    Otherwise, you already have svn write access as a
    project admin, you can adjust t the way you like.

    > Cheers,
    > Kai
    >
    > PS: Use of __MINGW32__ is no adequate check for environment.
    >

    I am not doing that, it is what ada itself does already.
    (If there is a confusion arising from the inlined patch,
    check with the attached new gsocket.h-2.diff which
    doesn't even touch such macro stuff.)

     
  • Kai Tietz

    Kai Tietz - 2010-04-07

    Well, this isn't a bug, but still an issue on our side IMHO. To what this leads what you are suggesting? People will see that their stuff is compiling fine by using cygwin and mingw.org, and be sure they won't query themself that this is maybe due a bug on their side. They will just notice that we don't compile and won't research reason for this. As by this fact, it is our issue.

    Kai

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    > Well, this isn't a bug, but still an issue on our side
    > IMHO. To what this leads what you are suggesting?

    I suggest that we receive problem reports as it happened
    with Rainer and Ruben, describe the reason for the failure
    and then present a simple solution.

    > People will see that their stuff is compiling fine by
    > using cygwin and mingw.org, and be sure they won't query
    > themself ....

    Well, do admit that we are used more for w64 targets than
    for w32 targets and the majority of the issues, if any,
    will arise from attempts at w64-targetted compilations which
    there is no cygwin or mingw.org alternative is available.
    Therefore, we _will_ get problem reports, and then we can
    lead them to proper solutions. Otherwise, with more trying
    to support other project's faulty implementations will be
    the failing of mingw-w64 project itself in the long run.
    For this particular case: Just look and see how many other
    compiler suits are out there providing a windows.h which
    includes winsock2.h and not winsock.h? OpenWatcom?
    Lcc-win32? Borland? None. I am not even mentioning that
    how many of them are typedef'ing SOCKET as a signed type:
    None.

     
  • Kai Tietz

    Kai Tietz - 2010-04-07

    Well, that you see it so
    "Well, do admit that we are used more for w64 targets than for w32 targets"
    isn't really true. 32-bit is a target we want and we do support, too. That the w64 is that one with more future is a different story.

    Don't mix here two diffent subjects - as you like to do - signess of SOCKET and co isn't subject of this thread.
    That other compilers are doing this is their beer, not ours. Do those other compilers have to support gcc and libraries ported from POSIX? For sure, no. So don't compare here apples with melons.
    I agree that is is interesting to fix those stuff where we see it, but it is a disadvantage that we simply fail on such "simple to fix" issue.
    It is already common to make order wrong - and this is for sure not our fault - and we have to find a way to handle this. It is never good to say "hey dude your code is broken. Fix it or die.", especially if are interested in supporting it. That we possibly emit an warning on compilation - which is in my opinion an adequate way to report such a fixable issue - when winsock2.h gets included after winsock.h is already, this is fine. But simply to fail to build is a different story. It is not always a argument to have right. When I walk over a street and the lights are green, I still take a look on traffic. As otherwise on my tomb-stone is written "he was right", but this won't help me anymore.

    Kai

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    [snip]
    > interested in supporting it. That we possibly emit an warning on
    > compilation - which is in my opinion an adequate way to report such a

    Something like this and not changing anything else?

    --- winsock2.h.orig 2010-04-07 10:36:13.000000000 +0300
    +++ winsock2.h 2010-04-07 13:12:17.000000000 +0300
    @@ -4,6 +4,14 @@
    * No warranty is given; refer to the file DISCLAIMER.PD within this package.
    */

    +#if defined(_WINSOCK2API_) && !defined(_WINSOCK2API_)
    +#warning Inclusion winsock2.h after winsock.h already
    +#warning is included (probably by way of windows.h).
    +#warning Fix the include order: making sure winsock2.h
    +#warning included *before* windows.h is one solution.
    +#define _WINSOCK2API_ /* do not error... */
    +#endif
    +
    #ifndef _WINSOCK2API_
    #define _WINSOCK2API_
    #define _WINSOCKAPI_

    ... if yes, I can agree with a *big fat* warning and not erroring.

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    [snip]
    > interested in supporting it. That we possibly emit an warning on
    > compilation - which is in my opinion an adequate way to report such a

    Something like this and not changing anything else?

    [EDIT: Fixing my typo..]

    --- winsock2.h.orig 2010-04-07 10:36:13.000000000 +0300
    +++ winsock2.h 2010-04-07 13:12:17.000000000 +0300
    @@ -4,6 +4,14 @@
    * No warranty is given; refer to the file DISCLAIMER.PD within this
    package.
    */

    +#if defined(_WINSOCKAPI_) && !defined(_WINSOCK2API_)
    +#warning Inclusion winsock2.h after winsock.h already
    +#warning is included (probably by way of windows.h).
    +#warning Fix the include order: making sure winsock2.h
    +#warning included *before* windows.h is one solution.
    +#define _WINSOCK2API_ /* do not error... */
    +#endif
    +
    #ifndef _WINSOCK2API_
    #define _WINSOCK2API_
    #define _WINSOCKAPI_

    ... if yes, I can agree with a *big fat* warning and not erroring.

     
  • Kai Tietz

    Kai Tietz - 2010-04-07

    Yes, something like this. We have to make sure that winsock2.h defines are really override the prior definitions, but that is a good solution in my opinion.
    It shows that there is a bug in source, but we *fix* it by emitting such a warning. People will see this warning and then change their source to get rid of it. So we have a win-win situation.

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    > Yes, something like this. We have to make sure that winsock2.h
    > defines are really override the prior definitions, but that is
    > a good solution in my opinion.
    > It shows that there is a bug in source, but we *fix* it by

    Well, we are *not* fixing anything, really, but we are not erroring
    either. Chances are that the broken source will still error if it
    also includes other winsock2-dependent hearders such as ws2tcpip.h
    but at least we will be telling them what their error is.

    > warning. People will see this warning and then change their
    > source to get rid of it. So we have a win-win situation.

    If it is OK, make final adjustments if necessary and I can commit
    it.

     
  • Kai Tietz

    Kai Tietz - 2010-04-07

    Well, I see. You still mis-understood me here. I said "We should allow that winsock2.h can override a prior winsock.h include." and this means we override definitions and not silently still continue using winsock V1 API.
    I don't want that *WE* getting failure reports about this, I want that users getting warning about "THIS IS A BUG IN YOUR CODE, AND PLEASE CHANGE IT TO GET RID OF THIS WARNING. BUT WE ARE THAT KIND TO FIX IT FOR YOU, SO THAT YOU CAN CONTINUE YOUR BUILD."

    Kai

     
  • Ozkan Sezer

    Ozkan Sezer - 2010-04-07

    > Well, I see. You still mis-understood me here. I said "We should
    > allow that winsock2.h can override a prior winsock.h include."
    > and this means we override definitions and not silently still
    > continue using winsock V1 API. I don't want that *WE* getting
    > failure reports about this, I want that users getting warning
    > about "THIS IS A BUG IN YOUR CODE, AND PLEASE CHANGE IT TO GET
    > RID OF THIS WARNING. BUT WE ARE THAT KIND TO FIX IT FOR YOU, SO
    > THAT YOU CAN CONTINUE YOUR BUILD."
    >
    > Kai
    >

    OK, I see, we want to say, "I what can but you're on your own",
    that's cool enough. What patch to commit? The one I gave as an
    example is OK?

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks