Зло живёт в функциях сравнения
Я утверждаю, что программисты очень часто допускают ошибки в достаточно простых функциях, которые предназначены для сравнения двух объектов. Это утверждение основано на опыте проверки нашей командой большого количества открытых проектов на языках C, C++ и C#.
Функции, о которых идёт речь, носят такие названия, как IsEqual, Equals, Compare, AreEqual и так далее. Или перегруженные операторы, такие как ==, !=.
Я заметил, что при написании статей мне очень часто попадаются ошибки, относящиеся к функциям сравнения. Я решил исследовать этот вопрос подробнее и изучил базу найденных нами ошибок. Для этого по базе был выполнен поиск функций с именами, содержащими слова Cmp, Equal, Compare и так далее. Результат оказался весьма впечатляющим и шокирующим.
В общем-то эта история схожа с написанием статьи «Эффект последней строки». Точно также я заметил аномалию и решил изучить её подробнее. К сожалению, в отличии от упомянутой статьи, я не знаю какие числа и статистику мне привести. Возможно, я придумаю: как и что можно посчитать — позже. Сейчас же, я руководствуюсь интуицией и могу только поделиться своими ощущениями. Они говорят, что ошибок в функциях сравнения много, и уверен, что у вас возникнет то же самое чувство, когда я покажу большое количество впечатляющих примеров.
Психология
На минутку вернемся к статье "Эффект последней строки". Кстати, если вы не читали её, то предлагаю сделать паузу и познакомиться с ней. Есть и более детальный разбор этой темы: "Объяснение эффекта последней строки".
В целом, можно сделать вывод, что причина ошибок в последних строках связана с тем, что разработчик уже мысленно перешел к следующим строкам/задачам, вместо того, чтобы сосредоточиться на завершении текущего фрагмента. Как результат — в конце написания однотипных блоков текста в последнем из них, он с большей вероятность допустит опечатку, чем в предыдущих.
Я считаю, что в случае написания функции сравнения, разработчик вообще часто не сосредотачивается на ней, считая её тривиальной. Другими словами, он пишет код автоматически, не задумываясь над ним. В противном случае, непонятно как можно сделать подобную ошибку:
Анализатор PVS-Studio обнаружил эту ошибку в коде проекта RunAsAdmin Explorer Shim (C++): 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
Анализатор PVS-Studio обнаружил эту ошибку в коде проекта MongoDB (С++): V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 44, 46. parallel.h 46
Всегда будет ложным, так как точно такая же проверка выполнялась двумя строчками выше. Корректный вариант кода:
Обнаруживается такая ошибка и в коде проекта Chromium (C++):
Предупреждение PVS-Studio: 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#).
Предупреждение PVS-Studio (C#): 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++.
Паттерн: член класса сравнивается сам с собойФункции сравнения часто состоят из последовательных сравнений полей структур/классов. Такой код тяготеет к ошибкам, когда член класса начинает сравниваться сам с собой. При этом можно выделить два подвида ошибки.
В первом случае, забывают указать имя объекта и пишут так:
Во втором случае, пишут одно и тоже имя объекта:
Давайте теперь познакомимся с этим паттерном ошибок на практике. Кстати, обратите внимание, что неправильное сравнение часто находится в последнем из однотипных блоков текста: привет «эффекту последней строки».
Ошибка найдена в коде проекта Unreal Engine 4 (C++):
Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '==' operator: AutoWritable == AutoWritable rendererinterface.h 180
Код проекта Samba (С):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '==' operator: getColor() == getColor() svggradientprimitive2d.hxx 61
Код проекта Chromium (C++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: V501 There are identical sub-expressions 'surfaceTwo->IsVRational()' to the left and to the right of the '!=' operator. modelrefine.cpp 780
Код проекта Serious Engine (C++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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 (С):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'value1.ClassName' to the left and to the right of the '==' operator. ThemeVisualStyles.cs 2141
Код проекта Mono (C#):
Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'u.TypeArguments.Length' to the left and to the right of the '!=' operator. generic.cs 3135
Код проекта MonoDevelop (C#):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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. Ошибка заключается в вычислении не размера объекта, а размера указателя. Пример:
Вместо размера структуры T вычисляется размер указателя. Размер указателя зависит от используемой модели данных, но обычно это 4 или 8 байт. В результате сравнивается больше или меньше байт памяти, чем занимает структура.
Правильный вариант кода:
Теперь давайте перейдём к практике. Вот как такая ошибка выглядит в коде проекта CryEngine V (C++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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)Функции сравнения часто вызывают другие функции сравнения. При этом одной из возможных ошибок является, что два раза передаётся ссылка/указатель на один и тот же объект. Пример:
Здесь объект A будет сравнён сам с собой, что, естественно, не имеет смысла.
Начнём мы с ошибки, найденный в коде отладчика GDB (С):
Предупреждение PVS-Studio: V549 The first argument of 'memcmp' function is equal to the second argument. psymtab.c 1580
Код проекта CryEngineSDK (C++):
Предупреждение PVS-Studio: V549 The first argument of 'stricmp' function is equal to the second argument. ishader.h 2089
Код проекта PascalABC.NET (C#):
Предупреждение PVS-Studio: 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:
Дело в том, что enum_consts[i] и this.enum_consts[i] это одно и тоже. Как я понимаю, правильный вызов должен быть таким:
Паттерн: повторяющиеся проверки A==B && A==BРаспространённой ошибкой при программировании является, когда одна и та же проверка выполняется дважды. Пример:
В подобных случаях возможны 2 варианта. Первый безобидный: одно сравнение лишнее, и его можно просто удалить. Второй хуже: хотели проверить какие-то другие переменные, но опечатались.
В любом случае подобный код заслуживает пристального внимания. Давайте я напугаю вас посильнее и покажу, что такую ошибку можно встретить даже в коде компилятора GCC (С):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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;
Также из этого следует, что результат нельзя сравнивать с константами 1 или -1. Другими словами, так делать неправильно:
Коварность описанных ошибок в том, что код может долгое время успешно работать. Ошибки могут начать проявлять себя при переходе на новую платформу или при смене версии компилятора.
Код проекта ReactOS (C++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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 проверяется не новая ссылка, а исходная. Рассмотрим синтетический пример:
Проверка if (obj == null) защищает от ситуации, если переменная obj содержала нулевую ссылку. Однако, нет никакой защиты от случая, если окажется, что оператор as вернёт нулевую ссылку. Правильный код должен выглядеть так:
Как правило, ошибка возникает из-за невнимательности программиста. Подобные ошибки возможны и в программах на языке С и C++, но я не нашел в базе найденных нами ошибок ни одного такого случая.
Код проекта MonoDevelop (C#):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'comparand', 'comparedCredentialKey'. CredentialCache.cs 4007
Код проекта Roslyn (C#):
Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'obj', 'd'. DiagnosticDescription.cs 201
Код проекта Roslyn (C#):
Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'other', 'otherResourceString'. LocalizableResourceString.cs 121
Код проекта MSBuild (C#):
Предупреждение PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'obj', 'name'. AssemblyRemapping.cs 64
Код проекта Mono (C#):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. tpplib peptide.cpp 191
Обратите внимание, что в условии используется оператор запятая. Код явно неверен, так как условие, стоящее слева от запятой, не учитывается. То есть условие слева вычисляется, но его результат никак не используется.
Код проекта Qt (C++):
Предупреждение PVS-Studio: V547 Expression '-- size >= 0' is always true. Unsigned type value is always >= 0. QtCLucene arrays.h 154
Код проекта CLucene (C++):
Предупреждение PVS-Studio: V547 Expression '-- size >= 0' is always true. Unsigned type value is always >= 0. arrays.h 154
Код проекта Mono (C#):
Предупреждение PVS-Studio: 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:
Паттерн: A = getA(), B = GetA()Часто в функциях сравнения приходится писать код вот такого типа:
Чтобы сократить размер условий или для оптимизации, используют промежуточные переменные:
При этом по невнимательности иногда допускают ошибку и инициализируют временные переменные одним и тем же значением:
Давайте посмотрим, как такие ошибки выглядят в коде реальных приложений.
Код проекта LibreOffice (C++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: 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#):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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++):
Предупреждение PVS-Studio: 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
Скорее всего, программист написал первое сравнение, потом второе и ему стало скучно. Поэтому он скопировал в буфер обмена блок текста:
И вставил его в текст программы нужное ему количество раз. Затем он менял индексы, но в одном месте ошибся, и получилось некорректное сравнение:
Примечание. Я подробнее рассуждаю об этой ошибке в мини-книге "Главный вопрос программирования, рефакторинга и всего такого" (см. главу «Не берите на себя работу компилятора»).
Паттерн: метод Equals некорректно обрабатывает нулевую ссылкуВ C# принято реализовывать методы Equals так, чтобы они корректно обрабатывали ситуацию, если в качестве аргумента приходит нулевая ссылка. К сожалению, реализация не всех методов соответствует этому правилу.
Код проекта GitExtensions (C#):
Предупреждение PVS-Studio: V3115 Passing 'null' to 'Equals(object obj)' method should not result in 'NullReferenceException'. Git.hub Organization.cs 14
Код проекта PascalABC.NET (C#):
Предупреждение PVS-Studio: V3115 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. ICSharpCode.SharpDevelop ServiceReferenceMapFile.cs 31
Разные другие ошибкиКод проекта G3D Content Pak (C++):
Предупреждение PVS-Studio: V575 The 'memcmp' function processes '0' elements. Inspect the 'third' argument. graphics3D matrix4.cpp 269
Одна закрывающаяся скобка стоит не на своём месте. В результате, количество сравниваемых байт вычисляется выражением sizeof(Matrix4) == 0. Размер любого класса больше 0, а значит, результат выражения равен 0. Таким образом, сравнивается 0 байт.
Код проекта Wolfenstein 3D (C++):
Предупреждение PVS-Studio: V648 Priority of the '&&' operation is higher than that of the '||' operation. math_quaternion.h 167
В одном месте, видимо, случайно написали оператор && вместо ||.
Код проекта FlightGear (С):
Предупреждение PVS-Studio: V595 The 'a' pointer was utilized before it was verified against nullptr. Check lines: 478, 479. codegen.c 478
Если в функцию передать в качестве первого аргумента NULL, то произойдёт разыменование нулевого указателя, хотя по задумке функция должна вернуть значение 0.
Код проекта WinMerge (C++):
Предупреждение PVS-Studio: V519 The 'code' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 79, 80. Merge timesizecompare.cpp 80
Код проекта ReactOS (C++):
Предупреждение PVS-Studio: V512 A call of the 'memcmp' function will lead to underflow of the buffer 'guidentry'. oleaut32 typelib2.c 320
В качестве первого аргумента макроса выступает указатель. В результате вычисляется адрес указателя, что не имеет никакого смысла.
Код проекта IronPython and IronRuby (C#):
Предупреждение PVS-Studio: 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 просто лишняя и код можно сократить до:
Код проекта Mono (C#):
Предупреждение PVS-Studio: 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
Как вообще эти программы работают?
Просматривая все эти ошибки, кажется удивительным, что все эти программы вообще работают. Ведь функции сравнения выполняют крайне важную и ответственную задачу в программах.
Есть несколько объяснений работоспособности программ с такими ошибками:
- Во многих функциях неправильно сравнивается только часть объекта. При этом частичного сравнения хватает для большинства задач в этой программе.
- Не возникает (пока) ситуаций, когда функция будет работать неправильно. Например, это относится к функциям, которые не защищены от нулевых указателей или те, где результат вызова функции memcmp помещается в переменную типа char. Программе просто везёт.
- Рассмотренная функция сравнения используется крайне редко или вообще не используется.
- А кто сказал, что программа работает? Многие программы действительно делают что-то неправильно!
Рекомендации
Я продемонстрировал, как много ошибок можно встретить в функциях сравнения. А отсюда следует, что работоспособность таких функций обязательно должна проверяться с помощью юнит-тестов.
Непременно пишите юнит-тесты для операторов сравнения, для функций Equals и так далее.
Уверен, что многим до прочтения этой статьи казалось, что такие тесты избыточны и всё равно никаких ошибок они не выявят: ведь функции сравнения на первый взгляд так просты… Что же, теперь я показал весь ужас, который может в них скрываться.
Обзоры кода и использования инструментов статического анализа тоже будут не лишними.
Заключение
В этой статье упоминалось множество именитых проектов, которые разрабатываются высококлассными специалистами. Эти проекты тщательно тестируются с помощью различных методологий. И все равно, это не помешало анализатору PVS-Studio найти в них ошибки. Это говорит о том, что PVS-Studio может стать отличным дополнением к другим методологиям, используемых для повышения качества и надёжности кода.
Приходите к нам на сайт и попробуйте PVS-Studio.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. The Evil within the Comparison Functions