Geekly Articles each Day
📜 ⬆️ ⬇️

The ideal way to implement a static code analyzer

Apple II emulator for Windows
One of the main difficulties with the use of static analysis tools is working with false positives. There are many ways to eliminate false positives using the analyzer settings or by changing the code. I took a small Apple II emulator for Windows project and show you how to work with the PVS-Studio static analyzer report in practice. I will show with examples how to correct errors and suppress false positives.

Introduction

I will describe the ideal implementation process for a static analysis methodology project. It consists in eliminating all false alarms of the analyzer and real errors so that the analyzer generates 0 warnings. This is the approach we used when working with the Unreal Engine 4 project.

In practice, the ideal approach is rarely possible. Therefore, in a large project, it is reasonable to take an alternative approach. You can hide all the current warnings and see only the messages related to the new or modified code. For this, there is a special mechanism in the PVS-Studio analyzer that saves information in a special database. Details can be found in the article: How to embed static analysis in a project with more than 10 megabytes of source code?

So, hiding all the messages, you can closely monitor the quality of the new code. Finding errors in the new code, you will quickly appreciate the power and benefit of the static analysis methodology. And when you have free time, you can return to the hidden warnings and gradually make the necessary edits to the code.
')
But back to the ideal happy world. Imagine that we have time, and we can safely deal with the warnings given by the PVS-Studio analyzer.

In this article I want to show how to work with analyzer warnings. And we will go the full way - from the first check to the window, in which there will be 0 warnings.

That is why I chose a small project. I could take a little more, but then I get tired of writing an article, and you read it. However, you still get tired. The article, even for a small project, will be large, but please pay attention to it. It can help you use our code analyzer more efficiently.

The test mouse was the Apple II emulator for Windows project. The choice fell on him by accident. Therefore, we will not dwell on it. I did not care what project to take, as long as it turned out to be small, but at the same time there was something interesting in it.

Project Features:

First start

After the first launch of the analyzer, we have the following warnings:

Figure 1. Alerts issued on the first run of the PVS-Studio analyzer for the Apple II emulator for Windows project.
Figure 1. Alerts issued on the first run of the PVS-Studio analyzer for the Apple II emulator for Windows project.

In the article I will discuss only warnings 1 and 2 levels related to general purpose analysis (GA). It would be possible to "win" and level 3, but then the article will drag on. We look at level 3 quite quickly, and I will give some explanations, but I will not correct anything.

Micro-optimizations (OP): not interested now.

About 64-bit diagnostics: there is no 64-bit configuration in the project. Not interested.

After checking the project, I sorted all the warnings by code. This can be done by clicking on the “Code” column (see Figure N2).

Figure 2. Window with PVS-Studio messages. Messages are sorted by diagnostic number.
Figure 2. Window with PVS-Studio messages. Messages are sorted by diagnostic number.

Sorting by code makes it easier to work with messages. There are groups of similar messages. Having understood the reason for the occurrence of one message, you can easily and quickly handle the rest.

Note. The reader may be asked why not immediately do such a sort. The fact is that I want to allow users to look at the messages in the process of analysis. If you sort them, the new messages will not be added to the end of the analysis, but to different places in the list. As a result, the messages will "jump." It will be impossible to work with such a "jerking" list.

Work with analyzer messages

There are three projects in the solution (they are visible in the Solution Explorer window in Figure N2). Two of them, zlib and zip_lib, are not interesting to check. Therefore, they should be excluded from the analysis. In fact, it suffices to exclude zip_lib, since zlib is already added to exceptions by default. This is done in the PVS-Studio settings window ( Don't Check Files section):

Figure N3. Exclude zip_lib from checking.
Figure N3. Exclude zip_lib from checking.

I excluded the unnecessary project in advance. However, this is easy to do after verification. And it is not necessary for this to go to the settings. In the context menu there is an item that allows you to conveniently hide all messages related to a file or a specific directory. It is very convenient. I recommend to get acquainted with the article " PVS-Studio for Visual C ++ ". It describes this and other features that will help you use the tool effectively.

So, everything is ready to start working on messages. Let's start with the diagnostics number V501 and go further. In total we will consider 32 + 49 = 81 posts. This is quite a lot. Therefore, in some places I will write in detail, and in some places I will be brief.

False triggers on xxxxxREG macros

The first 6 messages are caused by complex macros ADDXXREG, ADCHLREG, SBCHLREG, SBCHLREG. When disclosing them, redundant constructions arise, and the analyzer gives, for example, the following message:

V501 operator: (tmp >> 8) ^ reg_ixh ^ reg_ixh z80.cpp 3444

The ADDXXREG macro is very large and consists of other macros. Therefore, I will not give it in the article.

It is important to us that the XOR operation is performed twice with the variable reg_ixh. Accordingly, the expression can be simplified to (tmp >> 8). However, there is no mistake here. Just get a redundant expression when substituting certain macro arguments:

ADDXXREG (reg_ixh, reg_ixl, reg_ixh, reg_ixl, 15, 2);

These are false positives, and we must eliminate them. Suppress all warnings associated with them. To do this, in the header file where these macros are declared, I added the following comments:
Details about this mechanism for suppressing warnings can be found in the corresponding section of the documentation .

In principle, you can get by with one comment. In the names of all macros there is a “REG”. Therefore, you can write one comment: // - V: REG: 501. It will suppress V501 warnings in the lines where “REG” will occur. But this is bad, because you can accidentally hide a useful message that is not related to the macros listed above. It is possible to slightly improve the situation by adding a bracket for the search: // - V: REG (: 501. In this case, I think that it is not worth being lazy and you should write 4 comments.

Error in sprintf () function parameters

sprintf( sText, "%s %s = %s\n" , g_aTokens[ TOKEN_COMMENT_EOL ].sToken , g_aParameters[ PARAM_CATEGORY ].m_sName , g_aParameters[ eCategory ] );

Warning: V510 The 'sprintf' function is not expected to receive class-type variable as the fifth actual argument. debug.cpp 2300

Indeed, the fifth actual argument is a structure of type Command_t. Apparently, g_aParameters [eCategory] .m_sName should be used as an argument. Corrected the code.

Bad smelling ZeroMemory ()

The following message tells us that one array is not fully populated: V512: pHDD-> hd_buf. harddisk.cpp 491
 BYTE hd_buf[HD_BLOCK_SIZE+1]; // Why +1? ZeroMemory(pHDD->hd_buf, HD_BLOCK_SIZE);

The last byte is not reset. I can not say whether it is a mistake or not. Pay attention to the comment. Perhaps even the developers themselves do not know what size the array should be and whether it should be zeroed out entirely.

Such code is said to be smelled. It is not necessarily erroneous, but suspicious and may cause further errors.

I just suppress this warning with a comment. You can make changes to the file yourself, or you can use the context menu item “Mark selected messages as False Alarms”:

Figure 3. Adding a comment to the code to suppress the warning.
Figure 3. Adding a comment to the code to suppress the warning.

Having selected this item, the analyzer will insert a comment into the code itself:
 ZeroMemory(pHDD->hd_buf, HD_BLOCK_SIZE); //-V512

False triggering on memcpy () function call

 unsigned char random[ 256 + 4 ]; memcpy( &memmain[ iByte ], random, 256 );

The memcpy () function copies only part of the 'random' buffer. The analyzer considers this suspicious and honestly reports this. In this case, he is wrong, and there is no error. I suppressed the warning, as in the previous case, with the help of a comment. This is not very beautiful, but I don’t know how to do better in someone else’s code.

Extra action

 nAddress_ = 0; nAddress_ = (unsigned)*(LPBYTE)(mem + nStack); nStack++; nAddress_ += ((unsigned)*(LPBYTE)(mem + nStack)) << 8;

Warning: V519 The 'nAddress_' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 568, 569. debugger_assembler.cpp 569

The analyzer noticed that the variable nAddress_ is assigned different values ​​several times in a row. There is no error here, just an extra code. I deleted the first line, where the variable is assigned the value 0. Another option to get rid of the warning is to replace the second assignment with "+ =".

A similar situation can be observed in two more files:

The video.cpp file (see line 3310 and 3315). I deleted the extra operation “pSrc + = nLen;”.

The file Debug.cpp (see line 5867 and 5868). Replaced:
 char *p = sLine; p = strstr( sLine, ":" );
on
 char *p = strstr( sLine, ":" );

I will not dwell on these fragments in more detail.

Error in switch statement

The following diagnostic V519 already indicates a real serious error. Although the mistake is classical, and everyone knows about it, we meet it again and again in various programs.
 switch( c ) { case '\\': eThis = PS_ESCAPE; case '%': eThis = PS_TYPE; break; default: sText[ nLen++ ] = c; break; }

Warning: V519 The 'p' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 5867, 5868. debug.cpp 5868

After “eThis = PS_ESCAPE;” there is no 'break' operator. Because of this, the value of the variable 'eThis' will immediately change to PS_STYPE. This is an obvious mistake. To fix it, I added a 'break' operator.

Error: always false condition

 inline static ULONG ConvertZ80TStatesTo6502Cycles(UINT uTStates) { return (uTStates < 0) ? 0 : (ULONG) ((double)uTStates / uZ80ClockMultiplier); }

Warning: V547 Expression 'uTStates <0' is always false. Unsigned type value is never <0. z80.cpp 5507

The programmer wanted to protect himself from the case when a negative value is passed to the function. However, since the variable 'uTStates' is unsigned, protection will not work.

I added an explicit cast to the 'int' type:
 return ((INT)uTStates < 0) ? 0 : (ULONG) ((double)uTStates / uZ80ClockMultiplier);

Excessive caution analyzer

In the next function, the analyzer is worried that an overrun may occur.
 void SetCurrentImageDir(const char* pszImageDir) { strcpy(g_sCurrentDir, pszImageDir); int nLen = strlen( g_sCurrentDir ); if( g_sCurrentDir[ nLen - 1 ] != '\\' ) .... }

Warning: V557 Array underrun is possible. The value of 'nLen - 1' index could reach -1. applewin.cpp 553

If you pass an empty string to a function, its length will be zero. Then the array will go out of bounds: g_sCurrentDir [0 - 1].

The analyzer does not know whether such a situation is possible or not. Therefore, just in case, and warns.

I also do not know whether such a situation is possible or not. If possible, the analyzer found an error. If not, this is a false positive.

I decided to assume that this is a false positive. However, it would be more useful to improve the code, rather than adding a comment to suppress the warning. Therefore, let an additional check be added to the function:
 if (nLen == 0) return;

There is another place where, theoretically, it can occur beyond the array boundary. But we must try not to turn the article into a thick reference book. Therefore, I will not describe this warning and simply suppress it with the help of a comment. See the same file (line 556).

Assignment instead of comparison

 if ((bytenum == 3) && (byteval[1] = 0xAA)) {

Warning: V560 A part of conditional expression is always true: (byteval [1] = 0xAA). diskimagehelper.cpp 439

I’m sure that I wanted to perform the operation '==', not '='. If assignment is needed, it would be much more natural to write this:
 if (bytenum == 3) { byteval[1] = 0xAA;

Therefore, this is a mistake, and it should be fixed:
 if ((bytenum == 3) && (byteval[1] == 0xAA))

False alarms due to macros

 if ((TRACKS_MAX>TRACKS_STANDARD) && ....)

Warning: V560 A part of conditional expression is always true: ((35 + 5)> 35). diskimagehelper.cpp 548

If you open the macros, you get the expression ((35 + 5)> 35). The expression is always true, but it is not a mistake.

This is the case when I do not even know how best to proceed. Shalturu and just suppress the false positive with the comment: // - V560.

Extra variable

In the process of refactoring, sometimes “lost” variables remain. They are somehow used in the code, but in fact are no longer needed. Apparently this is exactly what happened with the bForeground variable:
 BOOL bForeground; .... bForeground = FALSE; .... if( bForeground ) dwCoopFlags |= DISCL_FOREGROUND; else dwCoopFlags |= DISCL_BACKGROUND; .... if( hr == DIERR_UNSUPPORTED && !bForeground && bExclusive )

And more, the 'bForeground' variable is not changed or used anywhere. This causes a warning: V560 A part of the conditional expression is always true:! BForeground. mouseinterface.cpp 690

This is an interesting example for philosophy. Is this a false positive or not? Even a person is difficult to answer. The analyzer is right, revealing an anomaly. But from the point of view of a person, this fragment may be code that is not complete. And then everything is fine.

We assume that this is another example of the "code with the smell." I removed the 'bForeground' variable from the code.

Behavior undefined

 *(mem+addr++) = (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4;

Warning: V567 Undefined behavior. The 'addr' variable is used while being between times sequence points. cpu.cpp 564

It is not known how the expression will be calculated:The correct code would be:
 *(mem+addr) = (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4; addr++;

Invalid arguments when calling wsprintf () and similar functions.

There are several errors associated with passing an incorrect amount of actual arguments in a formatted output function. A total of 10 such errors were found, but we will consider only one:
 wsprintf( sText, TEXT("%s full speed Break on Opcode: None") , sAction , g_iDebugBreakOnOpcode , g_aOpcodes65C02[ g_iDebugBreakOnOpcode ].sMnemonic );

Warning: V576 Incorrect format. A different number of actual arguments is expected while calling the 'wsprintfA' function. Expected: 3. Present: 5. debug.cpp 939

When forming the line, the last two parameters are not taken into account. As an outsider, it is difficult for me to say if these parameters are superfluous, or an error is made in the format string.

I decided that the parameters are redundant, and deleted them.

Similar problems are observed in the following sections of the code:
There are also a couple of places where% 08X is used to print the pointer values. On a 32-bit system, this works in practice. But in a 64-bit system, only part of the pointer will be printed. The correct solution is to use "% p". Relevant code sections:

False alarms on double comparisons

Although the analyzer is not to blame, it gave two false positives on repetitive conditions. Consider one case:
 if (nAddress <= _6502_STACK_END) { sprintf( sText,"%04X: ", nAddress ); PrintTextCursorX( sText, rect ); } if (nAddress <= _6502_STACK_END) { DebuggerSetColorFG( DebuggerGetColor( FG_INFO_OPCODE )); sprintf(sText, " %02X",(unsigned)*(LPBYTE)(mem+nAddress)); PrintTextCursorX( sText, rect ); }

Warning: V581 The conditional expressions of the 'if' are located alongside each other are identical. Check lines: 2929, 2935. debugger_display.cpp 2935

There is no error. The programmer simply divided the two sets of actions. From the point of view of the analyzer, such code is suspicious. Suddenly conditions should be different? However, with false alarms, you need to do something. I decided to combine two conditional operators into one:
 if (nAddress <= _6502_STACK_END) { sprintf( sText,"%04X: ", nAddress ); PrintTextCursorX( sText, rect ); DebuggerSetColorFG( DebuggerGetColor( FG_INFO_OPCODE )); sprintf(sText, " %02X",(unsigned)*(LPBYTE)(mem+nAddress)); PrintTextCursorX( sText, rect ); }

I think that the readability of the code did not suffer from this, but along the way we got rid of the false positives.

The second case is similar: V581 The conditional expressions of the 'if' are agreed alongside each other are identical. Check lines: 2237, 2245. debugger_display.cpp 2245

Picture 12
Figure 5. In the middle of a long article, it is recommended to insert a picture so that the reader can rest. I do not know what kind of article to insert an article. So here's your cat.

Pointer dereferencing before checking

In total, the analyzer issued 3 warnings on this topic. Unfortunately, the text of the program in these places is quite confused. Therefore, for simplicity, I will not provide real code, but pseudocode. For the first 2 warnings, the code looks like this:
 int ZEXPORT unzGetGlobalComment(char *szComment) { .... if (A) { *szComment='\0'; return UNZ_ERRNO; } .... if ((szComment != NULL) && X) .... }

Warning: V595 The 'szComment' pointer was used before it was verified against nullptr. Check lines: 1553, 1558. unzip.c 1553

As you can see, the passed 'szComment' pointer may be NULL. This is evidenced by a check (szComment! = NULL).

However, there is a code section in which the pointer is safely dereferenced without performing a check. Is it dangerous. It is possible that in practice situations never arise when the 'szComment' is equal to 0. But the code is dangerous and should be corrected.

Similarly: V595 The 'pToken_' pointer was used before it was verified against nullptr. Check lines: 811, 823. debugger_parser.cpp 811

But with the last, third case, everything is more complicated. I'm already tired of proving that such a code is incorrect and should be fixed. The function is short, so I will bring it all
 bool ArgsGetValue ( Arg_t *pArg, WORD * pAddressValue_, const int nBase ) { TCHAR *pSrc = & (pArg->sArg[ 0 ]); TCHAR *pEnd = NULL; if (pArg && pAddressValue_) { *pAddressValue_ = (WORD)(_tcstoul( pSrc, &pEnd, nBase) & _6502_MEM_END); return true; } return false; }

Warning: V595 The 'pArg' pointer was used before it was verified against nullptr. Check lines: 204, 207. debugger_parser.cpp 204

The 'pArg' pointer may be zero, as indicated by the “if (pArg && pAddressValue_)” condition. But before the pointer is checked, it is used in the expression:
 TCHAR *pSrc = & (pArg->sArg[ 0 ]);

This expression leads to undefined program behavior. You cannot dereference null pointers.

Many people object that in this code memory access does not occur, but a certain address is simply calculated. Therefore, there is no problem. However, this is too narrow an interpretation of undefined behavior. In general, no need to guess how the compiler will behave and how the code will work. So you can not write, and it makes no sense to discuss why.

Indefinite behavior in this code is not only referring to a zero memory address (which really may not be). For example, the compiler is quite right to reduce the test condition to “if (pAddressValue_)”. Since there is an expression “pArg-> xxx”, then the pointer is definitely not zero and should not be checked.

There is no sense in discussing this issue in more detail. I suggest to get acquainted with a special article: Dereferencing a null pointer leads to undefined behavior .

To fix the code is simple. It is enough to transfer the declaration of the variable inside the 'if'.

Scary expression

The analyzer confused the following expression:
 if ((cx > 4) & (cx <= 13))

Warning: V602 Consider inspecting the '(cx> 4)' expression. '>' possibly should not be replaced with '>>'. debug.cpp 8933

The analyzer sees that the '&' operator is used, whose operands are of the 'bool' type used. This is strange. Usually in such cases it is common to use the logical operator '&&'.

The operator '&' is used for bit operations. Therefore, the analyzer suggested the possibility that they wanted to work with bits here:
 if ((cx >> 4) & (cx <= 13))

The analyzer is too smart and wrong. However, the programmer’s fault is also there. This code is with a smell. Much more natural to write:
 if (cx > 4 && cx <= 13)

Unspeakable behavior and horror in macros

It is not clear what the result will be when you shift negative values ​​to the right. It’s better not to do this, since the behavior of the code may change on another compiler.
 const short SPKR_DATA_INIT = (short)0x8000; if (g_nSpeakerData == (SPKR_DATA_INIT >> 2))

Warning: V610 Unspecified behavior. Check the shift operator '>>'. The left operand 'SPKR_DATA_INIT' is negative. speaker.cpp 450

Output: you can declare the constant SPKR_DATA_INIT without a tank. True, then you will need to make a few more minor edits to prevent the compiler and analyzer from warning about comparing signed / unsigned numbers.

The analyzer found 3 more such dangerous places:
By the way, when I started to edit the last two warnings, I found 2 more errors. We can say that the analyzer helped to find them indirectly.

Here is how the macro is used:
 SET_PP_16(TFE_PP_ADDR_SE_BUSST, busst & ~0x180);

It opens in a long line. Therefore, I will show only its part:
 ..... = (busst & ~0x180 >> 8) & 0xFF; .....

The priority of the shift operator >> is greater than the priority of the & operator. See table: priorities of operations .

The programmer expected the code to work like this:
 ..... = ((busst & ~0x180) >> 8) & 0xFF; .....

And in fact, it works like this:
 ..... = (busst & (~0x180 >> 8)) & 0xFF; .....

That's why the PVS-Studio analyzer warns: “the left operand '~ 0x180' is negative”.

This is how dangerous it is to use macros!

Holes in safety

The project uses the sprintf (), wsprintf (), and so on functions in a very dangerous way. In brief, the functions are used as follows:
 sprintf(buf, STR);

If the string STR contains control characters such as "% s", the consequences will be unpredictable.

Usually such a code is considered as a security hole (see details ).

However, for the emulator, I think this is not critical. No one will attack him. However, such code is dangerous in itself. It can easily lead to the fall of the program or to its incorrect operation.

Correctly do this: sprintf (buf, "% s", STR);

The analyzer found quite a lot of dangerous function calls. In total, he issued 21 warnings .

Opposite conditions

 // TO DO: Need way of determining if DirectX init failed if (soundtype != SOUND_WAVE) { if (soundtype == SOUND_WAVE) soundtype = SOUND_SMART;

Warning: V637 The second condition is always false. Check lines: 270, 272. speaker.cpp 270

Judging by the comment, the code is not added. It is difficult to say how to deal with such a code. I decided to comment out the second meaningless 'if':
 if (soundtype != SOUND_WAVE) { //if (soundtype == SOUND_WAVE) // soundtype = SOUND_SMART;

Bad code alignment

The code looks as if both actions refer to the 'if' operator:
 { if ((Slot4 == CT_MockingboardC) || (Slot4 == CT_Phasor)) m_PropertySheetHelper.GetConfigNew().m_Slot[4] = CT_Empty; m_PropertySheetHelper.GetConfigNew().m_Slot[5] = CT_SAM; }

Warning: V640 The code's operational logic does not correspond with its formatting. The statement is indented. It is possible that curly brackets are missing. pagesound.cpp 229

As I understand it, there is no error in the code. However, this is not a false positive. The analyzer is absolutely right to warn about such code. It is necessary to correct alignment:
 { if ((Slot4 == CT_MockingboardC) || (Slot4 == CT_Phasor)) m_PropertySheetHelper.GetConfigNew().m_Slot[4] = CT_Empty; m_PropertySheetHelper.GetConfigNew().m_Slot[5] = CT_SAM; }

Incorrect operation with the strncat () function

 strncat( sText, CHC_DEFAULT, CONSOLE_WIDTH ); strncat( sText, pHelp , CONSOLE_WIDTH );

Warning: V645 The 'strncat' function call could lead to the 'sText' buffer overflow. It can hold. debugger_help.cpp 753

The third argument of the function is the number of characters that can still be added to the string. Therefore, it is right and safe to do so:
 strncat( sText, CHC_DEFAULT, sizeof(sText) - strlen(sText) - 1); strncat( sText, pHelp , sizeof(sText) - strlen(sText) - 1);

Details can be found by reviewing the description of the V645 diagnostic.

Extra checks

For a very long time, the 'new' operator has generated an exception std :: bad_alloc if it cannot allocate memory. However, programs can still meet unnecessary checks:
 BYTE* pNewImageBuffer = new BYTE [uNewImageSize]; _ASSERT(pNewImageBuffer); if (!pNewImageBuffer) return false;

Alert: V668 It is not a rule of reference for the pnewImageBuffer. The exception will be generated in the case of memory allocation error. diskimagehelper.cpp 197

_ASSERT and validation can and should be removed. There is no point in them.

Similar situation:

Independent declaration of system types

The project consistently defines some data types:
 typedef unsigned long ULONG; typedef void *LPVOID; typedef unsigned int UINT;

There is no obvious error here. We assume that this is a "code with a smell" and suppress warnings using the comment // - V677.

Violated the "Law of the Big Two"

For example, operator = is declared in the CConfigNeedingRestart class, but there is no copy constructor. This violates the Big Two Law .

The class is large enough, so I will not give code snippets here. Take a word.

In this class, all fields are simple types, so your own operator = is not needed at all. The class will be successfully copied automatically.

With the class Disk_t the situation is similar. And here and there, you can remove operator =.

Analyzer Warnings:

Typo

 int nHeight=nHeight=g_aFontConfig[ FONT_CONSOLE ]._nFontHeight;

Warning: V700 Consider inspecting the 'T foo = foo = ...' expression. It is odd that variable is initialized through itself. debugger_display.cpp 1226

Just a typo. Replaced with:
 int nHeight = g_aFontConfig[ FONT_CONSOLE ]._nFontHeight;

Unnecessary analyzer concern about enumerations

The enumeration 'AppMode_e' has the following named constants: MODE_LOGO, MODE_PAUSED, MODE_RUNNING, MODE_DEBUG, MODE_STEPPING.

The analyzer experiences that not all of them are used in this switch ():
 switch (g_nAppMode) { case MODE_PAUSED : _tcscat(.....); break; case MODE_STEPPING: _tcscat(.....); break; }

Warning: V719 The switch statement appMode_e 'enum: MODE_DEBUG, MODE_LOGO, MODE_RUNNING. frame.cpp 217

In this example, I am even a little ashamed of the analyzer. Empirical algorithms have failed. False alarm. There are several ways to eliminate it. For example, you can add a branch «default».
 switch (g_nAppMode) { case MODE_PAUSED : _tcscat(.....); break; case MODE_STEPPING: _tcscat(.....); break; default: break; }

Another similar false positive: V719 The switch statement doesn’t cover all values ​​of the 'AppMode_e' enum: MODE_DEBUG, MODE_LOGO. frame.cpp 1210

I promised to take a quick look at level three warnings.

We do not recommend (at least in the early stages) to even look at level 3. There are many false, uninteresting or specific messages. Now this is the situation.

For example, there are quite a few warnings V601.
 inline int IsDebugBreakpointHit() { if ( !g_bDebugNormalSpeedBreakpoints ) return false; return _IsDebugBreakpointHit(); }

: V601 The 'false' value is implicitly cast to the integer type. debug.h 210

'int'. «return false».

, , . 3 .

:
 double g_fClksPerSpkrSample; .... if ((double)g_nRemainderBufferSize != g_fClksPerSpkrSample)

: V550 An odd precise comparison. It's probably better to use a comparison with defined precision: fabs(A — B) > Epsilon. speaker.cpp 197

— , 'double'.

. , double , , . .

, (1 2 ), . — (. 6).

6. 1 2 .
6. 1 2 .

, . , , , . , , .

Let's sum up

, . . , . .

. , - , , 20 , . . , 90%.

, , . , « ». , , . .

. : « ? /?». , , .

, . , .

, PVS-Studio Apple II emulator for Windows::

Conclusion

PVS-Studio . : http://www.viva64.com/ru/pvs-studio-download/

. twitter . Thank.

PS /++, twitter: https://twitter.com/Code_Analysis

.


, : Andrey karpov. An Ideal Way to Integrate a Static Code Analyzer into a Project .

Read the article and have a question?
Often our articles are asked the same questions. We collected answers to them here: Answers to questions from readers of articles about PVS-Studio, version 2015 . Please review the list.

Source: https://habr.com/ru/post/263695/

More articles:


All Articles