Fix lots of compile warnings #1838
Conversation
Only comment function if needed in the future.
…urity]. Fix identical to 957aeec
|
just wanted to say thanks, i can't test this right now but i think it's really important to keep the number of warnings down so we don't miss the real bugs :) |
|
Fixed some more warnings, and an error in glInfoExample, while I was at it. I'm done now, reviews please. Rest of the 33 warnings are either in addon libraries or I don't know what to do about them. A list is here, sorry for the formatting, raw mode might look better. |
|
btw, the first batch of commits has been checked by @benben's build server, and the Bad Line Count has already been reduced from ~3400 to 1900! |
|
Ignore that "Good to merge" code style message - it's "bled over" from my fork's testing PR. Doesn't mean the PR isn't mergeable, though. ;-) |
Conflicts: examples/addons/threadedImageLoaderExample/src/testApp.cpp
|
We're post-release now, and I just fixed a merge conflict. Could I get some reviews, please, so we can start the merge process asap? |
|
i just scrolled through all these changes for a thorough review and can confirm this is good to merge, with one exception. i can't test (or see why) this line changed: GtkWidget* dialog = gtk_message_dialog_new (NULL, (GtkDialogFlags) 0, GTK_MESSAGE_QUESTION, (GtkButtonsType) GTK_BUTTONS_OK_CANCEL, "%s", question.c_str() );but i'm going to trust you on that one. if we can get one more review then it's ready to merge. actually, i think if no one has time to review this it's worth merging anyway -- it's really just a bunch of trivial changes that shouldn't change anything except what the compiler thinks. |
|
Did you see the associated commit: 5ca3d45 - Fix format not a string literal and no format arguments [-Wformat-security]? (Hint: best way to find out the assoc. commit: Hit "view file at", then history or blame view.) From one of the comments:
|
| // 2.0 / 3.0 * x1 + 1.0 / 3.0 * x2, | ||
| // 2.0 / 3.0 * y1 + 1.0 / 3.0 * y2, | ||
| // y1, y2); | ||
| //} | ||
|
|
There was a problem hiding this comment.
curious why this was commented out?
There was a problem hiding this comment.
from 073ab2e: Fix warning about unused function helper_quadratic_to(). Only comment function if needed in the future.
The function is static, and thus only visible in its file, where it's not used. therefore it's safe to remove it. I commented it instead of removing - in case we find out we do need something like this in the future, it's easily available.
There was a problem hiding this comment.
great - okay, makes sense.
thanks for going through these all - yay, less yellow triangles! ;)
feel free to merge!
There was a problem hiding this comment.
yay, less yellow triangles! ;)
do you even see those? ^^ I thought warnings were disabled on XCode (and the build server seems to agree)?
|
@bilderbuchi - all looks good to me except but apart from the questions - the rest looks good! |
|
Merging, thanks for the review! |
Fix lots of compile warnings
|
too late i guess :) but you can even remove that function if you want it's not really needed it goes from quadratic to cubic but we don't have anything that does quadratic bezier. i mistakingly added it when doing the cairo renderer and then forgot to remove it when i realized it wasn't necessary |
|
ok. I made a note over in the deprecated code issue, for the next cleanup pass. |
This fixes loads of (obvious) warnings.
Primarily code in the examples has been touched. Some warnings have been fixed within addons and OF core.
Warnings in addon libraries (i.e. external code) have been ignored. Most of the obvious warnings (un/signed-compare and unused-variables) should be out of the way now, so we can concentrate on the non-trivial ones (e.g. some narrowing warnings).
Linux examples compile script has been adapted to ignore the recently added
osx*examples.Would be great to get some review!