How PVS-Studio Prevents Rash Code Changes

Andrey Karpov
Articles: 565

You probably know we've just published a brief article about CovidSim. However, now we have a good excuse to think back to that project and demonstrate how regular PVS-Studio use can be beneficial. When we are in a hurry, concentration may fail us, and we may commit changes too quickly. That's where a static analyzer can be of big help.

0817_covid_unreachable_code/image1.png

Everything started with these two brief notes about an open project, COVID-19 CovidSim Model:

Then we pondered a bit and decided to monitor this project. We intended to show how important regular static code analysis may be. My teammate wrote about this in more detail here.

As soon as we started checking this project regularly, we got the results we expected :). Right now I'll show you an error caused by recent code changes. Someone must have been in a hurry. Of course, further on, we will not describe every minor bug or code imperfection the analyzer detected. We'll focus on something more intriguing.

Here's what happened after the latest changes to the CovidSim.cpp file:

0817_covid_unreachable_code/image2.png

The author chose the heap to store arrays, instead of the stack. However, the author was inattentive when editing code. Note that the memory is released after the return operator:

int GetXMLNode(....)
{
  char* buf = new char[65536];
  char* CloseNode = new char[2048];
  char* CloseParent = new char[2048];
  ....
  if (ResetFilePos) fseek(dat, CurPos, 0);
  return ret;                                // <=
  delete[] buf;
  delete[] CloseNode;
  delete[] CloseParent;
}

As a result, we have unreachable code. And a memory leak.

Thank God PVS-Studio warns us at once: V779 Unreachable code detected. It is possible that an error is present. CovidSim.cpp 675

Use static analysis regularly so that you can fix many errors at the earliest stage! You probably agree that it's easier to quickly fix a minor bug than sit and ponder why an app suddenly started to consume too much RAM.

The last thing. This error would have been impossible if the developers gave up manual memory allocation and release in favor of the RAII approach and smart pointers.

Below is the correct and reliable code:

std::unique_ptr<char[]> buf(new char[65536]);
std::unique_ptr<char[]> CloseNode(new char[2048]);
std::unique_ptr<char[]> CloseParent(new char[2048]);

Thank you for reading. Follow me into the world of C++ and bugs :). Twitter. Facebook.



Use PVS-Studio to search for bugs in C, C++, C# and Java

We offer you to check your project code with PVS-Studio. Just one bug found in the project will show you the benefits of the static code analysis methodology better than a dozen of the articles.

goto PVS-Studio;

Andrey Karpov
Articles: 565


Bugs Found

Checked Projects
427
Collected Errors
14 526
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site. Learn More →
Accept