Наше внимание недавно привлек репозиторий Electronic Arts на GitHub. Он очень маленький, и из двадцати трех доступных там проектов лишь несколько библиотек C++ показались интересными: EASTL, EAStdC, EABase, EAThread, EATest, EAMain и EAAssert. Сами проекты тоже очень маленькие (примерно по 10 файлов каждый), поэтому ошибки были обнаружены только в «самом большом» проекте из 20 файлов
Введение
Electronic Arts (EA) — американская компания по разработке видеоигр. У неё есть небольшой репозиторий на GitHub и несколько проектов на C++, а именно библиотеки C++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain и EAAssert. Они очень маленькие, и анализатор PVS-Studio смог обнаружить какие-либо ошибки только в «самом большом» проекте, EAStdC (20 файлов). При таких размерах невозможно достоверно оценить общее качество кода, поэтому просто взгляните на следующие пять предупреждений и решите сами.
Предупреждение 1
V524 Странно, что тело функции '>>' полностью эквивалентно телу функции '<<'. EAFixedPoint.h 287
шаблон <класс T,
int upShiftInt, int downShiftInt,
int upMulInt, int downDivInt>
структура 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;}
....
}
При перегрузке операторов сдвига программист допустил опечатку в одном из них, написав << вместо >>. Это очень похоже на ошибку копирования и вставки.
Предупреждение 2
V557 Возможно переполнение массива. Значение индекса 'nFormatLength' может достигать 16. EASprintfOrdered.cpp 246
static const int kSpanFormatCapacity = 16;
структура Span8
{
....
char mFormat[kSpanFormatCapacity];
....
};
static int OVprintfCore(....)
{
....
EA_ASSERT(nFormatLength < kSpanFormatCapacity);
if(nFormatLength < kSpanFormatCapacity)
spans[spanIndex].mFormat[nFormatLength++] = *p; // <=
еще
вернуть -1;
switch(*p)
{
case 'b': case 'd': case 'i': case 'u': case 'o': case 'x': case 'X':
case 'g': case 'G': case 'e': case 'E': case 'f': case 'F': case 'a':
case 'A': case 'p': case 'c': case 'C': case 's': case 'S': case 'n':
{
// Завершение обработки текущего фрагмента.
spans[spanIndex].mpEnd = p + 1;
spans[spanIndex].mFormat[nFormatLength] = 0; // <=
spans[spanIndex].mFormatChar = *p;
if(++spanIndex == kSpanCapacity)
перерыв;
....
}
Массив spans[spanIndex].mFormat состоит из 16 элементов, поэтому индекс последнего допустимого элемента равен 15. В текущем виде функция OVprintfCore увеличивает индекс nFormatLength до 16, если он равен максимально возможному индексу, то есть 15. После этого в операторе switch возникнет ошибка выхода за пределы массива .
Этот фрагмент был скопирован ещё два раза:
- V557 Возможно переполнение массива. Значение индекса 'nFormatLength' может достигать 16. EASprintfOrdered.cpp 614
- V557 Возможно переполнение массива. Значение индекса 'nFormatLength' может достигать 16. EASprintfOrdered.cpp 977
Предупреждение 3
V560 Часть условного выражения всегда истинна: (результат >= 0). EASprintfOrdered.cpp 489
static int OVprintfCore(....)
{
....
for(result = 1; (result >= 0) && (p < pEnd); ++p)
{
if(pWriteFunction8(p, 1, pWriteFunctionContext8, kWFSIntermediate) < 0)
вернуть -1;
nWriteCountSum += result;
}
....
}
Условие result >= 0 всегда истинно, поскольку переменная result нигде в цикле не изменяется. Код выглядит совершенно неправильно, и в нём, должно быть, есть какая-то ошибка.
Этот фрагмент был скопирован ещё два раза:
- V560 Часть условного выражения всегда истинна: (результат >= 0). EASprintfOrdered.cpp 852
- V560 Часть условного выражения всегда истинна: (результат >= 0). EASprintfOrdered.cpp 1215
Предупреждение 4
V1009 Проверьте инициализацию массива. Явно инициализируется только первый элемент. Остальные элементы инициализируются нулями. EASprintfOrdered.cpp 151
static int OVprintfCore(....)
{
....
int spanArgOrder[kArgCapacity] = { -1 };
....
}
Это не обязательно ошибка, но авторов следует предупредить, что только первый элемент массива spanArgOrder инициализируется значением -1 , в то время как все остальные будут установлены в 0.
Этот фрагмент был скопирован ещё два раза:
- V1009 Проверьте инициализацию массива. Явно инициализируется только первый элемент. Остальные элементы инициализируются нулями. EASprintfOrdered.cpp 518
- V1009 Проверьте инициализацию массива. Явно инициализируется только первый элемент. Остальные элементы инициализируются нулями. EASprintfOrdered.cpp 881
Предупреждение 5
V728 Избыточную проверку можно упростить. Выражение '(A && !B) || (!A && B)' эквивалентно выражению '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();
}
....
}
Я отформатировал этот пример для большей ясности, но в исходном виде это условие очень длинное и трудночитаемое. Однако мы можем значительно улучшить его, упростив условное выражение, как предлагает анализатор:
if( bDividendNegative != bDivisorNegative)
{
quotient.Negate();
}
Теперь код стал намного короче, что значительно упрощает понимание логики условия.