Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Cyberium
committed
Sep 17, 2015
1 parent
bb5dfcc
commit cf77ece
Showing
8 changed files
with
18 additions
and
11 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cf77ece
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.
About the CMake changes: I'm not in favor of hiding warnings, so if reasonably possible I would prefer to fix the actual code instead of setting up compiler flags to hide them.
The GCC changes don't really make sense to me. Why would you want to hide warnings for every architecture except (32-bit) x86? Am I detecting a quick copy/paste mistake there - the GCC file is very similar to the clang one, but not quite the same?
cf77ece
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.
omg i missed the "warning block"
About hiding its only about all case when we have something like
fread(..,size,.) and we dont realy need to check if size is same to the returned by function size.
cf77ece
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.
Thanks for providing some context about the warnings. I would assume we're using fread to actually read some data, in which case I'd say we do need to check the return value to catch any errors and make sure we got what we think we've read (or otherwise advance in the file as much as we expected if we don't care about the actual data).
I'll try reverting the flags locally and check how bad it would be to change the code.