Latest MinGW: build errors (undefined: __FD_ISSET)

[Albrecht Schlosser]

All,

we got a report in fltk.general in thread "fltk 1.3.4-2 compile error on
windows 7". It turned out that this is an issue of the latest MinGW
version which is incompatible with previous versions and our usage of
the MinGW/Windows headers and, last but not least, our attempt to load
the winsock dll (actually winsock2, ws2_32.lib/.dll) dynamically to
avoid link time dependencies.

The current MinGW headers have two issues, as I already wrote in
fltk.general:

(1) The first issue is minor and can be fixed easily, although I do no
yet understand why it happens. It looks as if the header ole2.h should
#include <winerror.h> which defines S_OK. This worked in older MinGW
versions but broke somehow in the latest version (or earlier, I don't
know exactly which one).

>> Fl_win32.cxx:2712:97: error: 'S_OK' was not declared in this scope

This was the report of the OP. It does not manifest with FLTK 1.4, but I
can confirm it exists with FLTK 1.3.4-svn. A simple patch is to #include
<winerror.h> explicitly.


(2) After issue (1) is fixed (FLTK 1.3 only) we get this linker error:

Linking CXX executable ../bin/fluid.exe
../lib/libfltk.a(Fl_win32.cxx.obj): In function `_FD_SET':
c:/mingw_xx/include/winsock.h:174: undefined reference to `__FD_ISSET'
c:/mingw_xx/include/winsock.h:174: undefined reference to `__FD_ISSET'
c:/mingw_xx/include/winsock.h:174: undefined reference to `__FD_ISSET'
collect2.exe: error: ld returned 1 exit status

This error exists for FLTK 1.3 and 1.4 although we don't explicitly call
FD_ISSET in our code.

Again, a "simple fix" would be to link FLTK against the winsock2 dll
(ws2_32.lib) but that's what we try to avoid to remove the static
dependency on ws2_32.dll. Before I go deeper into my investigations, let
me ask a question:

I understand that we wanted to avoid a dependency on old winsock or new
winsock2 (ws2_32) dll at the time of Windows 95/98/XP, but do we really
need this today? Or can we accept the dependency on the winsock2 dll and
just link the FLTK programs (fluid and test/demo programs) with winsock2?

More questions:

- What happens if we can't dynamically load the winsock dll?
- What is our fallback?
- Does "everything else" work, except maybe Fl::add_fd()?
- Does this all make sense today?
- Are there any Windows versions we can still support that lack
Winsock2 support?

Okay, back to the problem:

The issue is caused by the following code in winsock.h (with line numbers):

From winsock.h:
------
125 #ifndef FD_ISSET
126 int FD_ISSET (SOCKET, fd_set *);
127 #define FD_ISSET( __fd, __set ) __FD_ISSET ((__fd), (__set))
128
129 /* Microsoft provide this library function equivalent of the
FD_ISSET
130 * macro, and erroneously claim that it is neccessary to
implement the
131 * macro. We could just as easily implement it entirely inline...
132 */
133 int PASCAL __WSAFDIsSet (SOCKET, fd_set *);
134 /* ...but, given the availability of the library function, we
may just
135 * as well use it.
136 */
137 __CRT_ALIAS int __FD_ISSET( SOCKET __fd, fd_set *__set )
138 { return __WSAFDIsSet (__fd, __set); }
139 #endif /* ! defined FD_ISSET */
------
141 #ifndef FD_SET
142 ...
168 #define FD_SET( __fd, __set ) __FD_SET ((__fd), (__set))
169 __CRT_ALIAS void __FD_SET (SOCKET __fd, fd_set *__set)
170 { if( (__set->fd_count < FD_SETSIZE) && ! FD_ISSET (__fd, __set) )
171 __set->fd_array[__set->fd_count++] = __fd;
172 }
173 #endif /* ! defined FD_SET */
------

See also the full winsock.h (with line numbers) in the attachment.

We never use FD_ISSET, but we use FD_SET. The comment in line #129-131
and #134-135 is interesting: the MinGW (or maybe WINE?) folks assume
that we can inline the call to __WSAFDIsSet (__fd, __set); because of
"the availability of the library function". The real problem comes up
because we use the macro FD_SET (line #168) which uses __FD_SET (#169)
which uses (in line #170) the macro FD_ISSET (#127) which uses
__FD_ISSET (defined in line #137) that calls __WSAFDIsSet in line #138
which in defined in ws2_32.lib. Hence we have a dependency on ws2_32.dll.

Ian asked in fltk.general:

> Is there some way we can ifdef / undef our way out of this...?

I think it would be possible to #define FD_ISSET and maybe FD_SET before
we #include <winsock2.h>, but given the complexity of the implementation
and the internal dependencies (and the fact that this issue arises only
under MinGW) I wouldn't recommend to do this. I wouldn't even want to
investigate this further. IMHO it is always a bad idea to duplicate (and
modify) [parts of] system headers. These are implementation defined and
once you try to override it you're calling for trouble.

Anyway, I'd appreciate comments and suggestions. The current patch (see
attached file mingw-winsock-fltk-1.3_v1.patch) can be modified if
necessary but it's a working solution. However, it adds the dependency
on the winsock2 dll.

Further (and last) question: If we'd have to add the dependency on
winsock2 for MinGW builds, shouldn't we then add it to all Windows build
systems, just to keep the dependencies "the same" on all Windows
platforms? Maybe it would be confusing (at least) if programs linked
with one Windows toolchain (MinGW) depend on ws2_32.dll but others
(Cygwin, Visual Studio) do not?

[imm]

On 8 February 2018 at 19:43, Albrecht Schlosser wrote:

> Anyway, I'd appreciate comments and suggestions. The current patch (see
> attached file mingw-winsock-fltk-1.3_v1.patch) can be modified if necessary
> but it's a working solution. However, it adds the dependency on the winsock2
> dll.

I am "disappointed" that we need to make this change; but right now I
can not think of a better way to work around this.

I have to say it looks very much like someone in the mingw / win32api
team has filed to grasp the ramifications of their decisions here...

There are quite a few other DLL's that we load at runtime too - I
wonder if they plan on breaking them, too?

> Further (and last) question: If we'd have to add the dependency on winsock2
> for MinGW builds, shouldn't we then add it to all Windows build systems,
> just to keep the dependencies "the same" on all Windows platforms? Maybe it
> would be confusing (at least) if programs linked with one Windows toolchain
> (MinGW) depend on ws2_32.dll but others (Cygwin, Visual Studio) do not?

I don't think it makes sense to have the dll linked at runtime on
those toolchains that (still) support it, if we have to make the code
such that it is linked at compile time for the mingw case now?

I couldn't claim to be happy about this, though.

[Albrecht Schlosser]

On 10.02.2018 00:34 imm wrote:
> On 8 February 2018 at 19:43, Albrecht Schlosser wrote:
>
>> Anyway, I'd appreciate comments and suggestions. The current patch (see
>> attached file mingw-winsock-fltk-1.3_v1.patch) can be modified if necessary
>> but it's a working solution. However, it adds the dependency on the winsock2
>> dll.
>
> I am "disappointed" that we need to make this change; but right now I
> can not think of a better way to work around this.

#define FD_ISSET ... is the key to a patch that works for me with FLTK
1.4. See attached patch mingw-winsock-fltk-1.4_v2.patch. I called it
'_v2' because it's a different approach.

I don't really like it because we need to redefine not only FD_ISSET to
use "our version" but also some more function prototypes.

This was a quick shot. A little reordering of functions and prototypes
may help to make it shorter, but we have a catch-22 issue: We must
#define FD_ISSET before we #include winsock2.h so we don't _know_ the
correct definitions of SOCKET, fd_set, and maybe more.

Please check yourself.

> I have to say it looks very much like someone in the mingw / win32api
> team has filed to grasp the ramifications of their decisions here...

Yep, they didn't think of someone NOT linking with winsock2 (ws2_32.dll)
but loading it dynamically. That's what they actually broke.

> There are quite a few other DLL's that we load at runtime too - I
> wonder if they plan on breaking them, too?

Who knows? Note that I didn't check MinGW-w64 for similar issues yet.

>> Further (and last) question: If we'd have to add the dependency on winsock2
>> for MinGW builds, shouldn't we then add it to all Windows build systems,
>> just to keep the dependencies "the same" on all Windows platforms? Maybe it
>> would be confusing (at least) if programs linked with one Windows toolchain
>> (MinGW) depend on ws2_32.dll but others (Cygwin, Visual Studio) do not?
>
>
> I don't think it makes sense to have the dll linked at runtime on
> those toolchains that (still) support it, if we have to make the code
> such that it is linked at compile time for the mingw case now?

I did this for FLTK 1.4 with conditional code (__MINGW32__) but I'd like
to avoid this.

> I couldn't claim to be happy about this, though.

Can you explain why we _should_ load winsock2 dynamically? This is a
serious question. Do you know of any systems (we still support) that
lack support of winsock2 (ws2_32.dll)? So, what do we gain if we do,
what do we lose if we link directly with ws2_32?

I'm not against keeping it as-is (see my patch above, which should be
applicable to FLTK 1.3 in a similar way), but it's kinda hackish to
suppose we know all these types that would be defined by system supplied
macros otherwise.

The SOCKET type is even worse because we define it anyway, and it is
different in platforms: Linux (int) != Windows 32-bit != Windows 64-bit.

Just some thoughts...

[Ian MacArthur]

> On 11 Feb 2018, at 14:13, Albrecht Schlosser wrote:
>
> On 10.02.2018 00:34 imm wrote:
>> On 8 February 2018 at 19:43, Albrecht Schlosser wrote:
>>> Anyway, I'd appreciate comments and suggestions. The current patch (see
>>> attached file mingw-winsock-fltk-1.3_v1.patch) can be modified if necessary
>>> but it's a working solution. However, it adds the dependency on the winsock2
>>> dll.
>> I am "disappointed" that we need to make this change; but right now I
>> can not think of a better way to work around this.
>
> #define FD_ISSET ... is the key to a patch that works for me with FLTK 1.4. See attached patch mingw-winsock-fltk-1.4_v2.patch. I called it '_v2' because it's a different approach.
>
> I don't really like it because we need to redefine not only FD_ISSET to use "our version" but also some more function prototypes.
>
> This was a quick shot. A little reordering of functions and prototypes may help to make it shorter, but we have a catch-22 issue: We must #define FD_ISSET before we #include winsock2.h so we don't _know_ the correct definitions of SOCKET, fd_set, and maybe more.
>
> Please check yourself.

I looked at that briefly, but couldn’t see any way to do it that I liked. Your way is better than what I had cobbled together. If we do this, we need something that works for the “new” mingw but is still OK with older mingw32 and mingw64 and also with MS cl and... It’s not nice.

>
>> I have to say it looks very much like someone in the mingw / win32api
>> team has filed to grasp the ramifications of their decisions here...
>
> Yep, they didn't think of someone NOT linking with winsock2 (ws2_32.dll) but loading it dynamically. That's what they actually broke.
>
>> There are quite a few other DLL's that we load at runtime too - I
>> wonder if they plan on breaking them, too?
>
> Who knows? Note that I didn't check MinGW-w64 for similar issues yet.

The mingw64 I have is older, and does not exhibit this behaviour - I do not know if they will switch to using this newer variant of the win32api files at some point?

>
>>> Further (and last) question: If we'd have to add the dependency on winsock2
>>> for MinGW builds, shouldn't we then add it to all Windows build systems,
>>> just to keep the dependencies "the same" on all Windows platforms? Maybe it
>>> would be confusing (at least) if programs linked with one Windows toolchain
>>> (MinGW) depend on ws2_32.dll but others (Cygwin, Visual Studio) do not?
>> I don't think it makes sense to have the dll linked at runtime on
>> those toolchains that (still) support it, if we have to make the code
>> such that it is linked at compile time for the mingw case now?
>
> I did this for FLTK 1.4 with conditional code (__MINGW32__) but I'd like to avoid this.
>
>> I couldn't claim to be happy about this, though.
>
> Can you explain why we _should_ load winsock2 dynamically? This is a serious question. Do you know of any systems (we still support) that lack support of winsock2 (ws2_32.dll)? So, what do we gain if we do, what do we lose if we link directly with ws2_32?

TBH, I do not know of any current WinXX variant where this would be a problem; we did have issues early on, but I think those days are past now.

It still rankles though, but I now think our “best” option is to link in ws2_32 dll and be done with it...

>
> I'm not against keeping it as-is (see my patch above, which should be applicable to FLTK 1.3 in a similar way), but it's kinda hackish to suppose we know all these types that would be defined by system supplied macros otherwise.
>
> The SOCKET type is even worse because we define it anyway, and it is different in platforms: Linux (int) != Windows 32-bit != Windows 64-bit.

Unrelated: In other work, I got caught out by MS changing their SOCKET type between 32 and 64 recently... Not helpful!

[Albrecht Schlosser]

On 11.02.2018 18:41 Ian MacArthur wrote:
>
>> On 11 Feb 2018, at 14:13, Albrecht Schlosser wrote:
>>
>> #define FD_ISSET ... is the key to a patch that works for me with FLTK 1.4. See attached patch mingw-winsock-fltk-1.4_v2.patch. I called it '_v2' because it's a different approach.
>>
>> I don't really like it because we need to redefine not only FD_ISSET to use "our version" but also some more function prototypes.
>>
>> This was a quick shot. A little reordering of functions and prototypes may help to make it shorter, but we have a catch-22 issue: We must #define FD_ISSET before we #include winsock2.h so we don't _know_ the correct definitions of SOCKET, fd_set, and maybe more.
>>
>> Please check yourself.
>
> I looked at that briefly, but couldn’t see any way to do it that I liked. Your way is better than what I had cobbled together. If we do this, we need something that works for the “new” mingw but is still OK with older mingw32 and mingw64 and also with MS cl and... It’s not nice.

What I did should work on all Windows platforms, but I'm not sure if we
can get data type issues, maybe we'd have to add some casts or similar.
The proposed patch would (as it stands) only be for MinGW anyway, and
the solution *should* work on old and new mingw32 and mingw64 platforms.
I did not yet test all combinations though, but I have pretty old
mingw32 and mingw64 as well as absolutely new mingw32 and mingw64 (both
included in my new MSYS2 installation).

The question is whether the "original" MinGW + MSYS has different
headers - which seems to be the case since the OP and my "new"
installation of MinGW/MSYS had the issue whereas the
mingw32/mingw64/MSYS2 installation doesn't have the issue.

> The mingw64 I have is older, and does not exhibit this behaviour - I do not know if they will switch to using this newer variant of the win32api files at some point?

I don't know how these different forks maintain their Windows headers.
ISTR that I saw comments that refer to WINE, so the common root may be
Wine or Cygwin or whatever, but what current forks do and when and how
the synchronize again (if at all) I do not know.

We should be prepared for everything...

>>>> Further (and last) question: If we'd have to add the dependency on winsock2
>>>> for MinGW builds, shouldn't we then add it to all Windows build systems,
>>>> just to keep the dependencies "the same" on all Windows platforms? Maybe it
>>>> would be confusing (at least) if programs linked with one Windows toolchain
>>>> (MinGW) depend on ws2_32.dll but others (Cygwin, Visual Studio) do not?
>>> I don't think it makes sense to have the dll linked at runtime on
>>> those toolchains that (still) support it, if we have to make the code
>>> such that it is linked at compile time for the mingw case now?
>>
>> I did this for FLTK 1.4 with conditional code (__MINGW32__) but I'd like to avoid this.
>>
>>> I couldn't claim to be happy about this, though.
>>
>> Can you explain why we _should_ load winsock2 dynamically? This is a serious question. Do you know of any systems (we still support) that lack support of winsock2 (ws2_32.dll)? So, what do we gain if we do, what do we lose if we link directly with ws2_32?
>
> TBH, I do not know of any current WinXX variant where this would be a problem; we did have issues early on, but I think those days are past now.

I checked the code (by reading) and this is what I understood (but I'm
not sure and I don't know how to test this even if I wanted):

(1) Applications that don't use Fl::add_fd() will never try to load the
winsock2 dll dynamically, hence they will be able to run w/o ws2_32.dll
w/o issues.

(2) Applications that use Fl::add_fd() will work on systems that have
ws2_32.dll, of course.

(3) Applications that use Fl::add_fd() will *crash* on systems that
don't have ws2_32.dll. There is no fallback. At least that's what I read
in the code.

Back to the question "what do we lose?": Only case (1) is worth thinking
about. Most if not all of our test/*.{fl|cxx} demo programs would work,
as well as any GUI programs that don't use sockets (with or w/o add_fd()
if they use winsock2). Programs that use old winsock or any other socket
library would no longer work because they would not find ws2_32.dll
(linked in statically) at startup.

> It still rankles though, but I now think our “best” option is to link in ws2_32 dll and be done with it...

What about linking in ws2_32.dll in FLTK 1.4 but using my proposed FLTK
1.4 patch and the S_OK patch for FLTK 1.3.5 for MinGW only?

The advantages:

(1) Only *MinGW* builds of _FLTK 1.3.5_ (and later 1.3.x) would be
affected. Hence programs that don't use sockets could still successfully
run on "old" Windows systems w/o winsock2 even if built with MinGW. This
is the best we can do for backwards compatibility and w/o breaking the
ABI (new dll dependency for most apps).

(2) We can document the change that FLTK 1.4 requires ws2_32.dll [1] on
all Windows platforms, independent of the build system (MinGW, Visual
Studio, MSYS2, Linux cross compilation, whatever).

(3) We can remove the dynamic loading of winsock functions to:
(3a) make the code more "standard" (e.g. using FD_ISSET macro)
(3b) avoid "late crash" of programs using add_fd().

(4) This will unify all Windows platform to behave in the same way.

-----
[1] additionally to comctl32.dll and other standard Windows dll's

>> I'm not against keeping it as-is (see my patch above, which should be applicable to FLTK 1.3 in a similar way), but it's kinda hackish to suppose we know all these types that would be defined by system supplied macros otherwise.

Now, after thinking about the issue and in the light of possible ABI
changes I think the "split proposal" to

- patch 1.3.5 as described above and
- change 1.4.0 to link in ws2_32 statically

would be my preferred solution. No ABI change in 1.3.5, but "future
proof standard code" in 1.4.0 and later.

If there are no other proposals, then I suggest we vote:

+1 for "split proposal" described above.

>> The SOCKET type is even worse because we define it anyway, and it is different in platforms: Linux (int) != Windows 32-bit != Windows 64-bit.
>
> Unrelated: In other work, I got caught out by MS changing their SOCKET type between 32 and 64 recently... Not helpful!

Yep, and we have STR #3271 which I don't really know how to resolve:
http://www.fltk.org/str.php?L3271

OT:

The conflict is that we have the well-documented API Fl::add_fd(int
socket, ...) and we'd break this contract (API, not "only" ABI) if we'd
change this to Fl::add_fd(FL_SOCKET, ...) where FL_SOCKET is a platform
specific type that can be 'int', 'unsigned int', and 'unsigned __int64'
(see comment #1 - #6 of STR #3271). The same would apply to remove_fd().

But if we break the API (using FL_SOCKET), how would portable user
programs be affected? Would our API be backwards compatible? How many
compiler warnings (or even errors) would we get reported?

Comments welcome, preferred in the STR or, if necessary for discussion,
maybe in a new thread?

[Ian MacArthur]

> On 13 Feb 2018, at 12:26, Albrecht Schlosser wrote:
>

> Now, after thinking about the issue and in the light of possible ABI changes I think the "split proposal" to
>
> - patch 1.3.5 as described above and
> - change 1.4.0 to link in ws2_32 statically
>
> would be my preferred solution. No ABI change in 1.3.5, but "future proof standard code" in 1.4.0 and later.
>
> If there are no other proposals, then I suggest we vote:
>
> +1 for "split proposal" described above.

I think this is possibly our least-bad choice at the present time.

That being so, do we need to remove (or at least comment out for now) the code that loads ws2_32.dll on demand?

>
>>> The SOCKET type is even worse because we define it anyway, and it is different in platforms: Linux (int) != Windows 32-bit != Windows 64-bit.
>> Unrelated: In other work, I got caught out by MS changing their SOCKET type between 32 and 64 recently... Not helpful!
>
> Yep, and we have STR #3271 which I don't really know how to resolve:
> http://www.fltk.org/str.php?L3271
>
> OT:
>
> The conflict is that we have the well-documented API Fl::add_fd(int socket, ...) and we'd break this contract (API, not "only" ABI) if we'd change this to Fl::add_fd(FL_SOCKET, ...) where FL_SOCKET is a platform specific type that can be 'int', 'unsigned int', and 'unsigned __int64' (see comment #1 - #6 of STR #3271). The same would apply to remove_fd().
>
> But if we break the API (using FL_SOCKET), how would portable user programs be affected? Would our API be backwards compatible? How many compiler warnings (or even errors) would we get reported?
>
> Comments welcome, preferred in the STR or, if necessary for discussion, maybe in a new thread?

I already posted this to #3271, but I think we can (maybe) change to for 1.4, whilst the ABI is still “open”.
The impact that has on portability... Well, I do not know. I suspect in practice that the value in the SOCKET is likely to fit into a 32-bit value, even on 64-bit windows?

[Albrecht Schlosser]

On 13.02.2018 23:56 Ian MacArthur wrote:
>
>> On 13 Feb 2018, at 12:26, Albrecht Schlosser wrote:
>>
>> Now, after thinking about the issue and in the light of possible ABI changes I think the "split proposal" to
>>
>> - patch 1.3.5 as described above and
>> - change 1.4.0 to link in ws2_32 statically
>>
>> would be my preferred solution. No ABI change in 1.3.5, but "future proof standard code" in 1.4.0 and later.
>>
>> If there are no other proposals, then I suggest we vote:
>>
>> +1 for "split proposal" described above.
>
>
> I think this is possibly our least-bad choice at the present time.

Okay, I'll see if I can prepare two patches:

- one for 1.3 w/o changing the ABI and for MinGW (__MINGW32__) only

- one for 1.4 with "standard" code, using the macros (FD_ISSET etc.),
linking in ws2_32 directly on all Windows platforms

> That being so, do we need to remove (or at least comment out for now) the code that loads ws2_32.dll on demand?

Yep, I think so. Everything else would be confusing, and the
"standard-like" code using the macros directly would be simpler and less
error-prone as a side effect.

If we don't load the winsock dll dynamically we should also remove all
code that attempts to find and call functions like __WSAFDIsSet()
directly. The latter (i.e. the function name) is an implementation
detail we shouldn't [need to] know anyway, so it is better to use the
macros as defined and documented on all platforms (Windows and others).

>>>> The SOCKET type is even worse because we define it anyway, and it is different in platforms: Linux (int) != Windows 32-bit != Windows 64-bit.
>>> Unrelated: In other work, I got caught out by MS changing their SOCKET type between 32 and 64 recently... Not helpful!
>>
>> Yep, and we have STR #3271 which I don't really know how to resolve:
>> http://www.fltk.org/str.php?L3271
>>
>> OT:
>>
>> The conflict is that we have the well-documented API Fl::add_fd(int socket, ...) and we'd break this contract (API, not "only" ABI) if we'd change this to Fl::add_fd(FL_SOCKET, ...) where FL_SOCKET is a platform specific type that can be 'int', 'unsigned int', and 'unsigned __int64' (see comment #1 - #6 of STR #3271). The same would apply to remove_fd().
>>
>> But if we break the API (using FL_SOCKET), how would portable user programs be affected? Would our API be backwards compatible? How many compiler warnings (or even errors) would we get reported?
>>
>> Comments welcome, preferred in the STR or, if necessary for discussion, maybe in a new thread?
>
> I already posted this to #3271, but I think we can (maybe) change to for 1.4, whilst the ABI is still “open”.
> The impact that has on portability... Well, I do not know. I suspect in practice that the value in the SOCKET is likely to fit into a 32-bit value, even on 64-bit windows?

Definitely. If you have Visual Studio (and I'm sure you have) you can
look at the definition of the FD_SET macro (right click on FD_SET in
Fl_win32.cxx, select "Go To Definition (F12)". The definition is in
winsock2.h, but I won't post it here for copyright reasons. You'll see
that the macro uses a constant (FD_SETSIZE = 64) to verify that the
socket number is in the correct range etc..

Although I can see why MS want an unsigned type I can't see a reason to
use a 64-bit value.

Maybe they wanted to be prepared for extensions so programs can use more
than 64 sockets (seriously, OK), or another implementation that uses a
bit array... (less seriously) ;-)

WRT portability: I was thinking of the effects on portable programs that
store a socket number somewhere (in a struct or such). Currently they
would likely use 'int' which is portable across platforms. Windows users
may have issues with this when they use SOCKET instead or when they are
calling socket functions directly (to connect, read, write, etc.).

If we change the API and use FL_SOCKET we'd have a platform specific
data type and users should likely store socket values in structs etc. as
FL_SOCKET as well whose size will vary across platforms. I know it's
possible to do, but intermediate functions that deal with sockets (in
Unix they are all 'int') would have to use FL_SOCKET (or a similar
typedef) to be fully portable across platforms and not to get "lost
precision" warnings on some platforms.

Although this would be the user program's problem (not a FLTK problem),
this FLTK API change can introduce backwards compatibility issues that
must be "fixed" in the user's code.

[Manolo]

========

Now, after thinking about the issue and in the light of possible ABI
changes I think the "split proposal" to

- patch 1.3.5 as described above and
- change 1.4.0 to link in ws2_32 statically

would be my preferred solution. No ABI change in 1.3.5, but "future
proof standard code" in 1.4.0 and later.

If there are no other proposals, then I suggest we vote.
========

I'd like to vote 0 (abstain) this time for not having followed this issue enough
to have an informed position.

[Albrecht Schlosser]

Thanks for voting anyway. I take it we have now 3 votes with a total of
+2 (Ian and me) so I'll go ahead and prepare a patch.

I'd like to get Greg's opinion on this question for FLTK 1.4 though:
would it be okay to build FLTK on Windows with a (new) dependency on
Winsock2 (ws2_32.dll)? The current behavior to crash at runtime if
Fl::add_fd() is used is bad anyway.

Personally I believe it would be okay, but maybe there are reasons I
don't see.

[Greg Ercolano]

On 02/18/18 04:12, Albrecht Schlosser wrote:
> I'd like to get Greg's opinion on this question for FLTK 1.4 though:
> would it be okay to build FLTK on Windows with a (new) dependency on
> Winsock2 (ws2_32.dll)? The current behavior to crash at runtime if
> Fl::add_fd() is used is bad anyway.
>
> Personally I believe it would be okay, but maybe there are reasons I
> don't see.

Unless I'm missing something, I don't think I have a horse
in this race, so I abstain as well until there's more info
from those who are concerned about this change.

Someone who does I think would be Lauri; he was very concerned
with library size (started a thread here circa 2014 on that
subject), and by extension I think, concerned with dependencies
on system libraries (which I take it is what this change is about.

Would like to hear from him on this, if he has an opinion.

I do use add_fd() in my apps, as I'm often wanting to wake up
when child processes write to stdout/err through a pipe, which
I then update into the interface. However I *think* on windows
I switched to using threads an win32 calls, because the unixy
calls (popen()/read()) weren't good enough for efficient handling
of pipes.

[Lauri Kasanen]

On Mon, 19 Feb 2018 08:56:20 -0800
Greg Ercolano wrote:

> Unless I'm missing something, I don't think I have a horse
> in this race, so I abstain as well until there's more info
> from those who are concerned about this change.
>
> Someone who does I think would be Lauri; he was very concerned
> with library size (started a thread here circa 2014 on that
> subject), and by extension I think, concerned with dependencies
> on system libraries (which I take it is what this change is about.
>
> Would like to hear from him on this, if he has an opinion.

I rarely use Windows, and I don't know the effects a static winsock2
would have. Size-wise, linking against the winsock2 dll doesn't change
much. So I defer to the Windows experts.

- Lauri

[Albrecht Schlosser]

On 18.02.2018 13:12 Albrecht Schlosser wrote:
> On 18.02.2018 08:31 Manolo wrote:
>> ========
>> Now, after thinking about the issue and in the light of possible ABI
>> changes I think the "split proposal" to
>>
>> - patch 1.3.5 as described above and
>> - change 1.4.0 to link in ws2_32 statically
>>
>> would be my preferred solution. No ABI change in 1.3.5, but "future
>> proof standard code" in 1.4.0 and later.
>>
>> If there are no other proposals, then I suggest we vote.
>> ========
>>
>> I'd like to vote 0 (abstain) this time for not having followed this
>> issue enough
>> to have an informed position.
>
> Thanks for voting anyway. I take it we have now 3 votes with a total of
> +2 (Ian and me) so I'll go ahead and prepare a patch.

FYI: I created STR #3454 for this issue (especially for FLTK 1.3) and
committed the FLTK 1.3 patch to branch-1.3 w/o breaking the ABI (svn
r12679). I hope this fixes the issue for all Windows builds.

FLTK 1.4 is still open.

I built FLTK 1.3.x-svn (r12679) with all Windows build systems available
to me (except Cygwin and the bundled IDE's) and it seemed to work. I
used old and new MinGW installations (32-bit and 64-bit), old MinGW +
MinGW-w64 (64-bit), MSVC 2015 (64-bit, generated by CMake), and Msys2
(32-bit and 64-bit) and everything seemed to work.

I built and tested Greg's demo program
(examples/howto-add_fd-and-popen.cxx) and it worked. I'm surprised
because it uses Fl::add_fd() with a pipe to show the output of a command
(ping, in that case). Our docs say:

"Under UNIX any file descriptor can be monitored (files, devices, pipes,
sockets, etc.). Due to limitations in Microsoft Windows, WIN32
applications can only monitor sockets."

Am I missing something? I verified that select() "got something" so this
must have changed on newer Windows platforms. Does anyone know what's
going on? Here is our code:

Fl_win32.cxx:401: if (get_wsock_mod()&&
s_wsock_select(maxfd+1,&fdt[0],&fdt[1],&fdt[2],&t)) {
Fl_win32.cxx-402- // We got something - do the callback!
Fl_win32.cxx-403- for (int i = 0; i < nfds; i ++) {
Fl_win32.cxx-404- SOCKET f = fd[i].fd;
Fl_win32.cxx-405- short revents = 0;
Fl_win32.cxx-406- if (fl_wsk_fd_is_set(f, &fdt[0])) revents |=
FL_READ;
Fl_win32.cxx-407- if (fl_wsk_fd_is_set(f, &fdt[1])) revents |=
FL_WRITE;
Fl_win32.cxx-408- if (fl_wsk_fd_is_set(f, &fdt[2])) revents |=
FL_EXCEPT;
Fl_win32.cxx-409- if (fd[i].events & revents) fd[i].cb(f, fd[i].arg);
Fl_win32.cxx-410- }

So we're calling Winsock's select() in line 401, get "something", and
the "SOCKET f" = 3 (in my test run), verified by printf(). The demo
program worked as expected and printed a line every second or so.

BTW, I was using Windows 10.

FWIW: here are the relevant parts of the demo program code:

31-# define PING_CMD "ping -n 10 localhost" // 'slow command' under
windows
32-# ifdef _MSC_VER
33:# define popen _popen
34-# define pclose _pclose
35-# else /*_MSC_VER*/
--
62- Fl_Window win(600,600);
63- Fl_Multi_Browser brow(10,10,580,580);
64: if ( ( G_fp = popen(PING_CMD, "r") ) == NULL ) { // start the
external unix command
65: perror("popen failed");
66- return(1);
67- }
68: Fl::add_fd(fileno(G_fp), HandleFD, (void*)&brow); // setup a
callback for the popen()ed descriptor
69- win.resizable(brow);
70- win.show(argc, argv);

So we're opening a pipe with G_fp = popen() and using this in
Fl::add_fd(fileno(G_fp), ...); How does this qualify as a socket???

Comments, anybody?

Does anyone have a simple demo program with a Windows SOCKET to test
add_fd() directly with a "real" socket? I could perhaps write one, but
this would be useless effort if someone has a demo. TIA.

[Albrecht Schlosser]

On 19.02.2018 18:55 Lauri Kasanen wrote:
> On Mon, 19 Feb 2018 08:56:20 -0800
> Greg Ercolano wrote:
>
>> Unless I'm missing something, I don't think I have a horse
>> in this race, so I abstain as well until there's more info
>> from those who are concerned about this change.

The *only* concern is that _really_ _old_ Windows systems (XP or older)
might not have winsock2 (ws2_32.dll) and FLTK programs that don't use
sockets or Fl::add_fd() would no longer work on these old systems
because of the dependency. It seems that we only load the winsock dll
when Fl::add_fd() is called and later to select on the given sockets.
Programs that use add_fd() would crash (!) when add_fd() is called if
the winsock2 dll can't be loaded, other programs can run w/o it.

The new dependency would prevent *all* FLTK programs to run on systems
that don't have winsock2.

My patch for FLTK 1.3 avoids this new dependency, so this is the same
status as before.

>> Someone who does I think would be Lauri; he was very concerned
>> with library size (started a thread here circa 2014 on that
>> subject), and by extension I think, concerned with dependencies
>> on system libraries (which I take it is what this change is about.
>>
>> Would like to hear from him on this, if he has an opinion.
>
> I rarely use Windows, and I don't know the effects a static winsock2
> would have. Size-wise, linking against the winsock2 dll doesn't change
> much. So I defer to the Windows experts.

The code (program size) would likely be a few bytes shorter if we link
ws2_32.lib/.dll statically because we don't need the code to load the
dll dynamically and the code to find the function entries.

Besides this advantage we'd also get more standard (less error prone)
code because we're using system macros like FD_ISSET and FD_SET instead
of calling winsock functions directly, using implementation details
we're not supposed to (have to) know and which can be changed by Microsoft.

We get:

- smaller size
- less code (more standard, no dll loading)
- less error prone

Drawback:
- programs can't be started if ws2_32.dll is not available even if
they don't use sockets or Fl::add_fd()

The patch we need for MinGW makes it even harder to write and to
understand the code if we wanted to keep the dynamic loading of winsock
dll for such border cases. If we switched to static linking the code
would be even simpler and shorter.

The MinGW guys assumed that Winsock2 is always available, hence they
changed the winsock2 header file in a way that static linking is
required. The possible patch (used for FLTK 1.3 to maintain ABI
compatibility) defines the FD_ISSET macro before #include <winsock2.h>
to override the code in winsock2.h which is under condition "#ifndef
FD_ISSET". Nobody can guarantee that this will be "forever".

Hence I strongly suggest to use "standard" code for FLTK 1.4 with the
drawback that we must link statically to ws2_32.dll.

[Greg Ercolano]

On 02/19/18 10:33, Albrecht Schlosser wrote:
> On 19.02.2018 18:55 Lauri Kasanen wrote:
>> On Mon, 19 Feb 2018 08:56:20 -0800
>> Greg Ercolano wrote:
>>
>>> Unless I'm missing something, I don't think I have a horse
>>> in this race, so I abstain as well until there's more info
>>> from those who are concerned about this change.
>
> The *only* concern is that _really_ _old_ Windows systems (XP or older)
> might not have winsock2 (ws2_32.dll) and FLTK programs that don't use
> sockets or Fl::add_fd() would no longer work on these old systems
> because of the dependency. It seems that we only load the winsock dll
> when Fl::add_fd() is called and later to select on the given sockets.
> Programs that use add_fd() would crash (!) when add_fd() is called if
> the winsock2 dll can't be loaded, other programs can run w/o it.

I see -- and this patch change appears to be mingw build-specific.

If so, I'm unaffected, as I only build with microsoft's compiler.

But if the change affects Microsoft builds, I might be affected.

I definitely don't want to find I get a dll error when testing
my VS generated binary on the older windows versions (xp, nt).

[imm]

On 19 February 2018 at 18:33, Albrecht Schlosser wrote:
>
> The *only* concern is that _really_ _old_ Windows systems (XP or older)
> might not have winsock2 (ws2_32.dll)

FWIW, from memory (so quite possibly wrong...) I do not think XP is
affected by this (that is, I am pretty sure XP does always have
ws2_32.dll.)

I think you need to go older - pre Win98 maybe? Not sure about WinNT4...?

I see Greg says he builds for NT sometimes: if that works, albeit
built with the MS tools, and his code works and makes use of the
network layer, then that presumably tells us that ws2_32 is available
in that environment.


Note that, although the recent mingw API mods triggered this change,
the toolchain used is somewhat orthogonal to the issue; what really
matters is whether we can safely assume that the platform *will* have
ws2_32.dll available at runtime.

[Albrecht Schlosser]

On 20.02.2018 00:03 Greg Ercolano wrote:
> On 02/19/18 10:33, Albrecht Schlosser wrote:
>> On 19.02.2018 18:55 Lauri Kasanen wrote:
>>> On Mon, 19 Feb 2018 08:56:20 -0800
>>> Greg Ercolano wrote:
>>>
>>>> Unless I'm missing something, I don't think I have a horse
>>>> in this race, so I abstain as well until there's more info
>>>> from those who are concerned about this change.
>>
>> The *only* concern is that _really_ _old_ Windows systems (XP or older)
>> might not have winsock2 (ws2_32.dll) and FLTK programs that don't use
>> sockets or Fl::add_fd() would no longer work on these old systems
>> because of the dependency. It seems that we only load the winsock dll
>> when Fl::add_fd() is called and later to select on the given sockets.
>> Programs that use add_fd() would crash (!) when add_fd() is called if
>> the winsock2 dll can't be loaded, other programs can run w/o it.
>
> I see -- and this patch change appears to be mingw build-specific.

That's true for the 1.3.5 change. The idea being that we

(a) don't break the ABI and
(b) be as careful as possible and change only MinGW builds, just in
case ...

> If so, I'm unaffected, as I only build with microsoft's compiler.
>
> But if the change affects Microsoft builds, I might be affected.

The plan is to make all Windows builds the same. FLTK 1.4 should not be
build system / compiler specific. This might lead to inconsistencies
(different build behave differently) that are hard to diagnose in case
of error reports. We discussed it here (Ian and me) and came to the
conclusion that this would be the better choice.

If your application use Fl::add_fd() you're likely unaffected by the
change because that loads the winsock dll dynamically anyway and it
would fail (crash) if it is not available.

> I definitely don't want to find I get a dll error when testing
> my VS generated binary on the older windows versions (xp, nt).

Wow, XP might be an issue (I don't know for sure, maybe versions before
SP1 or SP2). Are you or your customers really still using NT? That would
likely be a problem (but again, I don't know, maybe there are versions
of ws2_32.dll installed?). Officially we don't support NT any more, but
it may still work.

If you can test on such systems a simple test would be to modify your
build system to add ws2_32.lib to the "additional linker libraries" or
whatever it's called in the linker setup of the VS IDE, just where you
would have added comctl32.lib. If your program is linked with ws2_32.lib
and the dll (ws2_32.dll) is not available on the target system it can't
be started at all (missing dll error, but the error message may be not
very helpful on these old systems). I believe the exit status code may
be 127 in such a case, but don't rely on this.

If that is the case (i.e. missing ws2_32.dll) and you really need to
support these old systems then we have a problem. I'd appreciate if you
could check this. TIA.

[Albrecht Schlosser]

On 20.02.2018 00:27 imm wrote:
> On 19 February 2018 at 18:33, Albrecht Schlosser wrote:
>>
>> The *only* concern is that _really_ _old_ Windows systems (XP or older)
>> might not have winsock2 (ws2_32.dll)
>
> FWIW, from memory (so quite possibly wrong...) I do not think XP is
> affected by this (that is, I am pretty sure XP does always have
> ws2_32.dll.)

This is from my old WinXP VM:

System: Windows XP Professional Version 2002 Service Pack 3

C:\Windows\system32\ws2_32.dll:

version: 5.1.2600.5512 (xpsp.080413-0852)

creation date/time: Wednesday, August 4, 2004, 13:00:00
modification date/time: Monday, April 14, 2008, 03:22:32

I can test if this works with a FLTK program built on Windows 10 with
MinGW and with Visual Studio 2015 (linked statically with ws2_32.dll)
but it's too late here today, and I'll be very busy tomorrow so it may
take a little longer (unless I can find a free hour or two).

> I think you need to go older - pre Win98 maybe? Not sure about WinNT4...?

I don't have active systems older than this XP VM. There's somewhere an
old Windows 98 or 95 system though (real hardware, don't know if it
would still work).

> I see Greg says he builds for NT sometimes: if that works, albeit
> built with the MS tools, and his code works and makes use of the
> network layer, then that presumably tells us that ws2_32 is available
> in that environment.

If it is on the system it's likely in C:\Windows\system32 but I don't
know if this is compatible in all cases. So to be sure we'd need to try.

> Note that, although the recent mingw API mods triggered this change,
> the toolchain used is somewhat orthogonal to the issue; what really
> matters is whether we can safely assume that the platform *will* have
> ws2_32.dll available at runtime.

Exactly. Unless we'd use the hackish fix I committed for FLTK 1.3. This
was even worse than for 1.4 because I had to patch two files (Fl_x.cxx
and Fl_win32.cxx) to get what we need. But it's ugly anyway and I'd like
to avoid it for 1.4 as you already know.

[Greg Ercolano]

On 02/19/18 15:27, imm wrote:
> On 19 February 2018 at 18:33, Albrecht Schlosser wrote:
>>
>> The *only* concern is that _really_ _old_ Windows systems (XP or older)
>> might not have winsock2 (ws2_32.dll)
>
> FWIW, from memory (so quite possibly wrong...) I do not think XP is
> affected by this (that is, I am pretty sure XP does always have
> ws2_32.dll.)
>
> I think you need to go older - pre Win98 maybe? Not sure about WinNT4...?
>
> I see Greg says he builds for NT sometimes: if that works, albeit
> built with the MS tools, and his code works and makes use of the
> network layer, then that presumably tells us that ws2_32 is available
> in that environment.

Just a note on my WinNT comment..

I've always built my commercial apps on VS7, because it seems
to generate the most flexible executables.

I always test the binaries on the oldest (WinNT) and latest (Win10
these days) versions of windows just to see if the day ever comes
that the executables won't run on one of them.

Hasn't happened yet, so color me impressed that the binaries continue
to run on such a wide span of operating system releases.

It's the one good thing I can say about MS windows; they did get
backwards compatibility right in a way that linux and OSX haven't.

[LM]

On Tuesday, February 13, 2018 at 7:27:01 AM UTC-5, Albrecht Schlosser wrote:

The question is whether the "original" MinGW + MSYS has different
headers - which seems to be the case since the OP and my "new"
installation of MinGW/MSYS had the issue whereas the
mingw32/mingw64/MSYS2 installation doesn't have the issue.

> The mingw64 I have is older, and does not exhibit this behaviour - I do not know if they will switch to using this newer variant of the win32api files at some point?

I don't know how these different forks maintain their Windows headers.
ISTR that I saw comments that refer to WINE, so the common root may be
Wine or Cygwin or whatever, but what current forks do and when and how
the synchronize again (if at all) I do not know.

We should be prepared for everything...

The mingw.org and the mingw-w64 project (for 32 and 64 bit) both maintain their own separate headers and source code.  They started with the same source but both diverged.  mingw-w64 may share some header information with the Wine project.  The mingw.org project wanted nothing to do with the modified mingw-w64 source (including headers) because they maintain that the project uses information that is not publicly documented.  The mingw-w64 project maintains that they've reverse engineered missing information using a clean room technique and so it's safe to use the information.  Unless the maintainers of the MinGW project change their opinions of the MinGW-w64 project, I do not think you'll see any synchronization between them and the MinGW-w64 project.  I maintained the original MinGW FAQ and documented some of the issues regarding the headers in my latest MinGW FAQ at http://www.distasis.com/cpp/mingw.htm

To make matters more complicated, both projects decided to relicense what used to be labeled as public domain headers under new terms such as the MIT license.  Some projects prefer to stay with the public domain version of the headers.  I left the MinGW project when they refused to consider dual licensing the headers so users who did not want to use the new MIT license could still use them as public domain or CC0.  I personally use a version of gcc (currently 4.9.2) for 32 bit platforms which I built from source based on the last public domain version of the MinGW headers.  I've made some fixes to the headers as I find bugs or things change in the MSDN documentation.  So, I have the original headers with fixes that the other two projects may not have included.  I also sometimes still use the older version of gcc (last version released by the MinGW project before the license change to headers).  The w32api patches I made work fine with either and fix a few issues where some Open Source projects no longer build without the modifications.

Last I checked, FLTK (1.3.x and 1.4.x) was building fine with my compilers (using 3.17-2 version of w32api headers or my patched headers), but I think I last built from source over a year ago.  If you need testing on systems with the older headers, let me know.  Would be happy to help when I have time especially if it would help maintain backward compatibility for users of older systems.