-
Notifications
You must be signed in to change notification settings - Fork 137
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
Enable suppressed unsigned warnings and fix offenders #229
Conversation
@@ -11,7 +11,10 @@ | |||
#ifndef NOMINMAX | |||
#define NOMINMAX | |||
#endif | |||
#ifdef FD_SETSIZE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not signed/unsigned related I just did this based on the windows build log from gitian.
So I forgot about some of those when writing the commit message. Debating if I should adjust it to accommodate the other entries. |
I can test Windows builds on a Windows OS. I also have Mac OS X but haven't taught myself building on that yet. And of course Linux qmake/make builds I can do. Will play around with it this weekend. |
Confirmed Windows binary building and running okay built using MinGW on a Windows OS. |
If we can confirm there are no ill effects on MAC from this I'd like to get it merged |
Still yet to get Mac builds working. Does @HyperCommNet want to test? You have Mac builds working right? |
This eliminates all non-compiler specific warnings when compiling on linux Compiler specific warnings include hardening related flags showing as unused in Clang and qt generated file prompting a warning when using GCC. Eliminates most warnings in linux-mingw makefile with the exception of boost related warnings and a windows header warning about the order includes are done in.
This has been rebased over master |
I want to break these up and have the different types of warning fixes in different commits to make any possible reverting easier. |
This is just me being curious, not me knowing any better but why do are you removing Also have you seen this bitcoin/bitcoin@f621326? It includes some further cleanups, is this something we could also look into. |
@MitchellCash yeah probably just forgot to update the mac makefiles as originally I was just doing signed/unsigned stuff. I'm going to redo this PR anyway. |
This eliminates all non-compiler specific warnings when compiling on linux
Compiler specific warnings include hardening related flags showing as unused
in Clang and qt generated file prompting a warning when using GCC.
Eliminates most warnings in linux-mingw makefile with the exception of boost
related warnings and a windows header warning about the order includes are
done in.
While this builds fine and shouldn't cause any issues I'd like people to play with it a bit before merging. Specifically windows builds. Also a OS-X build has not been attempted with these changes. @jwrb you think you could run an OS-X build of this for me?