Conversation

edited

Open Watcom pass-D_WINSOCKAPI_
option as#define _WINSOCKAPI_ 1
, but proper definition must be#define _WINSOCKAPI_
Correct compiler option must be -D_WINSOCKAPI_=
The problem must be with other compilers too.
Fix is only for Open Watcom tools but should be extend to other compilers.

This comment has been minimized.

At minimum Visual C++ and gcc define macro from command line by option -Dmacro as
#define macro 1
but in code it is
#define macro
what is different value even if in both cases macro is defined but value is different
I am sorry I don't have working configuration/installation for these compilers that I am not able to send you appropriate part of build log where should be this reported.
Open Watcom report this issue as error that I must fix it, but others probably only as warning.

This comment has been minimized.

Yes that's right. But is -DFOO= format accepted by all compilers? If it is we don't need a special WATCOM block for that, we can just do it that way. I'm not sure why we have the winsock api define like this, probably to prevent regular winsock from interfering if it's somehow included before winsock2.

Also what's with the other two line changes you made, they don't seem to change anything but they show up in github as changes. Did you change the line endings or something

This comment has been minimized.

I can not guarantee that it is accepted by all compilers. At minimum gcc and Visual C and probably clang compilers support it.
POSIX standard recommend it too, bellow is copy of POSIX standard specification for -D option

-D name[=value]
Define name as if by a C-language #define directive. If no = value is given, a value of 1 shall be used.

Sorry, the two other changes was created by github editor (end-of-line changes) from Web interface.

This comment has been minimized.

@jay: indeed, _WINSOCK_ define was added to prevent conflict between winsock2 and winsock1-specific headers (you cannot include both as they use the same names for classes, definitions, etc.). This is common workaround and widely used.

The other option would be to test headers in proper order. winsock2.h and ws2tcpip.h should be included before windows.h and winsock.h (note that windows.h includes winsock.h if WIN32_LEAN_AND_MEAN is not defined).

This comment has been minimized.

Changes just landed, thanks. If a compiler used by cmake doesn't support FOO= we'll wait until we hear about it and react from that.

Sorry, the two other changes was created by github editor (end-of-line changes) from Web interface.

I looked into this and it seems what happened is your editor (or github's editor?) converts the file to UTF-8 encoding from extended ascii. Since those two lines contain extended ascii characters they differ, so I ignored those changes.

The other option would be to test headers in proper order. winsock2.h and ws2tcpip.h should be included before windows.h and winsock.h (note that windows.h includes winsock.h if WIN32_LEAN_AND_MEAN is not defined).

Yes, I believe this came up before and some other thing we were testing for like OpenSSL included something that included something that included the first version of Winsock instead of the second, and so someone decided to just define _WINSOCKAPI_.

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.