Andrey Karpov

24 Mar 2021

Tags:

#Cpp

How PVS-Studio prevents rash code changes

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.

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

• Example of how new diagnostics appear in PVS-Studio
• COVID-19 research and uninitialized variable

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:

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.

#Cpp

278

0

5

0

SHARE

Tags:

#Cpp

We can email you a selection of our best articles once a month