Nossa atenção foi recentemente atraída pelo repositório de Artes Eletrônicas no GitHub. É pequeno e, dos 23 projetos disponíveis lá, apenas algumas bibliotecas C ++ pareciam interessantes: EASTL, EAStdC, EABase, EAThread, EATest, EAMain e EAAssert. Os projetos em si também são pequenos (cerca de 10 arquivos cada), então os erros foram encontrados apenas no "maior" projeto de 20 arquivos: D Mas nós os encontramos e eles parecem interessantes! Enquanto escrevia este post, também discutíamos animadamente os jogos da EA e a política da empresa: D
1. Introdução
A Electronic Arts (EA) é uma empresa americana de videogame. Ele possui um pequeno
repositório no GitHub e alguns projetos C ++, ou seja, bibliotecas C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain e EAAssert. Eles são pequenos e o analisador
PVS-Studio conseguiu encontrar todos os bugs apenas no "maior" projeto, o
EAStdC (20 arquivos). Em tamanhos como esse, você
não pode julgar com
segurança a qualidade geral do código; basta dar uma olhada nos cinco avisos a seguir e decidir por si mesmo.
Aviso 1
V524 É estranho que o corpo da função '>>' seja totalmente equivalente ao corpo da função '<<'. 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;} .... }
Ao sobrecarregar os operadores de turno, o programador digitou um deles digitando << em vez de >>. Isso se parece muito com um erro de copiar e colar.
Aviso 2
A saturação da matriz V557 é possível. O valor do índice 'nFormatLength' pode chegar 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;
A matriz
spans [spanIndex] .mFormat consiste em
16 elementos, portanto o índice do último elemento válido é
15 . Em sua forma atual, a função
OVprintfCore acaba incrementando o índice de
nFormatLength para
16 se tiver o maior índice possível, ou seja,
15 . Depois disso, ocorrerá um erro de matriz fora dos limites na instrução
switch .
Este fragmento foi copiado mais duas vezes:
- A saturação da matriz V557 é possível. O valor do índice 'nFormatLength' pode chegar a 16. EASprintfOrdered.cpp 614
- A saturação da matriz V557 é possível. O valor do índice 'nFormatLength' pode chegar a 16. EASprintfOrdered.cpp 977
Aviso 3
V560 Uma parte da expressão condicional é sempre verdadeira: (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; } .... }
A condição
resultado> = 0 sempre é verdadeira, pois a variável
resultado não é alterada em nenhum lugar do loop. O código não parece certo, e deve haver algum erro nele.
Este fragmento foi copiado mais duas vezes:
- V560 Uma parte da expressão condicional é sempre verdadeira: (resultado> = 0). EASprintfOrdered.cpp 852
- V560 Uma parte da expressão condicional é sempre verdadeira: (resultado> = 0). EASprintfOrdered.cpp 1215
Aviso 4
V1009 Verifique a inicialização do array. Somente o primeiro elemento é inicializado explicitamente. Os demais elementos são inicializados com zeros. EASprintfOrdered.cpp 151
static int OVprintfCore(....) { .... int spanArgOrder[kArgCapacity] = { -1 }; .... }
Isso não é necessariamente um bug, mas os autores devem ser avisados de que apenas o primeiro elemento da matriz
spanArgOrder é inicializado como
-1 , enquanto todo o restante será definido como 0.
Este fragmento foi copiado mais duas vezes:
- V1009 Verifique a inicialização do array. Somente o primeiro elemento é inicializado explicitamente. Os demais elementos são inicializados com zeros. EASprintfOrdered.cpp 518
- V1009 Verifique a inicialização do array. Somente o primeiro elemento é inicializado explicitamente. Os demais elementos são inicializados com zeros. EASprintfOrdered.cpp 881
Aviso 5
V728 Uma verificação excessiva pode ser simplificada. O '(A &&! B) || (! A && B) 'expressão é equivalente à expressão' 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(); } .... }
Formatei este exemplo para maior clareza, mas em sua forma original, essa condição é muito longa e difícil de ler. Mas podemos melhorar muito simplificando a expressão condicional, como sugere o analisador:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
O código é muito mais curto agora, o que facilita muito a compreensão da lógica da condição.
Conclusão
Como você deve ter notado, a maioria dos avisos possui mais duas duplicatas e todas essas duplicatas são encontradas no mesmo arquivo. A duplicação de código é um anti-padrão muito ruim, pois complica muito a manutenção do programa. Quando os bugs se infiltram nesse código, a estabilidade do programa diminui drasticamente porque se espalham por todo o código.
Felizmente, a EA fará o upload de outros projetos interessantes e visitaremos seu repositório mais uma vez :). Enquanto isso, faça o download do
PVS-Studio e experimente em seus próprios projetos.