# Библиотеки Electronic Arts почти хорошего качества

Наше внимание привлёк репозиторий Electronic Arts на GitHub\. Он очень маленький и из двадцати трёх проектов нас заинтересовали только несколько C\+\+ библиотек: EASTL, EAStdC, EABase, EAThread, EATest, EAMain и EAAssert\. Проекты оказались тоже очень маленькими \(около 10 файлов\), поэтому мы нашли ошибки только в "самом большом" из 20 файлов :D Но нашли, и интересные\! Пока писалась заметка, мы с коллегами также бурно обсудили игры компании EA и её стратегию :D\.

![0646_ElectornicArtsStdC_ru/image1.png](https://import.viva64.com/docx/blog/0646_ElectornicArtsStdC_ru/image1.png)

## Введение

[Electronic Arts \(EA\)](https://www.ea.com/) \- американская компания, которая занимается распространением видеоигр\. На сайте GitHub у неё есть небольшой [репозиторий](https://github.com/electronicarts) и несколько C\+\+ проектов\. Это C\+\+ библиотеки: EASTL, EAStdC, EABase, EAThread, EATest, EAMain и EAAssert\. Они очень маленькие и мы нашли интересные ошибки с помощью анализатора [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/) только в "самом большом" из них \- [EAStdC](https://github.com/electronicarts/EAStdC) \(20 файлов\)\. При таких объёмах [сложно](https://pvs-studio.ru/ru/blog/posts/0158/) говорить о качестве кода в целом\. Просто оцените 5 предупреждений и решите сами\.

## Предупреждение 1

[V524](https://pvs-studio.ru/ru/docs/warnings/v524/) It is odd that the body of '\>\>' function is fully equivalent to the body of '<<' function\. EAFixedPoint\.h 287

```cpp
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;}
  ....
}
```

При перегрузке операторов сдвига программист допустил опечатку в одном из них, перепутав операторы << и \>\>\. Скорее всего, это результат Copy\-Paste\-программирования\.

## Предупреждение 2

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'nFormatLength' index could reach 16\. EASprintfOrdered\.cpp 246

```cpp
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;                        // <=
  else
    return -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':
    {
      // Finalize the current span.
      spans[spanIndex].mpEnd = p + 1;
      spans[spanIndex].mFormat[nFormatLength] = 0;                         // <=
      spans[spanIndex].mFormatChar = *p;
      if(++spanIndex == kSpanCapacity)
        break;
  ....
}
```

Массив _spans\[spanIndex\]\.mFormat_ состоит из _16_ элементов, т\.е\. последний валидный элемент имеет индекс _15_\. Сейчас код функции _OVprintfCore_ написан так, что если индекс _nFormatLength_ будет иметь максимально возможный индекс \- _15_, то произойдёт инкремент до _16_\. Далее в операторе _switch_ возможен выход за границу массива\.

Этот фрагмент кода скопирован ещё в 2 места:

* V557 Array overrun is possible\. The value of 'nFormatLength' index could reach 16\. EASprintfOrdered\.cpp 614
* V557 Array overrun is possible\. The value of 'nFormatLength' index could reach 16\. EASprintfOrdered\.cpp 977

## Предупреждение 3

[V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: \(result \>\= 0\)\. EASprintfOrdered\.cpp 489

```cpp
static int OVprintfCore(....)
{
  ....
  for(result = 1; (result >= 0) && (p < pEnd); ++p)
  {
    if(pWriteFunction8(p, 1, pWriteFunctionContext8, kWFSIntermediate) < 0)
      return -1;
    nWriteCountSum += result;
  }
  ....
}
```

Условие _result \>\= 0_ всегда истинно, потому что переменная _result_ нигде в цикле не меняется\. Код выглядит очень подозрительно и, скорее всего, имеет место ошибка в этом коде\.

Этот фрагмент кода скопирован ещё в 2 места:

* V560 A part of conditional expression is always true: \(result \>\= 0\)\. EASprintfOrdered\.cpp 852
* V560 A part of conditional expression is always true: \(result \>\= 0\)\. EASprintfOrdered\.cpp 1215

## Предупреждение 4

[V1009](https://pvs-studio.ru/ru/docs/warnings/v1009/) Check the array initialization\. Only the first element is initialized explicitly\. The rest elements are initialized with zeros\. EASprintfOrdered\.cpp 151

```cpp
static int OVprintfCore(....)
{
  ....
  int spanArgOrder[kArgCapacity] = { -1 };
  ....
}
```

Возможно, это не является ошибкой, но разработчиков следует предупредить, что значением _\-1_ инициализируется только первый элемент массива _spanArgOrder_, а все остальные будут иметь значение 0\.

Этот фрагмент кода скопирован ещё в 2 места:

* V1009 Check the array initialization\. Only the first element is initialized explicitly\. The rest elements are initialized with zeros\. EASprintfOrdered\.cpp 518
* V1009 Check the array initialization\. Only the first element is initialized explicitly\. The rest elements are initialized with zeros\. EASprintfOrdered\.cpp 881

## Предупреждение 5

[V728](https://pvs-studio.ru/ru/docs/warnings/v728/) An excessive check can be simplified\. The '\(A && \!B\) \|\| \(\!A && B\)' expression is equivalent to the 'bool\(A\) \!\= bool\(B\)' expression\. int128\.h 1242

```cpp
inline void int128_t::Modulus(....) const
{
  ....
  bool bDividendNegative = false;
  bool bDivisorNegative = false;
  ....
  if(    (bDividendNegative && !bDivisorNegative)
     || (!bDividendNegative &&  bDivisorNegative))
  {
    quotient.Negate();
  }
  ....
}
```

Этот фрагмент кода был отформатирован для удобства, но в оригинале это очень длинное условие, которое сложно читать\. Другое дело, если мы упростим условное выражение, как советует анализатор:

```cpp
if( bDividendNegative != bDivisorNegative)
{
  quotient.Negate();
}
```

Код стал значительно короче, что сильно упростило понимание логики условного выражения\.

## Заключение

Как вы могли заметить, большинство подозрительных предупреждений продублированы в трех местах, причём в пределах одного и того же файла\. Дублирование кода является очень плохой практикой разработки, т\.к\. усложняет поддержку кода\. А если в такой код попадает ошибка, то стабильность программы резко снижается из\-за распространения ошибок по всему коду\.

Будем надеется, что опубликуют ещё что\-то интересное и мы вновь вернёмся в этот репозиторий :\)\. Ну а пока предлагаю желающим скачать [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/) и попробовать проверить собственные проекты\.