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

"Fixed" Command line warning D9025 : overriding '/W3' with '/W4' #1

Closed
wants to merge 3 commits into from

Conversation

sgeto
Copy link

@sgeto sgeto commented Feb 14, 2018

Hi

/W3 is part of CMake's default compiler flags.
Adding /W4 the way you did won't overwrite it.
All samples are being built with /W3 instead of /W4 and /WX.
Appveyor will break with this PR now, because of the two warnings that are now treated as errors (/W4 and /WX are now working)

C:\FindWDK\samples\WdmDriver\Main.c(5): warning C4100: 'driverObject': unreferenced formal parameter
C:\FindWDK\samples\WdmDriver\Main.c(11): warning C4100: 'registryPath': unreferenced formal parameter

Your module has the same issue with /GR vs. /GR-
The former is the default and must be removed first.
The module should probably start things off with something similar than my foreach(WARN_FLAG) loop to remove all default flags before adding new ones.

Hi

/W3 is part of CMake's default compiler flags.
Adding /W4 the way you did won't overwrite it.
All samples are being built with /W3 instead of /W4 and /WX.
Appveyor will break with this PR now, because of the two warnings that are now treated as errors (/W4 and /WX are now working)

```
C:\FindWDK\samples\WdmDriver\Main.c(5): warning C4100: 'driverObject': unreferenced formal parameter
C:\FindWDK\samples\WdmDriver\Main.c(11): warning C4100: 'registryPath': unreferenced formal parameter
```
Your module has the same issue with /GR vs. /GR-
The former is the default and must be removed first.
The module should probably start things off with something similar than my foreach(WARN_FLAG) loop to remove all default flags before adding new ones.
@SergiusTheBest
Copy link
Owner

Hi,

Thanks for the pull request!

It's interesting how did you get warning D9025? My compiler doesn't say anything bad. I am expecting that the latest options override previous ones and it's a normal and documented behaviour. But now I'm puzzled that the warning D9025 exists.

Indeed the warning level wasn't set for the C compiler. I've added set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W4 /WX") and fixed the warnings.

@sgeto
Copy link
Author

sgeto commented Feb 15, 2018

It's interesting how did you get warning D9025? My compiler doesn't say anything bad.

Change your Generator to NMake Makefiles and define -DCMAKE_VERBOSE_MAKEFILE=1 to see all flags and warnings. MSBuild isn't capable of that. 👍

I've added set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W4 /WX") and fixed the warnings.

I would advise you to

  1. Make your module handle this (not CMakeLists.txt) since this file is only used by your project. Users will face this problem again.

  2. Why not overwriting/clearing all default flags before adding yours? In my opinion defining -DWIN32 or -DWINDOWS can cause problems when building a driver. And there's also the issue with /GR vs. /GR-

I really like this module. I can easily imagine it to become the de facto standard. Keep it up!

@SergiusTheBest
Copy link
Owner

The idea is not to force users to /W4 /WX and let them choose. Also replacing directly in CMAKE_C_FLAGS will affect all other projects. But the approach is good. I'll take a look how to limit it only to WDK projects.

@sgeto
Copy link
Author

sgeto commented Feb 17, 2018

Thanks. I'll get back to you if there's anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants