Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mumble_pch.hpp: Improve MinGW compatibility. #2781

Merged

Conversation

@davidebeatrici
Copy link
Member

commented Jan 26, 2017

This PR implements various changes to improve the compatibility of mumble_pch.hpp with our MinGW build.

@davidebeatrici davidebeatrici changed the title Mumble pch.h include order mumble_pch.hpp: Include "winsock2.h" before "windows.h" Jan 26, 2017

@mkrautz
Copy link
Member

left a comment

Breaks MSVC build.

@mkrautz

This comment has been minimized.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mumble_pch.h-include-order branch from e603e2b to f2e664e Jan 26, 2017

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@davidebeatrici Please rebase this to current master. Then you get a free MSVC build.

If it builds there, I am OK to merge.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mumble_pch.h-include-order branch from f2e664e to f76581c Feb 13, 2017

mumble_pch.hpp: Include "winsock2.h" before "windows.h"
This fixes the following MinGW warning:
#warning Please include winsock2.h before windows.h [-Wcpp]

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mumble_pch.h-include-order branch 3 times, most recently from 674177d to afc11f9 Mar 6, 2017

@mkrautz

This comment has been minimized.

Copy link
Member

commented Mar 6, 2017

mumble_pch.h: Delete WINSOCKAPI definition

^- this commit has a wrong summary-

It should be `mumble_pch.hpp'...

@mkrautz

This comment has been minimized.

Copy link
Member

commented Mar 6, 2017

Please update your PR title and description to match the content.

It isn't only about winsock anymore, since you've also fixed, for example, errors in dsound.h.

Maybe something like:

Improve MinGW compatibility of mumble_pch.hpp.

This PR implements various changes to improve the compatibility of mumble_pch.hpp
with our MinGW build.

@davidebeatrici davidebeatrici changed the title mumble_pch.hpp: Include "winsock2.h" before "windows.h" mumble_pch.hpp: Improve MinGW compatibility. Mar 6, 2017

mumble_pch.hpp: Delete _WINSOCKAPI_ definition
The _WINSOCKAPI_ definition was needed when "winsock2.h" was being included after "windows.h", which checks if _WINSOCKAPI_ is defined to know if the header is included.
If it isn't, "winsock.h" (the previous version of winsock2.h) is included, which leads to a conflict and potential compilation fail, if the updated version of the header is included later.
mumble_pch.hpp: Include "mmreg.h" header
Fixes:
dsound.h(242): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
dsound.h(242): error C2143: syntax error: missing ';' before '*'
dsound.h(361): error C3646: 'lpwfxFormat': unknown override specifier
dsound.h(361): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
dsound.h(377): error C3646: 'lpwfxFormat': unknown override specifier
dsound.h(377): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
dsound.h(428): error C3646: 'lpwfxFormat': unknown override specifier
dsound.h(428): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
dsound.h(437): error C3646: 'lpwfxFormat': unknown override specifier
dsound.h(437): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
dsound.h(707): error C2061: syntax error: identifier 'LPWAVEFORMATEX'
dsound.h(718): error C2061: syntax error: identifier 'LPCWAVEFORMATEX'
dsound.h(793): error C2061: syntax error: identifier 'LPWAVEFORMATEX'
dsound.h(804): error C2061: syntax error: identifier 'LPCWAVEFORMATEX'
dsound.h(1066): error C2061: syntax error: identifier 'LPWAVEFORMATEX'
dsound.h(1125): error C2061: syntax error: identifier 'LPWAVEFORMATEX'

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mumble_pch.h-include-order branch from afc11f9 to cc00b21 Mar 6, 2017

@mkrautz
mkrautz approved these changes Mar 6, 2017

@davidebeatrici davidebeatrici merged commit 617975e into mumble-voip:master Mar 6, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.