Nos llamó la atención el repositorio de Electronic Arts en GitHub. Es muy pequeño y de veintitrés proyectos solo estábamos interesados en algunas bibliotecas de C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain y EAAssert. Los proyectos también eran muy pequeños (alrededor de 10 archivos), por lo que encontramos errores solo en el "mayor" de 20 archivos: D ¡Pero también encontramos errores interesantes! Mientras se escribía el artículo, mis colegas y yo también discutimos enérgicamente los juegos de EA y su estrategia: D
Introduccion
Electronic Arts (EA) es una empresa estadounidense que distribuye videojuegos. En el sitio web de GitHub, tiene un pequeño
repositorio y varios proyectos en C ++. Estas son bibliotecas C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain y EAAssert. Son muy pequeños y encontramos errores interesantes al usar el analizador
PVS-Studio solo en el "más grande" de ellos:
EAStdC (20 archivos). Con tales volúmenes,
es difícil hablar sobre la calidad del código en su conjunto. Simplemente califica 5 advertencias y decide por ti mismo.
Advertencia 1
V524 Es extraño que el cuerpo de la función '>>' sea completamente equivalente al cuerpo de la función '<<'. 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;} .... }
Al sobrecargar los operadores de turno, el programador cometió un error tipográfico en uno de ellos, confundiendo a los operadores << y >>. Lo más probable es que este sea el resultado de la programación de copiar y pegar.
Advertencia 2
V557 Array overrun es posible. El valor del índice 'nFormatLength' podría llegar a 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;
La
matriz spans [spanIndex] .mFormat consta de
16 elementos, es decir El último artículo válido tiene un índice de
15 . Ahora el
código de la función
OVprintfCore está escrito de modo que si el índice
nFormatLength tiene el índice máximo posible -
15 , se producirá un incremento de hasta
16 . Además en la
declaración de cambio, es posible ir más allá del límite de la matriz.
Este fragmento de código se copió a 2 lugares más:
- V557 Array overrun es posible. El valor del índice 'nFormatLength' podría llegar a 16. EASprintfOrdered.cpp 614
- V557 Array overrun es posible. El valor del índice 'nFormatLength' podría llegar a 16. EASprintfOrdered.cpp 977
Advertencia 3
V560 Una parte de la expresión condicional siempre es verdadera: (resultado> = 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; } .... }
El
resultado de la condición
> = 0 siempre
es verdadero, porque la variable de
resultado no cambia en ninguna parte del bucle. El código parece muy sospechoso y lo más probable es que haya un error en este código.
Este fragmento de código se copió a 2 lugares más:
- V560 Una parte de la expresión condicional siempre es verdadera: (resultado> = 0). EASprintfOrdered.cpp 852
- V560 Una parte de la expresión condicional siempre es verdadera: (resultado> = 0). EASprintfOrdered.cpp 1215
Advertencia 4
V1009 Verifique la inicialización de la matriz. Solo el primer elemento se inicializa explícitamente. Los elementos restantes se inicializan con ceros. EASprintfOrdered.cpp 151
static int OVprintfCore(....) { .... int spanArgOrder[kArgCapacity] = { -1 }; .... }
Esto puede no ser un error, pero se debe advertir a los desarrolladores que solo el primer elemento de la matriz
spanArgOrder se inicializa con un valor de
-1 , y todos los demás tendrán un valor de 0.
Este fragmento de código se copió a 2 lugares más:
- V1009 Verifique la inicialización de la matriz. Solo el primer elemento se inicializa explícitamente. Los elementos restantes se inicializan con ceros. EASprintfOrdered.cpp 518
- V1009 Verifique la inicialización de la matriz. Solo el primer elemento se inicializa explícitamente. Los elementos restantes se inicializan con ceros. EASprintfOrdered.cpp 881
Advertencia 5
V728 Se puede simplificar una verificación excesiva. El '(A &&! B) || (! A && B) 'expresión es equivalente a la expresión' 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(); } .... }
Este fragmento de código fue formateado por conveniencia, pero en el original es una condición muy larga que es difícil de leer. Otra cosa es si simplificamos la expresión condicional, como aconseja el analizador:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
El código se ha vuelto mucho más corto, lo que simplificó enormemente la comprensión de la lógica de la expresión condicional.
Conclusión
Como habrás notado, la mayoría de las advertencias sospechosas se duplican en tres lugares y dentro del mismo archivo. La duplicación de código es una práctica de desarrollo muy mala, ya que complica el soporte de código. Y si se produce un error en dicho código, la estabilidad del programa disminuye drásticamente debido a la propagación de errores en todo el código.
Esperemos que se publique algo más interesante y volveremos a este repositorio nuevamente :). Mientras tanto, sugiero a aquellos que deseen descargar
PVS-Studio e intentar verificar sus propios proyectos.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov.
Bibliotecas casi perfectas por Electronic Arts .