Notre attention a été attirée sur le référentiel Electronic Arts sur GitHub. Il est très petit et sur vingt-trois projets, nous n'étions intéressés que par quelques bibliothèques C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain et EAAssert. Les projets étaient également très petits (environ 10 fichiers), nous n'avons donc trouvé d'erreurs que dans le «plus grand» des 20 fichiers: D Mais nous en avons aussi trouvé des intéressants! Pendant la rédaction de l'article, mes collègues et moi avons également discuté vigoureusement des jeux d'EA et de sa stratégie: D
Présentation
Electronic Arts (EA) est une société américaine qui distribue des jeux vidéo. Sur le site Web de GitHub, elle a un petit
référentiel et plusieurs projets C ++. Ce sont des bibliothèques C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain et EAAssert. Ils sont très petits et nous avons trouvé des erreurs intéressantes en utilisant l'analyseur
PVS-Studio uniquement dans le "plus grand" d'entre eux -
EAStdC (20 fichiers). Avec de tels volumes,
il est difficile de parler de la qualité du code dans son ensemble. Évaluez simplement 5 avertissements et décidez par vous-même.
Avertissement 1
V524 Il est étrange que le corps de la fonction '>>' soit entièrement équivalent au corps de la fonction '<<'. EAFixedPoint.h 287
template <class T, int upShiftInt, int downShiftInt, int upMulInt, int downDivInt> struct FPTemplate { .... FPTemplate operator<<(int numBits) const { return value << numBits; } FPTemplate operator>>(int numBits) const { return value << numBits; } FPTemplate& operator<<=(int numBits) { value <<= numBits; return *this;} FPTemplate& operator>>=(int numBits) { value >>= numBits; return *this;} .... }
Lors de la surcharge des opérateurs de décalage, le programmeur a fait une faute de frappe dans l'un d'eux, confondant les opérateurs << et >>. Il s'agit très probablement du résultat de la programmation par copier-coller.
Avertissement 2
V557 Le dépassement de matrice est possible. La valeur de l'index 'nFormatLength' pourrait atteindre 16. EASprintfOrdered.cpp 246
static const int kSpanFormatCapacity = 16; struct Span8 { .... char mFormat[kSpanFormatCapacity]; .... }; static int OVprintfCore(....) { .... EA_ASSERT(nFormatLength < kSpanFormatCapacity); if(nFormatLength < kSpanFormatCapacity) spans[spanIndex].mFormat[nFormatLength++] = *p;
Le
tableau spans [spanIndex] .mFormat se compose de
16 éléments, c'est-à-dire le dernier élément valide a un indice de
15 . Maintenant, le
code de la fonction
OVprintfCore est écrit de sorte que si l'index
nFormatLength a l'index maximum possible -
15 , alors un incrément de jusqu'à
16 se produira. De plus, dans l'
instruction switch, il est possible d'aller au-delà de la limite du tableau.
Ce fragment de code a été copié dans 2 autres emplacements:
- V557 Le dépassement de matrice est possible. La valeur de l'index 'nFormatLength' pourrait atteindre 16. EASprintfOrdered.cpp 614
- V557 Le dépassement de matrice est possible. La valeur de l'index 'nFormatLength' pourrait atteindre 16. EASprintfOrdered.cpp 977
Avertissement 3
V560 Une partie de l'expression conditionnelle est toujours vraie: (résultat> = 0). EASprintfOrdered.cpp 489
static int OVprintfCore(....) { .... for(result = 1; (result >= 0) && (p < pEnd); ++p) { if(pWriteFunction8(p, 1, pWriteFunctionContext8, kWFSIntermediate) < 0) return -1; nWriteCountSum += result; } .... }
Le
résultat de la condition
> = 0 est toujours vrai, car la variable de
résultat ne change nulle part dans la boucle. Le code semble très suspect et il y a très probablement une erreur dans ce code.
Ce fragment de code a été copié dans 2 autres emplacements:
- V560 Une partie de l'expression conditionnelle est toujours vraie: (résultat> = 0). EASprintfOrdered.cpp 852
- V560 Une partie de l'expression conditionnelle est toujours vraie: (résultat> = 0). EASprintfOrdered.cpp 1215
Avertissement 4
V1009 Vérifiez l'initialisation de la baie. Seul le premier élément est initialisé explicitement. Les éléments restants sont initialisés avec des zéros. EASprintfOrdered.cpp 151
static int OVprintfCore(....) { .... int spanArgOrder[kArgCapacity] = { -1 }; .... }
Ce n'est peut-être pas une erreur, mais les développeurs doivent être avertis que seul le premier élément du tableau
spanArgOrder est initialisé avec une valeur de
-1 , et tous les autres auront une valeur de 0.
Ce fragment de code a été copié dans 2 autres emplacements:
- V1009 Vérifiez l'initialisation de la baie. Seul le premier élément est initialisé explicitement. Les éléments restants sont initialisés avec des zéros. EASprintfOrdered.cpp 518
- V1009 Vérifiez l'initialisation de la baie. Seul le premier élément est initialisé explicitement. Les éléments restants sont initialisés avec des zéros. EASprintfOrdered.cpp 881
Avertissement 5
V728 Un contrôle excessif peut être simplifié. Le '(A &&! B) || (! A && B) 'est équivalente à l'expression' bool (A)! = Bool (B) '. int128.h 1242
inline void int128_t::Modulus(....) const { .... bool bDividendNegative = false; bool bDivisorNegative = false; .... if( (bDividendNegative && !bDivisorNegative) || (!bDividendNegative && bDivisorNegative)) { quotient.Negate(); } .... }
Ce fragment de code a été formaté pour plus de commodité, mais dans l'original, il s'agit d'une condition très longue qui est difficile à lire. Une autre chose est de simplifier l'expression conditionnelle, comme le conseille l'analyseur:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
Le code est devenu beaucoup plus court, ce qui a grandement simplifié la compréhension de la logique de l'expression conditionnelle.
Conclusion
Comme vous l'avez peut-être remarqué, la plupart des avertissements suspects sont dupliqués à trois endroits et dans le même fichier. La duplication de code est une très mauvaise pratique de développement, complique la prise en charge du code. Et si une erreur se produit dans un tel code, la stabilité du programme diminue fortement en raison de la propagation des erreurs dans tout le code.
Espérons que quelque chose d'autre d'intéressant sera publié et nous reviendrons sur ce référentiel :). En attendant, je suggère à ceux qui souhaitent télécharger
PVS-Studio et essayer de vérifier leurs propres projets.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Svyatoslav Razmyslov.
Bibliothèques presque parfaites d'Electronic Arts .