# Зло живёт в функциях сравнения

Возможно, читатели помнят мою статью под названием "Эффект последней строки"\. В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текста\. Теперь я хочу рассказать о новом интересном наблюдении\. Оказывается, программисты тяготеют к тому, чтобы допустить ошибку в функциях сравнения двух объектов\. Такое утверждение выглядит неправдоподобным, однако, я покажу огромное количество примеров ошибок, которые шокируют читателя\. Читайте новое исследование, будет интересно и страшно\.

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

## Проблематика

Я утверждаю, что программисты очень часто допускают ошибки в достаточно простых функциях, которые предназначены для сравнения двух объектов\. Это утверждение основано на опыте проверки нашей командой большого количества открытых проектов на языках C, C\+\+ и C\#\.

Функции, о которых идёт речь, носят такие названия, как _IsEqual_, _Equals_, _Compare_, _AreEqual_ и так далее\. Или перегруженные операторы, такие как _\=\=_, _\!\=_\.

Я заметил, что при написании [статей](https://pvs-studio.ru/ru/blog/inspections/) мне очень часто попадаются ошибки, относящиеся к функциям сравнения\. Я решил исследовать этот вопрос подробнее и изучил [базу](https://pvs-studio.ru/ru/blog/examples/) найденных нами ошибок\. Для этого по базе был выполнен поиск функций с именами, содержащими слова _Cmp_, _Equal_, _Compare_ и так далее\. Результат оказался весьма впечатляющим и шокирующим\.

В общем\-то эта история схожа с написанием статьи "Эффект последней строки"\. Точно также я заметил аномалию и решил изучить её подробнее\. К сожалению, в отличии от упомянутой статьи, я не знаю какие числа и статистику мне привести\. Возможно, я придумаю как и что можно посчитать \- позже\. Сейчас же, я руководствуюсь интуицией и могу только поделиться своими ощущениями\. Они говорят, что ошибок в функциях сравнения много, и уверен, что у вас возникнет то же самое чувство, когда я покажу большое количество впечатляющих примеров\.

## Психология

На минутку вернемся к статье "[Эффект последней строки](https://pvs-studio.ru/ru/blog/posts/cpp/0260/)"\. Кстати, если вы не читали её, то предлагаю сделать паузу и ознакомиться с ней\. Есть и более детальный разбор этой темы: "[Объяснение эффекта последней строки](https://pvs-studio.ru/ru/blog/posts/0485/)"\. 

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

Я считаю, что в случае написания функции сравнения, разработчик вообще часто не сосредотачивается на ней, считая её тривиальной\. Другими словами, он пишет код автоматически, не задумываясь над ним\. В противном случае непонятно, как можно сделать подобную ошибку:

```cpp
bool IsLuidsEqual(LUID luid1, LUID luid2)
{
  return (luid1.LowPart == luid2.LowPart) &&
         (luid2.HighPart == luid2.HighPart);
}
```

Анализатор PVS\-Studio обнаружил эту ошибку в коде проекта **RunAsAdmin Explorer Shim** \(C\+\+\): [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: luid2\.HighPart \=\= luid2\.HighPart RAACommon raacommonfuncs\.cpp 1511

Опечатка\. Во второй строке должно быть написано: _luid1\.HighPart \=\= luid2\.HighPart_\.

Cогласитесь, код прост\. Видимо, простота кода как раз всё и портит\. Программист сразу воспринимает задачу по написанию такой функции стандартной и неинтересной\. Он моментально решает в голове, как написать функцию, и остаётся только реализовать код\. Это рутинный, но, к сожалению, необходимый процесс для того, чтобы перейти к написанию более важного, сложного и интересного кода\. Мыслями разработчик уже там и\.\.\. как результат \- допускает ошибку\.

Вдобавок программисты редко пишут юнит\-тесты для таких функций\. Опять подводит обманчивая простота этих функций\. Кажется избыточным их тестировать, ведь эти функции такие простые и однотипные\. Человек написал сотни таких функций за свою жизнь, неужели он может сделать в очередной функции ошибку?\! Может, и делает\.

При этом хочу обратить внимание, что речь в статье идёт не о коде студентов, которые учатся программировать\. Речь идёт об ошибках в коде таких проектов, как GCC, Qt, GDB, LibreOffice, Unreal Engine 4, CryEngine V, Chromium, MongoDB, Oracle VM Virtual Box, FreeBSD, WinMerge, CoreCLR, MySQL, Mono, CoreFX, Roslyn, MSBuild и т\.д\. Всё очень серьезно\.

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

## Ошибочные паттерны в функциях сравнения

Все ошибки в функциях сравнения будут разделены на несколько паттернов\. В статье речь идёт об ошибках в проектах на языках C, C\+\+ и C\#, но как\-то разделять эти языки смысла нет, так как многие паттерны одинаковы для разных языков\.

### Паттерн: A < B, B \> A

Очень часто в функциях сравнения надо выполнить такие проверки:

* A < B
* A \> B

Иногда программисты считают, что более красиво использовать тот же оператор <, но поменять местами имена переменных:

* A < B
* B < A

Однако, по невнимательности получаются вот такие проверки:

* A < B
* B \> A

На самом деле, здесь два раза выполняется одно и то же сравнение\. Возможно, вы не поняли о чём идёт речь, но сейчас мы перейдём к практическим примерам, и всё прояснится\.

```cpp
string _server;
....
bool operator<( const ServerAndQuery& other ) const {
  if ( ! _orderObject.isEmpty() )
    return _orderObject.woCompare( other._orderObject ) < 0;

  if ( _server < other._server )
    return true;
  if ( other._server > _server )
    return false;
  return _extra.woCompare( other._extra ) < 0;
}
```

Анализатор PVS\-Studio обнаружил эту ошибку в коде проекта **MongoDB** \(С\+\+\): [V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' operators situated alongside each other are identical\. Check lines: 44, 46\. parallel\.h 46

Это условие:

```cpp
if ( other._server > _server )
```

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

```cpp
if ( _server < other._server )
  return true;
if ( other._server < _server )
  return false;
```

Обнаруживается такая ошибка и в коде проекта **Chromium** \(C\+\+\):

```cpp
enum ContentSettingsType;
struct EntryMapKey {
  ContentSettingsType content_type;
  ...
};

bool OriginIdentifierValueMap::EntryMapKey::operator<(
    const OriginIdentifierValueMap::EntryMapKey& other) const {
  if (content_type < other.content_type)
    return true;
  else if (other.content_type > content_type)
    return false;
  return (resource_identifier < other.resource_identifier);
}
```

Предупреждение PVS\-Studio: [V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 61, 63\. browser content\_settings\_origin\_identifier\_value\_map\.cc 61

Я показал пример на языке C\+\+, теперь очередь C\#\. Следующая ошибка найдена в коде **IronPython and IronRuby** \(C\#\)\.

```cpp
public static int Compare(SourceLocation left,
                          SourceLocation right) {
  if (left < right) return -1;
  if (right > left) return 1;
  return 0;
}
```

Предупреждение PVS\-Studio \(C\#\): [V3021](https://pvs-studio.ru/ru/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless\. SourceLocation\.cs 156

Думаю, пояснения здесь излишни\.

Примечание\. Для C\# здесь только один пример ошибки, а для C\+\+ их было 2\. И вообще, ошибок для C\# кода в статье будет показано меньше, чем для C/C\+\+\. Однако, я не рекомендую спешить с выводом, что C\# намного безопаснее\. Дело в том, что анализатор PVS\-Studio научился анализировать C\# код относительно недавно, и мы просто проверили намного меньше проектов, написанных на C\#, чем на С и C\+\+\.

### Паттерн: член класса сравнивается сам с собой

Функции сравнения часто состоят из последовательных сравнений полей структур/классов\. Такой код тяготеет к ошибкам, когда член класса начинает сравниваться сам с собой\. При этом можно выделить два подвида ошибки\.

В первом случае, забывают указать имя объекта и пишут так:

```cpp
return m_x == foo.m_x &&
       m_y == m_y &&            // <=
       m_z == foo.m_z;
```

Во втором случае, пишут одно и тоже имя объекта:

```cpp
return zzz.m_x == foo.m_x &&
       zzz.m_y == zzz.m_y &&    // <=
       zzz.m_z == foo.m_z;
```

Давайте теперь ознакомимся с этим паттерном ошибок на практике\. Кстати, обратите внимание, что неправильное сравнение часто находится в последнем из однотипных блоков текста: привет "эффекту последней строки"\.

Ошибка найдена в коде проекта **Unreal Engine 4** \(C\+\+\):

```cpp
bool
Compare(const FPooledRenderTargetDesc& rhs, bool bExact) const
{
  ....
  return Extent == rhs.Extent
    && Depth == rhs.Depth
    && bIsArray == rhs.bIsArray
    && ArraySize == rhs.ArraySize
    && NumMips == rhs.NumMips
    && NumSamples == rhs.NumSamples
    && Format == rhs.Format
    && LhsFlags == RhsFlags
    && TargetableFlags == rhs.TargetableFlags
    && bForceSeparateTargetAndShaderResource ==
         rhs.bForceSeparateTargetAndShaderResource
    && ClearValue == rhs.ClearValue
    && AutoWritable == AutoWritable;           // <=
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: AutoWritable \=\= AutoWritable rendererinterface\.h 180

Код проекта **Samba** \(C\):

```cpp
static int compare_procids(const void *p1, const void *p2)
{
  const struct server_id *i1 = (struct server_id *)p1;
  const struct server_id *i2 = (struct server_id *)p2;

  if (i1->pid < i2->pid) return -1;
  if (i2->pid > i2->pid) return 1;
  return 0;
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\>' operator: i2\-\>pid \> i2\-\>pid brlock\.c 1901

Код проекта **MongoDB** \(C\+\+\):

```cpp
bool operator==(const MemberCfg& r) const {
  ....
  return _id==r._id && votes == r.votes &&
         h == r.h && priority == r.priority &&
         arbiterOnly == r.arbiterOnly &&
         slaveDelay == r.slaveDelay &&
         hidden == r.hidden &&
         buildIndexes == buildIndexes;        // <=
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: buildIndexes \=\= buildIndexes rs\_config\.h 101

Код проекта **Geant4 Software** \(C\+\+\):

```cpp
inline G4bool G4FermiIntegerPartition::
operator==(const G4FermiIntegerPartition& right)
{
  return (total == right.total &&
          enableNull == enableNull &&          // <=
          partition == right.partition);
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: enableNull \=\= enableNull G4hadronic\_deex\_fermi\_breakup g4fermiintegerpartition\.icc 58

Код проекта **LibreOffice** \(C\+\+\):

```cpp
class SvgGradientEntry
{
  ....
  bool operator==(const SvgGradientEntry& rCompare) const
  {
    return (getOffset() == rCompare.getOffset()
           && getColor() == getColor()            // <=
           && getOpacity() == getOpacity());      // <=
  }
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: getColor\(\) \=\= getColor\(\) svggradientprimitive2d\.hxx 61

Код проекта **Chromium** \(C\+\+\):

```cpp
bool FileIOTest::MatchesResult(const TestStep& a,
                               const TestStep& b) {
  ....
  return (a.data_size == a.data_size &&             // <=
          std::equal(a.data, a.data + a.data_size, b.data));
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: a\.data\_size \=\= a\.data\_size cdm\_file\_io\_test\.cc 367

Код проекта **FreeCAD** \(C\+\+\):

```cpp
bool FaceTypedBSpline::isEqual(const TopoDS_Face &faceOne,
                               const TopoDS_Face &faceTwo) const
{
  ....
  if (surfaceOne->IsURational() != 
      surfaceTwo->IsURational())
    return false;
  if (surfaceTwo->IsVRational() !=         // <= 
      surfaceTwo->IsVRational())           // <=
    return false;
  if (surfaceOne->IsUPeriodic() != 
      surfaceTwo->IsUPeriodic())
    return false;
  if (surfaceOne->IsVPeriodic() != 
      surfaceTwo->IsVPeriodic())
    return false;
  if (surfaceOne->IsUClosed() != 
      surfaceTwo->IsUClosed())
    return false;
  if (surfaceOne->IsVClosed() != 
      surfaceTwo->IsVClosed())
    return false;
  if (surfaceOne->UDegree() != 
      surfaceTwo->UDegree())
    return false;
  if (surfaceOne->VDegree() != 
      surfaceTwo->VDegree())
    return false;
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'surfaceTwo\-\>IsVRational\(\)' to the left and to the right of the '\!\=' operator\. modelrefine\.cpp 780

Код проекта **Serious Engine** \(C\+\+\):

```cpp
class CTexParams {
public:

  inline BOOL IsEqual( CTexParams tp) {
    return tp_iFilter     == tp.tp_iFilter &&
           tp_iAnisotropy == tp_iAnisotropy &&             // <=
           tp_eWrapU      == tp.tp_eWrapU &&
           tp_eWrapV      == tp.tp_eWrapV; };
  ....
};
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: tp\_iAnisotropy \=\= tp\_iAnisotropy gfx\_wrapper\.h 180

Код проекта **Qt** \(C\+\+\):

```cpp
inline bool qCompare(QImage const &t1, QImage const &t2, ....)
{
  ....
  if (t1.width() != t2.width() || t2.height() != t2.height()) {
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\!\=' operator: t2\.height\(\) \!\= t2\.height\(\) qtest\_gui\.h 101

Код проекта **FreeBSD** \(C\):

```cpp
static int
compare_sh(const void *_a, const void *_b)
{
  const struct ipfw_sopt_handler *a, *b;

  a = (const struct ipfw_sopt_handler *)_a;
  b = (const struct ipfw_sopt_handler *)_b;
  ....
  if ((uintptr_t)a->handler < (uintptr_t)b->handler)
    return (-1);
  else if ((uintptr_t)b->handler > (uintptr_t)b->handler) // <=
    return (1);
  
  return (0);
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(uintptr\_t\) b\-\>handler' to the left and to the right of the '\>' operator\. ip\_fw\_sockopt\.c 2893

Код проекта **Mono** \(C\#\):

```cpp
static bool AreEqual (VisualStyleElement value1, 
                      VisualStyleElement value2)
{
  return
    value1.ClassName == value1.ClassName && // <=
    value1.Part == value2.Part &&
    value1.State == value2.State;
}
```

Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'value1\.ClassName' to the left and to the right of the '\=\=' operator\. ThemeVisualStyles\.cs 2141

Код проекта **Mono** \(C\#\):

```cpp
public int ExactInference (TypeSpec u, TypeSpec v)
{
  ....
  var ac_u = (ArrayContainer) u;
  var ac_v = (ArrayContainer) v;
  ....
  var ga_u = u.TypeArguments;
  var ga_v = v.TypeArguments;
  ....
  if (u.TypeArguments.Length != u.TypeArguments.Length) // <=
    return 0;

  ....
}
```

Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'u\.TypeArguments\.Length' to the left and to the right of the '\!\=' operator\. generic\.cs 3135

Код проекта **MonoDevelop** \(C\#\):

```cpp
Accessibility DeclaredAccessibility { get; }
bool IsStatic { get; }

private bool MembersMatch(ISymbol member1, ISymbol member2)
{
  if (member1.Kind != member2.Kind)
  {
    return false;
  }

  if (member1.DeclaredAccessibility !=          // <=1
      member1.DeclaredAccessibility             // <=1
   || member1.IsStatic != member1.IsStatic)     // <=2
  {
    return false;
  }

  if (member1.ExplicitInterfaceImplementations().Any() ||  
      member2.ExplicitInterfaceImplementations().Any())
  {
    return false;
  }

  return SignatureComparer
    .HaveSameSignatureAndConstraintsAndReturnTypeAndAccessors(
       member1, member2, this.IsCaseSensitive);
}
```

Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'member1\.IsStatic' to the left and to the right of the '\!\=' operator\. CSharpBinding AbstractImplementInterfaceService\.CodeAction\.cs 545

Код проекта **Haiku** \(C\+\+\):

```cpp
int __CORTEX_NAMESPACE__ compareTypeAndID(....)
{
  int retValue = 0;
  ....
  if (lJack && rJack)
  {
    if (lJack->m_jackType < lJack->m_jackType)           // <=
    {
      return -1;
    }
    if (lJack->m_jackType == lJack->m_jackType)          // <=
    {
      if (lJack->m_index < rJack->m_index)
      {
        return -1;
      }
      else
      {
        return 1;
      }
    }
    else if (lJack->m_jackType > rJack->m_jackType)
    {
      retValue = 1;
    }
  }
  return retValue;
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '<' operator: lJack\-\>m\_jackType < lJack\-\>m\_jackType MediaJack\.cpp 783

Чуть ниже есть ещё одна точно такая же ошибка\. Как я понимаю, в обоих случаях забыли заменить _lJack_ на _rJack_\.

Код проекта **CryEngine V** \(C\+\+\):

```cpp
bool
CompareRotation(const Quat& q1, const Quat& q2, float epsilon)
{
  return (fabs_tpl(q1.v.x - q2.v.x) <= epsilon)
      && (fabs_tpl(q1.v.y - q2.v.y) <= epsilon)
      && (fabs_tpl(q2.v.z - q2.v.z) <= epsilon)     // <=
      && (fabs_tpl(q1.w - q2.w) <= epsilon);
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\-' operator: q2\.v\.z \- q2\.v\.z entitynode\.cpp 93

### Паттерн: вычисление размера указателя вместо размера структуры/класса

Данный вид ошибки возникает в программах на языках C и C\+\+ и связан с неправильным использованием оператора _sizeof_\. Ошибка заключается в вычислении не размера объекта, а размера указателя\. Пример:

```cpp
T *a = foo1();
T *b = foo2();
x = memcmp(a, b, sizeof(a));
```

Вместо размера структуры _T_ вычисляется размер указателя\. Размер указателя зависит от используемой [модели данных](https://pvs-studio.ru/ru/blog/terms/0012/), но обычно это 4 или 8 байт\. В результате сравнивается больше или меньше байт памяти, чем занимает структура\.

Правильный вариант кода:

```cpp
x = memcmp(a, b, sizeof(T));
```

или

```cpp
x = memcmp(a, b, sizeof(*a));
```

Теперь давайте перейдём к практике\. Вот как такая ошибка выглядит в коде проекта **CryEngine V** \(C\+\+\):

```cpp
bool
operator==(const SComputePipelineStateDescription& other) const
{
  return 0 == memcmp(this, &other, sizeof(this));
}
```

Предупреждение PVS\-Studio: [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) The memcmp function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. graphicspipelinestateset\.h 58

Код проекта **Unreal Engine 4** \(C\+\+\):

```cpp
bool FRecastQueryFilter::IsEqual(
  const INavigationQueryFilterInterface* Other) const
{
  // @NOTE: not type safe, should be changed when
  // another filter type is introduced
  return FMemory::Memcmp(this, Other, sizeof(this)) == 0;
}
```

Предупреждение PVS\-Studio: [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) The Memcmp function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. pimplrecastnavmesh\.cpp 172

### Паттерн: повторяющиеся аргументы вида Cmp\(A, A\)

Функции сравнения часто вызывают другие функции сравнения\. При этом одной из возможных ошибок является то, что два раза передаётся ссылка/указатель на один и тот же объект\. Пример:

```cpp
x = memcmp(A, A, sizeof(T));
```

Здесь объект _A_ будет сравнён сам с собой, что, естественно, не имеет смысла\. 

Начнём мы с ошибки, найденной в коде отладчика **GDB** \(C\):

```cpp
static int
psymbol_compare (const void *addr1, const void *addr2,
                 int length)
{
  struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
  struct partial_symbol *sym2 = (struct partial_symbol *) addr2;

  return (memcmp (&sym1->ginfo.value, &sym1->ginfo.value,    // <=
                  sizeof (sym1->ginfo.value)) == 0
          && sym1->ginfo.language == sym2->ginfo.language
          && PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2)
          && PSYMBOL_CLASS (sym1) == PSYMBOL_CLASS (sym2)
          && sym1->ginfo.name == sym2->ginfo.name);
}
```

Предупреждение PVS\-Studio: [V549](https://pvs-studio.ru/ru/docs/warnings/v549/) The first argument of 'memcmp' function is equal to the second argument\. psymtab\.c 1580

Код проекта **CryEngineSDK** \(C\+\+\):

```cpp
inline bool operator != (const SEfResTexture &m) const
{
  if (stricmp(m_Name.c_str(), m_Name.c_str()) != 0 ||   // <=
      m_TexFlags != m.m_TexFlags || 
      m_bUTile != m.m_bUTile ||
      m_bVTile != m.m_bVTile ||
      m_Filter != m.m_Filter ||
      m_Ext != m.m_Ext ||
      m_Sampler != m.m_Sampler)
    return true;
  return false;
}
```

Предупреждение PVS\-Studio: [V549](https://pvs-studio.ru/ru/docs/warnings/v549/) The first argument of 'stricmp' function is equal to the second argument\. ishader\.h 2089

Код проекта **PascalABC\.NET** \(C\#\):

```cpp
private List<string> enum_consts = new List<string>();
public override bool IsEqual(SymScope ts)
{
  EnumScope es = ts as EnumScope;
  if (es == null) return false;
  if (enum_consts.Count != es.enum_consts.Count) return false;
  for (int i = 0; i < es.enum_consts.Count; i++)
    if (string.Compare(enum_consts[i],
                       this.enum_consts[i], true) != 0)
      return false;
  return true;
}
```

Предупреждение PVS\-Studio: [V3038](https://pvs-studio.ru/ru/docs/warnings/v3038/) The 'enum\_consts\[i\]' argument was passed to 'Compare' method several times\. It is possible that other argument should be passed instead\. CodeCompletion SymTable\.cs 2206

Здесь немного поясню\. Ошибка в фактических аргументах функции _Compare_:

```cpp
string.Compare(enum_consts[i], this.enum_consts[i], true)
```

Дело в том, что _enum\_consts\[i\]_ и _this\.enum\_consts\[i\]_ \- это одно и то же\. Как я понимаю, правильный вызов должен быть таким:

```cpp
string.Compare(es.enum_consts[i], this.enum_consts[i], true)
```

или

```cpp
string.Compare(enum_consts[i], es.enum_consts[i], true)
```

### Паттерн: повторяющиеся проверки A\=\=B && A\=\=B

[Распространённой](https://pvs-studio.ru/ru/blog/examples/v501/) ошибкой при программировании является, когда одна и та же проверка выполняется дважды\. Пример:

```cpp
return A == B &&
       C == D &&   // <=
       C == D &&   // <=
       E == F;
```

В подобных случаях возможны 2 варианта\. Первый безобидный: одно сравнение лишнее, и его можно просто удалить\. Второй хуже: хотели проверить какие\-то другие переменные, но опечатались\.

В любом случае подобный код заслуживает пристального внимания\. Давайте я напугаю вас сильнее и покажу, что такую ошибку можно встретить даже в коде компилятора **GCC** \(C\):

```cpp
static bool
dw_val_equal_p (dw_val_node *a, dw_val_node *b)
{
  ....
  case dw_val_class_vms_delta:
    return (!strcmp (a->v.val_vms_delta.lbl1,
                     b->v.val_vms_delta.lbl1)
            && !strcmp (a->v.val_vms_delta.lbl1,
                        b->v.val_vms_delta.lbl1));
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\!strcmp\(a\-\>v\.val\_vms\_delta\.lbl1, b\-\>v\.val\_vms\_delta\.lbl1\)' to the left and to the right of the '&&' operator\. dwarf2out\.c 1428

Два раза вызывается функция _strcmp_ с одним и тем же набором аргументов\.

Код проекта **Unreal Engine 4** \(C\+\+\):

```cpp
FORCEINLINE
bool operator==(const FShapedGlyphEntryKey& Other) const
{
  return FontFace == Other.FontFace 
    && GlyphIndex == Other.GlyphIndex   // <=
    && FontSize == Other.FontSize
    && FontScale == Other.FontScale
    && GlyphIndex == Other.GlyphIndex;  // <=
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'GlyphIndex \=\= Other\.GlyphIndex' to the left and to the right of the '&&' operator\. fontcache\.h 139

Код проекта **Serious Engine** \(C\+\+\):

```cpp
inline BOOL CValuesForPrimitive::operator==(....)
{
  return (
 (....) &&
 (vfp_ptPrimitiveType == vfpToCompare.vfp_ptPrimitiveType) &&
 ....
 (vfp_ptPrimitiveType == vfpToCompare.vfp_ptPrimitiveType) &&
 ....
);
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(vfp\_ptPrimitiveType \=\= vfpToCompare\.vfp\_ptPrimitiveType\)' to the left and to the right of the '&&' operator\. worldeditor\.h 580

Код проекта **Oracle VM Virtual Box** \(C\+\+\):

```cpp
typedef struct SCMDIFFSTATE
{
  ....
  bool  fIgnoreTrailingWhite;
  bool  fIgnoreLeadingWhite;
  ....
} SCMDIFFSTATE;
/* Pointer to a diff state. */

typedef SCMDIFFSTATE *PSCMDIFFSTATE;

/* Compare two lines */
DECLINLINE(bool) scmDiffCompare(PSCMDIFFSTATE pState, ....)
{
  ....
  if (pState->fIgnoreTrailingWhite    // <=
   || pState->fIgnoreTrailingWhite)   // <=
    return scmDiffCompareSlow(....);
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'pState\-\>fIgnoreTrailingWhite' to the left and to the right of the '\|\|' operator\. scmdiff\.cpp 238

### Паттерн: неправильное использование значения, которое возвращает функция memcmp

Функция _memcmp_ возвращает следующие значения типа _int_:

* < 0 \- buf1 less than buf2;
* 0 \- buf1 identical to buf2;
* \> 0 \- buf1 greater than buf2;

Обратите внимание\. "Больше 0" означает любые числа, а вовсе не 1\. Этими числами могут быть: 2, 3, 100, 256, 1024, 5555, 65536 и так далее\. Это значит, что этот результат нельзя поместить в переменную типа _char_ или _short_\. Могут быть отброшены значащие биты, что приведет к нарушению логики выполнения программы\.

Также из этого следует, что результат нельзя сравнивать с константами 1 или \-1\. Другими словами, так делать неправильно:

```cpp
if (memcmp(a, b, sizeof(T)) == 1)
if (memcmp(x, y, sizeof(T)) == -1)
```

Правильные сравнения:

```cpp
if (memcmp(a, b, sizeof(T)) > 0)
if (memcmp(a, b, sizeof(T)) < 0)
```

Коварность описанных ошибок в том, что код может долгое время успешно работать\. Ошибки могут начать проявлять себя при переходе на новую платформу или при смене версии компилятора\.

Код проекта **ReactOS** \(C\+\+\):

```cpp
HRESULT WINAPI CRecycleBin::CompareIDs(....)
{
  ....
  return MAKE_HRESULT(SEVERITY_SUCCESS, 0,
   (unsigned short)memcmp(pidl1->mkid.abID,
                          pidl2->mkid.abID,
                          pidl1->mkid.cb));
}
```

Предупреждение PVS\-Studio: [V642](https://pvs-studio.ru/ru/docs/warnings/v642/) Saving the 'memcmp' function result inside the 'unsigned short' type variable is inappropriate\. The significant bits could be lost breaking the program's logic\. recyclebin\.cpp 542

Код проекта **Firebird** \(C\+\+\):

```cpp
SSHORT TextType::compare(ULONG len1, const UCHAR* str1,
                         ULONG len2, const UCHAR* str2)
{
  ....
  SSHORT cmp = memcmp(str1, str2, MIN(len1, len2));

  if (cmp == 0)
    cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0));
  return cmp;
}
```

Предупреждение PVS\-Studio: [V642](https://pvs-studio.ru/ru/docs/warnings/v642/) Saving the 'memcmp' function result inside the 'short' type variable is inappropriate\. The significant bits could be lost breaking the program's logic\. texttype\.cpp 338

Код проекта **CoreCLR** \(C\+\+\):

```cpp
bool operator( )(const GUID& _Key1, const GUID& _Key2) const
  { return memcmp(&_Key1, &_Key2, sizeof(GUID)) == -1; }
```

Предупреждение PVS\-Studio: [V698](https://pvs-studio.ru/ru/docs/warnings/v698/) Expression 'memcmp\(\.\.\.\.\) \=\= \-1' is incorrect\. This function can return not only the value '\-1', but any negative value\. Consider using 'memcmp\(\.\.\.\.\) < 0' instead\. sos util\.cpp 142

Код проекта **OpenToonz** \(C\+\+\):

```cpp
bool TFilePath::operator<(const TFilePath &fp) const
{
  ....
  char differ;
  differ = _wcsicmp(iName.c_str(), jName.c_str());
  if (differ != 0)
    return differ < 0 ? true : false;
  ....
}
```

Предупреждение PVS\-Studio: [V642](https://pvs-studio.ru/ru/docs/warnings/v642/) Saving the '\_wcsicmp' function result inside the 'char' type variable is inappropriate\. The significant bits could be lost, breaking the program's logic\. tfilepath\.cpp 328

### Паттерн: неправильная проверка нулевых ссылок

Данный паттерн ошибки свойственен программам на языке C\#\. В функциях сравнения иногда выполняют приведение типа с помощью оператора _as_\. Ошибка заключается в том, что по невнимательности на _null_ проверяется не новая ссылка, а исходная\. Рассмотрим синтетический пример:

```cpp
ChildT foo = obj as ChildT;
if (obj == null)
  return false;
if (foo.zzz()) {}
```

Проверка _if \(obj \=\= null\)_ защищает от ситуации, если переменная _obj_ содержала нулевую ссылку\. Однако, нет никакой защиты от случая, если окажется, что оператор _as_ вернёт нулевую ссылку\. Правильный код должен выглядеть так:

```cpp
ChildT foo = obj as ChildT;
if (foo == null)
  return false;
if (foo.zzz()) {}
```

Как правило, ошибка возникает из\-за невнимательности программиста\. Подобные ошибки возможны и в программах на языке С и C\+\+, но я не нашел в базе найденных нами ошибок ни одного такого случая\.

Код проекта **MonoDevelop** \(C\#\):

```cpp
public override bool Equals (object o)
{
  SolutionItemReference sr = o as SolutionItemReference;
  if (o == null)
    return false;
  return (path == sr.path) && (id == sr.id);
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'o', 'sr'\. MonoDevelop\.Core SolutionItemReference\.cs 81

Код проекта **CoreFX** \(C\#\):

```cpp
public override bool Equals(object comparand)
{
  CredentialHostKey comparedCredentialKey =
                                  comparand as CredentialHostKey;

  if (comparand == null)
  {
    // This covers also the compared == null case
    return false;
  }

  bool equals = string.Equals(AuthenticationType,
        comparedCredentialKey.AuthenticationType, ....
  ....
}
```

Предупреждение PVS\-Studio:  [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'comparand', 'comparedCredentialKey'\. CredentialCache\.cs 4007

Код проекта **Roslyn** \(C\#\):

```cpp
public override bool Equals(object obj)
{
  var d = obj as DiagnosticDescription;

  if (obj == null)
    return false;
    
  if (!_code.Equals(d._code))
    return false;
  ....
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'obj', 'd'\. DiagnosticDescription\.cs 201

Код проекта **Roslyn** \(C\#\):

```cpp
protected override bool AreEqual(object other)
{
  var otherResourceString = other as LocalizableResourceString;
  return
    other != null &&
    _nameOfLocalizableResource == 
      otherResourceString._nameOfLocalizableResource &&
    _resourceManager == otherResourceString._resourceManager &&
    _resourceSource == otherResourceString._resourceSource &&
    ....
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'other', 'otherResourceString'\. LocalizableResourceString\.cs 121

Код проекта **MSBuild** \(C\#\):

```cpp
public override bool Equals(object obj)
{
   AssemblyNameExtension name = obj as AssemblyNameExtension;
   if (obj == null)  // <=
   {
     return false;
   }
   ....
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'obj', 'name'\. AssemblyRemapping\.cs 64

Код проекта **Mono** \(C\#\):

```cpp
public override bool Equals (object o)
{
  UrlMembershipCondition umc = (o as UrlMembershipCondition);
  if (o == null)                                      // <=
    return false;

  ....

  return (String.Compare (u, 0, umc.Url, ....) == 0); // <=
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'o', 'umc'\. UrlMembershipCondition\.cs 111

Код проекта **Media Portal 2** \(C\#\):

```cpp
public override bool Equals(object obj)
{
  EpisodeInfo other = obj as EpisodeInfo;
  if (obj == null) return false;
  if (TvdbId > 0 && other.TvdbId > 0)
    return TvdbId == other.TvdbId;
  ....
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'obj', 'other'\. EpisodeInfo\.cs 560

Код проекта **NASA World Wind** \(C\#\):

```cpp
public int CompareTo(object obj)
{
  RenderableObject robj = obj as RenderableObject;
  if(obj == null)                                 // <=
    return 1;
  return this.m_renderPriority.CompareTo(robj.RenderPriority);
}
```

Предупреждение PVS\-Studio: [V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'obj', 'robj'\. RenderableObject\.cs 199

### Паттерн: Неправильные циклы

В некоторых функциях сравниваются коллекции элементов\. Естественно, что для их сравнения используются различные варианты циклов\. Если писать код невнимательно, как и бывает с функциями сравнения, то с циклами легко что\-то напутать\. Рассмотрим несколько таких ситуаций\.

Код проекта **Trans\-Proteomic Pipeline** \(C\+\+\):

```cpp
bool Peptide::operator==(Peptide& p) {
  ....
  for (i = 0, j = 0;
       i < this->stripped.length(), j < p.stripped.length();
       i++, j++) { 
  ....
}
```

Предупреждение PVS\-Studio: [V521](https://pvs-studio.ru/ru/docs/warnings/v521/) Such expressions using the ',' operator are dangerous\. Make sure the expression is correct\. tpplib peptide\.cpp 191

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

Код проекта **Qt** \(C\+\+\):

```cpp
bool equals( class1* val1, class2* val2 ) const
{
  ...
  size_t size = val1->size();
  ...
  while ( --size >= 0 ){
    if ( !comp(*itr1,*itr2) )
      return false;
    itr1++;
    itr2++;
  }
  ...
}
```

Предупреждение PVS\-Studio: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression '\-\- size \>\= 0' is always true\. Unsigned type value is always \>\= 0\. QtCLucene arrays\.h 154

Код проекта **CLucene** \(C\+\+\):

```cpp
class Arrays
{
  ....
   bool equals( class1* val1, class2* val2 ) const{
     static _comparator comp;
     if ( val1 == val2 )
       return true;
     size_t size = val1->size();
     if ( size != val2->size() )
       return false;
     _itr1 itr1 = val1->begin();
     _itr2 itr2 = val2->begin();
     while ( --size >= 0 ){
       if ( !comp(*itr1,*itr2) )
         return false;
       itr1++;
       itr2++;
     }
   return true;
  }
  ....
}
```

Предупреждение PVS\-Studio: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression '\-\- size \>\= 0' is always true\. Unsigned type value is always \>\= 0\. arrays\.h 154

Код проекта **Mono** \(C\#\):

```cpp
public override bool Equals (object obj)
{
  ....
  for (int i=0; i < list.Count; i++) {
    bool found = false;
    for (int j=0; i < ps.list.Count; j++) {     // <=
      if (list [i].Equals (ps.list [j])) {
        found = true;
        break;
      }
    }
    if (!found)
      return false;
  }
  return true; 
}
```

Предупреждение PVS\-Studio: [V3015](https://pvs-studio.ru/ru/docs/warnings/v3015/) It is likely that a wrong variable is being compared inside the 'for' operator\. Consider reviewing 'i' corlib\-net\_4\_x PermissionSet\.cs 607

По всей видимости допущена опечатка, и на самом деле в условии вложенного цикла должна использоваться переменная _j_, а не _i_:

```cpp
for (int j=0; j < ps.list.Count; j++)
```

### Паттерн: A \= getA\(\), B \= GetA\(\)

Часто в функциях сравнения приходится писать код вот такого типа:

```cpp
if (GetA().x == GetB().x && GetA().y == GetB().y)
```

Чтобы сократить размер условий или для оптимизации, используют промежуточные переменные:

```cpp
Type A = GetA();
Type B = GetB();
if (A.x == B.x && A.y == B.y)
```

При этом по невнимательности иногда допускают ошибку и инициализируют временные переменные одним и тем же значением:

```cpp
Type A = GetA();
Type B = GetA();
```

Давайте посмотрим, как такие ошибки выглядят в коде реальных приложений\.

Код проекта **LibreOffice** \(C\+\+\):

```cpp
bool CmpAttr(
  const SfxPoolItem& rItem1, const SfxPoolItem& rItem2)
{
  ....
  bool bNumOffsetEqual = false;
  ::boost::optional<sal_uInt16> oNumOffset1 =
        static_cast<const SwFmtPageDesc&>(rItem1).GetNumOffset();
  ::boost::optional<sal_uInt16> oNumOffset2 =
        static_cast<const SwFmtPageDesc&>(rItem1).GetNumOffset();

  if (!oNumOffset1 && !oNumOffset2)
  {
    bNumOffsetEqual = true;
  }
  else if (oNumOffset1 && oNumOffset2)
  {
    bNumOffsetEqual = oNumOffset1.get() == oNumOffset2.get();
  }
  else
  {
    bNumOffsetEqual = false;
  }
  ....
}
```

Предупреждение PVS\-Studio: [V656](https://pvs-studio.ru/ru/docs/warnings/v656/) Variables 'oNumOffset1', 'oNumOffset2' are initialized through the call to the same function\. It's probably an error or un\-optimized code\. Check lines: 68, 69\. findattr\.cxx 69

Код проекта **Qt** \(C\+\+\):

```cpp
AtomicComparator::ComparisonResult
IntegerComparator::compare(const Item &o1,
                           const AtomicComparator::Operator,
                           const Item &o2) const
{
  const Numeric *const num1 = o1.as<Numeric>();
  const Numeric *const num2 = o1.as<Numeric>();
 
  if(num1->isSigned() || num2->isSigned())
  ....
}
```

Предупреждение PVS\-Studio: [V656](https://pvs-studio.ru/ru/docs/warnings/v656/) Variables 'num1', 'num2' are initialized through the call to the same function\. It's probably an error or un\-optimized code\. Consider inspecting the 'o1\.as < Numeric \> \(\)' expression\. Check lines: 220, 221\. qatomiccomparators\.cpp 221

### Паттерн: неудачное копирование кода

Очень многие ошибки, приведённые ранее, можно назвать последствиями неудачного Copy\-Paste\. Они попадали под какой\-то ошибочный паттерн, и я решил, что их логично описать в соответствующих разделах\. Однако, у меня осталось несколько ошибок, которые явно возникли из\-за неудачного копирования кода, но которые я не знаю как классифицировать\. Поэтому я просто собрал эти ошибки здесь\.

Код проекта **CoreCLR** \(C\+\+\):

```cpp
int __cdecl Compiler::RefCntCmp(const void* op1, const void* op2)
{
  ....
  if (weight1)
  {
    ....
    if (varTypeIsGC(dsc1->TypeGet()))
    {
      weight1 += BB_UNITY_WEIGHT / 2;
    }
    if (dsc1->lvRegister)
    {
      weight1 += BB_UNITY_WEIGHT / 2;
    }
  }

  if (weight1)
  {
    ....
    if (varTypeIsGC(dsc2->TypeGet()))
    {
      weight1 += BB_UNITY_WEIGHT / 2;       // <=
    }
    if (dsc2->lvRegister)
    {
      weight2 += BB_UNITY_WEIGHT / 2;
    }
  }
  ....
}
```

Предупреждение PVS\-Studio: [V778](https://pvs-studio.ru/ru/docs/warnings/v778/) Two similar code fragments were found\. Perhaps, this is a typo and 'weight2' variable should be used instead of 'weight1'\. clrjit lclvars\.cpp 2702

Функция была длинная, поэтому она основательно сокращена для статьи\. Если рассматривать код этой функции, то заметно, что часть кода была скопирована, но в одном месте программист забыл заменить переменную _weight1_ на _weight2_\.

Код проекта **WPF samples by Microsoft** \(C\#\):

```cpp
public int Compare(GlyphRun a, GlyphRun b)
{
  ....
  if (aPoint.Y > bPoint.Y)      // <=
  {
    return -1;
  }
  else if (aPoint.Y > bPoint.Y) // <=
  {
    result = 1;
  }
  else if (aPoint.X < bPoint.X)
  {
    result = -1;
  }
  else if (aPoint.X > bPoint.X)
  {
    result = 1;
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 418, 422\. txtserializerwriter\.cs 418

Код проекта **PascalABC\.NET** \(C\#\):

```cpp
public void CompareInternal(....)
{
  ....
  else if (left is int64_const)
    CompareInternal(left as int64_const, right as int64_const);
  ....
  else if (left is int64_const)
    CompareInternal(left as int64_const, right as int64_const);  
  ....
}
```

Предупреждение PVS\-Studio: [V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 597, 631\. ParserTools SyntaxTreeComparer\.cs 597

Код проекта **SharpDevelop** \(C\#\):

```cpp
public int Compare(SharpTreeNode x, SharpTreeNode y)
{
  ....
  if (typeNameComparison == 0) {
    if (x.Text.ToString().Length < y.Text.ToString().Length)
      return -1;
    if (x.Text.ToString().Length < y.Text.ToString().Length)
      return 1;
  }  
  ....
}
```

Предупреждение PVS\-Studio: [V3021](https://pvs-studio.ru/ru/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless NamespaceTreeNode\.cs 87

Код проекта **Coin3D** \(C\+\+\):

```cpp
int
SbProfilingData::operator == (const SbProfilingData & rhs) const
{
  if (this->actionType != rhs.actionType) return FALSE;
  if (this->actionStartTime != rhs.actionStopTime) return FALSE;
  if (this->actionStartTime != rhs.actionStopTime) return FALSE;
  ....
}
```

Предупреждение PVS\-Studio: [V649](https://pvs-studio.ru/ru/docs/warnings/v649/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains function return\. This means that the second 'if' statement is senseless\. Check lines: 1205, 1206\. sbprofilingdata\.cpp 1206

Код проекта **Spring** \(C\+\+\):

```cpp
bool operator < (const aiFloatKey& o) const
  {return mTime < o.mTime;}
bool operator > (const aiFloatKey& o) const
  {return mTime < o.mTime;}
```

Предупреждение PVS\-Studio: [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\. assimp 3dshelper\.h 470

И последний особенно интересный фрагмент кода, который анализатор кода PVS\-Studio нашёл в проекте **MySQL** \(C\+\+\):

```cpp
static int rr_cmp(uchar *a,uchar *b)
{
  if (a[0] != b[0])
    return (int) a[0] - (int) b[0];
  if (a[1] != b[1])
    return (int) a[1] - (int) b[1];
  if (a[2] != b[2])
    return (int) a[2] - (int) b[2];
  if (a[3] != b[3])
    return (int) a[3] - (int) b[3];
  if (a[4] != b[4])
    return (int) a[4] - (int) b[4];
  if (a[5] != b[5])
    return (int) a[1] - (int) b[5]; // <=
  if (a[6] != b[6])
    return (int) a[6] - (int) b[6];
  return (int) a[7] - (int) b[7];
}
```

Предупреждение PVS\-Studio: [V525](https://pvs-studio.ru/ru/docs/warnings/v525/) The code containing the collection of similar blocks\. Check items '0', '1', '2', '3', '4', '1', '6' in lines 680, 682, 684, 689, 691, 693, 695\. sql records\.cc 680

Скорее всего, программист написал первое сравнение, потом второе и ему стало скучно\. Поэтому он скопировал в буфер обмена блок текста:

```cpp
if (a[1] != b[1])
  return (int) a[1] - (int) b[1];
```

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

```cpp
if (a[5] != b[5])
  return (int) a[1] - (int) b[5];
```

Примечание\. Я подробнее рассуждаю об этой ошибке в мини\-книге "[Главный вопрос программирования, рефакторинга и всего такого](https://pvs-studio.ru/ru/blog/posts/cpp/0391/)" \(см\. главу "Не берите на себя работу компилятора"\)\.

### Паттерн: метод Equals некорректно обрабатывает нулевую ссылку

В C\# принято реализовывать методы _Equals_ так, чтобы они корректно обрабатывали ситуацию, если в качестве аргумента приходит нулевая ссылка\. К сожалению, реализация не всех методов соответствует этому правилу\.

Код проекта **GitExtensions** \(C\#\):

```cpp
public override bool Equals(object obj)
{
  return GetHashCode() == obj.GetHashCode(); // <=
}
```

Предупреждение PVS\-Studio: [V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to 'Equals\(object obj\)' method should not result in 'NullReferenceException'\. Git\.hub Organization\.cs 14

Код проекта **PascalABC\.NET** \(C\#\):

```cpp
public override bool Equals(object obj)
{
  var rhs = obj as ServiceReferenceMapFile;
  return FileName == rhs.FileName;
}
```

Предупреждение PVS\-Studio: [V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. ICSharpCode\.SharpDevelop ServiceReferenceMapFile\.cs 31

### Разные другие ошибки

Код проекта **G3D Content Pak** \(C\+\+\):

```cpp
bool Matrix4::operator==(const Matrix4& other) const {
  if (memcmp(this, &other, sizeof(Matrix4) == 0)) {
    return true;
  }
  ...
}
```

Предупреждение PVS\-Studio: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memcmp' function processes '0' elements\. Inspect the 'third' argument\. graphics3D matrix4\.cpp 269

Одна закрывающаяся скобка стоит не на своём месте\. В результате количество сравниваемых байт вычисляется выражением _sizeof\(Matrix4\) \=\= 0_\. Размер любого класса больше 0, а значит результат выражения равен 0\. Таким образом, сравнивается 0 байт\.

Правильный вариант: 

```cpp
if (memcmp(this, &other, sizeof(Matrix4)) == 0) {
```

Код проекта **Wolfenstein 3D** \(C\+\+\):

```cpp
inline int operator!=( quat_t a, quat_t b )
{
  return ( ( a.x != b.x ) || ( a.y != b.y ) ||
           ( a.z != b.z ) && ( a.w != b.w ) );
}
```

Предупреждение PVS\-Studio: [V648](https://pvs-studio.ru/ru/docs/warnings/v648/) Priority of the '&&' operation is higher than that of the '\|\|' operation\. math\_quaternion\.h 167

В одном месте, видимо, случайно написали оператор _&&_ вместо _\|\|_\.

Код проекта **FlightGear** \(C\):

```cpp
static int tokMatch(struct Token* a, struct Token* b)
{
  int i, l = a->strlen;
  if(!a || !b) return 0;
  ....
}
```

Предупреждение PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'a' pointer was utilized before it was verified against nullptr\. Check lines: 478, 479\. codegen\.c 478

Если в функцию передать в качестве первого аргумента _NULL_, то произойдёт разыменование нулевого указателя, хотя по задумке функция должна вернуть значение _0_\.

Код проекта **WinMerge** \(C\+\+\):

```cpp
int TimeSizeCompare::CompareFiles(int compMethod,
                                  const DIFFITEM &di)
{
  UINT code = DIFFCODE::SAME;
  ...
  if (di.left.size != di.right.size)
  {
    code &= ~DIFFCODE::SAME;
    code = DIFFCODE::DIFF;
  }
  ...
}
```

Предупреждение PVS\-Studio: [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'code' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 79, 80\. Merge timesizecompare\.cpp 80

Код проекта **ReactOS** \(C\+\+\):

```cpp
#define IsEqualGUID(rguid1, rguid2) \
  (!memcmp(&(rguid1), &(rguid2), sizeof(GUID)))

static int ctl2_find_guid(....)
{
  MSFT_GuidEntry *guidentry;
  ...
  if (IsEqualGUID(guidentry, guid)) return offset;
  ...
}
```

Предупреждение PVS\-Studio: [V512](https://pvs-studio.ru/ru/docs/warnings/v512/) A call of the 'memcmp' function will lead to underflow of the buffer 'guidentry'\. oleaut32 typelib2\.c 320

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

Правильный вариант:

```cpp
if (IsEqualGUID(*guidentry, guid)) return offset;
```

Код проекта **IronPython and IronRuby** \(C\#\):

```cpp
public static bool Equals(float x, float y) {
  if (x == y) {
    return !Single.IsNaN(x);
  }
  return x == y;
}
```

Предупреждение PVS\-Studio: [V3024](https://pvs-studio.ru/ru/docs/warnings/v3024/) An odd precise comparison: x \=\= y\. Consider using a comparison with defined precision: Math\.Abs\(A \- B\) < Epsilon\. FloatOps\.cs 1048

Непонятно, в чём тут смысл специальной проверки на _NaN_\. Если условие _\(x \=\= y\)_ выполняется, то это значит, что и _x_, и _y_ отличны от _NaN_, потому что _NaN_ не равен никакому другому значению, в том числе и самому себе\. Похоже, что проверка на _NaN_ просто лишняя и код можно сократить до:

```cpp
public static bool Equals(float x, float y) {
  return x == y;
}
```

Код проекта **Mono** \(C\#\):

```cpp
public bool Equals (CounterSample other)
{
  return
    rawValue         == other.rawValue         &&
    baseValue        == other.counterFrequency &&   // <=
    counterFrequency == other.counterFrequency &&   // <=
    systemFrequency  == other.systemFrequency  &&
    timeStamp        == other.timeStamp        &&
    timeStamp100nSec == other.timeStamp100nSec &&
    counterTimeStamp == other.counterTimeStamp &&
    counterType      == other.counterType;
}
```

Предупреждение PVS\-Studio: [V3112](https://pvs-studio.ru/ru/docs/warnings/v3112/) An abnormality within similar comparisons\. It is possible that a typo is present inside the expression 'baseValue \=\= other\.counterFrequency'\. System\-net\_4\_x CounterSample\.cs 139

## Как вообще эти программы работают?

Просматривая все эти ошибки, кажется удивительным, что все эти программы вообще работают\. Ведь функции сравнения выполняют крайне важную и ответственную задачу в программах\.

Есть несколько объяснений работоспособности программ с такими ошибками:

1. Во многих функциях неправильно сравнивается только часть объекта\. При этом частичного сравнения хватает для большинства задач в этой программе\.
1. Не возникает \(пока\) ситуаций, когда функция будет работать неправильно\. Например, это относится к функциям, которые не защищены от нулевых указателей или тем, где результат вызова функции _memcmp_ помещается в переменную типа _char_\. Программе просто везёт\.
1. Рассмотренная функция сравнения используется крайне редко или вообще не используется\.
1. А кто сказал, что программа работает? Многие программы действительно делают что\-то неправильно\!

## Рекомендации

Я продемонстрировал, как много ошибок можно встретить в функциях сравнения\. А отсюда следует, что работоспособность таких функций обязательно должна проверяться с помощью юнит\-тестов\.

Непременно пишите юнит\-тесты для операторов сравнения, для функций _Equals_ и так далее\.

Уверен, что многим до прочтения этой статьи казалось, что такие тесты избыточны и всё равно никаких ошибок они не выявят: ведь функции сравнения на первый взгляд так просты\.\.\. Что же, теперь я показал весь ужас, который может в них скрываться\.

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

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

В этой статье упоминалось множество именитых проектов, которые разрабатываются высококлассными специалистами\. Эти проекты тщательно тестируются с помощью различных методологий\. И все равно, это не помешало анализатору PVS\-Studio найти в них ошибки\. Это говорит о том, что PVS\-Studio может стать отличным дополнением к другим методологиям, используемым для повышения качества и надёжности кода\.

Приходите к нам на сайт и попробуйте [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\.