{"id":397251,"date":"2024-06-29T13:20:04","date_gmt":"2024-06-29T13:20:04","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=397251"},"modified":"-0001-11-30T00:00:00","modified_gmt":"-0001-11-29T21:00:00","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=397251","title":{"rendered":"PVS-Studio searches for bugs in the DuckStation project<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
\n

We often check retro games. In our company, many developers like to find interesting projects for themselves. They feel nostalgic when they’re studying these projects. But we need to run retro games on something, right? This time we checked a project that helps to run old games on modern hardware.<\/p>\n

<\/p>\n

\"0881_duckstation\/image1.png\"<\/p>\n

<\/a> <\/p>\n

Introduction<\/h2>\n

<\/p>\n

DuckStation is an emulator of the Sony PlayStation console. The emulator, according to its website, has a version for Windows, Linux, and for Android smartphones. And recently it was launched on Xbox Series X and S. The project itself contains slightly less than a million lines of C and C++ code. DuckStation doesn’t release updates. Its developers regularly commit changes. So, we had to fixate the SHA of the commit: 13c5ee8<\/a><\/em>.<\/p>\n

<\/p>\n

We checked the project and found a lot of warnings \u2014 170 of the High level and 434 of the Medium level. Let’s look at the 10 most exciting of them.<\/p>\n

<\/p>\n

Check results<\/h2>\n

<\/p>\n

Warning N1<\/strong><\/p>\n

<\/p>\n

V726<\/a> An attempt to free memory containing the ‘wbuf’ array by using the ‘free’ function. This is incorrect as ‘wbuf’ was created on stack. log.cpp 216<\/p>\n

<\/p>\n

template<typename T> static ALWAYS_INLINE void FormatLogMessageAndPrintW(....) {   ....   wchar_t wbuf[512];   wchar_t* wmessage_buf = wbuf;   ....   if (wmessage_buf != wbuf)   {     std::free(wbuf);   }   if (message_buf != buf)   {     std::free(message_buf);   }   .... }<\/code><\/pre>\n
  <\/p>\n
Here the analyzer detected code with an error. In this code fragment, we see an attempt to delete an array allocated on the stack. Since the memory has not been allocated on the heap, you don’t need to call any special functions like std::free to clear it. When the object is destroyed, the memory is cleared automatically.<\/p>\n
  <\/p>\n
Also, when my colleague was editing this article, he considered this warning a false-positive. I described this interesting case in a separate article. So, I invite you to read it: How a PVS-Studio developer defended a bug in a checked project<\/a><\/em>.<\/p>\n
  <\/p>\n
Warning N2<\/strong><\/p>\n
  <\/p>\n
V547<\/a> Expression ‘i < pathLength’ is always true. file_system.cpp 454<\/p>\n
  <\/p>\n
void CanonicalizePath(const char *Path, ....) {   ....   u32 pathLength = static_cast<u32>(std::strlen(Path));   ....   for (i = 0; i < pathLength;)   {     ....     char nextCh = (i < pathLength) ? Path[i + 1] : '\\0';     ....   }   .... }<\/code><\/pre>\n
  <\/p>\n
The induction variable i<\/em> increases after the initialization of nextCh<\/em>. Judging by the fact that the strlen<\/em> function is used to determine the string length, the Path<\/em> string is null-terminated<\/a>. Then the i < pathLength<\/em> check is clearly redundant. You can skip the check since the condition will always be true. During the last loop iteration, we will get the null character anyway. Then the following code:<\/p>\n
  <\/p>\n
char nextCh = (i < pathLength) ? Path[i + 1] : '\\0';<\/code><\/pre>\n
  <\/p>\n
is the equivalent of:<\/p>\n
  <\/p>\n
char nextCh = Path[i + 1];<\/code><\/pre>\n
  <\/p>\n
However, even if the string wasn’t null-terminated, the check would be incorrect. During the final loop iteration, when trying to take the last character from Path[i + 1]<\/em>, you will get outside the array boundaries. In this case, the following code fragment would be better:<\/p>\n
  <\/p>\n
char nextCh = ((i + 1) < pathLength) ? Path[i + 1] : '\\0';<\/code><\/pre>\n
  <\/p>\n
Warnings N3, N4<\/strong><\/p>\n
  <\/p>\n
For this code snippet, the analyzer issued two warnings at once:<\/p>\n
  <\/p>\n