Unsere Aufmerksamkeit wurde kürzlich vom Electronic Arts Repository auf GitHub erregt. Es ist winzig und von den 23 dort verfügbaren Projekten schienen nur wenige C ++ - Bibliotheken interessant zu sein: EASTL, EAStdC, EABase, EAThread, EATest, EAMain und EAAssert. Die Projekte selbst sind ebenfalls winzig (jeweils etwa 10 Dateien), sodass Fehler nur im "größten" Projekt von 20 Dateien gefunden wurden: D Aber wir haben sie gefunden und sie sehen interessant aus! Während ich diesen Beitrag schrieb, hatten wir auch eine lebhafte Diskussion über EA-Spiele und die Unternehmensrichtlinien: D.
Einführung
Electronic Arts (EA) ist ein amerikanisches Videospielunternehmen. Es hat ein kleines
Repository auf GitHub und einige C ++ - Projekte, nämlich C ++ - Bibliotheken: EASTL, EAStdC, EABase, EAThread, EATest, EAMain und EAAssert. Sie sind winzig und der
PVS-Studio- Analysator konnte nur im "größten" Projekt,
EAStdC (20 Dateien),
Fehler finden . Bei solchen Größen können Sie die
Gesamtcodequalität nicht zuverlässig beurteilen. Schauen Sie sich also einfach die folgenden fünf Warnungen an und entscheiden Sie selbst.
Warnung 1
V524 Es ist seltsam, dass der Körper der Funktion '>>' dem Körper der Funktion '<<' vollständig entspricht. 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;} .... }
Beim Überladen der Schichtoperatoren machte der Programmierer einen Tippfehler in einem von ihnen, indem er << anstelle von >> schrieb. Dies sieht sehr nach einem Fehler beim Kopieren und Einfügen aus.
Warnung 2
V557 Array-Überlauf ist möglich. Der Wert des Index 'nFormatLength' könnte 16 erreichen. 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;
Das Array
spans [spanIndex] .mFormat besteht aus
16 Elementen, sodass der Index des letzten gültigen Elements
15 beträgt. In ihrer aktuellen Form
erhöht die
OVprintfCore- Funktion den Index von
nFormatLength auf
16, wenn sie den höchstmöglichen Index hat, d. H.
15 . Danach tritt in der
switch- Anweisung ein Array-out-of-bounds-Fehler auf.
Dieses Fragment wurde noch zweimal kopiert:
- V557 Array-Überlauf ist möglich. Der Wert des Index 'nFormatLength' könnte 16 erreichen. EASprintfOrdered.cpp 614
- V557 Array-Überlauf ist möglich. Der Wert des Index 'nFormatLength' könnte 16 erreichen. EASprintfOrdered.cpp 977
Warnung 3
V560 Ein Teil des bedingten Ausdrucks ist immer wahr: (Ergebnis> = 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; } .... }
Die Bedingung
result> = 0 ist immer wahr, da die
Ergebnisvariable an keiner Stelle in der Schleife geändert wird. Der Code sieht überhaupt nicht richtig aus und es muss ein Fehler darin sein.
Dieses Fragment wurde noch zweimal kopiert:
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: (Ergebnis> = 0). EASprintfOrdered.cpp 852
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: (Ergebnis> = 0). EASprintfOrdered.cpp 1215
Warnung 4
V1009 Überprüfen Sie die Array-Initialisierung. Nur das erste Element wird explizit initialisiert. Die restlichen Elemente werden mit Nullen initialisiert. EASprintfOrdered.cpp 151
static int OVprintfCore(....) { .... int spanArgOrder[kArgCapacity] = { -1 }; .... }
Dies ist nicht unbedingt ein Fehler, aber die Autoren sollten gewarnt werden, dass nur das erste Element des
spanArgOrder- Arrays auf
-1 initialisiert wird, während der Rest auf 0 gesetzt wird.
Dieses Fragment wurde noch zweimal kopiert:
- V1009 Überprüfen Sie die Array-Initialisierung. Nur das erste Element wird explizit initialisiert. Die restlichen Elemente werden mit Nullen initialisiert. EASprintfOrdered.cpp 518
- V1009 Überprüfen Sie die Array-Initialisierung. Nur das erste Element wird explizit initialisiert. Die restlichen Elemente werden mit Nullen initialisiert. EASprintfOrdered.cpp 881
Warnung 5
V728 Eine übermäßige Überprüfung kann vereinfacht werden. Das '(A &&! B) || (! A && B) 'Ausdruck entspricht dem Ausdruck' 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(); } .... }
Ich habe dieses Beispiel aus Gründen der Übersichtlichkeit formatiert, aber in seiner ursprünglichen Form ist dieser Zustand sehr lang und schwer zu lesen. Aber wir können es viel besser machen, indem wir den bedingten Ausdruck vereinfachen, wie der Analysator vorschlägt:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
Der Code ist jetzt viel kürzer, wodurch die Logik der Bedingung viel einfacher zu verstehen ist.
Fazit
Wie Sie vielleicht bemerkt haben, enthalten die meisten Warnungen zwei weitere Duplikate, und alle diese Duplikate befinden sich in derselben Datei. Die Vervielfältigung von Code ist ein sehr schlechtes Anti-Pattern, da dies die Programmwartung erheblich erschwert. Wenn sich Fehler in solchen Code einschleichen, sinkt die Stabilität des Programms drastisch, da sie sich über den gesamten Code verteilen.
Hoffentlich wird EA einige andere interessante Projekte hochladen und wir werden ihr Repository noch einmal besuchen :). In der Zwischenzeit können Sie
PVS-Studio herunterladen und in Ihren eigenen Projekten ausprobieren.