# 100 bugs in Open Source C/C\+\+ projects This article demonstrates the capabilities of the static code analysis methodology\. Readers are offered the chance to study samples of one hundred errors, found in open\-source projects in C/C\+\+\. All the errors have been found using the PVS\-Studio static code analyzer\. > This article is out of date\\\. An updatable list of articles about the projects we have checked is \[here\]\(https://pvs\-studio\.com/en/blog/inspections/\)\\\. ![a0079_bugs-100/image1.png](https://import.viva64.com/docx/blog/a0079_bugs-100/image1.png) ## Introduction We won't tire you programmers by making you read texts, and will get on to the error samples right away\. Those who want to know what [static code analysis](https://pvs-studio.com/en/blog/terms/0046/) is, please follow the link\. Those who want to know what PVS\-Studio is, and download the trial version, see [this page](https://pvs-studio.com/en/pvs-studio/)\. Oh yes, one more thing\. Please see our post "[FAQ for those who have read our articles](https://pvs-studio.com/en/blog/posts/0132/)"\. ## Samples of errors detected in various open\-source projects The samples of detected errors will be divided into several groups\. **This division is rather relative**\. One and the same error can often be referred to as a misprint, and incorrect array handling, at the same time\. Of course, we have taken just a few errors from each of the projects\. If we described all the defects found, it would be a reference book\. This is the list of analyzed projects: * Apache HTTP Server \- [http://httpd\.apache\.org/](http://httpd.apache.org/) * Audacity \- [http://audacity\.sourceforge\.net/](https://sourceforge.net/projects/audacity/) * Chromium \- [http://www\.chromium\.org/](http://www.chromium.org/) * Clang \- [http://clang\-analyzer\.llvm\.org/](http://clang-analyzer.llvm.org/) * CMake \- [http://www\.cmake\.org/](https://cmake.org/) * Crystal Space 3D SDK \- [http://www\.crystalspace3d\.org/main/Main\_Page](http://www.crystalspace3d.org/main/Main_Page) * eMule Plus \- [http://emuleplus\.info/](https://emuleplus.info/) * FAR Manager \- [http://www\.farmanager\.com/](https://www.farmanager.com/) * FCE Ultra \- [http://fceux\.com/web/home\.html](http://fceux.com/web/home.html) * Fennec Media Project \- [http://fennec\.sourceforge\.net/](http://fennec.sourceforge.net/) * G3D Content Pak \- [http://sourceforge\.net/projects/g3d\-cpp/](https://sourceforge.net/projects/g3d-cpp/) * IPP Samples \- [https://software\.intel\.com/en\-us/articles/code\-samples\-for\-intel\-integrated\-performance\-primitives\-library](https://software.intel.com/content/www/us/en/develop/home.html) * Lugaru \- [http://www\.wolfire\.com/lugaru](http://www.wolfire.com/lugaru) * Miranda IM \- [https://www\.miranda\-ng\.org/en/](https://www.miranda-ng.org/en/) * MySQL \- [http://www\.mysql\.com/](https://www.mysql.com/) * Newton Game Dynamics \- [http://newtondynamics\.com/forum/newton\.php](http://newtondynamics.com/forum/newton.php) * Notepad\+\+ \- [http://notepad\-plus\-plus\.org/](https://notepad-plus-plus.org/) * Pixie \- [http://www\.renderpixie\.com/](http://www.renderpixie.com/) * PNG library \- [http://libpng\.org/pub/png/](http://libpng.org/pub/png/) * Qt \- [https://www\.qt\.io/](https://www.qt.io/) * ReactOS \- [http://www\.reactos\.org/](https://reactos.org/) * Shareaza \- [http://shareaza\.sourceforge\.net/](http://shareaza.sourceforge.net/) * SMTP Client with SSL/TLS \- [http://www\.codeproject\.com/KB/IP/smtp\_ssl\.aspx](https://www.codeproject.com/articles/98355/smtp-client-with-ssl-tls) * StrongDC\+\+ \- [http://strongdc\.sourceforge\.net/index\.php?lang\=eng](http://strongdc.sourceforge.net/index.php?lang=eng) * Swiss\-Army Knife of Trace \- [http://www\.codeproject\.com/KB/trace/tracetool\.aspx](https://www.codeproject.com/articles/5498/tracetool-the-swiss-army-knife-of-trace) * TortoiseSVN \- [http://tortoisesvn\.net/](https://tortoisesvn.net/) * Ultimate TCP/IP \- [http://www\.codeproject\.com/KB/MFC/UltimateTCPIP\.aspx](https://www.codeproject.com/articles/20181/the-ultimate-tcp-ip-home-page) * VirtualDub \- [http://www\.virtualdub\.org/](http://www.virtualdub.org/) * WinMerge \- [http://winmerge\.org/](https://winmerge.org/) * Wolfenstein 3D \- [http://en\.wikipedia\.org/wiki/Wolfenstein\_3D](https://en.wikipedia.org/wiki/wolfenstein_3d) * Quake\-III\-Arena \- [https://github\.com/id\-Software/Quake\-III\-Arena](https://github.com/id-Software/Quake-III-Arena) * And some others\. ### Array errors and string handling Array errors, and string handling, are the largest class of defects in C/C\+\+ programs\. This is the price for the capability of effective low\-level memory handling available to programmers\. In the article we will show just a small part of these errors, found by the PVS\-Studio analyzer\. But we think any C/C\+\+ programmer understands how numerous and insidious they are\. **Example** **1\.** Wolfenstein 3D project\. Only part of an object is cleared\. ```cpp void CG_RegisterItemVisuals( int itemNum ) { ... itemInfo_t *itemInfo; ... memset( itemInfo, 0, sizeof( &itemInfo ) ); ... } ``` This error was found through the [V568](https://pvs-studio.com/en/docs/warnings/v568/) diagnostic: It's odd that the argument of sizeof\(\) operator is the '&itemInfo' expression\. cgame cg\_weapons\.c 1467\. The sizeof\(\) operator calculates the size of the pointer instead of the 'itemInfo\_t' structure's size\. It is "sizeof\(\*itemInfo\)" that must be written\. **Example** **2\.** Wolfenstein 3D project\. Only part of a matrix is cleared\. ```cpp ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) { memcpy( mat, src, sizeof( src ) ); } ``` This error was found through the [V511](https://pvs-studio.com/en/docs/warnings/v511/): The sizeof\(\) operator returns size of the pointer, and not of the array, in 'sizeof\(src\)' expression\. Splines math\_matrix\.h 94 Usually programmers expect 'sizeof\(src\)' to return the size of an array equal to "3\*3\*sizeof\(float\)" bytes\. But according to the language standard, 'src' is just a pointer, not an array\. Thus, the matrix will only be partly copied\. The 'memcpy' function will copy 4 or 8 bytes \(the pointer size\), depending on whether the code is 32\-bit or 64\-bit\. If you want the whole matrix to be copied, you may pass a reference to the array into the function\. This is the correct code: ```cpp ID_INLINE mat3_t::mat3_t( float (&src)[3][3] ) { memcpy( mat, src, sizeof( src ) ); } ``` **Example** **3\.** FAR Manager project\. Only part of an array is cleared\. ```cpp struct TreeItem { int *Last; size_t LastCount; ... void Clear() { strName.Clear(); memset(Last, 0, sizeof(Last)); Depth=0; } }; ``` This error was found through the [V579](https://pvs-studio.com/en/docs/warnings/v579/): diagnostic The memset function receives the pointer and its size as arguments\. It is probably a mistake\. Inspect the third argument\. far treelist\.hpp 66 Most likely, there is a missing operation of multiplication by the number of items being cleared, and the code should look as follows: "memset\(Last, 0, LastCount \* sizeof\(\*Last\)\);"\. **Example** **4\.** ReactOS project\. Incorrect calculation of a string length\. ```cpp static const PCHAR Nv11Board = "NV11 (GeForce2) Board"; static const PCHAR Nv11Chip = "Chip Rev B2"; static const PCHAR Nv11Vendor = "NVidia Corporation"; BOOLEAN IsVesaBiosOk(...) { ... if (!(strncmp(Vendor, Nv11Vendor, sizeof(Nv11Vendor))) && !(strncmp(Product, Nv11Board, sizeof(Nv11Board))) && !(strncmp(Revision, Nv11Chip, sizeof(Nv11Chip))) && (OemRevision == 0x311)) ... } ``` This error was found through the [V579](https://pvs-studio.com/en/docs/warnings/v579/) diagnostic: The strncmp function receives the pointer and its size as arguments\. It is probably a mistake\. Inspect the third argument\. vga vbe\.c 57 Calls of the 'strncmp' function in this code compare only the first several characters, not whole strings\. The error here is as follows: the sizeof\(\) operator, is used to calculate string lengths, which is absolutely inappropriate in this situation\. The sizeof\(\) operator actually calculates the pointer size instead of the number of bytes in a string\. What is most unpleasant and insidious about this error, is that this code almost works as intended\. In 99% of cases, comparison of the first several characters is enough\. But the remaining 1% can bring you much 'fun' and long hours of debugging\. **Example** **5\.** VirtualDub project\. Array overrun \(explicit index\)\. ```cpp struct ConvoluteFilterData { long m[9]; long bias; void *dyna_func; DWORD dyna_size; DWORD dyna_old_protect; BOOL fClip; }; static unsigned long __fastcall do_conv( unsigned long *data, const ConvoluteFilterData *cfd, long sflags, long pit) { long rt0=cfd->m[9], gt0=cfd->m[9], bt0=cfd->m[9]; ... } ``` This code was found through the [V557](https://pvs-studio.com/en/docs/warnings/v557/) diagnostic: Array overrun is possible\. The '9' index is pointing beyond array bound\. VirtualDub f\_convolute\.cpp 73 It is not a real error, but good diagnostic\. Explanation: [http://www\.virtualdub\.org/blog/pivot/entry\.php?id\=359](http://www.virtualdub.org/blog/pivot/entry.php?id=359)\. **Example** **6\.** CPU Identifying Tool project\. Array overrun \(index in a macro\)\. ```cpp #define FINDBUFFLEN 64 // Max buffer find/replace size ... int WINAPI Sticky (...) { ... static char findWhat[FINDBUFFLEN] = {'\0'}; ... findWhat[FINDBUFFLEN] = '\0'; ... } ``` This error was found through the [V557](https://pvs-studio.com/en/docs/warnings/v557/) diagnostic: Array overrun is possible\. The '64' index is pointing beyond array bound\. stickies stickies\.cpp 7947 This error is similar to the previous one\. The terminal null is written outside the array\. The correct code is: "findWhat\[FINDBUFFLEN \- 1\] \= '\\0';"\. **Example** **7\.** Wolfenstein 3D project\. Array overrun \(incorrect expression\)\. ```cpp typedef struct bot_state_s { ... char teamleader[32]; //netname of the team leader ... } bot_state_t; void BotTeamAI( bot_state_t *bs ) { ... bs->teamleader[sizeof( bs->teamleader )] = '\0'; ... } ``` This error was found through the [V557](https://pvs-studio.com/en/docs/warnings/v557/) diagnostic: Array overrun is possible\. The 'sizeof \(bs\-\>teamleader\)' index is pointing beyond array bound\. game ai\_team\.c 548 Here is one more example of an array overrun when using an explicitly declared index\. These samples show that errors which are so simple at first sight, are much more widely\-spread than it may seem\. The terminal null is written outside the 'teamleader' array\. This is the correct code: ```cpp bs->teamleader[ sizeof(bs->teamleader) / sizeof(bs->teamleader[0]) - 1 ] = '\0'; ``` **Example** **8\.** Miranda IM project\. Only part of a string is copied\. ```cpp typedef struct _textrangew { CHARRANGE chrg; LPWSTR lpstrText; } TEXTRANGEW; const wchar_t* Utils::extractURLFromRichEdit(...) { ... ::CopyMemory(tr.lpstrText, L"mailto:", 7); ... } ``` This error was found through the [V512](https://pvs-studio.com/en/docs/warnings/v512/) diagnostic: A call of the 'memcpy' function will lead to a buffer overflow or underflow\. tabsrmm utils\.cpp 1080 If Unicode\-strings are used, one character occupies 2 or 4 bytes \(depending on the data model being used in compiler\), instead of one byte\. Unfortunately, programmers easily forget about this, and you can often see defects like our example, in programs\. The 'CopyMemory' function will copy only part of the L"mailto:" string since it handles bytes, not characters\. You can fix the code by using a more appropriate function for string copying or, at least, multiplying number 7 by sizeof\(wchar\_t\)\. **Example** **9\.** CMake project\. Array overrun inside a loop\. ```cpp static const struct { DWORD winerr; int doserr; } doserrors[] = { ... }; static void la_dosmaperr(unsigned long e) { ... for (i = 0; i < sizeof(doserrors); i++) { if (doserrors[i].winerr == e) { errno = doserrors[i].doserr; return; } } ... } ``` This error was found through the [V557](https://pvs-studio.com/en/docs/warnings/v557/) diagnostic: Array overrun is possible\. The value of 'i' index could reach 367\. cmlibarchive archive\_windows\.c 1140, 1142 The error handler itself contains an error\. The sizeof\(\) operator returns the array size in bytes, and not the number of items inside it\. As a result, the program will try to search much more items than it should in the loop\. This is the correct loop: ```cpp for (i = 0; i < sizeof(doserrors) / sizeof(*doserrors); i++) ``` **Example** **10\.** CPU Identifying Tool project\. A string is printed into itself\. ```cpp char * OSDetection () { ... sprintf(szOperatingSystem, "%sversion %d.%d %s (Build %d)", szOperatingSystem, osvi.dwMajorVersion, osvi.dwMinorVersion, osvi.szCSDVersion, osvi.dwBuildNumber & 0xFFFF); ... sprintf (szOperatingSystem, "%s%s(Build %d)", szOperatingSystem, osvi.szCSDVersion, osvi.dwBuildNumber & 0xFFFF); ... } ``` This error was found through the [V541](https://pvs-studio.com/en/docs/warnings/v541/) diagnostic: It is dangerous to print the string 'szOperatingSystem' into itself\. stickies camel\.cpp 572, 603 An attempt at formatted printing of a string into itself can lead to faults\. The result of executing this code depends on the input data, and you cannot predict what will happen\. Most likely, the result will be a meaningless string or an [Access Violation](https://pvs-studio.com/en/blog/terms/0063/) will occur\. This error can be referred to the category of "code vulnerabilities"\. In some programs, by feeding special data to code, you can exploit such code fragments to cause a buffer overflow, or other effects an intruder needs\. **Example** **11\.** FCE Ultra project\. A string gets less memory than needed\. ```cpp int FCEUI_SetCheat(...) { ... if((t=(char *)realloc(next->name,strlen(name+1)))) ... } ``` This error was found through the [V518](https://pvs-studio.com/en/docs/warnings/v518/) diagnostic: The 'realloc' function allocates strange amount of memory calculated by 'strlen\(expr\)'\. Perhaps the correct variant is 'strlen\(expr\) \+ 1'\. fceux cheat\.cpp 609 This error is caused by a misprint\. It is the 'name' pointer instead of the "name\+1" expression that must be the argument of the strlen\(\) function\. As a result, the realloc function allocates 2 bytes less memory than needed: one byte is lost because 1 is not added to the string length; another byte is lost because the 'strlen' function calculates the string length skipping the first character\. **Example** **12\.** Notepad\+\+ project\. Partial array clearing\. ```cpp #define CONT_MAP_MAX 50 int _iContMap[CONT_MAP_MAX]; ... DockingManager::DockingManager() { ... memset(_iContMap, -1, CONT_MAP_MAX); ... } ``` This error was found through the [V512](https://pvs-studio.com/en/docs/warnings/v512/) diagnostic: A call of the memset function will lead to a buffer overflow or underflow\. notepadPlus DockingManager\.cpp 60 This is one more example of how the number of array items is mixed up with an array size\. A multiplication by sizeof\(int\) is missing\. We can go on and on showing you errors in array handling which we have found in various programs\. But we have to stop somewhere\. ### Undefined behavior A bit of theory first\. Undefined behavior is a property of certain programming languages \(most prominent in C and C\+\+\) wherein certain situations, a result is produced which depends on compiler implementation, or specified optimization switches\. In other words, the specification does not define the language's behavior in any possible situation but says: "at A condition, the result of B operation is undefined"\. It is considered a mistake to allow such a situation in your program, even if it is executed well by some particular compiler\. Such a program will not be cross platform and may cause failures on a different computer, operating system, and even within different compiler settings\. A sequence point defines any point in a computer program's execution, at which it is guaranteed that all side effects of previous evaluations will have been performed, and no side effects from subsequent evaluations have yet been performed\. To learn more about sequence points and cases of undefined behavior related to sequence points, see [this post](https://pvs-studio.com/en/blog/terms/0065/)\. **Example** **1\.** Chromium project\. Incorrect use of smart pointer\. ```cpp void AccessibleContainsAccessible(...) { ... auto_ptr child_array(new VARIANT[child_count]); ... } ``` This error was found through the [V554](https://pvs-studio.com/en/docs/warnings/v554/) diagnostic: Incorrect use of auto\_ptr\. The memory allocated with 'new \[\]' will be cleaned using 'delete'\. interactive\_ui\_tests accessibility\_win\_browsertest\.cc 171 This example demonstrates a case where using a smart pointer can cause undefined behavior\. It may be expressed through heap damage, program crash, incomplete object destruction or any other failure\. The error is as follows: memory is allocated by the new \[\] operator, and released by the delete operator in the 'auto\_ptr' class' destructor: ```cpp ~auto_ptr() { delete _Myptr; } ``` To fix these issues, you should use a more appropriate class, for instance, boost::scoped\_array\. **Example** **2\.** IPP Samples project\. Classic Undefined behavior\. ```cpp template void HadamardFwdFast(...) { Ipp32s *pTemp; ... for(j=0;j<4;j++) { a[0] = pTemp[0*4] + pTemp[1*4]; a[1] = pTemp[0*4] - pTemp[1*4]; a[2] = pTemp[2*4] + pTemp[3*4]; a[3] = pTemp[2*4] - pTemp[3*4]; pTemp = pTemp++; ... } ... } ``` This error was found through the [V567](https://pvs-studio.com/en/docs/warnings/v567/) diagnostic: Undefined behavior\. The 'pTemp' variable is modified while being used twice between sequence points\. me umc\_me\_cost\_func\.h 168 This is a classic example of undefined program behavior\. It is this construct which is used to demonstrate Undefined behavior in various articles\. It is unknown whether 'pTemp' will be incremented by one or not\. Two actions of changing pTemp variable's value are located in one sequence point\. It means that the compiler may create the following code: pTemp \= pTemp \+ 1; pTemp \= pTemp; Or it may create another version of the code: TMP \= pTemp; pTemp \= pTemp \+ 1; pTemp \= TMP; Which of the two code versions will be created depends on the compiler and optimization switches\. **Example** **3\.** Fennec Media Project project\. Complex expression\. ```cpp uint32 CUnBitArrayOld::DecodeValueRiceUnsigned(uint32 k) { ... while (!(m_pBitArray[m_nCurrentBitIndex >> 5] & Powers_of_Two_Reversed[m_nCurrentBitIndex++ & 31])) {} ... } ``` This error was found through the [V567](https://pvs-studio.com/en/docs/warnings/v567/) diagnostic: Undefined behavior\. The 'm\_nCurrentBitIndex' variable is modified while being used twice at single sequence point\. MACLib unbitarrayold\.cpp 78 There are no sequence points between two instances of using the 'm\_nCurrentBitIndex' variable\. It means that the standard does not specify the moment when this variable is incremented\. Correspondingly, this code may work differently depending on the compiler and optimization switches\. **Example** **4\.** Miranda IM project\. Complex expression\. ```cpp short ezxml_internal_dtd(ezxml_root_t root, char *s, size_t len) { ... while (*(n = ++s + strspn(s, EZXML_WS)) && *n != '>') { ... } ``` This error was found through the [V567](https://pvs-studio.com/en/docs/warnings/v567/) diagnostic: Undefined behavior\. The 's' variable is modified while being used twice between sequence points\.msne zxml\.c 371 Prefix increment of the variable is used here\. But it does not mean anything: it cannot be guaranteed that the 's' variable will be incremented before calling the strspn\(\) function\. ### Errors relating to operation priorities To make the understanding of these examples easier, let's recall the [operation priorities](https://pvs-studio.com/en/blog/terms/0064/) table\. **Example** **1\.** MySQL project\. Priorities of \! and & operations\. ```cpp int ha_innobase::create(...) { ... if (srv_file_per_table && !mysqld_embedded && (!create_info->options & HA_LEX_CREATE_TMP_TABLE)) { ... } ``` This error was found through the [V564](https://pvs-studio.com/en/docs/warnings/v564/) diagnostic: The '&' operator is applied to the bool type value\. You've probably forgotten to include parentheses, or intended to use the '&&' operator\. innobase ha\_innodb\.cc 6789 The programmer wanted a part of the expression to check that a certain bit in the 'create\_info\-\>options' variable is equal to zero\. But the priority of the '\!' operation is higher than that of the '&' operation, this is why the expression works by this algorithm: ```cpp ((!create_info->options) & HA_LEX_CREATE_TMP_TABLE) ``` We should use additional parentheses if we want the code to work properly: ```cpp (!(create_info->options & HA_LEX_CREATE_TMP_TABLE)) ``` Or, what we find nicer, write the code in the following way: ```cpp ((create_info->options & HA_LEX_CREATE_TMP_TABLE) == 0) ``` **Example** **2\.** eMule Plus project\. Priorities of \* and \+\+ operations\. ```cpp STDMETHODIMP CCustomAutoComplete::Next(..., ULONG *pceltFetched) { ... if (pceltFetched != NULL) *pceltFetched++; ... } ``` This error was found through the [V532](https://pvs-studio.com/en/docs/warnings/v532/) diagnostic: Consider inspecting the statement of '\*pointer\+\+' pattern\. What was probably meant: '\(\*pointer\)\+\+'\. emule customautocomplete\.cpp 277 If 'pceltFetched' is not a null pointer, the function must increment the variable of the ULONG type this pointer refers to\. The error is as follows: the priority of the '\+\+' operation is higher than that of '\*' operation \(pointer dereferencing\)\. The "\*pceltFetched\+\+;" line is identical to the following code: ```cpp TMP = pceltFetched + 1; *pceltFetched; pceltFetched = TMP; ``` Virtually it is just increment of the pointer\. To make the code correct, we must add parentheses: "\(\*pceltFetched\)\+\+;"\. **Example** **3\.** Chromium project\. Priorities of & and \!\= operations\. ```cpp #define FILE_ATTRIBUTE_DIRECTORY 0x00000010 bool GetPlatformFileInfo(PlatformFile file, PlatformFileInfo* info) { ... info->is_directory = file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0; ... } ``` This error was found through the [V564](https://pvs-studio.com/en/docs/warnings/v564/) diagnostic: The '&' operator is applied to the bool type value\. You've probably forgotten to include parentheses or intended to use the '&&' operator\. base platform\_file\_win\.cc 216 Programmers easily forget that the priority of the '\!\=' operation is higher than that of '&'\. This is what happened in our case\. As a result, we have the following expression: ```cpp info->is_directory = file_info.dwFileAttributes & (0x00000010 != 0); ``` Let's simplify the expression: ```cpp info->is_directory = file_info.dwFileAttributes & (true); ``` Let's simplify it once again: ```cpp info->is_directory = file_info.dwFileAttributes & 1; ``` It turns out that we have tested the first bit instead of the fifth bit\. To fix this, we need to add parentheses\. **Example** **4\.** BCmenu project\. IF and ELSE mixed up\. ```cpp void BCMenu::InsertSpaces(void) { if(IsLunaMenuStyle()) if(!xp_space_accelerators) return; else if(!original_space_accelerators) return; ... } ``` This error was found through the [V563](https://pvs-studio.com/en/docs/warnings/v563/) diagnostic: It is possible that this 'else' branch must apply to the previous 'if' statement\. fire bcmenu\.cpp 1853 This is not an error of operation priorities, but one related to it\. The programmer did not take into account that the 'else' branch refers to the nearest 'if' operator\. We can see the code justification, as if it works like the following algorithm: ```cpp if(IsLunaMenuStyle()) { if(!xp_space_accelerators) return; } else { if(!original_space_accelerators) return; } ``` But actually it is equivalent to the following construct: ```cpp if(IsLunaMenuStyle()) { if(!xp_space_accelerators) { return; } else { if(!original_space_accelerators) return; } } ``` **Example** **5\.** IPP Samples project\. Priorities of ?: and \| operations\. ```cpp vm_file* vm_file_fopen(...) { ... mds[3] = FILE_ATTRIBUTE_NORMAL | (islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING; ... } ``` This error was found through the [V502](https://pvs-studio.com/en/docs/warnings/v502/) diagnostic: Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\|' operator\. vm vm\_file\_win\.c 393 Depending on the 'islog' variable's value, the expression must be either equal to "FILE\_ATTRIBUTE\_NORMAL" or "FILE\_ATTRIBUTE\_NORMAL \| FILE\_FLAG\_NO\_BUFFERING"\. But it does not happen\. Priority of the '?:' operation is lower than that of '\|'\. As a result, the code acts as follows: ```cpp mds[3] = (FILE_ATTRIBUTE_NORMAL | (islog == 0)) ? 0 : FILE_FLAG_NO_BUFFERING; ``` Let's simplify the expression: ```cpp mds[3] = (0x00000080 | ...) ? 0 : FILE_FLAG_NO_BUFFERING; ``` Since FILE\_ATTRIBUTE\_NORMAL equals 0x00000080, the condition is always true\. It means that 0 will always be written into mds\[3\]\. **Example** **6\.** Newton Game Dynamics project\. Priorities of ?: and \* operations\. ```cpp dgInt32 CalculateConvexShapeIntersection (...) { ... den = dgFloat32 (1.0e-24f) * (den > dgFloat32 (0.0f)) ? dgFloat32 (1.0f) : dgFloat32 (-1.0f); ... } ``` This error was found through the [V502](https://pvs-studio.com/en/docs/warnings/v502/) diagnostic: Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\*' operator\. physics dgminkowskiconv\.cpp 1061 The error in this code again relates to the low priority of the '?:' operation\. The condition for the '?:' operator is expressed by a meaningless subexpression "dgFloat32 \(1\.0e\-24f\) \* \(den \> dgFloat32 \(0\.0f\)\)"\. Adding parentheses will solve the issue\. By the way, programmers often forget how cunning the '?:' operator is\. Here is a post on this topic: "How to make fewer errors at the stage of code writing\. [Part N2](https://pvs-studio.com/en/blog/posts/cpp/a0072/)"\. ### Formatted output errors Examples of these errors are boring and all pretty similar, so we will examine only a few samples\. The point is that functions with a variable number of arguments accept actual arguments incompatible with the format string\. Any programmer who uses such functions as printf\(\) is familiar with this type of error\. **Example** **1\.** ReactOS project\. Incorrect printing of a WCHAR\-character\. ```cpp static void REGPROC_unescape_string(WCHAR* str) { ... default: fprintf(stderr, "Warning! Unrecognized escape sequence: \\%c'\n", str[str_idx]); ... } ``` This error was found through the [V576](https://pvs-studio.com/en/docs/warnings/v576/) diagnostic: Incorrect format\. Consider checking the third actual argument of the 'fprintf' function\. The char type argument is expected\. regedit regproc\.c 293 The fprinf\(\) function must print a character of the char type\. But the third argument is a character of the WCHAR type\. The user will get an incorrectly generated message\. To fix the code, we should replace '%c' with '%C' in the format string\. **Example** **2\.** Intel AMT SDK project\. Character '%' missing\. ```cpp void addAttribute(...) { ... int index = _snprintf(temp, 1023, "%02x%02x:%02x%02x:%02x%02x:%02x%02x:" "%02x%02x:02x%02x:%02x%02x:%02x%02x", value[0],value[1],value[2],value[3],value[4], value[5],value[6],value[7],value[8], value[9],value[10],value[11],value[12], value[13],value[14],value[15]); ... } ``` This error was found through the [V576](https://pvs-studio.com/en/docs/warnings/v576/) diagnostic: Incorrect format\. A different number of actual arguments is expected while calling '\_snprintf' function\. Expected: 18\. Present: 19\. mod\_pvs mod\_pvs\.cpp 308 It is not easy to find an error here at first glance\. However, the PVS\-Studio analyzer does not get tired, and notices that the function takes more actual arguments than specified in the format string\. The reason is that the '%' character is missing in one place\. Let's single out this fragment: ```cpp "%02x%02x:[HERE]02x%02x:%02x%02x:%02x%02x", ``` **Example** **3\.** Intel AMT SDK project\. Unused argument\. ```cpp bool GetUserValues(...) { ... printf("Error: illegal value. Aborting.\n", tmp); return false; } ``` This error was found through the [V576](https://pvs-studio.com/en/docs/warnings/v576/) diagnostic: Incorrect format\. A different number of actual arguments is expected while calling 'printf' function\. Expected: 1\. Present: 2\. RemoteControlSample remotecontrolsample\.cpp 792 The error is as follows: the 'tmp' variable is not used in any way when printing the information message\. **Example** **4\.** G3D Content Pak project\. Printing of meaningless data\. ```cpp class Matrix3 { ... inline float* operator[] (int iRow) { ... }; void AnyVal::serialize(G3D::TextOutput& t) const { ... const Matrix3& m = *(Matrix3*)m_value; ... t.printf("%10.5f, %10.5f, %10.5f,\n %10.5f, %10.5f, %10.5f,\n %10.5f, %10.5f, %10.5f)", m[0, 0], m[0, 1], m[0, 2], m[1, 0], m[1, 1], m[1, 2], m[2, 0], m[2, 1], m[2, 2]); ... } ``` This error was found through the [V520](https://pvs-studio.com/en/docs/warnings/v520/) diagnostic: The comma operator ',' in array index expression '\[0, 0\]'\. graphics3D anyval\.cpp 275 The program prints meaningless values instead of the matrix\. You may write such a code when you work with different programming languages, and sometimes forget how to access an item in a two\-dimensional array in the C language\. Let's see how the 'm\[0, 1\]' expression works\. At first, expression"0, 1" is calculated\. The result of this expression is 1\. Then the 'operator\[\]' function is called in the Matrix3 class\. The function takes the actual argument 1, and returns the pointer to the first string in the matrix\. It is the value of this pointer that will be printed by the 'printf\(\)' function though it expects a value of the float\-type\. This is the correct code: ```cpp t.printf("%10.5f, %10.5f, %10.5f,\n %10.5f, %10.5f, %10.5f,\n %10.5f, %10.5f, %10.5f)", m[0][0], m[0][1], m[0][2], m[1][0], m[1][1], m[1][2], m[2][0], m[2][1], m[2][2]); ``` ### Examples of misprints found in code A lot of programming errors are caused by misprints\. Most of these errors are quickly detected at the early stages of testing\. But there are some defects of this kind that remain in code for a long time, causing troubles both to programmers and users\. You can decrease the frequency of these errors using the PVS\-Studio analyzer\. It will find them before testing starts, which will significantly reduce the cost of defect detection and elimination\. **Example** **1\.** Miranda IM project\. Assignment inside IF\. ```cpp void CIcqProto::handleUserOffline(BYTE *buf, WORD wLen) { ... else if (wTLVType = 0x29 && wTLVLen == sizeof(DWORD)) ... } ``` This error was found through the [V560](https://pvs-studio.com/en/docs/warnings/v560/) diagnostic: A part of conditional expression is always true: 0x29\. icqoscar8 fam\_03buddy\.cpp 632 Because of a misprint, there is an assignment taking place inside the condition of the 'if' operator\. This is the correct condition: "if \(wTLVType \=\= 0x29 && wTLVLen \=\= sizeof\(DWORD\)\)"\. **Example** **2\.** ReactOS project\. Assignment error\. ```cpp BOOL WINAPI GetMenuItemInfoA(...) { ... mii->cch = mii->cch; ... } ``` This error was found through the [V570](https://pvs-studio.com/en/docs/warnings/v570/) diagnostic: The 'mii\-\>cch' variable is assigned to itself\. user32 menu\.c 4347 The value of the variable is assigned to itself\. The programmer apparently intended to write it in this way: "mii\-\>cch \= miiW\-\>cch;"\. **Example** **3\.** Clang project\. Object name misprinted\. ```cpp static Value *SimplifyICmpInst(...) { ... case Instruction::Shl: { bool NUW = LBO->hasNoUnsignedWrap() && LBO->hasNoUnsignedWrap(); bool NSW = LBO->hasNoSignedWrap() && RBO->hasNoSignedWrap(); ... } ``` This error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions 'LBO\-\>hasNoUnsignedWrap \(\)' to the left and to the right of the '&&' operator\. LLVMAnalysis instructionsimplify\.cpp 1891 There is a misprint when using variables with similar names\. In the first line, both LBO and RBO variables must be used\. This is the correct code: ```cpp bool NUW = LBO->hasNoUnsignedWrap() && RBO->hasNoUnsignedWrap(); ``` **Example** **4\.** Notepad\+\+ project\. Incorrect state test\. ```cpp bool _isPointXValid; bool _isPointYValid; ... bool isPointValid() { return _isPointXValid && _isPointXValid; }; ``` This error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions to the left and to the right of the '&&' operator\. \_isPointXValid && \_isPointXValid The name '\_isPointXValid' is used twice\. The function must actually return this code: "\_isPointXValid && \_isPointYValid"\. **Example** **5\.** StrongDC\+\+ project\. Unsuccessful check of \\r\\n\. ```cpp static void getContentLengthAndHeaderLength(...) { ... while(line[linelen] != '\r' && line[linelen] != '\r') ... } ``` The error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions 'line \[linelen\] \!\= '\\r'' to the left and to the right of the '&&' operator\. miniupnpc miniupnpc\.c 153 Because of a misprint, presence of the '\\r' character is checked twice\. Actually presence of the '\\n' character must be checked too\. **Example** **6\.** G3D Content Pak project\. A closing parenthesis in the wrong place\. ```cpp bool Matrix4::operator==(const Matrix4& other) const { if (memcmp(this, &other, sizeof(Matrix4) == 0)) { return true; } ... } ``` This error was found through the [V575](https://pvs-studio.com/en/docs/warnings/v575/) diagnostic: The 'memcmp' function processes '0' elements\. Inspect the 'third' argument\. graphics3D matrix4\.cpp 269 One closing parenthesis is in the wrong place\. It turns out that the size of the memory area being compared, is calculated by the "sizeof\(Matrix4\) \=\= 0" expression\. This expression always has the 'false' result\. Then 'false' turns into an integer value equal to 0\. This is the correct code: ```cpp if (memcmp(this, &other, sizeof(Matrix4)) == 0) { ``` **Example** **7\.** QT project\. Error in structure member copying\. ```cpp PassRefPtr Structure::getterSetterTransition(Structure* structure) { ... transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity; transition->m_hasGetterSetterProperties = transition->m_hasGetterSetterProperties; transition->m_hasNonEnumerableProperties = structure->m_hasNonEnumerableProperties; transition->m_specificFunctionThrashCount = structure->m_specificFunctionThrashCount; ... } ``` This error was found through the [V570](https://pvs-studio.com/en/docs/warnings/v570/) diagnostic: The 'transition\-\>m\_hasGetterSetterProperties' variable is assigned to itself\. QtScript structure\.cpp 512 It is not easy to find an error looking at this code\. But it _is_ there\. The field 'm\_hasGetterSetterProperties' is copied into itself\. This is the correct code: ```cpp transition->m_hasGetterSetterProperties = structure->m_hasGetterSetterProperties; ``` **Example** **8\.** Apache HTTP Server project\. Extra sizeof operator\. ```cpp PSECURITY_ATTRIBUTES GetNullACL(void) { PSECURITY_ATTRIBUTES sa; sa = (PSECURITY_ATTRIBUTES) LocalAlloc(LPTR, sizeof(SECURITY_ATTRIBUTES)); sa->nLength = sizeof(sizeof(SECURITY_ATTRIBUTES)); ... } ``` This error was found through the [V568](https://pvs-studio.com/en/docs/warnings/v568/) diagnostic: It's odd that the argument of sizeof\(\) operator is the 'sizeof \(SECURITY\_ATTRIBUTES\)' expression\. libhttpd util\_win32\.c 115 The field 'nLength' must contain the size of the 'SECURITY\_ATTRIBUTES' structure\. There is a misprint in the code: the 'sizeof' operator is used twice\. As a result, the field 'nLength' stores a size of the 'size\_t' type\. This is the correct code: ```cpp sa->nLength = sizeof(SECURITY_ATTRIBUTES); ``` **Example** **9\.** FCE Ultra project\. Double variable declaration\. ```cpp int iNesSaveAs(char* name) { ... fp = fopen(name,"wb"); int x = 0; if (!fp) int x = 1; ... } ``` This error was found through the [V561](https://pvs-studio.com/en/docs/warnings/v561/) diagnostic: It's probably better to assign value to 'x' variable than to declare it anew\. Previous declaration: ines\.cpp, line 960\. fceuxines\.cpp 962 The 'x' variable must store information whether or not a file was opened successfully\. Because of a misprint, a new variable named 'x' is created and initialized instead of assigning 1 to the existing variable\. This is how the correct code must look: ```cpp if (!fp) x = 1; ``` **Example** **10\.** Notepad\+\+ project\. Using && operator instead of &\. ```cpp TCHAR GetASCII(WPARAM wParam, LPARAM lParam) { ... result=ToAscii(wParam, (lParam >> 16) && 0xff, keys,&dwReturnedValue,0); ... } ``` This error was found through the [V560](https://pvs-studio.com/en/docs/warnings/v560/) diagnostic: A part of the conditional expression is always true: 0xff\. notepadPlus babygrid\.cpp 694 The "\(lParam \>\> 16\) && 0xff" expression is meaningless and is always equal to 1 \(true\)\. A misprint here is in using the '&&' operator instead of '&'\. **Example** **11\.** WinDjView project\. Incomplete condition\. ```cpp inline bool IsValidChar(int c) { return c == 0x9 || 0xA || c == 0xD || c >= 0x20 && c <= 0xD7FF || c >= 0xE000 && c <= 0xFFFD || c >= 0x10000 && c <= 0x10FFFF; } ``` This error was found through the [V560](https://pvs-studio.com/en/docs/warnings/v560/) diagnostic: A part of the conditional expression is always true: 0xA\. WinDjView xmlparser\.cpp 45 False The IsValidChar function always returns 'true'\. Comparison is missing in one place because of a misprint: "\.\.\. \|\| 0xA \|\| \.\.\."\. **Example** **12\.** Fennec Media Project project\. Extra semicolon\. ```cpp int settings_default(void) { ... for(i=0; i<16; i++); for(j=0; j<32; j++) { settings.conversion.equalizer_bands.boost[i][j] = 0.0; settings.conversion.equalizer_bands.preamp[i] = 0.0; } } ``` This error was found through the [V529](https://pvs-studio.com/en/docs/warnings/v529/) diagnostic: Odd semicolon ';' after 'for' operator\. settings\.c 483 All the C and C\+\+ programmers know how dangerous an extra semicolon ';' is\. Unfortunately, this knowledge does not prevent them from making such misprints\. There is an extra semicolon after the first 'for' operator, which makes this program fragment unable to execute\. **Example** **13\.** QT project\. Missing break operator\. ```cpp int QCleanlooksStyle::pixelMetric(...) { ... case PM_SpinBoxFrameWidth: ret = 3; break; case PM_MenuBarItemSpacing: ret = 6; case PM_MenuBarHMargin: ret = 0; break; ... } ``` This error was found through the [V519](https://pvs-studio.com/en/docs/warnings/v519/) diagnostic: The 'ret' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 3765, 3767\. QtGui qcleanlooksstyle\.cpp 3767 This is a classic error \- 'break' is missing inside the 'switch' operator\. I don't think there's need for any further comments here\. **Example** **14\.** Miranda IM project\. Assignment instead of comparison\. ```cpp int FindItem(...) { ... int ret; ret=FindItem(hwnd,dat,hItem, (struct ClcContact ** )&z, (struct ClcGroup ** )&isv,NULL); if (ret=0) {return (0);} ... } ``` This error was found through the [V559](https://pvs-studio.com/en/docs/warnings/v559/) diagnostic: Suspicious assignment inside the condition expression of 'if' operator: ret \= 0\. clist\_mw clcidents\.c 179 There is a misprint inside the condition of the 'if' operator: '\=' is written instead of '\=\='\. The function will handle the situation incorrectly when a certain item is not found\. **Example** **15\.** IPP Samples project\. Incorrect index\. ```cpp struct AVS_MB_INFO { ... Ipp8u refIdx[AVS_DIRECTIONS][4]; ... }; void AVSCompressor::GetRefIndiciesBSlice(void){ ... if (m_pMbInfo->predType[0] & predType) { m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][0]; iRefNum += 1; } if (m_pMbInfo->predType[1] & predType) { m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][1]; iRefNum += 1; } if (m_pMbInfo->predType[2] & predType) { m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][2]; iRefNum += 1; } if (m_pMbInfo->predType[3] & predType) { m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][30]; iRefNum += 1; } ... } ``` This error was found through the [V557](https://pvs-studio.com/en/docs/warnings/v557/) diagnostic: Array overrun is possible\. The '30' index is pointing beyond array boundaries\. avs\_enc umc\_avs\_enc\_compressor\_enc\_b\.cpp 495 Consider this fragment: "m\_pMbInfo\-\>refIdx\[dir\]\[30\]"\. Because of a misprint, number 30 is written instead of index 3\. By the way, this sample shows well how relative our division of errors into categories is\. This error might well be referred to the category "Errors in array and string handling"\. The division is relative, and is made to show the diversity of errors the PVS\-Studio analyzer can detect\. **Example** **16\.** ReactOS project\. Misprint in a macro\. ```cpp #define SWAP(a,b,c) c = a;\ a = b;\ a = c ``` This error was found through the [V519](https://pvs-studio.com/en/docs/warnings/v519/) diagnostic: The 'v2' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 343, 343\. win32k gradient\.c 343 It is a rather funny misprint in a macro intended to swap values in two variables\. Look closely at the code, and you will see what I mean\. This is the correct code: ```cpp #define SWAP(a,b,c) c = a;\ a = b;\ b = c ``` **Example** **17\.** Quake\-III\-Arena project\. Misprint\. Comma instead of multiplication operator\. ```cpp void Q1_AllocMaxBSP(void) { ... q1_allocatedbspmem += Q1_MAX_MAP_CLIPNODES * sizeof(q1_dclipnode_t); ... q1_allocatedbspmem += Q1_MAX_MAP_EDGES , sizeof(q1_dedge_t); ... q1_allocatedbspmem += Q1_MAX_MAP_MARKSURFACES * sizeof(unsigned short); ... } ``` The error has been found with rule [V521](https://pvs-studio.com/en/docs/warnings/v521/): Such expressions using the ',' operator are dangerous\. Make sure the expression is correct\. bspc l\_bsp\_q1\.c 136 It's a funny misprint\. Look at the line in the middle of the code\. ',' written instead of '\*'\. As a result, the 'sizeof\(q1\_dedge\_t\)' value is always added to the 'q1\_allocatedbspmem' variable\. I have no suggestions how this misprint could have occurred\. **Example** **18\.** LibXml project\. Misprint \=\+\. ```cpp static int xmlXPathCompOpEvalFirst(...) { ... total += xmlXPathCompOpEvalFirst(...); ... total =+ xmlXPathCompOpEvalFilterFirst(ctxt, op, first); ... } ``` This error has been found with rule [V588](https://pvs-studio.com/en/docs/warnings/v588/): The expression of the 'A \=\+ B' kind is utilized\. Consider reviewing it, as it is possible that 'A \+\= B' was meant\. libxml xpath\.c 12676 In one place, "\=\+" is written instead of "\+\=" by mistake\. They look similar but the result is quite different\. Such errors are rather difficult to find just by reviewing the code\. Many errors in software are caused by misprints\. There are many more errors of this kind, than programmers think\. We could go on and on in this section, but we've decide to stop at the 18th 11:06:AM example\. ### Incorrect use of base functions and classes **Example** **1\.** Fennec Media Project\. Two terminal nulls absent\. ```cpp int JoiningProc(HWND hwnd,UINT uMsg, WPARAM wParam,LPARAM lParam) { ... OPENFILENAME lofn; memset(&lofn, 0, sizeof(lofn)); ... lofn.lpstrFilter = uni("All Files (*.*)\0*.*"); ... } ``` This error was found through the [V540](https://pvs-studio.com/en/docs/warnings/v540/) diagnostic: Member 'lpstrFilter' should point to string terminated by two 0 characters\. base windows\.c 5309 In Windows API, there are structures in which pointers to strings must end with two null characters\. It is that very kind of string the 'lpstrFilter' member in the OPENFILENAME structure points to\. Description of 'lpstrFilter' in MSDN: _LPCTSTR_ _A buffer containing pairs of null\-terminated filter strings\. The last string in the buffer must be terminated by two NULL characters\._ If you forget to write an additional null at the end, the dialogue of file handling may contain garbage in the filter fields\. This is the correct code: ```cpp lofn.lpstrFilter = uni("All Files (*.*)\0*.*\0"); ``` **Example** **2\.** TortoiseSVN project\. Incorrect use of 'remove' function\. ```cpp STDMETHODIMP CShellExt::Initialize(....) { ... ignoredprops = UTF8ToWide(st.c_str()); // remove all escape chars ('\\') std::remove(ignoredprops.begin(), ignoredprops.end(), '\\'); break; ... } ``` This error was found through the [V530](https://pvs-studio.com/en/docs/warnings/v530/) diagnostic: The return value of the function 'remove' is required to be utilized\. contextmenu\.cpp 442 The std::remove function does not remove items from the container\. It only shifts the items, and returns the iterator to the beginning of trash\. Assume we have a vector container that contains items 1,2,3,1,2,3,1,2,3\. If we execute the code "remove\( v\.begin\(\), v\.end\(\), 2 \)", the container will contain items 1,3,1,3,X,X,X, where X is trash\. The function will return the iterator to the first trash item, so if we want to remove these trash items, we need to write the code: "v\.erase\(remove\(v\.begin\(\), v\.end\(\), 2\), v\.end\(\)\)"\. **Example** **3\.** TortoiseSVN project\. Using 'empty' function instead of 'clear'\. ```cpp CMailMsg& CMailMsg::SetFrom(string sAddress, string sName) { if (initIfNeeded()) { // only one sender allowed if (m_from.size()) m_from.empty(); m_from.push_back(TStrStrPair(sAddress,sName)); } return *this; } ``` This error was found through the [V530](https://pvs-studio.com/en/docs/warnings/v530/) diagnostic: The return value of function 'empty' is required to be utilized\. mailmsg\.cpp 40 The error here is as follows: the vector::empty\(\) function is called by mistake instead of vector::clear\(\), and the array's contents remain the same\. It is a very frequent error because the words 'clear' and 'empty' are rather close in meaning, and you might easily mix them up\. **Example** **4\.** WinMerge project\. Using 'empty' function instead of 'clear'\. ```cpp void CDirView::GetItemFileNames(int sel, String& strLeft, String& strRight) const { UINT_PTR diffpos = GetItemKey(sel); if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS) { strLeft.empty(); strRight.empty(); } else { ... } } ``` This error was found through the [V530](https://pvs-studio.com/en/docs/warnings/v530/) diagnostic: The return value of function 'empty' is required to be utilized WinMerge DirActions\.cpp 1307, 1308 Again, the reason is in using the empty\(\) function instead of clear\(\)\. We could cite examples of such errors from other projects as well: InstantVNC, IPP Samples, Chromium, Intel AMT SDK, etc\. Unfortunately, all these samples are alike, and there is nothing interesting about examining them\. But trust me, you can see these defects in serious projects developed by professional programmers\. **Example** **5\.** Pixie project\. Using 'alloca' function inside loops\. ```cpp inline void triangulatePolygon(...) { ... for (i=1;inodes[iad->nodes_allocated_size], (size_grow - iad->nodes_allocated_size) * sizeof(IMAGE_ARRAY_DATA_NODE), 0); ... } ``` This error was found through the [V575](https://pvs-studio.com/en/docs/warnings/v575/) diagnostic: Function receives an odd argument\. clist\_modern modern\_image\_array\.cpp 59 The 'memset' function handles 0 items, i\.e\. it actually does nothing\. The reason, is in mixed up arguments\. This is how the correct call of the memset function should be written: ```cpp memset(&iad->nodes[iad->nodes_allocated_size], 0, (size_grow - iad->nodes_allocated_size) * sizeof(IMAGE_ARRAY_DATA_NODE)); ``` ### Examples of meaningless code **Example** **1\.** IPP Samples project\. Incomplete condition\. ```cpp void lNormalizeVector_32f_P3IM(Ipp32f *vec[3], Ipp32s* mask, Ipp32s len) { Ipp32s i; Ipp32f norm; for(i=0; i 3 && pBytes[0] == 0xEF && pBytes[1] == 0xBB && pBytes[2] == 0xBF ) { pBytes += 3; nBytes -= 3; bBOM = true; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'pBytes \[ 0 \] \=\= 0xEF' is always false\. The value range of signed char type: \[\-128, 127\]\. Shareaza remote\.cpp 350 In this code, the 'TCHAR' type is the 'char' type\. The value range of char is from \-128 to 127 inclusive\. Value 0xEF in the variable of the char type is nothing other than number \-17\. When comparing the char variable with number 0xEF, its type is extended up to the 'int' type\. But the value still lies inside the range \[\-128\.\.127\]\. The "pBytes\[0\] \=\= 0xEF" \("\-17 \=\= 0xEF"\) condition is always false, and the program does not work as intended\. This is the correct comparison: ```cpp if ( nBytes > 3 && pBytes[0] == TCHAR(0xEF) && pBytes[1] == TCHAR(0xBB) && pBytes[2] == TCHAR(0xBF) ) ``` **Example** **2\.** TortoiseSVN project\. Value range of char type\. ```cpp BOOL TortoiseBlame::OpenFile(const TCHAR *fileName) { ... // check each line for illegal utf8 sequences. // If one is found, we treat // the file as ASCII, otherwise we assume // an UTF8 file. char * utf8CheckBuf = lineptr; while ((bUTF8)&&(*utf8CheckBuf)) { if ((*utf8CheckBuf == 0xC0)|| (*utf8CheckBuf == 0xC1)|| (*utf8CheckBuf >= 0xF5)) { bUTF8 = false; break; } ... } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression '\* utf8CheckBuf \=\= 0xC0' is always false\. The value range of signed char type: \[\-128, 127\]\. tortoiseblame\.cpp 310 While the defect in the previous example seems to be caused through mere inattention, in this case it is not so\. Here is another identical example where a condition is always false\. This is a very widely\-spread type of error in various projects\. **Example** **3\.** VirtualDub project\. Unsigned type is always \>\= 0\. ```cpp typedef unsigned short wint_t; ... void lexungetc(wint_t c) { if (c < 0) return; g_backstack.push_back(c); } ``` The error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'c < 0' is always false\. Unsigned type value is never < 0\. Ami lexer\.cpp 225 The "c < 0" condition is always false because the variable of the unsigned type is always above or equal to 0\. **Example** **4\.** Swiss\-Army Knife of Trace project\. Socket handling\. ```cpp static UINT_PTR m_socketHandle; void TTrace::LoopMessages(void) { ... // Socket creation if ( (m_socketHandle = socket(AF_INET,SOCK_STREAM,0)) < 0) { continue; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression '\(m\_socketHandle \= socket \(2, 1, 0\)\) < 0' is always false\. Unsigned type value is never < 0\. Vs8\_Win\_Lib tracetool\.cpp 871 An attempt to check that a socket was created successfully is performed incorrectly\. If a socket cannot be created, this situation is not handled in any way\. To make the check work correctly, we should use the INVALID\_SOCKET constant: ```cpp m_socketHandle = socket(AF_INET,SOCK_STREAM, 0); if (m_socketHandle == INVALID_SOCKET) ... ``` **Example** **5\.** Chromium project\. Time handling\. ```cpp IdleState CalculateIdleState(...) { ... DWORD current_idle_time = 0; ... // Will go -ve if we have been idle for // a long time (2gb seconds). if (current_idle_time < 0) current_idle_time = INT_MAX; ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'current\_idle\_time < 0' is always false\. Unsigned type value is never < 0\. browser idle\_win\.cc 23 To handle time, a variable of the unsigned type is used\. As a result, checking of too large values does not work\. This is the correct code: ```cpp if (current_idle_time > INT_MAX) current_idle_time = INT_MAX; ``` **Example** **6\.** ICU project\. Error in condition\. ```cpp U_CDECL_BEGIN static const char* U_CALLCONV _processVariableTop(...) { ... if(i == locElementCapacity && (*string != 0 || *string != '_')) { *status = U_BUFFER_OVERFLOW_ERROR; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression '\*string \!\= 0 \|\| \*string \!\= '\_'' is always true\. Probably the '&&' operator should be used here\. icui18n ucol\_sit\.cpp 242 The condition contains a logical error\. The "\(\*string \!\= 0 \|\| \*string \!\= '\_'\)" subexpression is always true\. It is impossible that one and the same string character is not equal to 0 and '\_' at a time\. **Example** **7\.** QT project\. Dangerous loop\. ```cpp bool equals( class1* val1, class2* val2 ) const{ { ... size_t size = val1->size(); ... while ( --size >= 0 ){ if ( !comp(*itr1,*itr2) ) return false; itr1++; itr2++; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression '\-\-size \>\= 0' is always true\. Unsigned type value is always \>\= 0\. QtCLucene arrays\.h 154 The \(\-\-size \>\= 0\) condition is always true, since the size variable has the unsigned type\. It means that if two sequences being compared are alike, we will get an overflow which will in its turn cause Access Violation, or other program failures\. This is the correct code: ```cpp for (size_t i = 0; i != size; i++){ if ( !comp(*itr1,*itr2) ) return false; itr1++; itr2++; } ``` **Example** **8\.** MySQL project\. Error in condition\. ```cpp enum enum_mysql_timestamp_type str_to_datetime(...) { ... else if (str[0] != 'a' || str[0] != 'A') continue; /* Not AM/PM */ ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'str \[0\] \!\= 'a' \|\| str \[0\] \!\= 'A'' is always true\. Probably the '&&' operator should be used here\. clientlib my\_time\.c 340 The condition is always true because the character is always either not equal to 'a' or to 'A'\. This is the correct check: ```cpp else if (str[0] != 'a' && str[0] != 'A') ``` **Example** **9\.** QT project\. Incorrect count of references\. ```cpp STDMETHODIMP QEnumPins::QueryInterface(const IID &iid,void **out) { ... if (S_OK) AddRef(); return hr; } ``` This error was found through the [V545](https://pvs-studio.com/en/docs/warnings/v545/) diagnostic: Such conditional expression of 'if' operator is incorrect for the HRESULT type value '\(HRESULT\) 0L'\. The SUCCEEDED or FAILED macro should be used instead\. phonon\_ds9 qbasefilter\.cpp 60 The check condition is represented by the S\_OK constant\. Since S\_OK is 0, the AddRef\(\) function will never be called\. This is how this check should look: if \(hr \=\= S\_OK\)\. **Example** **10\.** TickerTape project\. Incorrect tornado\. ```cpp void GetWindAtSingleTornado(...) { ... if(radius < THRESH * 5) *yOut = THRESH * 10 / radius; else if (radius < THRESH * 5) *yOut = -3.0f / (THRESH * 5.0f) * (radius - THRESH * 5.0f) + 3.0f; else *yOut = 0.0f; ... } ``` This error was found through the [V517](https://pvs-studio.com/en/docs/warnings/v517/) diagnostic: The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. TickerTape wind\.cpp 118 The second condition is always false\. The reason is that the first condition coincides with the second\. There must be a misprint here\. **Example** **11\.** Apache HTTP Server project\. Error of socket handling in Windows\. ```cpp typedef UINT_PTR SOCKET; static unsigned int __stdcall win9x_accept(void * dummy) { SOCKET csd; ... do { clen = sizeof(sa_client); csd = accept(nsd, (struct sockaddr *) &sa_client, &clen); } while (csd < 0 && APR_STATUS_IS_EINTR(apr_get_netos_error())); ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'csd < 0' is always false\. Unsigned type value is never < 0\. libhttpd child\.c 404 Socket handling errors very often emerge in cross platform programs built under Windows\. In Linux, socket descriptors are represented by the signed type, while in Windows it is the unsigned type\. Programmers often forget about this, and check the error status by comparing the value to 0\. This is incorrect; you must use specialized constants\. **Example** **12\.** QT project\. Misprint in comparisons\. ```cpp QStringList ProFileEvaluator::Private::values(...) { ... else if (ver == QSysInfo::WV_NT) ret = QLatin1String("WinNT"); else if (ver == QSysInfo::WV_2000) ret = QLatin1String("Win2000"); else if (ver == QSysInfo::WV_2000) <<-- ret = QLatin1String("Win2003"); else if (ver == QSysInfo::WV_XP) ret = QLatin1String("WinXP"); ... } ``` This error was found through the [V517](https://pvs-studio.com/en/docs/warnings/v517/) diagnostic: The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 2303, 2305\. lrelease profileevaluator\.cpp 2303 In the string we have marked, there must be the text "ver \=\= QSysInfo::WV\_2003"\. Because of this error, the "ret \= QLatin1String\("Win2003"\)" statement will never be executed\. ### Code vulnerabilities Of course, errors leading to code vulnerabilities are actually misprints, incorrect conditions, and incorrect array handling\. But we decided to single out certain errors into a separate group, because they relate to the notion of software vulnerabilities\. An intruder, using such errors, can try to disturb program operation, perform an attack to gain extended rights, or carry out any other actions he/she needs\. **Example** **1\.** Ultimate TCP/IP project\. Incorrect check of an empty string\. ```cpp char *CUT_CramMd5::GetClientResponse(LPCSTR ServerChallenge) { ... if (m_szPassword != NULL) { ... if (m_szPassword != '\0') { ... } ``` This error was found through the [V528](https://pvs-studio.com/en/docs/warnings/v528/) diagnostic: It is odd that the pointer to the 'char' type is compared with the '\\0' value\. What was probably meant: \*m\_szPassword \!\= '\\0'\. UTMail ut\_crammd5\.cpp 333 This code fragment must check that the pointer to the password is not equal to NULL, and that the string is not empty\. But instead, the code checks twice that the pointer is not equal to NULL\. The check of the string does not work\. The "if \(m\_szPassword \!\= '\\0'\)" condition was intended to check that there is a terminal null in the very beginning of the string, which means that the string is empty\. But a pointer dereferencing operation is missing here, and it is the pointer itself which is compared to zero\. This is the correct code: ```cpp if (m_szPassword != NULL) { ... if (*m_szPassword != '\0') ``` **Example** **2\.** Chromium project\. Null pointer handling\. ```cpp bool ChromeFrameNPAPI::Invoke(...) { ChromeFrameNPAPI* plugin_instance = ChromeFrameInstanceFromNPObject(header); if (!plugin_instance && (plugin_instance->automation_client_.get())) return false; ... } ``` This error was found through the [V522](https://pvs-studio.com/en/docs/warnings/v522/) diagnostic: Dereferencing of the null pointer 'plugin\_instance' might take place\. Check the logical condition\. chrome\_frame\_npapi chrome\_frame\_npapi\.cc 517 The condition that checks the null pointer is written incorrectly\. As a result, we have a [segmentation error](https://pvs-studio.com/en/blog/terms/0063/)\. This is the correct code: ```cpp if (plugin_instance && (plugin_instance->automation_client_.get())) return false; ``` **Example** **3\.** SMTP Client with SSL/TLS project\. Incomplete buffer clearing\. ```cpp void MD5::finalize () { ... uint1 buffer[64]; ... // Zeroize sensitive information memset (buffer, 0, sizeof(*buffer)); ... } ``` This error was found through the [V512](https://pvs-studio.com/en/docs/warnings/v512/) diagnostic: A call of the 'memset' function will lead to a buffer overflow or underflow\. CSmtp md5\.cpp 212 For security purposes, the function tries to clear the buffer containing sensitive information\. But it fails\. Only the first byte will be cleared in the buffer\. The error is this: the 'sizeof' operator calculates the size of the 'uint1' type instead of buffer\. This is the correct code: ```cpp memset (buffer, 0, sizeof(buffer)); ``` Generally, errors of incomplete memory clearing are rather frequent\. Consider some other cases like this\. **Example** **4\.** Chromium\. Incomplete buffer clearing\. ```cpp void Time::Explode(..., Exploded* exploded) const { ... ZeroMemory(exploded, sizeof(exploded)); ... } ``` This error was found through the [V512](https://pvs-studio.com/en/docs/warnings/v512/) diagnostic: A call of the 'memset' function will lead to underflow of the buffer '\(exploded\)'\. base time\_win\.cc 227 The ZeroMemory function clears only part of the Exploded structure\. The reason is that the 'sizeof' operator returns the pointer size\. To fix the error, we must dereference the pointer: ```cpp ZeroMemory(exploded, sizeof(*exploded)); ``` **Example** **5\.** Apache HTTP Server project\. Incomplete buffer clearing\. ```cpp #define MEMSET_BZERO(p,l) memset((p), 0, (l)) void apr__SHA256_Final(..., SHA256_CTX* context) { ... MEMSET_BZERO(context, sizeof(context)); ... } ``` This error was found through the [V512](https://pvs-studio.com/en/docs/warnings/v512/) diagnostic: A call of the 'memset' function will lead to underflow of the buffer '\(context\)'\. apr sha2\.c 560 The error is completely identical to the previous one\. The 'sizeof' operator calculates the pointer size\. To fix it, we must write: "sizeof\(\*context\)"\. **Example** **6\.** Miranda IM project\. Incorrect string handling\. ```cpp static char *_skipblank(char * str) { char * endstr=str+strlen(str); while ((*str==' ' || *str=='\t') && str!='\0') str++; while ((*endstr==' ' || *endstr=='\t') && endstr!='\0' && endstr 79) { png_warning(png_ptr, "keyword length must be 1 - 79 characters"); new_key[79] = '\0'; key_len = 79; } ... } ``` This error was found through the [V527](https://pvs-studio.com/en/docs/warnings/v527/) diagnostic: It is odd that the '\\0' value is assigned to the 'char' type pointer\. What was probably meant: \*new\_key \[79\] \= '\\0'\. graphics3D pngwutil\.c 1283 This sample demonstrates a mistake where the programmer accidentally clears the pointer instead of truncating the string length\. The point is that 'new\_key' is a pointer to a string\. And it means that we should write our code as follows, to truncate it to 79 characters: ```cpp (*new_key)[79] = '\0'; ``` **Example** **8\.** Intel AMT SDK project\. Unverified user name\. ```cpp static void wsman_set_subscribe_options(...) { ... if (options->delivery_certificatethumbprint || options->delivery_password || options->delivery_password) { ... } ``` This error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions 'options\-\>delivery\_password' to the left and to the right of the '\|\|' operator\. OpenWsmanLib wsman\-client\.c 631 Because of the developer's lack of attention, presence of password is checked twice, while presence of user name is not checked at all\. This is the correct code: ```cpp if (options->delivery_certificatethumbprint || options->delivery_username || options->delivery_password) { ``` **Example** **9\.** Ultimate TCP/IP project\. Incorrect handling of empty strings\. ```cpp void CUT_StrMethods::RemoveCRLF(LPSTR buf) { // v4.2 changed to size_t size_t len, indx = 1; if(buf != NULL){ len = strlen(buf); while((len - indx) >= 0 && indx <= 2) { if(buf[len - indx] == '\r' || buf[len - indx] == '\n') buf[len - indx] = 0; ++indx; } } } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression '\(len \- indx\) \>\= 0' is always true\. Unsigned type value is always \>\= 0\. UTDns utstrlst\.cpp 58 The "len \- indx" expression has the unsigned type 'size\_t' and is always \>\= 0\. Let's see what it will result in, if we send an empty string to the input\. If the string is empty, then: len \= 0, indx \= 1\. The len \- indx expression is equal to 0xFFFFFFFFu\. Since 0xFFFFFFFFu \> 0 and indx <\= 2, an array access is performed "buf\[len \- indx\]"\. The "buf\[0xFFFFFFFFu\]" operation will cause Access Violation\. **Example** **10\.** Miranda IM project\. Underflow protection does not work\. ```cpp void Append( PCXSTR pszSrc, int nLength ) { ... UINT nOldLength = GetLength(); if (nOldLength < 0) { // protects from underflow nOldLength = 0; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'nOldLength < 0' is always false\. Unsigned type value is never < 0\. IRC mstring\.h 229 The check "if \(nOldLength < 0\)" does not work since the nOldLength variable has the unsigned type\. **Example** **11\.** Apache HTTP Server project\. Incorrect handling of negative values\. ```cpp typedef size_t apr_size_t; APU_DECLARE(apr_status_t) apr_memcache_getp(...) { ... apr_size_t len = 0; ... len = atoi(length); ... if (len < 0) { *new_length = 0; *baton = NULL; } else { ... } } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'len < 0' is always false\. Unsigned type value is never < 0\. aprutil apr\_memcache\.c 814 The check "if \(len < 0\)" does not work because the 'len' variable has the unsigned type\. **Example** **12\.** Ultimate TCP/IP project\. Incorrect condition of loop termination\. ```cpp void CUT_StrMethods::RemoveSpaces(LPSTR szString) { ... size_t loop, len = strlen(szString); // Remove the trailing spaces for(loop = (len-1); loop >= 0; loop--) { if(szString[loop] != ' ') break; } ... } ``` This error was found through the [V547](https://pvs-studio.com/en/docs/warnings/v547/) diagnostic: Expression 'loop \>\= 0' is always true\. Unsigned type value is always \>\= 0\. UTDns utstrlst\.cpp 430 Suppose the whole string consists only of spaces\. While searching the characters, the program will reach the null item of the string, and the 'loop' variable will equal zero\. Then it will be decremented once again\. Since this variable is of unsigned type, its value will be 0xFFFFFFFFu or 0xFFFFFFFFFFFFFFFFu \(depending on the architecture\)\. This value is 'naturally \>\= 0', and a new loop iteration will start\. There will be an attempt at memory access by szString\[0xFFFFFFFFu\] address \- the consequences of this are familiar to every C/C\+\+ programmer\. **Example** **13\.** Crypto\+\+ project\. Private data clearing error\. ```cpp void CAST256::Base::UncheckedSetKey(const byte *userKey, unsigned int keylength, const NameValuePairs &) { AssertValidKeyLength(keylength); word32 kappa[8]; ... memset(kappa, 0, sizeof(kappa)); } ``` This error has been found with rule [V597](https://pvs-studio.com/en/docs/warnings/v597/): The compiler could delete the 'memset' function call, which is used to flush 'kappa' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. cryptlib cast\.cpp 293 The problem is in the memset\(\) function\. The arguments passed into the function are correct\. If a programmer looks at how the debug\-version of this code works in the debugger, he/she won't notice the trouble either\. The error occurs in the release version of the project\. The data that should have been cleared will remain in memory\. The reason is that the compiler has the right to delete the call of the memset\(\) function during optimization, and this is what it does\. If you want know why it happens, read the article "[Overwriting memory \- why?](https://pvs-studio.com/en/blog/posts/cpp/k0041/)"\. ### Copy\-Paste Developers should not also underestimate [Copy\-Paste](https://pvs-studio.com/en/blog/terms/0068/) errors, as well as common misprints\. They are very, very common\. Programmers spend a lot of time debugging these errors\. Of course, misprints and Copy\-Paste errors are similar, but there is a difference between them, which caused us to place them into different groups in this article\. Misprints often result in using the wrong variable instead of the needed one\. And in the case of copy\-paste, programmers simply forget to edit copied and pasted lines\. **Example** **1\.** Fennec Media Project project\. Mistake while handling array items\. ```cpp void* tag_write_setframe(char *tmem, const char *tid, const string dstr) { ... if(lset) { fhead[11] = '\0'; fhead[12] = '\0'; fhead[13] = '\0'; fhead[13] = '\0'; } ... } ``` This error was found through the [V525](https://pvs-studio.com/en/docs/warnings/v525/) diagnostic: The code containing the collection of similar blocks\. Check items '11', '12', '13', '13' in lines 716, 717, 718, 719\. id3 editor\.c 716 The four similar lines must have appeared in the code through the copy\-paste method\. When the programmer started editing the indices, he/she made a mistake that causes zero to be written into 'fhead\[13\] ' twice, and not be written into 'fhead\[14\] '\. **Example** **2\.** MySQL project\. Mistake while handling array items\. ```cpp static int rr_cmp(uchar *a,uchar *b) { if (a[0] != b[0]) return (int) a[0] - (int) b[0]; if (a[1] != b[1]) return (int) a[1] - (int) b[1]; if (a[2] != b[2]) return (int) a[2] - (int) b[2]; if (a[3] != b[3]) return (int) a[3] - (int) b[3]; if (a[4] != b[4]) return (int) a[4] - (int) b[4]; if (a[5] != b[5]) return (int) a[1] - (int) b[5]; if (a[6] != b[6]) return (int) a[6] - (int) b[6]; return (int) a[7] - (int) b[7]; } ``` This error was found through the [V525](https://pvs-studio.com/en/docs/warnings/v525/) diagnostic: The code containing the collection of similar blocks\. Check items '0', '1', '2', '3', '4', '1', '6' in lines 680, 682, 684, 689, 691, 693, 695\. sql records\.cc 680 It is not apparent at first sight, so let's single it out: ```cpp return (int) a[1] - (int) b[5]; ``` Actually the code should be: ```cpp return (int) a[5] - (int) b[5]; ``` **Example** **3\.** TortoiseSVN project\. File name not corrected\. ```cpp BOOL GetImageHlpVersion(DWORD &dwMS, DWORD &dwLS) { return(GetInMemoryFileVersion(("DBGHELP.DLL"), dwMS, dwLS)) ; } BOOL GetDbgHelpVersion(DWORD &dwMS, DWORD &dwLS) { return(GetInMemoryFileVersion(("DBGHELP.DLL"), dwMS, dwLS)) ; } ``` This error was found through the [V524](https://pvs-studio.com/en/docs/warnings/v524/) diagnostic: It is odd that the 'GetDbgHelpVersion' function is fully equivalent to the 'GetImageHlpVersion' function \(SymbolEngine\.h, line 98\)\. symbolengine\.h 105 The 'GetImageHlpVersion' function must have appeared through copying and pasting the 'GetInMemoryFileVersion' function\. The error is as follows: the programmer forgot to fix the file name in the copied and pasted function\. This is the correct code: ```cpp BOOL GetImageHlpVersion(DWORD &dwMS, DWORD &dwLS) { return(GetInMemoryFileVersion(("IMAGEHLP.DLL"), dwMS, dwLS)) ; } ``` **Example** **4\.** Clang project\. Identical function bodies\. ```cpp MapTy PerPtrTopDown; MapTy PerPtrBottomUp; void clearBottomUpPointers() { PerPtrTopDown.clear(); } void clearTopDownPointers() { PerPtrTopDown.clear(); } ``` This error was found through the [V524](https://pvs-studio.com/en/docs/warnings/v524/) diagnostic: It is odd that the body of 'clearTopDownPointers' function is fully equivalent to the body of 'clearBottomUpPointers' function \(ObjCARC\.cpp, line 1318\)\. LLVMScalarOpts objcarc\.cpp 1322 The body of the clearBottomUpPointers function seems to be incorrect; this function should be written as follows: ```cpp void clearBottomUpPointers() { PerPtrBottomUp.clear(); } ``` **Example** **5\.** QT\. Unsuccessful swap\. ```cpp bool qt_testCollision(...) { ... t=x1; x1=x2; x2=t; t=y1; x1=y2; y2=t; ... } ``` This error was found through the [V519](https://pvs-studio.com/en/docs/warnings/v519/) diagnostic: The 'x1' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 2218, 2219\. Qt3Support q3canvas\.cpp 2219 The first line is absolutely correct, and swaps values in the x1 and x2 variables\. In the second line, variables y1 and y2 must be swapped\. This line is probably a copy of the previous one\. All the 'x' letters must be replaced with letters 'y'\. Unfortunately, the programmer forgot to do that in one place: "\.\.\. x1\=y2; \.\.\."\. Correct code: ```cpp t=x1; x1=x2; x2=t; t=y1; y1=y2; y2=t; ``` **Example** **6\.** Crystal Space 3D SDK project\. Identical subexpressions\. ```cpp inline_ bool Contains(const LSS& lss) { return Contains(Sphere(lss.mP0, lss.mRadius)) && Contains(Sphere(lss.mP0, lss.mRadius)); } ``` This error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions to the left and to the right of the '&&' operator\. plgcsopcode icelss\.h 69 The error is this: the 'lss\.mP0\.' variable is used twice here\. There must be 'lss\.mP1' in the first part of the expression\. **Example** **7\.** Notepad\+\+ project\. Setting an incorrect style\. ```cpp void KeyWordsStyleDialog::updateDlg() { ... Style & w1Style = _pUserLang->_styleArray.getStyler(STYLE_WORD1_INDEX); styleUpdate(w1Style, _pFgColour[0], _pBgColour[0], IDC_KEYWORD1_FONT_COMBO, IDC_KEYWORD1_FONTSIZE_COMBO, IDC_KEYWORD1_BOLD_CHECK, IDC_KEYWORD1_ITALIC_CHECK, IDC_KEYWORD1_UNDERLINE_CHECK); Style & w2Style = _pUserLang->_styleArray.getStyler(STYLE_WORD2_INDEX); styleUpdate(w2Style, _pFgColour[1], _pBgColour[1], IDC_KEYWORD2_FONT_COMBO, IDC_KEYWORD2_FONTSIZE_COMBO, IDC_KEYWORD2_BOLD_CHECK, IDC_KEYWORD2_ITALIC_CHECK, IDC_KEYWORD2_UNDERLINE_CHECK); Style & w3Style = _pUserLang->_styleArray.getStyler(STYLE_WORD3_INDEX); styleUpdate(w3Style, _pFgColour[2], _pBgColour[2], IDC_KEYWORD3_FONT_COMBO, IDC_KEYWORD3_FONTSIZE_COMBO, IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_UNDERLINE_CHECK); Style & w4Style = _pUserLang->_styleArray.getStyler(STYLE_WORD4_INDEX); styleUpdate(w4Style, _pFgColour[3], _pBgColour[3], IDC_KEYWORD4_FONT_COMBO, IDC_KEYWORD4_FONTSIZE_COMBO, IDC_KEYWORD4_BOLD_CHECK, IDC_KEYWORD4_ITALIC_CHECK, IDC_KEYWORD4_UNDERLINE_CHECK); ... } ``` This error was found through the [V525](https://pvs-studio.com/en/docs/warnings/v525/) diagnostic: The code contains a collection of similar blocks\. Check items '7', '7', '6', '7' in lines 576, 580, 584, 588 It is almost unreal to expect find this error by sight, so let's abridge the text to single out the most interesting fragments: ```cpp styleUpdate(... IDC_KEYWORD1_BOLD_CHECK, IDC_KEYWORD1_ITALIC_CHECK, ...); styleUpdate(... IDC_KEYWORD2_BOLD_CHECK, IDC_KEYWORD2_ITALIC_CHECK, ...); styleUpdate(... IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_BOLD_CHECK, <<-- ...); styleUpdate(... IDC_KEYWORD4_BOLD_CHECK, IDC_KEYWORD4_ITALIC_CHECK, ...); ``` By mistake, IDC\_KEYWORD3\_BOLD\_CHECK is used instead of IDC\_KEYWORD3\_ITALIC\_CHECK\. **Example** **8\.** ReactOS object\. Choosing a wrong object\. ```cpp void CardButton::DrawRect(HDC hdc, RECT *rect, bool fNormal) { ... HPEN hhi = CreatePen(0, 0, MAKE_PALETTERGB(crHighlight)); HPEN hsh = CreatePen(0, 0, MAKE_PALETTERGB(crShadow)); ... if(fNormal) hOld = SelectObject(hdc, hhi); else hOld = SelectObject(hdc, hhi); ... } ``` This error was found through the [V523](https://pvs-studio.com/en/docs/warnings/v523/) diagnostic: The 'then' statement is equivalent to the 'else' statement\. cardlib cardbutton\.cpp 83 The 'hsh' object is not used, while 'hhi' is used twice\. This is the correct code: ```cpp if(fNormal) hOld = SelectObject(hdc, hhi); else hOld = SelectObject(hdc, hsh); ``` **Example** **9\.** IPP Samples project\. Incorrect check\. ```cpp Status VC1VideoDecoder::ResizeBuffer() { ... if(m_pContext && m_pContext->m_seqLayerHeader && m_pContext->m_seqLayerHeader->heightMB && m_pContext->m_seqLayerHeader->heightMB) ... } ``` This error was found through the [V501](https://pvs-studio.com/en/docs/warnings/v501/) diagnostic: There are identical sub\-expressions 'm\_pContext\-\>m\_seqLayerHeader\-\>heightMB' to the left and to the right of the '&&' operator\. vc1\_dec umc\_vc1\_video\_decoder\.cpp 1347 Correct code: ```cpp if(m_pContext && m_pContext->m_seqLayerHeader && m_pContext->m_seqLayerHeader->heightMB && m_pContext->m_seqLayerHeader->widthMB) ``` **Example** **10\.** ReactOS project\. Mistake in a variable name\. ```cpp BOOL APIENTRY GreStretchBltMask(...) { ... MaskPoint.x += DCMask->ptlDCOrig.x; MaskPoint.y += DCMask->ptlDCOrig.x; ... } ``` This error was found through the [V537](https://pvs-studio.com/en/docs/warnings/v537/) diagnostic: Consider reviewing the correctness of 'x' item's usage\. win32k bitblt\.c 670 This is a very good example, where you can see that a line was copied and pasted\. After that, the programmer fixed the first name 'x' but forgot to fix the second\. This is the correct code: ```cpp MaskPoint.x += DCMask->ptlDCOrig.x; MaskPoint.y += DCMask->ptlDCOrig.y; ``` ### Late check of null pointers C/C\+\+ programmers have to check numerous pointers all the time, to make sure that they are not equal to zero\. Since there are many of these check to be made, the chance of making a mistake is also quite high\. It often happens that a pointer is used first, and only then is compared to NULL\. Errors of this type reveal themselves very rarely\. Usually the program works correctly in standard mode, and fails only in the case of a non\-standard situation\. Instead of correctly processing a null pointer in normal mode, an [Access Violation](https://pvs-studio.com/en/blog/terms/0063/) will occur, and an exception will be thrown\. **Example** **1\.** Quake\-III\-Arena project\. Late check\. ```cpp void Item_Paint(itemDef_t *item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (item == NULL) { return; } ... } ``` This error has been found with rule [V595](https://pvs-studio.com/en/docs/warnings/v595/): The 'item' pointer was utilized before it was verified against nullptr\. Check lines: 3865, 3869\. cgame ui\_shared\.c 3865 The 'item' pointer is used first, and only then is compared to NULL\. **Example** **2\.** LAME Ain't an MP3 Encoder project\. Late check\. ```cpp static int check_vbr_header(PMPSTR mp, int bytes) { ... buf = buf->next; pos = buf->pos; if(!buf) return -1; /* fatal error */ ... } ``` This error has been found with rule [V595](https://pvs-studio.com/en/docs/warnings/v595/): The 'buf' pointer was utilized before it was verified against nullptr\. Check lines: 226, 227\. mpglib interface\.c 226 If 'buf' equals NULL, an exception will be thrown instead of returning the error code\. And if exceptions are not used, the program will crash\. **Example** **3\.** daoParanoia library project\. Late check\. ```cpp static long i_stage2_each(root_block *root, v_fragment *v, void(*callback)(long,int)) { cdrom_paranoia *p=v->p; long dynoverlap=p->dynoverlap/2*2; if (!v || !v->one) return(0); ... } ``` This error has been found with rule [V595](https://pvs-studio.com/en/docs/warnings/v595/): The 'v' pointer was utilized before it was verified against nullptr\. Check lines: 532, 535\. daoParanoia paranoia\.c 532 The situation here is identical to the previous ones\. **Example** **4\.** TrinityCore project\. Late check\. ```cpp bool OnCheck(Player* player, Unit* /*target*/) { bool checkArea = player->GetAreaId() == AREA_ARGENT_TOURNAMENT_FIELDS || player->GetAreaId() == AREA_RING_OF_ASPIRANTS || player->GetAreaId() == AREA_RING_OF_ARGENT_VALIANTS || player->GetAreaId() == AREA_RING_OF_ALLIANCE_VALIANTS || player->GetAreaId() == AREA_RING_OF_HORDE_VALIANTS || player->GetAreaId() == AREA_RING_OF_CHAMPIONS; return player && checkArea && player->duel && player->duel->isMounted; } ``` This error has been found with rule [V595](https://pvs-studio.com/en/docs/warnings/v595/): The 'player' pointer was utilized before it was verified against nullptr\. Check lines: 310, 312\. scripts achievement\_scripts\.cpp 310 As you can see from the "player && \.\.\." condition, the 'player' pointer can be equal to zero\. However, this check, like in all the previous examples, is too late\. We could cite many examples of such errors, but they are all alike\. If you have seen a few such errors, be sure you've seen them all\. ### Miscellaneous **Example** **1\.** Image Processing SDK project\. Octal number\. ```cpp inline void elxLuminocity(const PixelRGBus& iPixel, LuminanceCell< PixelRGBus >& oCell) { oCell._luminance = uint16(0.2220f*iPixel._red + 0.7067f*iPixel._blue + 0.0713f*iPixel._green); oCell._pixel = iPixel; } inline void elxLuminocity(const PixelRGBi& iPixel, LuminanceCell< PixelRGBi >& oCell) { oCell._luminance = 2220*iPixel._red + 7067*iPixel._blue + 0713*iPixel._green; oCell._pixel = iPixel; } ``` This error was found through the [V536](https://pvs-studio.com/en/docs/warnings/v536/) diagnostic: Be advised that the utilized constant value is represented by an octal form\. Oct: 0713, Dec: 459\. IFF plugins pixelservices\.inl 146 If you examine the second function, you will see that the programmer intended to use number 713, not 0713\. Number 0713 is declared in the octal numeral system\. You can easily forget about it if you seldom use octal constants\. **Example** **2\.** IPP Samples project\. One variable for two loops\. ```cpp JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void) { ... for(c = 0; c < m_scan_ncomps; c++) { block = m_block_buffer + (DCTSIZE2*m_nblock*(j+(i*m_numxMCU))); // skip any relevant components for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++) { block += (DCTSIZE2*m_ccomp[c].m_nblocks); } ... } ``` This error was found through the [V535](https://pvs-studio.com/en/docs/warnings/v535/) diagnostic: The variable 'c' is being used for this loop, and for the outer loop\. jpegcodec jpegdec\.cpp 4652 One and the same variable is used for the outer loop and the inner loop\. As a result, this code will handle only part of the data, or cause an infinite loop\. **Example** **3\.** Quake\-III\-Arena project\. Missing return\. ```cpp static ID_INLINE int BigLong(int l) { LongSwap(l); } ``` This error has been found with rule [V591](https://pvs-studio.com/en/docs/warnings/v591/): Non\-void function should return a value\. botlib q\_shared\.h 155 This code is written in C\. This means that the compiler doesn't require that return should be necessarily present\. But it is absolutely necessary here\. However, the code can work well, due to sheer luck\. Everything depends on what the EAX register contains\. But it's just luck and nothing more\. The function body should have been written this way: \{ return LongSwap\(l\); \}\. **Example** **4\.** Notepad\+\+ project\. Odd condition\. ```cpp int Notepad_plus::getHtmlXmlEncoding(....) const { ... if (langT != L_XML && langT != L_HTML && langT == L_PHP) return -1; ... } ``` This error has been found with rule [V590](https://pvs-studio.com/en/docs/warnings/v590/): Consider inspecting this expression\. The expression is excessive, or contains a misprint\. Notepad\+\+ notepad\_plus\.cpp 853 Perhaps this error is just a misprint, but it could also have appeared during factoring\. However, it is obvious\. The condition can be simplified: if \(langT \=\= L\_PHP\)\. It means that the code must have looked this way: ```cpp if (langT != L_XML && langT != L_HTML && langT != L_PHP) ``` ## References 1. [PVS\-Studio Main Product Page](https://pvs-studio.com/en/pvs-studio/)\. 1. [Download the fully functional trial](https://pvs-studio.com/en/pvs-studio/download/)\. 1. [Buy PVS\-Studio](https://pvs-studio.com/en/order/)\. 1. PVS\-Studio [Documentation](https://pvs-studio.com/en/docs/)\. 1. [Feedback](https://pvs-studio.com/en/about-feedback/)\. 1. Twitter\. [http://twitter\.com/Code\_Analysis](https://twitter.com/code_analysis)