Some source code issues

[GiveOneMoreChance]

1. https://github.com/coelckers/gzdoom/blo ... s.cpp#L941
Uninitialized variable fbestdist

2. https://github.com/coelckers/gzdoom/blo ... ors.h#L651
Non-void function should return a value

3. https://github.com/coelckers/gzdoom/blo ... .cpp#L1226
Copypast ? box.right > 0 && box.right > 0

4. https://github.com/coelckers/gzdoom/blo ... w.cpp#L194
if (next_random == FUZZ_RANDOM_X_SIZE) always false

5. https://github.com/coelckers/gzdoom/blo ... e.cpp#L589
Expression always false

6. https://github.com/coelckers/gzdoom/blo ... e.cpp#L113
Expression always true

7. https://github.com/coelckers/gzdoom/blo ... .cpp#L9028
Expression always false

8.https://github.com/coelckers/gzdoom/blo ... T.cpp#L526
if (size >= 623), else if (size >= 68), else if (size >= 39)
Expression always false

9. https://github.com/coelckers/gzdoom/blo ... .cpp#L2836
Expression always true

10. https://github.com/coelckers/gzdoom/blo ... s.cpp#L198 and
https://github.com/coelckers/gzdoom/blo ... s.cpp#L208
Expressions always true

11. https://github.com/coelckers/gzdoom/blo ... .cpp#L5515 and
https://github.com/coelckers/gzdoom/blo ... .cpp#L6314
Expressions always true

12.https://github.com/coelckers/gzdoom/blo ... .cpp#L6348
Some part of expression always true: mobj

13.https://github.com/coelckers/gzdoom/blo ... .cpp#L6527
Some part of expression always true: !checker

[Edward-san]

Did you use a tool for this?

[GiveOneMoreChance]

I can check with PVS-studio and send Log file
This ten errors i found randomly tring adding IMGUI

[Rachael]

Why are you using different commit numbers for each and every line here? Most of these are more nitpicks than they are real problems. What is the purpose of this report?

[_mental_]

Those are commits when corresponding files were changed the last time.
And I wouldn’t say that it’s just a nitpicking. Most of them seem to be logic errors.

[Rachael]

Fixed a few

I am not touching the ones I either cannot follow, or seem to be safety bars, or seem to be compatibility hacks. For any of the latter two, I think it should be Graf, exclusively, who deals with those.

For example, the PCX thing, we could simplify the statement quite a bit by removing the check, but at the same time it may be there for a reason despite the check always being false.

[Graf Zahl]

Looking at those 3, I think you did one wrong and one not optimally.

If you want to set a double to the maximum, use DBL_MAX, not some hard coded constant.
And the third one should check height instead of the second 'width'.

The rest: Most should not be changed, unless the message indicates a missing pointer check outside the always true/false condition.

[dpJudas]

The one in r_draw.cpp should be changed to the following:

static int next_random = 0;

I must have been blind when coding the upscaled fuzz demon version for not noticing this bug.

[Rachael]

Looking at those 3, I think you did one wrong and one not optimally.

If you want to set a double to the maximum, use DBL_MAX, not some hard coded constant.
And the third one should check height instead of the second 'width'.

The rest: Most should not be changed, unless the message indicates a missing pointer check outside the always true/false condition.

Fixed

[Rachael]

The one in r_draw.cpp should be changed to the following:

static int next_random = 0;

I must have been blind when coding the upscaled fuzz demon version for not noticing this bug.

Done

[GiveOneMoreChance]

Good job guys!
I checked Gzdoom project in PVS-studio, here results, maybe it's useful info
https://www.dropbox.com/s/g8d3dgjiyznnn ... .html?dl=0

[dpJudas]

Thanks for trying to help.

Unfortunately we'd rather not have people spamming us with output from random tools unless it serves a very specific purpose. I quote from the code submission guidelines:

Code sanitizer/static analyzer is not an excuse to contribute

If you don't know what you're doing, do not use these tools. We typically find that people who use static analyzers as an excuse to contribute apply the wrong fix. Similarly reporting the errors to us isn't especially helpful as it can become a cat and mouse game. We're not saying they're bad tools, but use them in your own projects first.

[Rachael]

I realize you are trying to be helpful, but running a code analyzer is anything but. We have access to such tools, too, and will use them as needed. No one asked you for your PVS-studio report, and if they wanted it they would have asked.

For the most part I still feel like a lot of this stuff here is nit-picky - some of the code is the way it is for a reason, and while it is good to be aware that it is that way, it does not do us a lot of good with a project that has so many contributors. Believe it or not, some redundancy that you pointed out is actually needed.

In the future I would ask you bear this in mind - if you are going to run PVS-studio over GZDoom then you need to keep 3 things in mind:
a) First, you must be willing to change the code yourself.
b) Second, you must understand the reason why the code is the way it is. If changing it defeats the purpose, then we will not accept the change.
c) Lastly, you must also ensure that any changes made would not ultimately break anything. When running PVS-studio or something else that big which makes such a massive report, the room for error is just too significant in order to solve all the problems. It would take a combined effort of the 4 of us to fix all such issues, and - to put it lightly - "ain't nobody got time for that".

We welcome assistance, but any sort of "here's 2000 problems, gotta go bye!" is not what we're looking for. If you think you can whittle these problems down, you're welcome to make a pull request. But otherwise, I feel this report has been addressed to a satisfactory degree, and I am going to close it.