Hello,
as a test, I've tried PVS-Studio on SFML (master) and, considering I did it correctly, there is not many things to say (which is a good thing).
- Most of the output was about c-style casts (mainly in glloader.cpp).
- Several "variable assigned twice", like:
sfogl_StrToExtMap *entry = NULL;
entry = FindExtEntry(extensionName);
- "The '(FT_RENDER_MODE_NORMAL)' named constant with the value of 0 is used in the bitwise operation. font.cpp:485"
- SoundSource has a copy constructor but no assignment operator.
- And a few about optimization, like : use !string.empty() instead of string != "".
- Last, an UB in stb_image.h. There are bitwise shifts on negative numbers.
That's all folks!
Except the missing operator=(), the rest can be "forgotten" since it's mainly syntax choices¹ (but why the double assignment?)
++
¹: I know C-casts are discouraged, but with C structs and function, the static_cast should "win".
- "The '(FT_RENDER_MODE_NORMAL)' named constant with the value of 0 is used in the bitwise operation. font.cpp:485"
How does that line look? Because in line 485 (master) (https://github.com/SFML/SFML/blob/master/src/SFML/Graphics/Font.cpp#L485), there are different constants:
if (FT_Load_Char(face, codePoint, FT_LOAD_TARGET_NORMAL | FT_LOAD_FORCE_AUTOHINT) != 0)
(but why the double assignment?)
The separate declaration and initialization is of course pointless, it can be fixed too.
Do you have all the occurrences in text format?
I guess now that I realize that there might be reason to look into it more, I might go ahead and fix up most of those warnings.
Don't we have to do everything again if we change something and regenerate GLLoader.cpp? If it doesn't take long, fine, but let's not waste our time on fixing generated files just for cosmetic reasons :P
As for null string testing, I'd rather keep it that way since it's more idiomatic. But does someone disagree?
Is it really more idiomatic? After all, the empty() method exists for only this purpose ;)