# Ноль, один, два, Фредди заберёт тебя Перед вами продолжение серии статей, которую можно озаглавить "ужасы для программистов"\. В этот раз речь пойдёт о типовом паттерне опечаток, связанном с использованием чисел 0, 1, 2\. Неважно, пишете вы на C, C\+\+, C\# или Java\. Если вы используете константы 0, 1, 2, или если эти числа содержатся в именах переменных, то, скорее всего, Фредди заглянет к вам ночью в гости\. Читайте и не говорите потом, что вас не предупреждали\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image1.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image1.png) ## Введение Я продолжаю серию статей, посвященных замеченным закономерностям в том, как люди допускают ошибки\. Предыдущие публикации: 1. [Эффект последней строки](https://pvs-studio.ru/ru/blog/posts/cpp/0260/) 1. [Самая опасная функция в мире C/C\+\+](https://pvs-studio.ru/ru/blog/posts/cpp/0360/) 1. [Зло живёт в функциях сравнения](https://pvs-studio.ru/ru/blog/posts/cpp/0509/) В этот раз закономерность заметил не я, а мой коллега Святослав Размыслов\. Он обратил внимание, что постоянно в своих [статьях](https://pvs-studio.ru/ru/blog/posts/?author=sviatoslav-razmyslov&page=1) описывает проблемы, в которых фигурируют переменные, содержащие в своём имени числа 1 и 2\. Святослав предложил мне подробнее исследовать эту тему и она действительно оказалась весьма плодотворной\. Выяснилось, что в нашей [коллекции ошибок](https://pvs-studio.ru/ru/blog/examples/) содержится большое число фрагментов кода, неправильных из\-за того, что люди путаются в индексах 0, 1, 2 или в именах переменных, содержащих такие числа\. Выявлена новая интересная закономерность, о которой и пойдёт речь ниже\. Я благодарен Святославу за подсказку исследовать эту тему и посвящаю эту статью ему\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image2.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image2.png) _Святослав Размыслов, менеджер, внимательный [ловец](https://habr.com/ru/company/pvs-studio/blog/483742/) [багов](https://habr.com/ru/company/pvs-studio/blog/486706/) и просто талантливый человек\._ Какова цель этой статьи? Показать, как легко все мы ошибаемся и допускаем опечатки\. Если программисты будут предупреждены \- они будут внимательнее в процессе обзоров кода, сосредотачивая своё внимание на злополучных 0, 1, 2\. Также программисты лучше смогут ощутить ценность статических анализаторов кода, которые помогают заметить такие ошибки\. Дело не в рекламе PVS\-Studio \(хотя в ней тоже :\)\. До сих пор многие программисты считают статический анализ лишним, предпочитая концентрироваться на собственной аккуратности и обзорах кода\. К сожалению, стараться писать код без ошибок \- это хорошо, но недостаточно\. Эта статья в очередной раз это ярко продемонстрирует\. Никто не застрахован от таких ошибок\. Ниже вы увидите эпичные ляпы в даже таких известных проектах как Qt, Clang, Hive, LibreOffice, Linux Kernel, \.NET Compiler Platform, XNU kernel, Mozilla Firefox\. Причем это не какие\-то экзотические редкие ошибки, а самые что ни на есть частые\. Не убедил? Тогда приступим\! _"Болтовня ничего не стоит\! Покажите мне баги\!"_ _\(c\) переделанная цитата Linus Torvalds\._ ## Опечатки в константах при индексации массивов Как правило, в наших статьях мы приводим предупреждения, с помощью которых найдены ошибки\. В этот раз я опущу эти предупреждения, так как и без них ошибка будет легко заметна и понятна\. Впрочем, хотя эти ошибки сразу бросаются в глаза в коротком фрагменте кода, они отлично умеют прятаться в коде проектов\. Начнём с ошибок, когда возникает путаница с числовыми литералами, используемыми для индексации массивов\. Несмотря на банальность этих ошибок, их много и обнаруживаются они отнюдь не в лабораторных работах студентов\. **Проект XNU kernel, язык C** ```cpp uint32_t gss_krb5_3des_unwrap_mbuf(....) { .... for (cflag = 1; cflag >= 0; cflag--) { *minor = gss_krb5_3des_token_get( ctx, &itoken, wrap, &hash, &offset, &length, reverse); if (*minor == 0) break; wrap.Seal_Alg[0] = 0xff; wrap.Seal_Alg[0] = 0xff; } .... } ``` Строчку скопировали, но поправить индекс забыли\. Как я понимаю, здесь должно быть написано: ```cpp wrap.Seal_Alg[0] = 0xff; wrap.Seal_Alg[1] = 0xff; ``` **Проект LibreOffice, язык C\+\+** ```cpp Sequence< OUString > FirebirdDriver:: getSupportedServiceNames_Static() throw (RuntimeException) { Sequence< OUString > aSNS( 2 ); aSNS[0] = "com.sun.star.sdbc.Driver"; aSNS[0] = "com.sun.star.sdbcx.Driver"; return aSNS; } ``` Как и в предыдущем случае, строчку скопировали, но забыли исправить 0 на 1\. Исправили только строковый литерал\. Можно задаться философским вопросом, как можно допустить такую ошибку в функции из четырех строк? Можно и всё\. Вот такое оно, программирование\. **Проект Quake\-III\-Arena, язык C** ```cpp int VL_FindAdjacentSurface(....) { .... if (fabs(dir[0]) > test->radius || fabs(dir[1]) > test->radius || fabs(dir[1]) > test->radius) { .... } ``` В скопированной строчке забыли заменить _dir\[1\]_ на _dir\[2\]_\. Как результат \- значение по оси Z не контролируется\. **Проект OpenCOLLADA, язык C\+\+** ```cpp struct short2 { short values[2]; short2(short s1, short s2) { values[0] = s1; values[2] = s2; } .... }; ``` Да, даже в таком коротком конструкторе можно умудриться выйти за границу массива при его инициализации\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image3.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image3.png) **Проект Godot Engine, язык C\+\+** ```cpp Array PhysicsDirectSpaceState::_cast_motion(....) { .... Array ret(true); ret.resize(2); ret[0]=closest_safe; ret[0]=closest_unsafe; return ret; } ``` Комментария не требуется\. **Проект Asterisk, язык C** ```cpp static void sip_threadinfo_destructor(void *obj) { struct sip_threadinfo *th = obj; struct tcptls_packet *packet; if (th->alert_pipe[1] > -1) { // <= close(th->alert_pipe[0]); } if (th->alert_pipe[1] > -1) { close(th->alert_pipe[1]); } th->alert_pipe[0] = th->alert_pipe[1] = -1; .... } ``` При написании однотипных блоков, ошибка, как правило, располагается в нижележащем\. До этого все рассмотренные случаи были как раз такими\. Здесь же опечатка находится в необычном месте, а именно, в первом блоке\. Почему так получилось, сказать сложно\. Мне не остаётся ничего иного, как просто привести картинку с единорогом, пожимающим плечами: ![0713_Zero_one_two_Freddy_coming_for_you_ru/image4.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image4.png) **Проект Open CASCADE Technology, язык C\+\+** ```cpp inline void Prepend(const Standard_Integer theIndex) { if (myIndex[1] >= 0) Standard_OutOfRange::Raise ("BRepMesh_PairOfIndex...."); myIndex[1] = myIndex[0]; myIndex[1] = theIndex; } ``` Два раза в одну и туже ячейку массива копируются разные значения\. Явная ошибка, но как её исправить мне было непонятно, так как код проекта мне не знаком\. Поэтому я просто посмотрел, как разработчики поправили код после того, как наша команда указала им на эту ошибку\. Правильный вариант: ```cpp myIndex[1] = myIndex[0]; myIndex[0] = theIndex; ``` **Проект Trans\-Proteomic Pipeline, язык C\+\+** ```cpp void ASAPRatio_getProDataStrct(proDataStrct *data, char **pepBofFiles) { .... if (data->indx == -1) { data->ratio[0] = -2.; data->ratio[0] = 0.; // <= data->inv_ratio[0] = -2.; data->inv_ratio[1] = 0.; return; } .... } ``` Я переживаю, что пакеты для научных исследований содержат такие ошибки\. Trans\-Proteomic Pipeline предназначен для решения задач в сфере биологии\. Можно такого нарешать и "наисследовать"\. В этом пакете вообще находилось много интересного: [проверка 2012 года](https://pvs-studio.ru/ru/blog/posts/cpp/0156/), [проверка 2013 года](https://pvs-studio.ru/ru/blog/posts/cpp/0212/)\. Пожалуй, можно вновь попробовать взглянуть на этот проект\. **Проект ITK, язык C\+\+** Перед нами другой проект для проведения исследований в области медицины: Medicine Insight Segmentation and Registration Toolkit \(ITK\)\. Проект другой, а ошибки те же самые\. ```cpp template< typename TCoordRepType > void VoronoiDiagram2D< TCoordRepType >::SetOrigin(PointType vorsize) { m_VoronoiBoundaryOrigin[0] = vorsize[0]; m_VoronoiBoundaryOrigin[0] = vorsize[1]; } ``` **Проект ITK, язык C\+\+** ```cpp int itkPointSetToSpatialObjectDemonsRegistrationTest(....) { .... // Set its position EllipseType::TransformType::OffsetType offset; offset[0]=50; offset[1]=50; offset[1]=50; .... } ``` Чистый Copy\-Paste\. **Проект ReactOS, язык C\+\+** ```cpp HPALETTE CardWindow::CreateCardPalette() { .... //include button text colours cols[0] = RGB(0, 0, 0); cols[1] = RGB(255, 255, 255); //include the base background colour cols[1] = crBackgnd; //include the standard button colours... cols[3] = CardButton::GetHighlight(crBackgnd); cols[4] = CardButton::GetShadow(crBackgnd); cols[5] = CardButton::GetFace(crBackgnd); .... } ``` По всей видимости, константа _crBackgnd_ должна записываться в ячейку _cols\[2\]_\. **Проект Coin3D, язык C\+\+** ```cpp SoVRMLInline::GLRender(SoGLRenderAction * action) { .... if ((size[0] >= 0.0f && size[1] >= 0.0f && size[1] >= 0.0f) && ((vis == ALWAYS) || (vis == UNTIL_LOADED && child == NULL))) { .... } ``` Дважды проверятся элемент массива _size\[1\]_, а элемент _size\[2\]_ не проверятся\. Вот так и появляются странные артефакты на изображениях\. **Проект OpenCV, язык C\+\+** ```cpp bool Jpeg2KDecoder::readHeader() { .... cmptlut[0] = .... cmptlut[1] = .... cmptlut[2] = .... if( cmptlut[0] < 0 || cmptlut[1] < 0 || cmptlut[0] < 0 ) result = false; .... } ``` Прям чувствуется, как выражение _cmptlut\[0\] < 0_ было дважды размножено копированием, но поправили ноль только в одном месте\. **Проект Visualization Toolkit \(VTK\), язык C\+\+** ```cpp void vtkImageStencilRaster::PrepareForNewData(....) { .... if (allocateExtent && allocateExtent[1] >= allocateExtent[1]) .... } ``` Здесь и далее я не буду комментировать многие подобные ошибки\. Что тут комментировать? Главное, разглядывая подобные фрагменты кода, прочувствовать, что хотя ошибка простая, это не значит, что она будет замечена программистом\. **Проект Visualization Toolkit \(VTK\), язык C\+\+** ```cpp template void vtkDataSetAttributesCopyValues(....) { .... inZPtr += (outExt[0] - outExt[0])*inIncs[0] * data_type_size + (outExt[2] - outExt[2])*inIncs[1] * data_type_size + (outExt[4] - outExt[4])*inIncs[2] * data_type_size; .... } ``` Здесь программист явно торопился побыстрее написать код\. По\-другому сложно объяснить, как он ошибся трижды\. Элементы массива вычитаются сами из себя\. В результате получается, что этот код эквивалентен: ```cpp inZPtr += (0)*inIncs[0] * data_type_size + (0)*inIncs[1] * data_type_size + (0)*inIncs[2] * data_type_size; ``` Впрочем, этот код можно сократить ещё больше: ```cpp inZPtr += 0; ``` Великолепно\. В коде есть длинное серьёзно выглядящее выражение, которое, на самом деле, ничего не делает\. Люблю такие случаи\. **Проект Visualization Toolkit \(VTK\), язык C\+\+** Аналогичный случай спешного написания кода\. ```cpp void vtkPiecewiseControlPointsItem::SetControlPoint( vtkIdType index, double* newPos) { double oldPos[4]; this->PiecewiseFunction->GetNodeValue(index, oldPos); if (newPos[0] != oldPos[0] || newPos[1] != oldPos[1] || newPos[2] != oldPos[2] || newPos[2] != oldPos[2]) { this->PiecewiseFunction->SetNodeValue(index, newPos); } } ``` Дважды повторяется сравнение _newPos\[2\] \!\= oldPos\[2\]_\. **Проект ADAPTIVE Communication Environment \(ACE\), язык C\+\+** ```cpp bool URL_Base::strip_scheme (ACE_CString& url_string) { .... ACE_CString::size_type pos = url_string.find (':'); if (pos > 0 && url_string[pos+1] == '/' && url_string[pos+1] == '/') { .... // skip '://' url_string = url_string.substr (pos+3); } .... } ``` В условии должно проверяться, что встречены две косые черты, стоящие после двоеточия\. Другими словами, ищется подстрока "://"\. Из\-за опечатки, проверка "подслеповата" и готова второй косой чертой посчитать любой символ\. **Проект IPP Samples, язык C\+\+** ```cpp void MeBase::MakeVlcTableDecision() { .... Ipp32s BestMV = IPP_MIN(IPP_MIN(m_cur.MvRate[0],m_cur.MvRate[1]), IPP_MIN(m_cur.MvRate[2],m_cur.MvRate[3])); Ipp32s BestAC = IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]), IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2])); .... } ``` Опечатка кроется вот здесь, в передаваемых в макрос аргументах: ```cpp IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2]) ``` Получается, что выбирается минимум из двух одинаковых значений\. На самом деле, должно быть написано: ```cpp IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3]) ``` Кстати, этот код может продемонстрировать пользу стандартной библиотеки\. Если написать так: ```cpp Ipp32s BestMV = std::min_element(begin(m_cur.MvRate), end(m_cur.MvRate)); Ipp32s BestAC = std::min_element(begin(m_cur.AcRate), end(m_cur.AcRate)); ``` То код станет короче, и меньше подвержен ошибкам\. Собственно, чем меньше однотипного кода, тем больше шансов, что он будет написан правильно\. **Проект Audacity, язык C\+\+** ```cpp sampleCount VoiceKey::OnBackward (....) { .... int atrend = sgn(buffer[samplesleft - 2]- buffer[samplesleft - 1]); int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]- buffer[samplesleft - WindowSizeInt-2]); .... } ``` Правильное выражение: ```cpp int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]- buffer[samplesleft - WindowSizeInt-1]); ``` **Проект PDFium, язык C\+\+** ```cpp void sycc420_to_rgb(opj_image_t* img) { .... opj_image_data_free(img->comps[0].data); opj_image_data_free(img->comps[1].data); opj_image_data_free(img->comps[2].data); img->comps[0].data = d0; img->comps[1].data = d1; img->comps[2].data = d2; img->comps[1].w = yw; // 1 img->comps[1].h = yh; // 1 img->comps[2].w = yw; // 1 img->comps[2].h = yh; // 1 img->comps[1].w = yw; // 2 img->comps[1].h = yh; // 2 img->comps[2].w = yw; // 2 img->comps[2].h = yh; // 2 img->comps[1].dx = img->comps[0].dx; img->comps[2].dx = img->comps[0].dx; img->comps[1].dy = img->comps[0].dy; img->comps[2].dy = img->comps[0].dy; } ``` Дублируется ряд действий по инициализации структуры\. Те строчки, что помечены комментарием _//2_, можно удалить, и ничего не изменится\. Я сомневался, включать ли этот фрагмент кода в статью\. Здесь не совсем ошибка, и не совсем с индексами\. Однако этот лишний код, скорее всего, появился именно из\-за того, что программист запутался во всех этих членах класса и индексах 1, 2\. Так что, думаю, этот фрагмент кода подходит для демонстрации, как легко путаться в числах\. **Проект CMake, С** Рассматриваемый далее код написан не разработчиками CMake, а позаимствован\. Судя по комментарию в начале файла, функция _utf8\_encode_ была написана Tim Kientzle ещё в 2007\. С тех пор эта функция кочует из проекта в проект и много где встречается\. Я не стал изучать вопрос первоисточника, так как это сейчас не принципиально\. Раз этот код есть в проекте CMake, то и ошибка относится к CMake\. ```cpp static char * utf8_encode(const wchar_t *wval) { .... p[0] = 0xfc | ((wc >> 30) & 0x01); p[1] = 0x80 | ((wc >> 24) & 0x3f); p[1] = 0x80 | ((wc >> 18) & 0x3f); p[2] = 0x80 | ((wc >> 12) & 0x3f); p[3] = 0x80 | ((wc >> 6) & 0x3f); p[4] = 0x80 | (wc & 0x3f); p += 6; .... } ``` Как видите, есть какая\-то путаница с индексами\. Дважды происходит запись в элемент массива _p\[1\]_\. Если изучить код по соседству, то становится понятно, что правильный код должен быть таким: ```cpp p[0] = 0xfc | ((wc >> 30) & 0x01); p[1] = 0x80 | ((wc >> 24) & 0x3f); p[2] = 0x80 | ((wc >> 18) & 0x3f); p[3] = 0x80 | ((wc >> 12) & 0x3f); p[4] = 0x80 | ((wc >> 6) & 0x3f); p[5] = 0x80 | (wc & 0x3f); p += 6; ``` **Примечание** Обратите внимание, что все ошибки, рассмотренные в этой главе, относятся к коду на языке C или C\+\+\. Нет кода на C\# или Java\! Это очень интересно, я не ожидал подобного\. На мой взгляд, рассмотренные опечатки никак не зависят от языка\. И в следующих главах действительно появятся ошибки в коде на других языках\. Думаю, это просто случайное совпадение\. Анализатор PVS\-Studio начал гораздо позже поддерживать языки C\#/Java по сравнению с C/C\+\+, и мы просто не успели накопить в базе соответствующее примеры ошибок\. Тем не менее, наблюдение всё равно интересное\. Видимо, C и C\+\+ программисты больше любят использовать числа 0, 1 и 2 при работе с массивами :\)\. ## Опечатки в именах Это будет самый большой раздел\. Людям очень легко запутаться в таких именах, как _a1_ и _a2_\. Кажется, как тут можно запутаться? Можно\. Легко\. И сейчас читатель сможет в этом убедиться\. **Проект Hive, язык Java** ```cpp @Override public List getAllInstancesOrdered() { List list = new LinkedList<>(); list.addAll(instances.values()); Collections.sort(list, new Comparator() { @Override public int compare(ServiceInstance o1, ServiceInstance o2) { return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity()); } }); return list; } ``` Функция сравнения _compare_ принимает два объекта: _o1_ и _o2_\. Но из\-за опечатки далее используется только _o2_\. Что интересно, благодаря Copy\-Paste эта ошибка мигрировала в другую функцию: ```cpp @Override public List getAllInstancesOrdered() { List list = new LinkedList<>(); readLock.lock(); try { list.addAll(instances.values()); } finally { readLock.unlock(); } Collections.sort(list, new Comparator() { @Override public int compare(ServiceInstance o1, ServiceInstance o2) { return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity()); } }); return list; } ``` ![0713_Zero_one_two_Freddy_coming_for_you_ru/image5.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image5.png) **Проект Infer\.NET, язык C\#** ```cpp private void MergeParallelTransitions() { .... if (double.IsInfinity(transition1.Weight.Value) && double.IsInfinity(transition1.Weight.Value)) .... } ``` **Проект Doom 3, язык C\+\+** ```cpp uint AltOp::fixedLength() { uint l1 = exp1->fixedLength(); uint l2 = exp1->fixedLength(); if (l1 != l2 || l1 == ~0u) return ~0; return l1; } ``` Если кто\-то сразу не заметил опечатку, то нужно посмотреть на строку, где инициализируется переменная _l2_\. Следовало использовать _exp2_\. **Проект Source Engine SDK, язык C\+\+** ```cpp void GetFPSColor( int nFps, unsigned char ucColor[3] ) { .... int nFPSThreshold1 = 20; int nFPSThreshold2 = 15; if (IsPC() && g_pMaterialSystemHardwareConfig->GetDXSupportLevel() >= 95) { nFPSThreshold1 = 60; nFPSThreshold1 = 50; } .... } ``` Правильно: ```cpp nFPSThreshold1 = 60; nFPSThreshold2 = 50; ``` **Проект Linux Kernel, язык C** Кстати, опечатки могут быть не только в именах переменных, но и в именах макросов\. Сейчас будет несколько таких примеров\. ```cpp int private_ioctl(struct vnt_private *pDevice, struct ifreq *rq) { .... if (sStartAPCmd.byBasicRate & BIT3) { pMgmt->abyIBSSSuppRates[2] |= BIT7; pMgmt->abyIBSSSuppRates[3] |= BIT7; pMgmt->abyIBSSSuppRates[4] |= BIT7; pMgmt->abyIBSSSuppRates[5] |= BIT7; } else if (sStartAPCmd.byBasicRate & BIT2) { pMgmt->abyIBSSSuppRates[2] |= BIT7; pMgmt->abyIBSSSuppRates[3] |= BIT7; pMgmt->abyIBSSSuppRates[4] |= BIT7; } else if (sStartAPCmd.byBasicRate & BIT1) { // <= pMgmt->abyIBSSSuppRates[2] |= BIT7; pMgmt->abyIBSSSuppRates[3] |= BIT7; } else if (sStartAPCmd.byBasicRate & BIT1) { // <= pMgmt->abyIBSSSuppRates[2] |= BIT7; } else { /* default 1,2M */ pMgmt->abyIBSSSuppRates[2] |= BIT7; pMgmt->abyIBSSSuppRates[3] |= BIT7; } .... } ``` Как видите, дважды используется маска с именем _BIT1_, что делает вторую проверку бессмысленной\. Тело второго, помеченного комментарием, условного оператора никогда не будет выполнено\. **Проект CMaNGOS, язык C\+\+** ```cpp void AttackedBy(Unit* pAttacker) override { .... DoScriptText(urand(0, 1) ? SAY_BELNISTRASZ_AGGRO_1 : SAY_BELNISTRASZ_AGGRO_1, m_creature, pAttacker); .... } ``` В игре планировалось случайное поведение, но всегда выбирается одна и та же константа _SAY\_BELNISTRASZ\_AGGRO\_1_\. **Проект Vangers: One For The Road, язык C\+\+** ```cpp const char* iGetJoyBtnNameText(int vkey,int lang) { .... if (vkey >= VK_STICK_SWITCH_1 && vkey <= VK_STICK_SWITCH_9) { ret = (lang) ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1] : iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1]; return ret; } .... } ``` Судя по написанному рядом коду, правильный вариант должен быть таким: ```cpp ret = (lang) ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1] : iJoystickStickSwitch1[vkey - VK_STICK_SWITCH_1]; ``` **Проект RT\-Thread, язык C** ```cpp uint8_t can_receive_message_length(uint32_t can_periph, uint8_t fifo_number) { uint8_t val = 0U; if(CAN_FIFO0 == fifo_number){ val = (uint8_t)(CAN_RFIFO0(can_periph) & CAN_RFIFO_RFL0_MASK); }else if(CAN_FIFO0 == fifo_number){ val = (uint8_t)(CAN_RFIFO1(can_periph) & CAN_RFIFO_RFL0_MASK); }else{ /* illegal parameter */ } return val; } ``` RT\-Thread \- это операционная система с открытым исходным кодом в реальном времени для встроенных устройств\. Здесь мы видим путаницу между FIFO 0 и FIFO 1\. И где\-то кто\-то столкнётся с глючным устройством\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image6.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image6.png) Ошибка здесь: ```cpp if (CAN_FIFO0 == fifo_number){ .... }else if(CAN_FIFO0 == fifo_number){ Вторая проверка всегда даёт false. Правильно: if (CAN_FIFO0 == fifo_number){ .... }else if(CAN_FIFO1 == fifo_number){ ``` **Проект Hive, язык Java** ```cpp private void generateDateTimeArithmeticIntervalYearMonth(String[] tdesc) throws Exception { String operatorName = tdesc[1]; String operatorSymbol = tdesc[2]; String operandType1 = tdesc[3]; String colOrScalar1 = tdesc[4]; String operandType2 = tdesc[5]; String colOrScalar2 = tdesc[6]; .... if (colOrScalar1.equals("Col") && colOrScalar1.equals("Column")) { .... } else if (colOrScalar1.equals("Col") && colOrScalar1.equals("Scalar")) { .... } ``` Анализатор PVS\-Studio указывает сразу на 2 ошибки: 1. Строка, хранящаяся в _colOrScalar1,_ не может одновременно равняться строкам "Col" и "Column"; 1. Строка, хранящаяся в _colOrScalar1,_ не может одновременно равняться строкам "Col" и "Scalar"\. Явно присутствует путаница в именах переменных\. **Проект Shareaza, язык C\+\+** ```cpp void CDownloadWithSources::MergeMetadata(const CXMLElement* pXML) { CQuickLock pLock( Transfers.m_pSection ); CXMLAttribute* pAttr1 = m_pXML->GetAttribute(CXMLAttribute::schemaName); CXMLAttribute* pAttr2 = pXML->GetAttribute(CXMLAttribute::schemaName); if (pAttr1 && pAttr2 && !pAttr1->GetValue().CompareNoCase(pAttr1->GetValue())) .... } ``` Правильно: ```cpp pAttr1->GetValue().CompareNoCase(pAttr2->GetValue()) ``` **Примечание** Давайте сделаем небольшую паузу\. Есть опасение, что просматривая гору банальных ошибок, мы забудем, зачем мы это делаем\. Задача не в том, чтобы посмеяться над чужим кодом\. Всё это не повод тыкать пальцем и приговаривать: "Ха\-ха, ну надо же"\. Это повод задуматься\! [Публикации](https://pvs-studio.ru/ru/blog/inspections/) нашей команды призваны показать, что никто из нас не застрахован от ошибок\. Ошибки, описываемые в статье, появляются в коде намного чаще, чем можно ожидать\. Ещё важно, что вероятность запутаться в 0, 1, 2 почти не зависит от квалификации программиста\. Полезно осознать, что людям свойственно ошибаться\. Без этого нельзя сделать следующий шаг в повышении качества и надёжности кода\. Понимая, что все мы ошибаемся, люди начинают стараться выявить ошибки на самых ранних этапах, используя стандарты кодирования, обзоры кода, юнит\-тесты, статические и динамические анализаторы\. Это очень хорошо\. Зачем написаны и так понятные вещи? К сожалению, общаясь с большим количеством разработчиков, мы вынуждены констатировать, что не всегда это так уж всем понятно\. У многих слишком завышена самооценка и они просто не допускают мысли, что способны совершать простые ошибки\. Это грустно\. Если вы тимлид/менеджер, то приглашаю заодно ознакомиться вот с этой [заметкой](https://pvs-studio.ru/ru/blog/posts/0498/)\. **Проект Qt, язык C\+\+** ```cpp AtomicComparator::ComparisonResult IntegerComparator::compare(const Item &o1, const AtomicComparator::Operator, const Item &o2) const { const Numeric *const num1 = o1.as(); const Numeric *const num2 = o1.as(); if(num1->isSigned() || num2->isSigned()) .... } ``` Правильно: ```cpp const Numeric *const num2 = o2.as(); ``` **Проект Android, язык C\+\+** ```cpp static inline bool isAudioPlaybackRateEqual( const AudioPlaybackRate &pr1, const AudioPlaybackRate &pr2) { return fabs(pr1.mSpeed - pr2.mSpeed) < AUDIO_TIMESTRETCH_SPEED_MIN_DELTA && fabs(pr1.mPitch - pr2.mPitch) < AUDIO_TIMESTRETCH_PITCH_MIN_DELTA && pr2.mStretchMode == pr2.mStretchMode && pr2.mFallbackMode == pr2.mFallbackMode; } ``` Сразу две опечатки, из\-за которых переменные _pr2\.mStretchMode_ и _pr2\.mFallbackMode_ сравниваются сами с собой\. **Проект Boost, язык C\+\+** ```cpp point3D operator/(const point3D &p1, const point3D &p2) { return point3D(p1.x/p2.x, p1.y/p2.y, p1.z/p1.z); } ``` В самом конце опечатались и поделили переменную _p1\.z_ саму на себя\. **Проект Clang, язык C\+\+** ```cpp bool haveSameType(QualType Ty1, QualType Ty2) { return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) || (Ty2->isIntegerType() && Ty2->isIntegerType())); } ``` Да, да, подобные ошибки анализатор PVS\-Studio находит и в компиляторах\. Правильно: ```cpp (Ty1->isIntegerType() && Ty2->isIntegerType()) ``` **Проект Clang, язык C\+\+** ```cpp Instruction *InstCombiner::visitXor(BinaryOperator &I) { .... if (Op0I && Op1I && Op0I->isShift() && Op0I->getOpcode() == Op1I->getOpcode() && Op0I->getOperand(1) == Op1I->getOperand(1) && (Op1I->hasOneUse() || Op1I->hasOneUse())) { .... } ``` Правильно: ```cpp (Op0I->hasOneUse() || Op1I->hasOneUse()) ``` **Проект Qt, язык C\+\+** ```cpp inline bool qCompare(QImage const &t1, QImage const &t2, ....) { .... if (t1.width() != t2.width() || t2.height() != t2.height()) { .... } ``` **Проект NCBI Genome Workbench, язык C\+\+** ```cpp static bool s_PCRPrimerSetLess(const CPCRPrimerSet& s1, const CPCRPrimerSet& s2) { if (!s1.IsSet() && s1.IsSet()) { return true; } else if (s1.IsSet() && !s2.IsSet()) { return false; } else if (!s1.IsSet() && !s2.IsSet()) { return false; } else if (s1.Get().size() < s2.Get().size()) { return true; } else if (s1.Get().size() > s2.Get().size()) { return false; } else { ..... } ``` Ошибка в самой первой проверке\. Должно быть написано: ```cpp if (!s1.IsSet() && s2.IsSet()) { ``` **Проект NCBI Genome Workbench, язык C\+\+** ```cpp CRef CNWAligner::Run(CScope &scope, const CSeq_loc &loc1, const CSeq_loc &loc2, bool trim_end_gaps) { if ((!loc1.IsInt() && !loc1.IsWhole()) || (!loc1.IsInt() && !loc1.IsWhole())) { NCBI_THROW(CException, eUnknown, "Only whole and interval locations supported"); } .... } ``` Была размножена первая строчка условия, но затем программист поспешил и забыл заменить _loc1_ на _loc2_\. **Проект FlashDevelop, С\#** ```cpp public void SetPrices(....) { UInt32 a0 = _choice.GetPrice0(); UInt32 a1 = _choice.GetPrice1(); UInt32 b0 = a1 + _choice2.GetPrice0(); // <= UInt32 b1 = a1 + _choice2.GetPrice1(); .... } ``` **Проект FreeCAD, язык C\+\+** ```cpp inline void insEdgeVec(std::map > &map, int n1, int n2) { if(n1 oNumOffset1 = static_cast(rItem1).GetNumOffset(); ::boost::optional oNumOffset2 = static_cast(rItem1).GetNumOffset(); .... } ``` И ещё одна классическая Copy\-Paste ошибка :\)\. В одном месте 1 на 2 поправили, а в другом забыли\. **Проект LibreOffice, язык C\+\+** ```cpp XMLTransformerOOoEventMap_Impl::XMLTransformerOOoEventMap_Impl( XMLTransformerEventMapEntry *pInit, XMLTransformerEventMapEntry *pInit2 ) { if( pInit ) AddMap( pInit ); if( pInit ) AddMap( pInit2 ); } ``` Здесь не ошибка в замене 1 на 2, а просто не добавили 2 во второе условие\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image7.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image7.png) Возможно, вы немного устали\. Поэтому предлагаю заварить чай или кофе, и мы продолжим знакомство с миром чисел 0, 1 и 2\. **Проект Geant4 software, язык C\+\+** ```cpp void G4VTwistSurface::GetBoundaryLimit(G4int areacode, G4double limit[]) const { .... if (areacode & sC0Min1Max) { limit[0] = fAxisMin[0]; limit[1] = fAxisMin[1]; } else if (areacode & sC0Max1Min) { limit[0] = fAxisMax[0]; limit[1] = fAxisMin[1]; } else if (areacode & sC0Max1Max) { limit[0] = fAxisMax[0]; limit[1] = fAxisMax[1]; } else if (areacode & sC0Min1Max) { limit[0] = fAxisMin[0]; limit[1] = fAxisMax[1]; } .... } ``` Надеюсь, вы воспользовались советом и отдохнули\. Готовы теперь найти в этом коде ошибку? Поздравляю читателей, которые заметили ошибку\. Вы молодцы\! Кто поленился искать, тех я тоже понимаю\. Обзор подобного кода очень утомителен и есть желание как\-то побыстрее пойти проверять что\-то более интересное\. Вот здесь статические анализаторы отлично помогают, так как не устают\. Ошибка в том, что эти две проверки совпадают: ```cpp if (areacode & sC0Min1Max) { } else if (areacode & sC0Min1Max) { ``` Если изучить код, то становится понятно, что ошибочной является самая первая проверка\. Правильно: ```cpp if (areacode & sC0Min1Min) { } else if (areacode & sC0Max1Min) { } else if (areacode & sC0Max1Max) { } else if (areacode & sC0Min1Max) { ``` **Проект 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); } ``` **Проект TortoiseGit, язык C\+\+** ```cpp void CGitStatusListCtrl::OnContextMenuList(....) { .... if( (!this->m_Rev1.IsEmpty()) || (!this->m_Rev1.IsEmpty()) ) .... } ``` **Проект Geant4 software, язык C\+\+** ```cpp G4double G4MesonAbsorption:: GetTimeToAbsorption(const G4KineticTrack& trk1, const G4KineticTrack& trk2) { .... if(( trk1.GetDefinition() == G4Neutron::Neutron() || trk1.GetDefinition() == G4Neutron::Neutron() ) && sqrtS>1.91*GeV && pi*distance>maxChargedCrossSection) return time; .... } ``` **Проект MonoDevelop, С\#** ```cpp private bool MembersMatch(ISymbol member1, ISymbol member2) { .... if (member1.DeclaredAccessibility != member1.DeclaredAccessibility || member1.IsStatic != member1.IsStatic) { return false; } .... } ``` Как видите, пока фрагменты кода идут без пояснений\. Собственно, тут и пояснять нечего\. Можно только повздыхать\. **Проект Dolphin Emulator, язык C\+\+** ```cpp bool IRBuilder::maskedValueIsZero(InstLoc Op1, InstLoc Op2) const { return (~ComputeKnownZeroBits(Op1) & ~ComputeKnownZeroBits(Op1)) == 0; } ``` **Проект RunAsAdmin Explorer Shim, язык C\+\+** ```cpp bool IsLuidsEqual(LUID luid1, LUID luid2) { return (luid1.LowPart == luid2.LowPart) && (luid2.HighPart == luid2.HighPart); } ``` **Проект IT\+\+, язык C\+\+** ```cpp Gold::Gold(const ivec &mseq1_connections, const ivec &mseq2_connections) { .... it_assert(mseq1.get_length() == mseq1.get_length(), "Gold::Gold(): dimension mismatch"); } ``` **Проект QuantLib, язык C\+\+** ```cpp Distribution ManipulateDistribution::convolve( const Distribution& d1, const Distribution& d2) { .... QL_REQUIRE (d1.xmin_ == 0.0 && d1.xmin_ == 0.0, "distributions offset larger than 0"); .... } ``` **Проект Samba, язык C\+\+** ```cpp static bool samu_correct(struct samu *s1, struct samu *s2) { .... } else if (s1_len != s1_len) { DEBUG(0, ("Password history not written correctly, " "lengths differ, want %d, got %d\n", s1_len, s2_len)); .... } ``` **Проект Mozilla Firefox, язык C\+\+** ```cpp static PRBool IsZPositionLEQ(nsDisplayItem* aItem1, nsDisplayItem* aItem2, void* aClosure) { if (!aItem1->GetUnderlyingFrame()->Preserves3D() || !aItem1->GetUnderlyingFrame()->Preserves3D()) { return IsContentLEQ(aItem1, aItem2, aClosure); } .... } ``` **Проект Haiku Operation System, язык C\+\+** ```cpp void trans_double_path::reset() { m_src_vertices1.remove_all(); m_src_vertices2.remove_all(); m_kindex1 = 0.0; // <= m_kindex1 = 0.0; // <= m_status1 = initial; m_status2 = initial; } ``` **Проект Qt, язык C\+\+** Ok, теперь давайте чуть\-чуть посложнее\. Попробуйте ради интереса найти ошибку здесь: ```cpp static ShiftResult shift(....) { .... qreal l = (orig->x1 - orig->x2)*(orig->x1 - orig->x2) + (orig->y1 - orig->y2)*(orig->y1 - orig->y1) * (orig->x3 - orig->x4)*(orig->x3 - orig->x4) + (orig->y3 - orig->y4)*(orig->y3 - orig->y4); .... } ``` Картинка, чтобы не увидеть ответ сразу, и была возможность подумать\. ![0713_Zero_one_two_Freddy_coming_for_you_ru/image8.png](https://import.viva64.com/docx/blog/0713_Zero_one_two_Freddy_coming_for_you_ru/image8.png) Правильно, вместо _orig\-\>y1 \- orig\-\>y1_ должно быть написано _orig\-\>y1 \- orig\-\>y2_\. **Проект \.NET Compiler Platform, язык С\#** ```cpp public void IndexerMemberRace() { .... for (int i = 0; i < 20; i++) { .... if (i % 2 == 0) { thread1.Start(); thread2.Start(); } else { thread1.Start(); thread2.Start(); } .... } .... } ``` Интересный случай\. В целях тестирования требуется запускать потоки в различном порядке\. Однако из\-за опечатки потоки стартуют всегда одинаково, вследствие чего тест проверяет меньше чем должен\. Правильно: ```cpp if (i % 2 == 0) { thread1.Start(); thread2.Start(); } else { thread2.Start(); thread1.Start(); } ``` **Проект Samba, язык С** ```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; } ``` Функция сравнения никогда не вернёт 1, так как условие _i2\-\>pid \> i2\-\>pid_ не имеет смысла\. Естественно, это обыкновенная опечатка, и на самом деле должно быть написано: ```cpp if (i1->pid > i2->pid) return 1; ``` **Проект ChakraCore, язык C\+\+** Последний случай в этой главе\. Ура\! ```cpp bool Lowerer::GenerateFastBrSrEq(...., IR::RegOpnd * srcReg1, IR::RegOpnd * srcReg2, ....) { .... else if (srcReg1 && (srcReg1->m_sym->m_isStrConst)) .... else if (srcReg1 && (srcReg1->m_sym->m_isStrConst)) .... } ``` ## Прочие ошибки Теперь поговорим о менее многочисленных паттернах ошибок, связанных с использованием чисел 0, 1, 2\. ### Опечатки в условиях, где явно используется константа 0/1/2 **Проект ROOT, язык C\+\+** ```cpp Int_t TProofMonSenderML::SendSummary(TList *recs, const char *id) { .... if (fSummaryVrs == 0) { if ((dsn = recs->FindObject("dataset"))) recs->Remove(dsn); } else if (fSummaryVrs == 0) { .... } ``` Странно дважды сравнивать переменную _fSummaryVrs_ с 0\. **Проект \.NET CoreCLR, язык С\#** ```cpp void PutIA64Imm22(UINT64 * pBundle, UINT32 slot, INT32 imm22) { if (slot == 0) // <= { .... } else if (slot == 1) { .... } else if (slot == 0) // <= { .... } .... } ``` **Проект FFmpeg, язык C** ```cpp static int imc_decode_block(....) { .... if (stream_format_code & 0x1) imc_decode_level_coefficients_raw(....); else if (stream_format_code & 0x1) imc_read_level_coeffs_raw(....); .... } ``` ### Индекс / имя Ранее мы рассматривали случаи, когда неверен индекс или имя\. А вот ситуация, когда сразу и не скажешь, как классифицировать ошибку\. Этот пример можно было отнести как к одной, так и к другой главе\. Поэтому я решил привести его отдельно\. **Проект Mesa 3D Graphics Library, язык C\+\+** ```cpp bool ir_algebraic_visitor::reassociate_constant(....) { .... if (ir1->operands[0]->type->is_matrix() || ir1->operands[0]->type->is_matrix() || ir2->operands[1]->type->is_matrix() || ir2->operands[1]->type->is_matrix()) return false; .... } ``` Этот код можно поправить так: ```cpp if (ir1->operands[0]->type->is_matrix() || ir1->operands[1]->type->is_matrix() || ir2->operands[0]->type->is_matrix() || ir2->operands[1]->type->is_matrix()) ``` И можно поправить так: ```cpp if (ir1->operands[0]->type->is_matrix() || ir2->operands[0]->type->is_matrix() || ir1->operands[1]->type->is_matrix() || ir2->operands[1]->type->is_matrix()) ``` ### Лишний 0 Иногда 0 оказывается лишним и вредным\. Из\-за него число может превратиться в восьмеричное там, где это не нужно\. Или портят форматную строку\. Названные ошибки не подходят для этой статьи, но я считаю, что про них стоит упомянуть\. Не буду приводить в статье код с этими ошибками, но если интересно, то на них можно посмотреть здесь: * [V536](https://pvs-studio.ru/ru/docs/warnings/v536/) Be advised that the utilized constant value is represented by an octal form, [примеры](https://pvs-studio.ru/ru/blog/examples/v536/); * [V638](https://pvs-studio.ru/ru/docs/warnings/v638/) A terminal null is present inside a string\. The '\\0xNN' characters were encountered\. Probably meant: '\\xNN', [примеры](https://pvs-studio.ru/ru/blog/examples/v638/)\. ### Забыли написать \+1 **Проект Haiku Operation System, язык C\+\+** ```cpp int UserlandFS::KernelEmu::new_path(const char *path, char **copy) { .... // append a dot, if desired if (appendDot) { copiedPath[len] = '.'; copiedPath[len] = '\0'; } .... } ``` Правильный вариант: ```cpp copiedPath[len] = '.'; copiedPath[len + 1] = '\0'; ``` Примечание\. Ситуация, когда забыли добавить единицу, вовсе не редка\. Я точно помню, что не раз встречал такие случаи\. Однако, когда я захотел набрать подобных примеров для статьи, я нашёл только этот пример\. Мне жаль, что я не могу напугать вас этими ошибками побольше\. Прошу прощения\. ### Ошибки форматирования \(C\#\) Чаще всего функции для построения строк оперируют с небольшим количеством аргументов\. Вот и получается, что чаще всего ошибки связаны с использованием \{0\}, \{1\} или \{2\}\. **Проект Azure PowerShell, язык C\#** ```cpp protected override void ProcessRecordInternal() { .... if (this.ShouldProcess(this.Name, string.Format("Creating Log Alert Rule '{0}' in resource group {0}", this.Name, this.ResourceGroupName))) { .... } .... } ``` Опечатались и дважды написали \{0\}\. В результате в строку будет дважды вставлено имя _this\.Name_\. А вот имя _this\.ResourceGroupName_ не попадёт в создаваемую строку\. **Проект Mono, язык C\#** ```cpp void ReadEntropy () { if (reader.IsEmptyElement) throw new XmlException ( String.Format ("WS-Trust Entropy element is empty.{2}", LineInfo ())); .... } ``` Это вообще что\-то странное\. Нужно вставить то, чего нет\. Скорее всего, этот код подвергся неудачному рефакторингу и оказался сломан\. **Проект Xenko, язык C\#** ```cpp public string ToString(string format, IFormatProvider formatProvider) { if (format == null) return ToString(formatProvider); return string.Format( formatProvider, "Red:{1} Green:{2} Blue:{3}", R.ToString(format, formatProvider), G.ToString(format, formatProvider), B.ToString(format, formatProvider)); } ``` Программист забыл, что нумерация начинается с \{0\}, а не с \{1\}\. Правильный код: ```cpp return string.Format( formatProvider, "Red:{0} Green:{1} Blue:{2}", R.ToString(format, formatProvider), G.ToString(format, formatProvider), B.ToString(format, formatProvider)); ``` **Проект \.NET Compiler Platform, язык C\#** ```cpp private void DumpAttributes(Symbol s) { .... Console.WriteLine("{0} {1} {2}", pa.ToString()); .... } ``` Аргументов явно недостаточно\. ## Выводы и рекомендации Мне пришлось продемонстрировать очень много примеров, чтобы показать, что опечатки, связанные с 0, 1 и 2, заслуживают отдельного внимания\. Если бы я просто сказал: "Легко перепутать o1 и o2", вы бы согласились, но не придали этому то значение, какое придадите сейчас, прочитав или хотя бы пролистав статью\. Теперь вы предупреждены, и это хорошо\. Предупреждён — значит вооружён\. Теперь вы будете более внимательны на обзорах кода и уделите дополнительное внимание переменным, в именах которых увидите 0, 1, 2\. Дать какие\-то рекомендации по оформлению кода, чтобы избежать подобных ошибок, сложно\. Как вы видели, ошибки встречаются даже в таком простом коде, где и оформлять\-то нечего\. Поэтому я не стану призывать избегать 0, 1, 2 и давать переменным длинные имена\. Если вместо чисел начать писать First/Second/Left/Right и так далее, то соблазн скопировать имя или выражение будет ещё больше\. Возможно, такая рекомендация в конечном итоге вовсе не сократит, а увеличит количество ошибок\. Тем не менее, когда вы пишете много однотипного кода, по\-прежнему актуальна рекомендация "табличного оформления кода"\. Табличное форматирование не гарантирует отсутствие опечаток, но позволяет их легче и быстрее заметить\. См\. главу N13 в мини\-книге "[Главный вопрос программирования, рефакторинга и всего такого](https://pvs-studio.ru/ru/blog/posts/cpp/0391/)"\. Есть ещё одна хорошая новость\. Все рассмотренные в этой статье ошибки найдены с помощью статического анализатора кода [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. Соответственно, внедряя в процесс разработки инструменты статического анализа, вы сможете выявить многие опечатки на самом раннем этапе\. Спасибо за внимание\. Надеюсь, вам было интересно и страшно\. Желаю надёжного кода и поменьше ошибаться с 0, 1, 2, чтобы Фредди не пришёл к вам\.