To get a trial key
fill out the form below
Team license
Enterprise license
** By clicking this button you agree to our Privacy Policy statement

Request our prices
New License
License Renewal
--Select currency--
USD
EUR
RUB
* By clicking this button you agree to our Privacy Policy statement

Free PVS-Studio license for Microsoft MVP specialists
** By clicking this button you agree to our Privacy Policy statement

To get the licence for your open-source project, please fill out this form
** By clicking this button you agree to our Privacy Policy statement

I am interested to try it on the platforms:
** By clicking this button you agree to our Privacy Policy statement

Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Analyzing the Quake III Arena GPL proje…

Analyzing the Quake III Arena GPL project

Feb 06 2012
Author:

As you know, the id Software company has laid out source codes of many of their games. We already checked some of these projects earlier. This time we decided to analyze the Quake III Arena GPL source code. Analysis was performed with PVS-Studio 4.54.

Unfortunately, the post about the check appeared to be bare and without detailed comments. We haven't found such errors in Quake III Arena GPL as to write an interesting article about. Moreover, some of the found errors we have already seen when checking the Doom 3 game's code.

Below we will cite code fragments with various errors and corresponding diagnostic messages that helped us to find them. As usually, we want to note that these are far not all the errors the PVS-Studio analyzer can detect in this project:

  • If an error is found several times, only one case is described.
  • The article does not contain descriptions of unessential errors.
  • We do not cite code fragments about which we cannot decide quickly whether or not there is an error.

Readers can study diagnostic messages generated by PVS-Studio themselves if they wish. The new trial mode allows you to do that easily.

Fragment N1.

Diagnostic message V511.

The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof(src)' expression.

ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
  memcpy( mat, src, sizeof( src ) );
}

Correct code:

memcpy( mat, src, sizeof(float) * 3 * 3);

Fragment N2.

Diagnostic message V501.

There are identical sub-expressions '(result->flags & 64)' to the left and to the right of the '||' operator.

void BotMoveToGoal(....)
{
  ...
  if ((result->flags & MOVERESULT_ONTOPOF_FUNCBOB) ||
      (result->flags & MOVERESULT_ONTOPOF_FUNCBOB))
  {
    ms->reachability_time = AAS_Time() + 5;
  }
  ...
}

Fragment N3.

Diagnostic message V510.

The 'ScriptError' function is not expected to receive class-type variable as third actual argument.

typedef struct punctuation_s
{
  char *p;
  int n;
  struct punctuation_s *next;
} punctuation_t;

punctuation_t *punctuations;

int PS_ExpectTokenType(script_t *script, ....)
{
  ...
  ScriptError(script, "expected %s, found %s",
    script->punctuations[subtype], token->string);
  ...
}

Fragment N4.

Diagnostic message V570.

The 'p->org[0]' variable is assigned to itself.

void CG_ParticleSnowFlurry (qhandle_t pshader, centity_t *cent)
{
  ...
  p->org[0] = p->org[0];
  p->org[1] = p->org[1];
  p->org[2] = p->org[2];
  ...
}

Fragment N5.

Diagnostic message V568.

It's odd that the argument of sizeof() operator is the '& itemInfo' expression.

void CG_RegisterItemVisuals( int itemNum ) {
  itemInfo_t  *itemInfo;
  ...
  memset( itemInfo, 0, sizeof( &itemInfo ) );
}

Correct code:

memset( itemInfo, 0, sizeof( *itemInfo ) );

Fragment N6.

Diagnostic message V595.

The 'item' pointer was utilized before it was verified against nullptr. Check lines: 3865, 3869.

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;
  }
  ...
}

Fragment N7.

Diagnostic message V557.

Array overrun is possible. The 'sizeof (bs->teamleader)' index is pointing beyond array bound.

typedef struct bot_activategoal_s
{
  ...
  float leadbackup_time;
  char teamleader[32];
  float askteamleader_time;
  ...
} bot_state_t;

void BotTeamAI(bot_state_t *bs) {
  ...
  bs->teamleader[sizeof(bs->teamleader)] = '\0';
  ...
}

Fragment N8.

Diagnostic message V579.

The Com_Memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument.

void Cvar_Restart_f( void ) {
  cvar_t *var;
  ...
  // clear the var completely, since we
  // can't remove the index from the list
  Com_Memset( var, 0, sizeof( var ) );
  ...
}

Correct code:

Com_Memset( var, 0, sizeof( *var ) );

Fragment N9.

Diagnostic message V557.

Array overrun is possible. The '3' index is pointing beyond array bound.

void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors )
{
  int *pColors = ( int * ) dstColors;
  unsigned char invModulate[3];
  int c;
  ...
  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];
  ...  
}

Fragment N10.

Diagnostic message V521.

Such expressions using the ',' operator are dangerous. Make sure the expression is correct.

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);
  ...
}

Correct code:

Q1_MAX_MAP_EDGES * sizeof(q1_dedge_t);

Fragment N11.

Diagnostic message V595.

The 'node' pointer was utilized before it was verified against nullptr. Check lines: 769, 770.

void FloodPortals_r (node_t *node, int dist)
{
  ...
  if (node->occupied)
    Error("FloodPortals_r: node already occupied\n");
  if (!node)
  {
    Error("FloodPortals_r: NULL node\n");
  }
  ...
}

Fragment N12.

Diagnostic message V501.

There are identical sub-expressions 'fabs(dir[1]) > test->radius' to the left and to the right of the '||' operator.

int VL_FindAdjacentSurface(....)
{
  ...
  if (fabs(dir[0]) > test->radius ||
      fabs(dir[1]) > test->radius ||
      fabs(dir[1]) > test->radius)
  {
  ...
}

Fragment N13.

Diagnostic message V517.

The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 3333, 3335.

void CMainFrame::OnClipSelected() 
{
  ...
  if (g_bPatchBendMode)
    Patch_BendHandleENTER();
  else if (g_bPatchBendMode)
    Patch_InsDelHandleENTER();
  ...
}

Fragment N14.

Diagnostic message V579.

The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument.

void CXYWnd::Paste()
{
  ...
  char* pBuffer = new char[nLen+1];
  memset( pBuffer, 0, sizeof(pBuffer) );
  ...
}
Correct code:
memset( pBuffer, 0, (nLen+1) * sizeof(char) );

Fragment N15.

Diagnostic message V519.

The 'numQuadCels' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1004, 1006.

static void setupQuad( long xOff, long yOff )
{
    ...
  numQuadCels  = (.....);
  numQuadCels += numQuadCels/4 + numQuadCels/16;
  numQuadCels += 64;
  numQuadCels  = (.....);
  numQuadCels += numQuadCels/4;
  numQuadCels += 64;
  ...
}

Fragment N16.

Diagnostic message V537.

Consider reviewing the correctness of 'scale_x' item's usage.

void Terrain_AddMovePoint(....) {
  ...
  x = ( v[ 0 ] - p->origin[ 0 ] ) / p->scale_x;
  y = ( v[ 1 ] - p->origin[ 1 ] ) / p->scale_x;
  ...
}

Correct code:

y = ( v[ 1 ] - p->origin[ 1 ] ) / p->scale_y;

Fragment N17.

Diagnostic message V557.

Array overrun is possible. The value of 'i' index could reach 3.

int   numteamVotingClients[2];

void CalculateRanks( void ) {
  ...
  for ( i = 0; i < TEAM_NUM_TEAMS; i++ ) {
    level.numteamVotingClients[i] = 0;
  }
  ...
}

Fragment N18.

Diagnostic message V591.

Non-void function should return a value.

static ID_INLINE int BigLong(int l)
{ LongSwap(l); }
Correct code:
static ID_INLINE int BigLong(int l)
{ return LongSwap(l); }

Conclusion

It took me about three hours to find all these errors. This time includes downloading the source codes, the analysis itself, analyzing the results and copying out the most interesting code fragments. As you see, a static analyzer is a very efficient tool of error search. However, it is even more efficient when used regularly.

Popular related articles
In the world of anthropomorphic animals: PVS-Studio checks Overgrowth

Date: Jun 23 2022

Author: Vladislav Stolyarov

Recently, Wolfire Games released Overgrowth's source code. We couldn't but check the game's quality with the help of PVS-Studio. Let's see where you can find the coolest action: in the game or in its…
Unreal baselining: PVS-Studio's enhancements for Unreal Engine projects

Date: Apr 26 2022

Author: Valery Komarov

The PVS-Studio static analyzer is constantly evolving. We enhance various mechanisms, integrate the analyzer with game engines, IDEs, CI/CD instruments, and other systems and services. A few years ag…
Checking the Ogre3D framework with the PVS-Studio static analyzer

Date: Mar 16 2022

Author: Grigory Semenchev

Developers like graphics engines because they are easy to work with. The PVS-Studio team likes graphics engines because we often find interesting code fragments. One of our readers asked us to analyz…
Thanks, Mario, but the code needs fixing — checking TheXTech

Date: Nov 22 2021

Author: Alexey Govorov

It's cool when enthusiastic developers create a working clone of a famous game. It's even cooler when people are ready to continue the development of such projects! In this article, we check TheXTech…
How the Carla car simulator helped us level up the static analysis of Unreal Engine 4 projects

Date: Nov 19 2021

Author: Konstantin Kochkin

One of the mechanisms of static analysis is method annotations of popular libraries. Annotations provide more information about functions during errors detecting. CARLA is an impressive open-source p…

Comments (0)

Next comments
Unicorn with delicious cookie
Our website uses cookies to enhance your browsing experience. Would you like to learn more?
Accept