When developers do make mistakes, it's often accidental or because the developers are in a hurry. These errors often make their way into small edits to the code. Let's review one of these cases: a developer fixes an error and introduces a new one simultaneously.
Actually, the image above already demonstrates everything. You do not even need to read any further :). However, I would still like to talk about what we see here, so that's what I'm going to do.
Back in 2021, I started monitoring Blender - an open-source project - for errors. I employed the PVS-Studio analyzer to scan it and find errors. Since Blender is developing at a quick pace, I was getting PVS-Studio's warnings to new code quite frequently. Unfortunately, I could not react to them in a timely manner, because I was busy with everything else. Lots of meetings, one article after the other - and some else in between. So I basically missed most of these warnings :( . As a result, during that year, I only posted a couple of notes about fresh errors - though I could have written more.
Yesterday I noticed a new message with two warnings. So, I thought, why not take a look? Especially since I had a couple of minutes. One warning was not all that interesting, while the second one was quite a find. It was definitely a sign for me to focus and write how PVS-Studio can detect errors if used regularly :).
So, @Antonioya committed two new lines that were intended to fix the following bug: Fix T94903: GPencil: Copying keys doesn't preserve Keyframe Type.
The developer was in a hurry and never noticed that the pointer he worked with could be null. The project's code contains a nullptr check that proves it:
gpf->key_type = gpfs->key_type;
if (gpf) {
In its turn, the PVS-Studio analyzer detected an anomaly and issued a warning: V595 [CWE-476]: The 'gpf' pointer was utilized before it was verified against nullptr. Check lines: 458, 459. editaction_gpencil.c
And that's a sure way to notice many mistakes and save time one'd spend fixing them. It's way easier for that same developer to come back to the code they wrote and fix it than for the company for fix this bug when it gets to QA or users.
P.S. First I wanted to simply name the article "How PVS-Studio prevents rash code changes", but then I discovered we already have an article with this name. So I added "example N2". I hope that with time we will write more and more of such articles. Thank you for your time - and have a shot at incorporating PVS-Studio into your development process!
Thanks for catch up the bug. Yes, I was fixing other bug and received this bug and fixed and did not notice the possible error. Really in this case the gpf pointer will have a valid value in 99,9999...% I have moved the code inside the `if`, but maybe, the null checking could be removed.
About compiler and warning tools, they are really great, but the problem with Visual Studio warning system mis that if you enabled it, you get hundreds or maybe more warnings, and the "real" warnings are missed between a lot of useless warnings, so it is not very practical.
I understand what you’re talking about. It can be difficult to enable more warnings in the compiler or start using a static code analyzer. All such tools inevitably generate false positives and it is sometimes unclear what to do with all of them.
However professional tools, such as PVS-Studio, differ favorably from compiler warnings or classic linters. These tools take into account such issues.
Firstly, PVS-Studio is not that "furious", it has a lot of settings for diagnostics.
Secondly, there are various ways to specifically suppress warnings, or, for example, mark macros as safe.
Thirdly, and most importantly, there is a thoughtful way to introduce the analyzer into the development process. There is a special mode when we leave technical debt for later and work only with warnings issued for new or changed code. Actually, this is how I noticed this error, and did not look through 100500 warnings :). How it all works is written in detail in the article "How to introduce a static code analyzer in a legacy project and not to discourage the team".
English
Français
340