It's winter outside, the year is coming to an end, which means it's time to review the most notable errors the PVS-Studio analyzer detected in 2020. In the past year, we introduced many new diagnostic rules that detected these errors and placed them at the top. We've also enhanced the analyzer's core and added new use case scenarios. You can learn more about this in our . Let me remind you that our analyzer also supports C# and Java. Check out my colleagues' articles for more information on those languages. Now let's move on to the most memorable bugs PVS-Studio found in open source projects over the past year. blog No. 10. Modulo division by one The modulo by 1 operation is meaningless. The result will always be zero. llvm-stress.cpp 631 V1063 The developer intended to get a random value between 0 and 1 from a modulo operation. However, the operation of type always returns 0. In this case, it would be correct to rewrite the condition as follows: X%1 More information on this bug is available in the following article: " ". Checking Clang 11 with PVS-Studio No 9. Four checks After processing the code snippet below, PVS-Studio generated four warning messages: A part of conditional expression is always true: x >= 0. editor.cpp 1137 V560 A part of conditional expression is always true: y >= 0. editor.cpp 1137 V560 A part of conditional expression is always true: x < 40. editor.cpp 1137 V560 A part of conditional expression is always true: y < 30. editor.cpp 1137 V560 The last statement triggered all four warnings. The problem is that the statement performs four checks that always returns . I would call this bug amusing rather than major. These checks are redundant, and you can remove them. if true This error got here from the following article: VVVVVV??? VVVVVV!!! No 8. delete instead of delete[] The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] poke_data;'. CCDDE.CPP 410 V611 The analyzer detected that memory is freed in a way that is incompatible with how memory was allocated. To free the memory allocated for the array, use the operator instead of . delete[] delete For more information on this bug, check out the following article: " " The Code of the Command & Conquer Game: Bugs from the 90's. Volume two No. 7. Buffer overflow Let's take a look at the function. net_hostname_get The option from the branch is selected during preprocessing. The preprocessed file reflects this as follows: #else The function returns a pointer to a 7-byte array that contains the string and a null terminator. Now let's take a look at the code that produces the buffer overflow. PVS-Studio warning: [CWE-119] A call of the 'memcpy' function will lead to the 'net_hostname_get()' buffer becoming out of range. log_backend_net.c 114 V512 After preprocessing expands as follows: MAX_HOSTNAME_LEN When the data is copied, string literal overflow occurs. This causes undefined behavior. For more information on this bug, see " ". Checking the Code of Zephyr Operating System No. 6. Something super weird PVS-Studio warning: [CWE-628] The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. shell.c 427 V575 Here someone failed to emulate the function. strdup Let's start with the analyzer's warning. The analyzer reports that the function copied the string but didn't copy the null terminator. memcpy The following line of code seems to copy the null terminator: However, it does not. There is a typo here, and the null terminator is assigned to itself. Note that the value is recorded to the array instead of . As a result, the function returns a string that lacks the null terminator. mntpt cpy_mntpt mntpt_prepare We see that the programmer intended to write the statement below: However, there is still no reason to make the line so complex. Let's simplify the code: See " " for more details. Checking the Code of Zephyr Operating System No. 5. Meaningless overflow protection [CWE-570] Expression 'rel_wait < 0' is always false. Unsigned type value is never < 0. os_thread_windows.c 359 V547 In the code above, take a look at the variable. It is of the unsigned type. This means that the statement always returns FALSE and has no practical value. rel_wait DWORD rel_wait < 0 The error itself is ordinary. However, its fix is more intriguing. The developers simplified the code but failed to fix the bug. You can read the entire case in my colleague's article: " ". Why PVS-Studio Doesn't Offer Automatic Fixes For more details on this error, see the following article: " ". Static code analysis of the PMDK library collection by Intel and errors that are not actual errors No. 4. Don't expand std, bro Extending the 'std' namespace may result in undefined behavior. sized_iterator.hh 210 V1061 You can read more on this example and why this is a poor practice in the following article: " ". Checking the Code of DeepSpeech, or Why You Shouldn't Write in namespace std No. 3. The little scrollbar that could not There are identical sub-expressions to the left and to the right of the '-' operator: bufferHeight - bufferHeight TermControl.cpp 592 V501 This is what's called "history-dependent activation". In this case, the Windows Terminal failed to show its scrollbar because of an error. My colleague researched the bug and figured out what happened. Curious? Here's his article: " ". The Little Scrollbar That Could Not No. 2. Radius and height mixed up And once again we'll talk about the analyzer's several warnings: Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 791 V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 833 V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 884 V764 This is how the function is called: And this is its definition: You can see that when the developer called the function, the arguments were mixed up. Read more on this error in the following article: " " A Second Check of Newton Game Dynamics with PVS-Studio No. 1. Overwriting the result The 'color_name' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 621, 627. string.cpp 627 V519 The function above analyzes the color name with its transparency parameter and returns the color's hexadecimal code. If the string contains the transparency parameter, this parameter is split from the string and the color is recorded to the variable. Otherwise, the variable is assigned the original color string. color_name color_name The problem arises when the lowercase() function is called. The programmer passed the wrong parameter into this function. If the variable contains a substring of , then this substring will always be rewritten. Thus, we won't get what we expected from parseNamedColorString() function. color_name value This is how we can fix this line: For more details on this error see: " ". PVS-Studio: analyzing pull requests in Azure DevOps using self-hosted agents Conclusion Over the past year, we found many errors in open source projects. These were ordinary copy-paste bugs, incorrect constants, memory leaks, and many other problems. This year's Top 10 bugs include several ones detected by our new algorithms and prove that our analyzer keeps evolving. I hope you enjoyed reading my selection of memorable bugs as much as I enjoyed assembling this list. Of course, if you read our or looked through warning lists PVS-Studio produced after scanning open source projects, you may have your own Top-10. blog Previously published at https://www.viva64.com/en/b/0784/