Nuestra atención fue atraída recientemente por el repositorio de Electronic Arts en GitHub. Es pequeño, y de los veintitrés proyectos disponibles allí, solo unas pocas bibliotecas de C ++ parecían interesantes: EASTL, EAStdC, EABase, EAThread, EATest, EAMain y EAAssert. Los proyectos en sí también son pequeños (alrededor de 10 archivos cada uno), por lo que los errores se encontraron solo en el proyecto "más grande" de 20 archivos: D ¡Pero los encontramos y parecen interesantes! Mientras escribía esta publicación, también teníamos una discusión animada sobre los juegos de EA y la política de la compañía: D
Introduccion
Electronic Arts (EA) es una compañía estadounidense de videojuegos. Tiene un pequeño
repositorio en GitHub y algunos proyectos de C ++, a saber, bibliotecas de C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain y EAAssert. Son pequeños, y el analizador
PVS-Studio logró encontrar cualquier error solo en el proyecto "más grande",
EAStdC (20 archivos). Con tamaños como ese, no
puede juzgar de
manera confiable la calidad general del código, así que solo eche un vistazo a las siguientes cinco advertencias y decida usted 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 escribiendo << en lugar de >>. Esto se parece mucho a un error 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, por lo que el índice del último elemento válido es
15 . En su forma actual, la función
OVprintfCore termina incrementando el índice de
nFormatLength a
16 si tiene el índice más alto posible, es decir,
15 . Después de eso, se producirá un error de matriz fuera de los límites en la instrucción de
cambio .
Este fragmento fue copiado dos veces 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; } .... }
La condición de
resultado> = 0 siempre es verdadera ya que la variable de
resultado no se cambia en ninguna parte del bucle. El código no se ve bien en absoluto, y debe haber algún error en él.
Este fragmento fue copiado dos veces 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 no es necesariamente un error, pero se debe advertir a los autores que solo el primer elemento de la matriz
spanArgOrder se inicializa en
-1 , mientras que el resto se establecerá en 0.
Este fragmento fue copiado dos veces 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(); } .... }
He formateado este ejemplo para mayor claridad, pero en su forma original, esta condición es muy larga y difícil de leer. Pero podemos hacerlo mucho mejor simplificando la expresión condicional, como sugiere el analizador:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
El código es mucho más corto ahora, lo que hace que la lógica de la condición sea mucho más fácil de entender.
Conclusión
Como habrás notado, la mayoría de las advertencias tienen dos duplicados más, y todos esos duplicados se encuentran en el mismo archivo. La duplicación de código es un anti-patrón muy malo ya que complica mucho el mantenimiento del programa. Cuando los errores se introducen en dicho código, la estabilidad del programa cae drásticamente porque se extienden por todo el código.
Con suerte, EA cargará algunos otros proyectos interesantes y visitaremos su repositorio una vez más :). Mientras tanto, bienvenido a descargar
PVS-Studio y probarlo en sus propios proyectos.