Avant la sortie d'Amnesia: Rebirth, Fractional Games a publié le code source du légendaire Amnesia: The Dark Descent et de sa suite, Amnesia: A Machine For Pigs. Pourquoi ne pas utiliser un outil d'analyse statique pour voir à quel point de terribles erreurs se cachent à l'intérieur de ces jeux d'horreur cultes?
Quand j'ai vu la nouvelle sur Reddit que le code source des jeux " Amnesia: The Dark Descend " et " Amnesia: A Machine for Pigs " était publié, je ne pouvais pas passer et vérifier ce code en utilisant PVS-Studio, et en même temps écrire à propos de cet article. D'autant plus que le 20 octobre (et au moment de la publication de cet article, il était déjà sorti) une nouvelle partie de cette série: " Amnesia: Rebirth ".
Amnesia: The Dark Descent est sorti en 2010 et est devenu un jeu d'horreur de survie culte. Pour être honnête, je n'ai jamais pu le passer, même un petit peu, puisque je joue à des jeux d'horreur en utilisant le même algorithme: parier, entrer pendant cinq minutes, sortir par "alt + f4" au premier moment de fluage et supprimer le jeu.Mais j'ai aimé regarder le passage de ce jeu sur YouTube.
Et au cas où quelqu'un ne serait pas encore familier avec PVS-Studio, il s'agit d'un analyseur statique qui recherche les erreurs et les endroits suspects dans le code source des programmes.
J'aime particulièrement regarder dans le code source des jeux, donc si vous vous demandez quelles sont les erreurs qui y sont faites, vous pouvez lire mes articles précédents. Eh bien, ou jetez un œil aux articles de mes collègues sur la vérification du code source des jeux.
Après vérification, il s'est avéré qu'une grande partie du code de "The Dark Descend" et "A Machine For Pigs" se chevauchaient, et les rapports pour les deux projets étaient très similaires. Donc presque toutes les erreurs que je citerai ci-dessous sont contenues dans les deux projets.
Et la plus grande couche d'erreurs de tous les analyseurs détectés dans ces projets était la couche d'erreurs de «copier-coller». D'où le titre de l'article. La principale raison de ces erreurs est le " dernier effet de ligne ".
Eh bien, passons aux choses sérieuses.
Erreurs de copier-coller
Il y avait pas mal d'endroits suspects, similaires à la copie inattentive. Certains cas, peut-être, sont en quelque sorte causés par la logique interne du jeu lui-même. Mais s'ils m'embarrassaient à la fois moi et l'analyseur, cela valait au moins la peine de les commenter. Après tout, les autres développeurs peuvent être aussi lents que moi.
Extrait 1.
Commençons par un exemple où toute la fonction consiste à comparer les résultats des méthodes et des champs de deux objets aObjectDataA et aObjectDataB . Je vais donner cette fonction entièrement pour plus de clarté. Essayez de remarquer par vous-même où l'erreur a été commise dans la fonction:
static bool SortStaticSubMeshesForBodies(const ....& aObjectDataA,
const ....& aObjectDataB)
{
//Is shadow caster check
if( aObjectDataA.mpObject->GetRenderFlagBit(....)
!= aObjectDataB.mpObject->GetRenderFlagBit(....))
{
return aObjectDataA.mpObject->GetRenderFlagBit(....)
< aObjectDataB.mpObject->GetRenderFlagBit(....);
}
//Material check
if( aObjectDataA.mpPhysicsMaterial != aObjectDataB.mpPhysicsMaterial)
{
return aObjectDataA.mpPhysicsMaterial < aObjectDataB.mpPhysicsMaterial;
}
//Char collider or not
if( aObjectDataA.mbCharCollider != aObjectDataB.mbCharCollider)
{
return aObjectDataA.mbCharCollider < aObjectDataB.mbCharCollider;
}
return aObjectDataA.mpObject->GetVertexBuffer()
< aObjectDataA.mpObject->GetVertexBuffer();
}Une photo, pour ne pas espionner accidentellement la réponse:
Pouvez-vous trouver l'erreur? Oui, le dernier retour est une comparaison utilisant aObjectDataA des deux côtés. Notez que toutes les expressions du code original ont été écrites sur une ligne, ici j'ai coupé les mots pour que tout rentre exactement dans la largeur de la ligne. Imaginez ce qu'un tel défaut recherchera à la fin de la journée de travail. Et l'analyseur le trouvera immédiatement après avoir assemblé et exécuté l'analyse incrémentielle.
V501 Il existe des sous-expressions identiques 'aObjectDataA.mpObject-> GetVertexBuffer ()' à gauche et à droite de l'opérateur '<'. WorldLoaderHplMap.cpp 1123
Par conséquent, une telle erreur sera trouvée presque au moment de l'écriture du code, au lieu de se cacher dans les profondeurs du code à partir de plusieurs étapes de contrôle qualité, ce qui rendra votre recherche plusieurs fois plus difficile.
Note de son collègue Andrey Karpov. Oui, c'est une "erreur de dernière ligne" classique. Cependant, il s'agit également d'un modèle d'erreur classique lors de la comparaison de deux objets. Voir l'article " Evil Lives in Comparison Functions ".Fragment 2.
Jetons un coup d'œil au code qui a provoqué l'avertissement:
Je présente le code avec une capture d'écran pour plus de clarté.
L'avertissement lui-même ressemble à ceci:
V501 Il existe des sous-expressions identiques 'lType == eLuxJournalState_OpenNote' à gauche et à droite de '||' opérateur. LuxJournal.cpp 2262
L'analyseur a détecté une erreur lors de la vérification de la valeur de la variable lType . L'égalité est vérifiée deux fois avec le même membre de l'énumérateur eLuxJournalState_OpenNote .
Tout d'abord, j'aimerais qu'une telle condition soit écrite sous forme «tabulaire» pour améliorer la lisibilité. Voir le chapitre 13 dans le mini-livre " La plus grande question de la programmation, de la refactorisation et tout ça ".
if(!( lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenDiary
|| lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenNarratedDiary))
return false;Sous cette forme, il devient beaucoup plus facile de repérer une erreur même sans analyseur.
Cependant, la question se pose: une telle vérification erronée conduit-elle à une distorsion de la logique du programme? Après tout, vous devez peut-être vérifier une autre valeur de lType , mais la vérification a été manquée en raison d'une erreur de copier-coller. Jetons un coup d'œil à l'énumération elle-même:
enum eLuxJournalState
{
eLuxJournalState_Main,
eLuxJournalState_Notes,
eLuxJournalState_Diaries,
eLuxJournalState_QuestLog,
eLuxJournalState_OpenNote,
eLuxJournalState_OpenDiary,
eLuxJournalState_OpenNarratedDiary,
eLuxJournalState_LastEnum,
};Il n'y a que trois significations qui ont le mot «Ouvrir» dans leur nom. Et tous les trois sont présents dans le chèque. Très probablement, il n'y a pas de distorsion de la logique ici, mais il est toujours impossible de le dire avec certitude. Ainsi, l'analyseur a soit trouvé une erreur logique que le développeur du jeu pouvait corriger, soit trouvé un endroit écrit "laid" qui aurait dû être réécrit pour une meilleure clarté.
Fragment 3.
Le cas suivant est généralement l'exemple le plus évident d'une erreur de copier-coller.
V778 Deux fragments de code similaires ont été trouvés. Il s'agit peut-être d'une faute de frappe et la variable «mvSearcherIDs» devrait être utilisée à la place de «mvAttackerIDs». LuxSavedGameTypes.cpp 615
void cLuxMusicHandler_SaveData::ToMusicHandler(....)
{
....
// Enemies
//Attackers
for(size_t i=0; i<mvAttackerIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap
->GetEntityByID(mvAttackerIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
//Searchers
for(size_t i=0; i<mvSearcherIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap->GetEntityByID(mvSearcherIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
}Dans le premier cycle, le travail s'accompagne du pointeur pEntity , qui a été reçu via mvAttackerIDs, et si la condition n'est pas remplie, un message de débogage est émis pour les mêmes mvAttackerIDs . Cependant, dans la boucle suivante, qui est stylée exactement de la même manière que la section précédente du code, pEntity est obtenu à l'aide de mvSearcherIDs . Mais l'avertissement est toujours émis avec la mention de mvAttackerIDs .
Très probablement, le bloc de code signé "Searchers" a été copié à partir du bloc "Attackers", mvAttackerIDs a été remplacé par mvSearcherIDs , mais le bloc else n'a pas été modifié. Par conséquent, le message d'erreur utilise un élément du mauvais tableau.
Cette erreur n'affecte pas la logique du jeu, mais de cette façon, vous pouvez mettre un porc sérieux sur une personne qui devra déboguer cet endroit et perdre du temps à travailler avec le mauvais élément mvSearcherIDs .
Fragment 4.
L'analyseur a indiqué l'endroit suspect suivant avec trois avertissements:
- L' expression V547 'pEntity == 0' est toujours fausse. LuxScriptHandler.cpp 2444
- V649 Il existe deux instructions «if» avec des expressions conditionnelles identiques. La première instruction «if» contient un retour de fonction. Cela signifie que la deuxième déclaration «si» est insensée. Vérifiez les lignes: 2433, 2444. LuxScriptHandler.cpp 2444
- V1051 Envisagez de vérifier les erreurs d'impression. Il est possible que «pTargetEntity» soit vérifié ici. LuxScriptHandler.cpp 2444
Regardons le code:
void __stdcall cLuxScriptHandler::PlaceEntityAtEntity(....)
{
cLuxMap *pMap = gpBase->mpMapHandler->GetCurrentMap();
iLuxEntity *pEntity = GetEntity(....);
if(pEntity == NULL) return;
if(pEntity->GetBodyNum() == 0)
{
....
}
iPhysicsBody *pBody = GetBodyInEntity(....);
if(pBody == NULL) return;
iLuxEntity *pTargetEntity = GetEntity(....);
if(pEntity == NULL) return; // <=
iPhysicsBody *pTargetBody = GetBodyInEntity(....);
if(pTargetBody == NULL) return;
....
}Un avertissement V547 a été émis pour la deuxième vérification pEntity == NULL . Pour l'analyseur, cette vérification sera toujours fausse , car si cette condition était vraie , la fonction quitterait plus haut en raison de la vérification similaire précédente.
L'avertissement suivant, V649 a été émis précisément parce que nous avons deux contrôles identiques. Habituellement, un tel cas peut ne pas être une erreur. Du coup, dans une partie du code, une logique est implémentée, et dans une autre partie du code, selon le même contrôle, quelque chose d'autre doit être implémenté. Mais dans ce cas, le corps du premier chèque consiste en retour, donc la deuxième vérification, si la condition s'avère vraie, n'atteindra même pas le point. En traçant une telle logique, l'analyseur réduit le nombre de faux messages pour le code suspect et les émet uniquement pour une logique très étrange.
L'erreur indiquée par le dernier avertissement est intrinsèquement très similaire à l'exemple précédent. Très probablement, tous les contrôles ont été dupliqués à partir du premier contrôle if (pEntity == NULL) , puis l'objet vérifié a été remplacé par celui requis. Dans le cas des objets pBody et pTargetBody, le remplacement a été effectué, mais l'objet pTargetEntity a été oublié. En conséquence, cet objet n'est pas vérifié.
Dans l'exemple que nous considérons, si vous approfondissez un peu le code, il s'avère qu'une telle erreur en général n'affectera pas le déroulement du programme. Le pointeur pTargetBody obtient sa valeur de la fonction GetBodyInEntity :
iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity,
asTargetBodyName);Le premier argument ici est juste un pointeur non coché, qui n'est utilisé nulle part ailleurs. Et heureusement, à l'intérieur de cette fonction, il y a une vérification du premier argument pour NULL :
iPhysicsBody* ....::GetBodyInEntity(iLuxEntity* apEntity, ....)
{
if(apEntity == NULL){
return NULL;
}
....
}Par conséquent, ce code fonctionne correctement à la fin, bien qu'il contienne une erreur.
Fragment 5.
Et encore un endroit suspect avec le copier-coller!
Cette méthode efface les champs de l'objet de la classe cLuxPlayer .
void cLuxPlayer::Reset()
{
....
mfRoll=0;
mfRollGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
mfLeanRoll=0;
mfLeanRollGoal=0;
mfLeanRollSpeedMul=0;
mfLeanRollMaxSpeed=0;
mvCamAnimPos =0;
mvCamAnimPosGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
....
}Mais pour une raison quelconque, les deux variables mfRollSpeedMul et mfRollMaxSpeed sont annulées deux fois:
- V519 La variable 'mfRollSpeedMul' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 298, 308. LuxPlayer.cpp 308
- V519 La variable 'mfRollMaxSpeed' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 299, 309. LuxPlayer.cpp 309
Examinons la classe elle-même et voyons quels champs elle contient:
class cLuxPlayer : ....
{
....
private:
....
float mfRoll;
float mfRollGoal;
float mfRollSpeedMul;
float mfRollMaxSpeed;
float mfLeanRoll;
float mfLeanRollGoal;
float mfLeanRollSpeedMul;
float mfLeanRollMaxSpeed;
cVector3f mvCamAnimPos;
cVector3f mvCamAnimPosGoal;
float mfCamAnimPosSpeedMul;
float mfCamAnimPosMaxSpeed;
....
}Fait intéressant, il existe trois blocs similaires de variables avec des noms associés: mfRoll , mfLeanRoll et mvCamAnimPos . Dans Reset, ces trois blocs sont effacés, à l'exception des deux dernières variables du troisième bloc, mfCamAnimPosSpeedMul et mfCamAnimPosMaxSpeed . Et c'est exactement à la place de ces deux variables que se trouvent les affectations dupliquées. Très probablement, toutes ces affectations ont été copiées à partir du premier bloc d'affectation, puis les noms de variables ont été remplacés par ceux requis.
Il se peut que deux variables manquantes n'aient pas dû être remises à zéro, mais le contraire est également très probable. Et les réaffectations n'aideront évidemment pas à maintenir ce code. Comme vous pouvez le voir, dans une longue toile de pied des mêmes actions, vous ne remarquerez peut-être pas une telle erreur, et l'analyseur aide ici.
Fragment 5.5.
Cet exemple est très similaire au précédent. Je citerai immédiatement un fragment de code et un avertissement de l'analyseur pour cela.
V519 La variable 'mfTimePos' reçoit des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 49, 53. AnimationState.cpp 53
cAnimationState::cAnimationState(....)
{
....
mfTimePos = 0;
mfWeight = 1;
mfSpeed = 1.0f;
mfBaseSpeed = 1.0f;
mfTimePos = 0;
mfPrevTimePos=0;
....
}La variable mfTimePos a reçu deux fois la valeur 0. Comme dans l'exemple précédent, regardons la déclaration de ce champ:
class cAnimationState
{
....
private:
....
//Properties of the animation
float mfLength;
float mfWeight;
float mfSpeed;
float mfTimePos;
float mfPrevTimePos;
....
}Vous pouvez voir que ce bloc de déclaration correspond également à l'ordre d'affectation dans l'extrait de code d'erreur, comme dans l'exemple précédent. Ici, dans l'affectation, au lieu de la variable mfLength, la valeur obtient mfTimePos . Mais ici l'erreur ne peut pas être expliquée en copiant le bloc et "l'effet de la dernière ligne". Il se peut que mfLength n'ait pas besoin de se voir attribuer une nouvelle valeur, mais cet emplacement est toujours suspect.
Fragment 6.
L'analyseur a émis tout un tas d'avertissements pour le prochain fragment de code de "Amnesia: A Machine For Pigs". Je ne donnerai qu'une partie du code pour lequel des erreurs du même genre ont été émises:
void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep)
{
....
if(prevMoveState != mMoveState)
{
....
//Backward
if(mMoveState == eLuxEnemyMoveState_Backward)
{
....
}
....
//Walking
else if(mMoveState == eLuxEnemyMoveState_Walking)
{
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;
....
}
....
}
}Où est l'erreur ici?
L'analyseur a émis les avertissements suivants:
- V768 La constante d'énumération 'eLuxEnemyMoveState_Jogging' est utilisée comme variable de type booléen. LuxEnemyMover.cpp 672
- V768 La constante d'énumération 'eLuxEnemyMoveState_Walking' est utilisée comme variable de type booléen. LuxEnemyMover.cpp 680
- V768 La constante d'énumération 'eLuxEnemyMoveState_Jogging' est utilisée comme variable de type booléen. LuxEnemyMover.cpp 688
La chaîne if-else-if est répétée dans le code d'origine, puis ces avertissements ont simplement été émis pour chaque corps de else if .
Considérez la ligne pointée par l'analyseur:
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;Il n'est pas surprenant qu'une erreur se soit glissée dans une telle expression, qui est également écrite sur une ligne dans l'original. Et vous l'avez probablement déjà remarqué. Le membre de l'énumération eLuxEnemyMoveState_Jogging n'est comparé à rien; sa valeur est vérifiée. Très probablement, l'expression «prevMoveState == eLuxEnemyMoveState_Jogging» était implicite.
Une telle erreur peut sembler totalement inoffensive. Mais dans mon autre article, sur la vérification du Bullet Engine , parmi les commits du projet, j'ai trouvé un correctif de boguedu même type, entraînant l'application de forces sur les objets du mauvais côté. Et dans ce cas, cette erreur a été commise à plusieurs reprises. Eh bien, je note aussi que la condition ternaire n'a aucun sens, puisqu'elle sera remplie, en dernier lieu, aux résultats booléens des opérateurs logiques.
Fragment 7.
Et enfin, les deux derniers exemples d'erreurs de copier-coller. Cette fois encore dans une instruction conditionnelle. L'analyseur a émis un avertissement pour le morceau de code suivant:
void iParticleEmitter::SetSubDivUV(const cVector2l &avSubDiv)
{
//Check so that there is any subdivision
// and that no sub divison axis is
//equal or below zero
if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x >0 && avSubDiv.y >0))
{
....
}
....
}Je pense qu'il est assez facile de remarquer un endroit suspect dans un tel fragment, séparé du code entier. Cependant, cette erreur a réussi à se cacher des développeurs de ce jeu.
L'analyseur a émis l'avertissement suivant:
V501 Il existe des sous-expressions identiques à gauche et à droite du '||' opérateur: avSubDiv.x> 1 || avSubDiv.x> 1 ParticleEmitter.cpp 199
La deuxième parenthèse de la condition indique que les champs x et y sont cochés . Mais dans la première parenthèse, pour une raison quelconque, ce moment a été manqué et seul le champ x est vérifié... De plus, à en juger par le commentaire de validation, les deux champs devraient avoir été validés. Ici, ce n'est pas "l'effet de la dernière ligne" qui a fonctionné, mais plutôt le premier, car dans la première parenthèse, ils ont oublié de remplacer l'appel au champ x par l'appel au champ y .
De telles erreurs sont donc assez insidieuses, car dans ce cas, le développeur n'a même pas été aidé en écrivant un commentaire explicatif sur la condition.
Je recommanderais dans de tels cas de prendre l'habitude de consigner les chèques connexes sous forme de tableau. Il est plus facile de modifier de cette façon et la faille sera beaucoup plus visible:
if( (avSubDiv.x > 1 || avSubDiv.x > 1)
&& (avSubDiv.x > 0 && avSubDiv.y > 0))Fragment 7.5.
Absolument identique, en fait, l'erreur a été trouvée dans un endroit complètement différent:
static bool EdgeTriEqual(const cTriEdge &edge1, const cTriEdge &edge2)
{
if(edge1.tri1 == edge2.tri1 && edge1.tri2 == edge2.tri2)
return true;
if(edge1.tri1 == edge1.tri1 && edge1.tri2 == edge2.tri1)
return true;
return false;
}Eh bien, comment avez-vous réussi à voir immédiatement où elle se cachait? Ce n'est pas pour rien que tant d'exemples ont déjà été triés :)
L'analyseur a émis l'avertissement suivant:
V501 Il y a des sous-expressions identiques à gauche et à droite de l'opérateur '==': edge1.tri1 == edge1.tri1 Math.cpp 2914 Analysons
ceci fragment dans l'ordre. Evidemment, le premier contrôle vérifie l'égalité des champs edge1.tri1 et edge2.tri2 , et en même temps l'égalité de edge1.tri2 et edge2.tri2 :
edge1.tri1 -> edge2.tri1
edge1.tri2 -> edge2.tri2Et dans la deuxième vérification, à en juger par la partie correcte de la vérification 'edge1.tri2 == edge2.tri1', il était nécessaire de vérifier l'égalité de ces champs "en croix":
Mais au lieu de vérifier edge1.tri1 == edge2.tri2 , une vérification sans signification a été effectuée edge1.tri1 == edge1.tri1 . Mais c'est tout le contenu de la fonction, je n'en ai rien supprimé. Et tout de même, une telle erreur est entrée dans le code.
Autres erreurs
Fragment 1.
Je vais donner le fragment de code suivant avec les retraits d'origine.
void iCharacterBody::CheckMoveCollision(....)
{
....
/////////////////////////////////////
//Forward velocity reflection
//Make sure that new velocity points in the right direction
//and that it is not too large!
if(mfMoveSpeed[eCharDir_Forward] != 0)
{
vForwardVel = ....;
float fForwardSpeed = vForwardVel.Length();
if(mfMoveSpeed[eCharDir_Forward] > 0)
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
else
if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}
....
}Avertissement PVS-Studio: V563 Il est possible que cette branche «else» s'applique à l'instruction «if» précédente. CharacterBody.cpp 1591
Cet exemple peut prêter à confusion. Sinon, pourquoi le retrait est-il le même que le si le plus extérieur ? Est-ce l' autre pour la condition extérieure? Eh bien, vous devez placer les parenthèses, sinon else fait référence au if directement précédent .
if(mfMoveSpeed[eCharDir_Forward] > 0)
{
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
}
else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
{
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}Ou pas? Au cours de la rédaction de cet article, j'ai changé d'avis plusieurs fois sur la variante la plus probable de la séquence d'actions conçue pour ce code.
Si nous approfondissons un peu ce code, il s'avère que la variable fForwardSpeed , avec laquelle la comparaison est effectuée dans le cas inférieur , ne peut pas avoir une valeur inférieure à zéro, car elle reçoit une valeur de la méthode Length :
inline T Length() const
{
return sqrt( x * x + y * y + z * z);
}Ensuite, très probablement, l'essence de ces vérifications est que nous vérifions d'abord si l'élément mfMoveSpeed est supérieur à zéro, puis nous vérifions sa valeur par rapport à fForwardSpeed . De plus, les deux derniers ifs se correspondent dans leur formulation.
Dans ce cas, le code original fonctionnera comme prévu! Mais cela rendra certainement la personne qui viendra le modifier / le refactoriser pour le puzzle.
Je pensais que je ne rencontrerais jamais du code qui ressemble à ceci. Par intérêt, j'ai regardé dans notre base de données des erreurs trouvées dans des projets open source et décrites dans des articles. Et des exemples de cette erreur ont été trouvés dans d'autres projets - vous pouvez les examiner vous-même .
Et n'écrivez pas comme ça, même si vous êtes vous-même clair dans ce cas. Ou des accolades ou une indentation correcte, ou mieux, les deux. Ne plongez pas dans la souffrance ceux qui en viennent à comprendre votre code, et vous-même dans le futur;)
Fragment 2.
La prochaine erreur m'a un peu troublé, et j'ai essayé de trouver la logique ici pendant longtemps. Mais en fin de compte, il me semble toujours que c'est probablement une erreur, et plutôt grossière.
Jetons un coup d'œil au code:
bool cBinaryBuffer::DecompressAndAdd(char *apSrcData, size_t alSize)
{
....
///////////////////////////
// Init decompression
int ret = inflateInit(&zipStream);
if (ret != Z_OK) return false;
///////////////////////////
// Decompress, chunk by chunk
do
{
//Set current output chunk
zipStream.avail_out = lMaxChunkSize;
....
//Decompress as much as possible to current chunk
int ret = inflate(&zipStream, Z_NO_FLUSH);
if(ret != Z_OK && ret != Z_STREAM_END)
{
inflateEnd(&zipStream);
return false;
}
....
}
while (zipStream.avail_out == 0 && ret != Z_STREAM_END);
....
return true;
}V711 Il est dangereux de créer une variable locale dans une boucle avec le même nom qu'une variable contrôlant cette boucle. BinaryBuffer.cpp 371
Nous avons donc une variable ret , qui contrôle la sortie de la boucle do-while . Mais à l'intérieur de cette boucle, au lieu d'assigner une nouvelle valeur à cette variable externe, une nouvelle variable nommée ret est déclarée . Par conséquent, il remplace la variable externe ret et la variable qui est vérifiée dans la condition de boucle ne changera jamais.
Dans une malheureuse combinaison de circonstances, un tel cycle pourrait devenir infini. Très probablement, dans ce cas, ce code enregistre une condition interne qui vérifie la valeur de la variable interne ret et conduit à quitter la fonction.
Conclusion
Très souvent, les développeurs n'utilisent pas régulièrement l'analyse statique, mais avec de longues pauses. Ou ils exécutent même le projet via l'analyseur une fois. Du fait de cette approche, l'analyseur ne détecte souvent rien de sérieux ou trouve quelque chose comme les exemples que nous considérons, ce qui n'a peut-être pas particulièrement affecté la fonctionnalité du jeu. Il semble que l'analyseur ne soit pas particulièrement utile. Eh bien, il a trouvé de tels endroits, mais fonctionne toujours.
Et le fait est que des endroits similaires, mais dans lesquels l'erreur n'a pas été masquée, mais clairement conduit à un bogue dans le programme, ont déjà été corrigés par de longues heures de débogage, de tests et du service de test. Par conséquent, lors de la vérification d'un projet, l'analyseur n'affiche que les problèmes qui ne se sont en aucun cas manifestés. Parfois, parmi ces problèmes, il y a aussi des moments graves qui ont vraiment influencé le fonctionnement du programme, mais la probabilité que le programme suive leur chemin était faible, et donc cette erreur était inconnue des développeurs.
Par conséquent, il est extrêmement important d'évaluer l'utilité de l'analyse statique uniquement après son utilisation régulière. Si une exécution unique de PVS-Studio révélait de tels endroits suspects et inexacts dans le code de ce jeu, alors combien d'erreurs évidentes de ce type devaient être localisées et corrigées au cours du processus de développement.
Utilisez régulièrement un analyseur statique!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Victoria Khanieva. Amnesia: The Dark Descent ou Comment oublier de réparer le copier-coller .