Команда разработчиков PVS-Studio сравнила свой статический анализатор кода PVS-Studio с открытым (free, open source) статическим анализатором кода Cppcheck. Для сравнения были выбраны исходные коды трех открытых проектов id Software: Doom 3, Quake 3: Arena, Wolfenstein: Enemy Territory. В статье приведена методика сравнения, списки обнаруженных ошибок. В заключении приведены "не выводы", так как мы сознательно не делаем никаких выводов. Каждый может сам повторить наше сравнение и сделать свои выводы.
Задача сравнения статических анализаторов кода - очень сложная и неблагодарная. Прежде всего, потому что надо разработать методику сравнения, иметь доступ к инструментам, хорошую базу примеров ошибок (причем лучше реальных, а не синтетических). Кроме этого сравнение вообще стоит делать по двум отдельным категориям: диагностические возможности и удобство использования. Сравнение диагностических возможностей "по-хорошему" должно учитывать количество обнаруженных ошибок, количество необнаруженных ошибок, количество ложных срабатываний. Сравнение удобства использования - вообще трудно оценить в цифрах. Кому-то нужна command line версия, а кому-то интегрированная в среду разработки (а ведь их много разных). Кому-то нужно использование в команде, а кому-то - индивидуальное. А если еще вспомнить про разные программные (Windows, Linux) и аппаратные (x86, AMR) платформы... Короче говоря, здесь вполне вакантно место для независимой некоммерческой организации, которая будет заниматься сравнением инструментов анализа кода аналогично компаниям, тестирующим разные антивирусные продукты (например, австрийская AV-Comparatives). Есть, конечно же, Gartner с их (Magic Quadrant for Static Application Security Testing), но этого явно недостаточно. В общем, ниша свободна.
Авторы и контрибьюторы Cppcheck, а особенно, Daniel Marjamäki - крутые ребята, делающие полезное дело. Их инструмент - достоин внимания, а сами они - молодцы. Тем более что их продукт полностью free, а значит каждый, кто интересуется статическим анализом кода, может очень легко его посмотреть.
Наш бизнес - это инструмент статического анализа C/C++ кода PVS-Studio. И раз уж так получилось, что мы являемся конкурентами, то мне приходится отвечать на вопросы наших пользователей: "А вы сравнивали PVS-Studio с Cppcheck?". На такие вопросы мне приходится отвечать одним из следующих вариантов:
В любом случае все эти ответы хоть и правильные, но не всегда устраивают людей. Поэтому мы вынуждены выполнить сравнение PVS-Studio и Cppcheck, и опубликовать его, чтобы было, что отвечать людям более конкретно.
Эта статья НЕ служит целью показать, что определенные функции одного продукта хороши, а другие плохи. Это просто один из способов сравнить анализаторы, чтобы читатели могли САМИ сделать выбор в пользу наиболее подходящего им инструмента.
Для сравнения PVS-Studio 4.61 и Cppcheck 1.54 мы взяли исходный код трех проектов id Software с GitHub: Doom 3, Quake 3: Arena, Wolfenstein: Enemy Territory. Прогнали на нем оба анализатора, получили список ошибок. Затем просмотрели (вручную) весь список. И отобрали те ошибки, которые являются реальными. Мы НЕ отбирали неудачный код, возможно некорректные конструкции и т.п. Только явные ошибки.
У нашего сравнения есть недостатки:
Таким образом, кому интересно РЕАЛЬНО сравнить PVS-Studio и Cppcheck, мы предлагаем самостоятельно скачать инструменты, скачать исходники и выполнить сравнение. Trial-версии PVS-Studio будет достаточно, чтобы найти и просмотреть все ошибки. А Cppcheck вообще бесплатен. Только так вы будете знать, какой из этих двух инструментов лучше подходит именно для вас.
Фрагмент 1
..\..\[Build]\Doom3\id-Software-DOOM-3-a9c49da\neo\idlib\hashing\MD5.cpp(252):
Using size of pointer ctx instead of size of its data.
void MD5_Final( MD5_CTX *ctx, unsigned char digest[16] ) {
...
memset( ctx, 0, sizeof( ctx ) ); /* In case it's sensitive */Здесь должно быть sizeof(*ctx). Как написано сейчас - передается размер указателя и обнуляется объект не полностью.
Фрагмент 2
..\..\[Build]\Doom3\id-Software-DOOM-3-a9c49da\neo\renderer\Image_init.cpp(2214)
Mismatching allocation and deallocation: sortIndex
void idImageManager::PrintMemInfo( MemInfo_t *mi ) {
int *sortIndex;
...
sortIndex = new int[images.Num()];
...
delete sortIndex;Ошибка связана с тем, что память выделяется как для массива, а освобождается как от одного элемента. Конкретно в этом случае (для int) это не проблема, но если бы создавался массив объектов с деструктором, то вызван бы был только один деструктор, а не все. Правильный вариант delete [] sortIndex.
Фрагмент 3
..\..\[Build]\Doom3\id-Software-DOOM-3-a9c49da\neo\renderer\MegaTexture.cpp(542)
Using size of pointer newBlock instead of size of its data.
void idMegaTexture::GenerateMegaMipMaps(
megaTextureHeader_t *header, idFile *outFile ) {
...
byte *newBlock = (byte *)_alloca( tileSize );
...
memset( newBlock, 0, sizeof( newBlock ) );Такая же ошибка, как и во фрагменте 1 - надо писать sizeof(*newBlock));
Фрагмент 4
..\..\[Build]\Doom3\id-Software-DOOM-3-a9c49da\neo\sys\win32\win_shared.cpp(177)
memset() called to fill 0 bytes of '&'
void Sys_GetCurrentMemoryStatus( sysMemoryStats_t &stats ) {
...
memset( &statex, sizeof( statex ), 0 );Здесь перепутаны местами второй и третий аргументы - должно быть memset(&statex, 0, sizeof( statex)). Особенность данной ошибки в том, что глазами ее заметить очень сложно.
Фрагмент 1
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 524, 533. anim_blend.cpp(524)
const char *idAnim::AddFrameCommand(
const idDeclModelDef *modelDef,
int framenum, idLexer &src, const idDict *def ) {
...
} else if ( token == "muzzle_flash" ) {
if( !src.ReadTokenOnLine( &token ) ) {
return "Unexpected end of line";
}
...
} else if ( token == "muzzle_flash" ) {
fc.type = FC_MUZZLEFLASH;
fc.string = new idStr( "" );
...В этой функции две одинаковые ветки if с разным содержанием. Скорее всего, в одной из них опечатка.
Фрагмент 2
V556 The values of different enum types are compared. af.cpp 895
class idDeclAF_Constraint {
...
declAFConstraintType_t type;
...
};
constraintType_t GetType( void ) const { return type; }
bool idAF::Load( idEntity *ent, const char *fileName ) {
...
if (
file->constraints[j]->name.Icmp(
constraint->GetName() ) == 0 &&
file->constraints[j]->type == constraint->GetType() )
{
...В этом фрагменте кода сравниваются значения разных типов, т.е. принадлежащих разным enum. Хотя в каких-то отдельных случаях это может и работать, но это - явная ошибка.
Фрагмент 3
V528 It is odd that pointer to 'char' type is compared with the '\0' value.
Probably meant: *classname != '\0'. game_local.cpp 1250
const char *classname = mapEnt->epairs.GetString( "classname" );
if ( classname != '\0' ) {
FindEntityDef( classname, false );
}Здесь хотели проверить строку classname, чтобы убедиться, что она не пустая. Однако сравнение не отрабатывает, так как надо разыменовать указатель.
Фрагмент 4
V528 It is odd that pointer to 'char' type is compared with the '\0' value.
Probably meant: *soundShaderName != '\0'. game_local.cpp 1619
soundShaderName = dict->GetString( "s_shader" );
if (soundShaderName != '\0' && dict->GetFloat("s_shakes") != 0.0f){
soundShader = declManager->FindSound( soundShaderName );Ошибка повторяет фрагмент 3 - нужно разыменование указателя.
Фрагмент 5
V514 Dividing sizeof a pointer 'sizeof (clientInPVS)' by another value. There is a probability of logical error presence. game_network.cpp 686
void idGameLocal::ServerWriteSnapshot(
int clientNum, int sequence, idBitMsg &msg,
byte *clientInPVS, int numPVSClients ) {
...
memcpy( clientInPVS, snapshot->pvs,
( numPVSClients + 7 ) >> 3 );
LittleRevBytes( clientInPVS, sizeof( int ),
sizeof( clientInPVS ) / sizeof ( int ) );
}Здесь видна целая история из жизни данного фрагмента кода. Когда-то clientInPVS был локальным массивом и sizeof(clientInPVS)/sizeof(int) вычислял действительно количество элементов. Однако со временем clientInPVS стали передавать как параметр в функцию. Но код остался прежним. В результате значение sizeof(clientInPVS)/sizeof(int) всегда равняется или 1 для 32-битной, или 2 для 64-битной платформы. Для исправления надо явно передавать количество элементов.
Фрагмент 6
V599 The destructor was not declared as a virtual one, although the 'BOBrick' class contains virtual functions. gamebustoutwindow.cpp 509
class BOBrick {
...
virtual void WriteToSaveGame( idFile *savefile );
virtual void ReadFromSaveGame( idFile *savefile,
idGameBustOutWindow *game );
};
BOBrick *paddle;
void idGameBustOutWindow::ReadFromSaveGame( idFile *savefile ) {
idWindow::ReadFromSaveGame( savefile );
// Clear out existing paddle and entities from GUI load
delete paddle;В данном фрагменте класс содержит виртуальные функции, но не содержит виртуального деструктора. И хотя это не всегда проблема, лучше всегда заводить виртуальный деструктор в таком случае, чтобы проблема не возникла позже.
Фрагмент 7
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1931, 1933. gamessdwindow.cpp 1931
void idGameSSDWindow::FireWeapon(int key) {
...
} else
if(gameStats.levelStats.targetEnt->type == SSD_ENTITY_ASTRONAUT) {
HitAstronaut(static_cast<SSDAstronaut*>(
gameStats.levelStats.targetEnt), key);
} else
if(gameStats.levelStats.targetEnt->type == SSD_ENTITY_ASTRONAUT) {Опять проверяется одно и то же условие в разных ветках кода. Скорее всего просто неудачно скопировано.
Фрагмент 8
V535 The variable 'i' is being used for this loop and for the outer loop. matrix.cpp 3128
bool idMatX::IsOrthonormal( const float epsilon ) const
{
for ( int i = 0; i < numRows; i++ ) {
...
for ( i = 1; i < numRows; i++ ) {Странность кода заключается в том, что счетчик цикла i используется и для внешнего, и для внутреннего цикла.
Фрагмент 9
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. md5.cpp 252
void MD5_Final( MD5_CTX *ctx, unsigned char digest[16] ) {
...
memset( ctx, 0, sizeof( ctx ) ); /* In case it's sensitive */Здесь должно быть sizeof(*ctx), а как написано сейчас - передается размер указателя и обнуляется объект не полностью.
Фрагмент 10
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. model_ase.cpp 731
typedef struct {
...
} aseMesh_t;
aseMesh_t *currentMesh;
...
ase.currentMesh = &ase.currentObject->mesh;
memset( ase.currentMesh, 0, sizeof( ase.currentMesh ) );Не первый раз встречающаяся ошибка, вместо размера объекта в функцию memset передается размер указателя, который всегда один и тот же.
Фрагмент 11
V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. model_lwo.cpp 1251
int sgetI1( unsigned char **bp )
{
...
*bp++;Не менее распространенная проблема - увеличение значения указателя вместо увеличения значения объекта, на который он указывает. Правильный вариант - (*bp)++.
В этом же файле есть еще две похожие ошибки, которые не включены в отчет.
Фрагмент 12
V533 It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'j'. surface_polytope.cpp 65
void idSurface_Polytope::FromPlanes(
const idPlane *planes, const int numPlanes ) {
for ( j = 0; j < w.GetNumPoints(); j++ ) {
for ( k = 0; k < verts.Num(); j++ ) {Здесь внутренний цикл идет по переменной k, однако увеличивается переменная j. Издержки копирования кода.
Фрагмент 13
V535 The variable 'i' is being used for this loop and for the outer loop. weapon.cpp 2533
const char *idWeapon::GetAmmoNameForNum( ammo_t ammonum )
{
...
for ( i = 0; i < 2; i++ ) {
...
for( i = 0; i < num; i++ ) {Вновь для внутреннего и внешнего счетчиков цикла используется одна и та же переменная.
Фрагмент 14
V575 The 'memset' function processes '0' elements. Inspect the third argument. win_shared.cpp 177
void Sys_GetCurrentMemoryStatus( sysMemoryStats_t &stats ) {
...
memset( &statex, sizeof( statex ), 0 );Здесь перепутаны местами второй и третий аргументы - должно быть memset(&statex, 0, sizeof( statex)). Особенность данной ошибки в том, что глазами ее заметить очень сложно.
Фрагмент 15
V512 A call of the 'memset' function will lead to underflow of the buffer '& cluster'. aasfile.cpp 1312
void idAASFileLocal::DeleteClusters( void ) {
aasPortal_t portal;
aasCluster_t cluster;
...
// first portal is a dummy
memset( &portal, 0, sizeof( portal ) );
portals.Append( portal );
// first cluster is a dummy
memset( &cluster, 0, sizeof( portal ) );
clusters.Append( cluster );
}Очень красивая ошибка. Копирование кода до добра не доводит. Во втором блоке забыли заменить sizeof(portal) на sizeof(cluster).
Фрагмент 16
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. megatexture.cpp 542
void idMegaTexture::GenerateMegaMipMaps(
megaTextureHeader_t *header, idFile *outFile ) {
...
byte *newBlock = (byte *)_alloca( tileSize );
...
memset( newBlock, 0, sizeof( newBlock ) );Здесь надо писать sizeof(*newBlock), иначе используется размер указателя.
Фрагмент 17
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. target.cpp 257
#define BIT( num ) ( 1 << ( num ) )
const int BUTTON_ATTACK = BIT(0);
void idTarget_WaitForButton::Think( void ) {
idPlayer *player;
...
if ( player && ( !player->oldButtons & BUTTON_ATTACK ) &&
( player->usercmd.buttons & BUTTON_ATTACK ) ) {
player->usercmd.buttons &= ~BUTTON_ATTACK;Здесь из-за приоритета оператора "!" (который выше, чем у "&") получилось некорректное условие. Хотели проверить, что младший бит равен нулю, а проверили, что все биты равны нулю.
Обнаружено ошибок с помощью Cppcheck: 4.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 3.
Обратите внимание, что возможно набор проверяемых файлов не полностью идентичен.
Фрагмент 1
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\code\q3_ui\ui_servers2.c 580
Using sizeof with a numeric constant as function argument might not be what you intended.
static void ArenaServers_Remove( void )
{
...
memcpy( &g_arenaservers.favoriteaddresses[i],
&g_arenaservers.favoriteaddresses[i+1],
(g_arenaservers.numfavoriteaddresses - i - 1)*
sizeof(MAX_ADDRESSLENGTH));В этом коде используется странное выражение sizeof(MAX_ADDRESSLENGTH). Это всегда будет размер типа этой переменной, а не ее значение. Вероятно, должно быть просто MAX_ADDRESSLENGTH без sizeof().
Фрагмент 2
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\code\qcommon\files.c 549
Memory leak: buf
static void FS_CopyFile( char *fromOSPath, char *toOSPath ) {
...
byte *buf;
...
buf = malloc( len );
if (fread( buf, 1, len, f ) != len)
Com_Error( ERR_FATAL, "Short read in FS_Copyfiles()\n" );
fclose( f );
if( FS_CreatePath( toOSPath ) ) {
return;
}
...
}Здесь вполне возможна ситуация, когда выделенная для buf память так и останется не освобожденной. Классический пример того, зачем в C++ придумали умные указатели.
Фрагмент 3
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\code\renderer\tr_shade_calc.c 628
Array 'invModulate[3]' index 3 out of bounds
void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors )
{
...
unsigned char invModulate[3];
...
invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
// this trashes alpha, but the AGEN block fixes itОпять промахиваемся с размером массива и количеством элементов.
Фрагмент 4
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\code\server\sv_rankings.c 947
Assert statement modifies 'j'.
assert( (j++) < 68 );Странный фрагмент - ведь assert в release-сборке обычно отсутствует. Таким образом, не понятно, то ли j++ должно быть ВНЕ assert, то ли это код действительно только для debug-версии.
Фрагмент 5
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\code\splines\math_matrix.h 87
Using sizeof for array given as function argument returns the size of pointer.
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью. Переменная src - это просто указатель.
Фрагмент 6
..\..\[Build]\Quake3\id-Software-Quake-III-Arena-dbe4ddb\lcc\src\2html.c 131
printf format string has 2 parameters but 3 are given
static void do_uid(int x) {
printf("<a href='#%d'>%d</a>", x, x, x);
}Здесь printf печатает два числа, а параметров передается три. То ли лишний параметр передают, то ли забыли его напечатать.
Фрагмент 1
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (src)' expression. math_matrix.h 87
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью.
Фрагмент 2
V523 The 'then' statement is equivalent to the 'else' statement. be_aas_sample.c 864
int AAS_TraceAreas(vec3_t start, vec3_t end, int *areas,
vec3_t *points, int maxareas)
{
...
if (front < 0)
frac = (front)/(front-back);
else
frac = (front)/(front-back);Здесь переменная frac рассчитывается одинаковым образом, хотя перед этим проверяется условие. Возможно, она должна рассчитываться по-разному.
Фрагмент 3
V568 It's odd that the argument of sizeof() operator is the '& itemInfo' expression. cg_weapons.c 849
void CG_RegisterItemVisuals( int itemNum ) {
...
itemInfo_t *itemInfo;
memset( itemInfo, 0, sizeof( &itemInfo ) );Третьим аргументом memset оказывается размер указателя, а не размер объекта.
Фрагмент 4
V557 Array overrun is possible. The 'sizeof (bs->teamleader)' index is pointing beyond array bound. ai_cmd.c 1311
char teamleader[32]; //netname of the team leader
void BotMatch_StartTeamLeaderShip(
bot_state_t *bs, bot_match_t *match)
{
...
bs->teamleader[sizeof(bs->teamleader)] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 5
V557 Array overrun is possible. The value of 'i' index could reach 3. g_main.c 776
int numteamVotingClients[2];// set by CalculateRanks
typedef enum {
TEAM_FREE,
TEAM_RED,
TEAM_BLUE,
TEAM_SPECTATOR,
TEAM_NUM_TEAMS
} team_t;
void CalculateRanks( void ) {
...
for ( i = 0; i < TEAM_NUM_TEAMS; i++ ) {
level.numteamVotingClients[i] = 0;
}Массив состоит всего лишь из двух элементов, а значения enum, используемые в качестве счетчика явно больше этого. Из-за чего естественно возникает выход за границы массива.
Фрагмент 6
V579 The Com_Memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cvar.c 763
void Cvar_Restart_f( void ) {
...
cvar_t *var;
...
Com_Memset( var, 0, sizeof( var ) );Опять передается не размер объекта, а размер указателя. Правильный вариант sizeof(*var).
Фрагмент 7
V557 Array overrun is possible. The '3' index is pointing beyond array bound. tr_shade_calc.c 628
void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors )
{
...
unsigned char invModulate[3];
...
invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
// this trashes alpha, but the AGEN block fixes itПромахиваемся по массиву из-за того, что элементов не 4, а 3.
Обнаружено ошибок с помощью Cppcheck: 6.
Обнаружено ошибок с помощью PVS-Studio: 7.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 2.
Обратите внимание, что возможно набор проверяемых файлов не полностью идентичен.
Фрагмент 1
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\curl-7.12.2\docs\examples\sepheaders.c 76
Resource leak: bodyfile
bodyfile = fopen( bodyfilename,"w" );
...
// no fclose for bodyfileЗдесь классическая утечка ресурсов - файл открывается, но не закрывается. Конечно, этот код находится в файле examples, что снимает с него вину. Но утечка от этого не пропадает.
Фрагмент 2
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\curl-7.12.2\src\main.c 3765
Undefined behavior: variable is used as parameter and destination in s[n]printf().
sprintf( dirbuildup,"%s%s%s",dirbuildup, DIR_CHAR, tempdir );Строка печатается сама в себя. Это может привести к проблемам в большинстве случаев.
Фрагмент 3
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\game\bg_animation.c 585
Using sizeof for array given as function argument returns the size of pointer.
void BG_ParseConditionBits( char **text_pp,
animStringItem_t *stringTable, int condIndex, int result[2] ) {
...
memset( result, 0, sizeof( result ) );Одним из аргументов функции является массив. Его размер пытаются вычислить с помощью sizeof(), хотя надо явно или передавать размер (что более правильно), или жестко написать размер "2 * sizeof(int)", раз в этом коде он все равно прописан.
Фрагмент 4
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\game\bg_animation.c 776
Using size of pointer command instead of size of its data.
static void BG_ParseCommands( char **input,
animScriptItem_t *scriptItem, animModelInfo_t *animModelInfo,
animScriptData_t *scriptData )
{
...
// TTimo gcc: might be used uninitialized
animScriptCommand_t *command = NULL;
...
memset( command, 0, sizeof( command ) );Здесь вместо размера объекта вычисляется размер указателя.
Фрагмент 5
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\qcommon\cvar.c 905
Using size of pointer var instead of size of its data.
void Cvar_Restart_f( void ) {
cvar_t *var;
...
memset( var, 0, sizeof( var ) );Снова и снова вместо размера объекта используется размер указателя.
Фрагмент 6
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\splines\math_matrix.h 94
Using sizeof for array given as function argument returns the size of pointer.
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью.
Фрагмент 7
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\game\bg_pmove.c 4097
Redundant assignment of "fwdmove_knockback" in switch
switch ( pm->ps->weapon ) {
case WP_MOBILE_MG42: fwdmove_knockback = 4000.f;
fwdmove_knockback = 400.f;
break;
case WP_PANZERFAUST: fwdmove_knockback = 32000.f;
bckmove_knockback = 1200.f;
break;
case WP_FLAMETHROWER: fwdmove_knockback = 2000.f;
bckmove_knockback = 40.f;
break;
}Здесь в ветке WP_MOBILE_MG42 два раза присваивается значение одной и той же переменной.
Фрагмент 8
..\..\[Build]\Enemy Territory\id-Software-Enemy-Territory-40342a9\src\game\q_math.c 422
Array 'pnt[3]' index 3 out of bounds
typedef vec_t vec3_t[3];
void RotatePointAroundVertex( vec3_t pnt,
float rot_x, float rot_y, float rot_z, const vec3_t origin )
{
...
// rotate point
pnt[0] = ( tmp[3] * ( tmp[8] - tmp[9] ) + pnt[3] * tmp[2] );Обращение к pnt[3] приводит к промаху мимо массива.
Фрагмент 1
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (src)' expression. math_matrix.h 94
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью.
Фрагмент 2
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (result)' expression. bg_animation.c 585
void BG_ParseConditionBits( char **text_pp,
animStringItem_t *stringTable, int condIndex, int result[2] )
{
...
memset( result, 0, sizeof( result ) );Одним из аргументов функции является массив. Его размер пытаются вычислить с помощью sizeof(), хотя надо явно или передавать размер (что более правильно), или жестко написать размер "2", раз в этом коде он все равно прописан.
Фрагмент 3
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. bg_animation.c 776
static void BG_ParseCommands( char **input,
animScriptItem_t *scriptItem, animModelInfo_t *animModelInfo,
animScriptData_t *scriptData )
{
// TTimo gcc: might be used uninitialized
animScriptCommand_t *command = NULL;
...
memset( command, 0, sizeof( command ) );Здесь вместо размера объекта вычисляется размер указателя.
Фрагмент 4
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. bg_pmove.c 3257
static void PM_Weapon( void ) {
...
if ( !pm->ps->pm_flags & PMF_LIMBO ) {
PM_CoolWeapons();
}Приоритет операторов приводит к тому, что выражение вычисляется не так, как ожидал программист.
Фрагмент 5
V523 The 'then' statement is equivalent to the 'else' statement. bg_pmove.c 4115
static void PM_Weapon( void ) {
...
if ( DotProduct( pml.forward, pm->ps->velocity ) > 0 )
{
VectorScale( pml.forward, -1.f * ( fwdmove_knockback / mass ),
kvel ); // -1 as we get knocked backwards
} else {
VectorScale( pml.forward, -1.f * ( fwdmove_knockback / mass ),
kvel ); // -1 as we get knocked backwards
}Вне зависимости от условия выполняется одинаковая ветка кода. Возможно, здесь должна быть другая ветка.
Фрагмент 6
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cg_character.c 308
static qboolean CG_CheckForExistingAnimModelInfo(
const char *animationGroup, const char *animationScript,
animModelInfo_t **animModelInfo )
{
...
memset( *animModelInfo, 0, sizeof( *animModelInfo ) );Вычисляется размер указателя, а не размер объекта, так как в функцию передается указатель на указатель.
Фрагмент 7
V519 The 'backColor[2]' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3180, 3181. cg_draw.c 3181
typedef vec_t vec4_t[4];
static void CG_DrawObjectiveInfo( void ) {
...
vec4_t backColor;
backColor[0] = 0.2f;
backColor[1] = 0.2f;
backColor[2] = 0.2f;
backColor[2] = 1.f;Вместо четвертого элемента дважды записывают в третий.
Фрагмент 8
V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: ... }. cg_newdraw.c 720
typedef enum {qfalse, qtrue} qboolean;
qboolean eventHandling;
void CG_MouseEvent( int x, int y ) {
switch ( cgs.eventHandling ) {
case CGAME_EVENT_SPEAKEREDITOR:
case CGAME_EVENT_GAMEVIEW:
case CGAME_EVENT_CAMPAIGNBREIFING:
case CGAME_EVENT_FIRETEAMMSG:В switch и в case используются разные enum.
Фрагмент 9
V568 It's odd that the argument of sizeof() operator is the '& itemInfo' expression. cg_weapons.c 1631
void CG_RegisterItemVisuals( int itemNum ) {
itemInfo_t *itemInfo;
...
memset( itemInfo, 0, sizeof( &itemInfo ) );Третьим аргументом memset оказывается размер указателя, а не размер объекта.
Фрагмент 10
V557 Array overrun is possible. The '3' index is pointing beyond array bound. q_math.c
typedef vec_t vec3_t[3];
void RotatePointAroundVertex( vec3_t pnt, float rot_x,
float rot_y, float rot_z, const vec3_t origin )
{
...
// rotate point
pnt[0] = ( tmp[3] * ( tmp[8] - tmp[9] ) + pnt[3] * tmp[2] );Обращение к pnt[3] приводит к промаху мимо массива.
Фрагмент 11
V557 Array overrun is possible. The 'sizeof (bs->teamleader)' index is pointing beyond array bound. ai_cmd.c 1037
char teamleader[32]; //netname of the team leader
...
bs->teamleader[sizeof( bs->teamleader )] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 12
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. ai_dmq3.c
if ( !g_entities[client].r.svFlags & SVF_BOT ) {
return;
}Из-за приоритета операторов вычисление выражения произойдет не так, как ожидал программист.
Фрагмент 13
V562 It's odd to compare 0 or 1 with a value of 2. ai_main.c 2659
if ( !level.clients[0].pers.connected == CON_CONNECTED ) {
return;
}Приоритет операторов снова меняет суть выражения.
Фрагмент 14
V557 Array overrun is possible. The value of 'i' index could reach 4. g_systemmsg.c 157
#define NUM_PLAYER_CLASSES 5
void G_CheckForNeededClasses( void ) {
qboolean playerClasses[NUM_PLAYER_CLASSES - 1][2];
...
for ( i = 0; i < NUM_PLAYER_CLASSES; i++ ) {
if ( !playerClasses[i][0] ) {
cnt++;
}
}Доступ вне границ массива.
Фрагмент 15
V557 Array overrun is possible. The '3' index is pointing beyond array bound. tr_shade_calc.c 679
void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors ) {
...
unsigned char invModulate[3];
...
invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
// this trashes alpha, but the AGEN block fixes itОпять промахиваемся с размером массива и количеством элементов.
Фрагмент 16
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cvar.c 905
void Cvar_Restart_f( void ) {
cvar_t *var;
...
memset( var, 0, sizeof( var ) );Опять передается не размер объекта, а размер указателя. Правильный вариант sizeof(*var).
Фрагмент 17
V519 The 'fwdmove_knockback' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 4097, 4098. bg_pmove.c 4098
static void PM_Weapon( void ) {
...
if ( !( pm->ps->eFlags & EF_PRONE ) && (
pml.groundTrace.surfaceFlags & SURF_SLICK ) ) {
float fwdmove_knockback = 0.f;
float bckmove_knockback = 0.f;
switch ( pm->ps->weapon ) {
case WP_MOBILE_MG42: fwdmove_knockback = 4000.f;
fwdmove_knockback = 400.f;
break;
case WP_PANZERFAUST: fwdmove_knockback = 32000.f;
bckmove_knockback = 1200.f;
break;
case WP_FLAMETHROWER: fwdmove_knockback = 2000.f;
bckmove_knockback = 40.f;
break;
}Здесь в ветке WP_MOBILE_MG42 два раза присваивается значение одной и той же переменной.
Обнаружено ошибок с помощью Cppcheck: 8.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 6.
Обратите внимание, что возможно набор проверяемых файлов не полностью идентичен.
Обнаружено ошибок с помощью Cppcheck: 4.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 3.
Обнаружено ошибок с помощью Cppcheck: 6.
Обнаружено ошибок с помощью PVS-Studio: 7.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 2.
Обнаружено ошибок с помощью Cppcheck: 8.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Cppcheck, и PVS-Studio): 6.
Обратите внимание, что возможно набор проверяемых файлов не полностью идентичен.
Я не хочу делать никакие выводы из результатов этого сравнения. Эти результаты не значат, что один инструмент лучше. Просто мы запустили два инструмента, и нашли вот такие ошибки - и все. Выводы каждый делает сам. И лучше - проверив проекты самостоятельно. Возможно, мы даже что-то даже пропустили. Зачем же тогда эта статья? Просто теперь мне есть, что отвечать пользователям на вопрос: "А вы сравнивались с Cppcheck?".