Распространенные шаблоны опечаток в программировании

У разработчиков есть бесконечное количество способов допустить ошибки при написании кода. Однако иногда можно обнаружить очевидные и интересные закономерности в том, как и где разработчики допускают ошибки. Давайте поговорим о коде как о «магните» для опечаток.

История исследования

Для тестирования и продвижения статического анализатора кода PVS-Studio мы проверяем различные проекты с открытым исходным кодом. Если мы находим ошибки, то сообщаем о них авторам проектов. Мы собираем ошибки и пишем статьи о самых интересных случаях.

Просматривая все эти ошибки, я замечаю различные повторяющиеся шаблоны опечаток. За некоторыми исключениями, они независимы от языка программирования. По крайней мере, они являются общими для кода, написанного на C, C++, C# и Java. В этой статье я обрисую 7 закономерностей, которые я уже заметил:

1. Эффект последней строки
2. Самая опасная функция memset
3. Неправильные функции сравнения
4. Неправильные функции копирования
5. Ошибки, связанные с данными и временем
6. Несчастливые числа: 0, 1, 2.
7. Ошибка отклонения на единицу

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

Эффект последней строки

Это моя первая найденная выкройка. Впервые я описал это в статье 2014 года. Вот описание закономерности: при написании однотипных блоков кода можно допустить ошибку в последнем блоке.

Поясню это на примере кода из проекта Godot Engine (C++).

String SoftBody::get_configuration_warning() const {
  ....
  Transform t = get_transform();
  if ((ABS(t.basis.get_axis(0).length() - 1.0) > 0.05 ||
       ABS(t.basis.get_axis(1).length() - 1.0) > 0.05 ||
       ABS(t.basis.get_axis(0).length() - 1.0) > 0.05)) {
    if (!warning.empty())
  ....
}

Это классическая ошибка копипаста. Разработчики не хотят перепечатывать похожие строки кода. Поэтому они просто удваивают строку кода:

ABS(t.basis.get_axis(0).length() - 1.0) > 0.05

После этого во второй строке заменяют 0 на 1, а в последней строке забывают заменить 0 на 2. Когда мы отдельно просматриваем такой фрагмент кода, кажется удивительным обнаружить столь простую ошибку. Но именно так выглядит этот узор.

Вот еще один пример, найденный в модульных тестах проекта LLVM (C++).

TEST(RegisterContextMinidump, ConvertMinidumpContext_x86_64) {
  MinidumpContext_x86_64 Context;
  ....
  Context.rax = 0x0001020304050607;
  Context.rbx = 0x08090a0b0c0d0e0f;
  ....
  Context.eflags = 0x88898a8b;
  Context.cs = 0x8c8d;
  Context.fs = 0x8e8f;
  Context.gs = 0x9091;
  Context.ss = 0x9293;    // <=
  Context.ds = 0x9495;
  Context.ss = 0x9697;    // <=
  llvm::ArrayRef<uint8_t> ContextRef(reinterpret_cast<uint8_t *>(&Context),
                                     sizeof(Context));
  ....
}

Давайте посмотрим на строки кода, выделенные комментариями. Этот код вряд ли был написан методом копирования-вставки, но в последней строке все равно есть ошибка. Разработчик поторопился и по невнимательности допустил опечатку при неоднократной записи значения в регистр ss вместо регистра es.

Примечание. Еще одна интересная вещь: ошибка показывает, как статический анализатор дополняет модульное тестирование. Модульные тесты также могут содержать ошибки, которые вызывают определенные непроверенные или не полностью протестированные сценарии. Однако писать модульные тесты для модульных тестов непрактично. Здесь вам помогут анализаторы кода.

Чтобы не сложилось впечатление, что такие ошибки встречаются только в языках C и C++, я покажу вам пример кода Java. Я обнаружил следующую опечатку в проекте IntelliJ IDEA Community Edition (Java).

private static boolean isBeforeOrAfterKeyword(String str, boolean trimKeyword) {
  return (trimKeyword ? LoadingOrder.BEFORE_STR.trim() :
           LoadingOrder.BEFORE_STR).equalsIgnoreCase(str) ||
         (trimKeyword ? LoadingOrder.AFTER_STR.trim() :
           LoadingOrder.AFTER_STR).equalsIgnoreCase(str) ||
         LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase(str) ||         // <=
         LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase(str);           // <=
}

Здесь мы замечаем очевидную закономерность. Другие подобные примеры вы можете увидеть в старой статье Эффект последней строки.

Почему я назвал этот узор именно так? «Эффект последних метров» из мира альпинизма — это мой источник вдохновения. Я читал наблюдение, что альпинисты часто допускают ошибку в конце восхождения. Причина не в том, что они устали — они менее сконцентрированы. Они ошибочно думают, что уже достигли вершины горы и скоро смогут отдохнуть и насладиться завершением путешествия. Альпинист теряет концентрацию, спешит закончить восхождение и делает что-то не так.

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

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

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

Совет. При просмотре кода следует уделять больше внимания последним строкам кода.

Самая опасная функция memset

Помимо общих шаблонов, существуют шаблоны, специфичные для конкретного языка. Например, функция memset вызывает большое количество ошибок. Чаще всего он используется в программах на языке C, но его часто можно встретить и в коде на C++.

void* memset( void* dest, int ch, size_t count );           // C
void* std::memset( void* dest, int ch, std::size_t count ); // C++

Функция memset копирует значение (unsigned char) ch в каждый из первых символов count объекта, на который указывает dest. Описание:

• Memset — это вариант функции на языке C.
• std::memset — это вариант функции C++.

На мой взгляд, эта функция является лидером языков C и C++ по привлечению к себе ошибок. Чаще всего ошибка связана с неверным расчетом размера буфера.

Опечатки возникают, когда используется ненужный оператор косвенности (*) или когда он отсутствует там, где он необходим. Или в избыточном операторе адреса (&). Теперь рассмотрим примеры таких ошибок.

Разработчик забыл разыменовать указатель (*). Фрагмент кода взят из проекта FAR Manager v2 для Linux (Far2l, C++). Из-за опечатки оператор sizeof оценивает размер указателя, а не размер структуры.

int64_t FileList::VMProcess(int OpCode,
                            void *vParam,
                            int64_t iParam)
{
  ....
  PluginInfo *PInfo = (PluginInfo *)vParam;
  memset(PInfo, 0, sizeof(PInfo));
  ....
}

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

memset(PInfo, 0, sizeof(*PInfo));

Ненужный оператор адреса (&). Вот код из проекта Energy Checker SDK (C). Оператор sizeof оценивает размер указателя вместо размера структуры WIN32_FIND_DATA. В результате обнуляются только первые 4 или 8 байт структуры. Это зависит от того, создается ли приложение Win32 или Win64.

int plh_read_pl_folder(PPLH_PL_FOLDER_INFO pconfig) {
  ....
  WIN32_FIND_DATA file_data;
  ....
  memset(
    &file_data,
    0,
    sizeof(&file_data)
  );
  ....
}

Правильная версия: sizeof(file_data). А еще лучше вообще избавиться от функции memset. Для этого можно инициализировать объект нулями в объявлении:

WIN32_FIND_DATA file_data = { };

Разработчик перепутал & с *. Пример взят из проекта Wolfenstein 3D (C). Здесь разработчику пришлось разыменовать указатель, чтобы правильно оценить размер структуры, вместо использования оператора адреса.

void CG_RegisterItemVisuals( int itemNum ) {
  ....
  itemInfo_t *itemInfo;
  ....
  memset( itemInfo, 0, sizeof( &itemInfo ) );
  ....
}

Переполнение буфера. В проекте эмулятора PS4 с открытым исходным кодом (C++) разработчик неосторожно перепутал аргумент sizeof с аргументом countof. Они думали, что оператор sizeof оценит количество элементов. Поэтому они умножили это значение на размер одного элемента.

struct GnmCmdPSShader
{
  ....
  uint32_t reserved[27];
};

int PS4API sceGnmSetPsShader350(....)
{
  ....
  memset(param->reserved, 0, sizeof(param->reserved) * sizeof(uint32_t)); 
  return SCE_OK;
}

Правильная версия кода:

memset(param->reserved, 0, sizeof(param->reserved));

Примечание. Кстати, функция memset вызывает не только опечатки, но и другие ошибки. О них вы можете прочитать в статье: Самая опасная функция в мире C/C++». Например, потенциальные уязвимости возникают, когда разработчики используют функцию memset в конце для перезаписи личных данных. Вот пример из проекта WebRTC (C++).

void AsyncSocksProxySocket::SendAuth() {
  ....
  char * sensitive = new char[len];
  pass_.CopyTo(sensitive, true);
  request.WriteString(sensitive);  // Password
  memset(sensitive, 0, len);
  delete [] sensitive;
  ....
}

Дело в том, что компиляторы удалят вызов функции memset для оптимизации кода. С точки зрения компилятора, поскольку буфер освобожден, мы можем его не заполнять. В результате личные данные останутся в памяти. Эта тема выходит за рамки статьи об опечатках. Итак, если вы хотите получить более подробную информацию, я предлагаю вам нажать на следующие ссылки:

• V597: диагностика PVS-Studio
• CWE-14: Удаление кода компилятором для очистки буферов
• Безопасное удаление личных данных

Совет. Внимательно просмотрите код, содержащий вызов функции memset. Более того, во время ревью кода можно попробовать использовать другой способ заполнения памяти.

Функция memset часто используется во время инициализации для заполнения всех полей структуры нулевыми значениями. Вот абстрактный пример:

MyStruct foo;
memset(foo, 0, sizeof(foo));

Проще и безопаснее сразу при объявлении заполнять структуры нулями:

MyStruct foo = { };

Если вам нужно обнулить массив в C++, лучше использовать функцию std::fill. Позвольте мне объяснить это на примере проекта Notepad++ (C++).

#define CONT_MAP_MAX 50
int _iContMap[CONT_MAP_MAX];
....
DockingManager::DockingManager()
{
  ....
  memset(_iContMap, -1, CONT_MAP_MAX);
  ....
}

Разработчики думают, что они инициализируют массив типа int, содержащий 50 элементов, значением -1. Но функция memset работает с байтами. Фактически, значение 0xFFFFFFFF инициализирует первые 12 элементов. Они будут заполнены правильным значением -1, но это просто удача. Еще один элемент получит значение 0x0000FFFF. Остальные элементы массива останутся неинициализированными.

Если мы используем функцию std::fill, допустить ошибку сложнее:

// since C++11
std::fill(std::begin(_iContMap), std::end(_iContMap), -1);

// since C++20
std::ranges::fill(_iContMap, -1);

Неправильные функции сравнения

Писать функции сравнения скучно. На самом деле этот вспомогательный код прост и неинтересен. Так что здесь нет места творчеству. Нам просто нужно сравнить все переменные друг с другом.

Вот почему разработчики спешат писать такие функции. Они также спешат при просмотре кода. Трудно сосредоточиться на таком простом коде. Такие функции просматриваются по диагонали во время проверки кода. Это приводит к множеству незамеченных опечаток.

Начнем с классической опечатки в большом блоке однотипных сравнений. Пример взят из проекта Apache Flink (Java).

@Override
public boolean equals(Object o) 
{
  ....
  CheckpointStatistics that = (CheckpointStatistics) o;
  return id == that.id &&
    savepoint == that.savepoint &&
    triggerTimestamp == that.triggerTimestamp &&
    latestAckTimestamp == that.latestAckTimestamp &&
    stateSize == that.stateSize &&
    duration == that.duration &&
    alignmentBuffered == that.alignmentBuffered &&
    processedData == processedData &&
    persistedData == that.persistedData &&
    numSubtasks == that.numSubtasks &&
    numAckSubtasks == that.numAckSubtasks &&
    status == that.status &&
    Objects.equals(checkpointType, that.checkpointType) &&
    Objects.equals(
      checkpointStatisticsPerTask, 
      that.checkpointStatisticsPerTask);
}

Нашли ошибку? Думаю, нет. Этот код скучно писать и скучно читать. Вы, вероятно, не хотели его внимательно изучать. Вот строка с опечаткой.

processedData == processedData

Переменная сравнивается сама с собой. Правильная версия:

processedData == that.processedData

Те же опечатки мы можем найти в более мелких функциях из проекта eShopOnContainers (C#).

private bool CheckSameOrigin(string urlHook, string url)
{
  var firstUrl = new Uri(urlHook, UriKind.Absolute);
  var secondUrl = new Uri(url, UriKind.Absolute);

  return firstUrl.Scheme == secondUrl.Scheme &&
         firstUrl.Port == secondUrl.Port &&
         firstUrl.Host == firstUrl.Host;
}

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

Но оказывается, что даже в такой короткой функции сравнения можно допустить опечатку. Вот пример из проекта Skia Graphics Engine (C++).

inline bool operator==(const SkPDFCanon::BitmapGlyphKey& u,
                       const SkPDFCanon::BitmapGlyphKey& v)
{
  return memcmp(&u, &u, sizeof(SkPDFCanon::BitmapGlyphKey)) == 0;
}

Объект u сравнивается с самим собой побайтно.

Однако ошибки в функциях сравнения не ограничиваются чем-то, что сравнивается само с собой. Ошибки разнообразны. В качестве примера я взял интересную опечатку из проекта Azure Service Fabric (C++).

template <typename TK, typename TV>
static bool MapCompare(const std::map<TK, TV>& lhs,
                       const std::map<TK, TV>& rhs)
{
  if (lhs.size() != rhs.size()) { false; }

  return std::equal(lhs.begin(), lhs.end(), rhs.begin());
}

Взгляните на одинокое ключевое слово false. Код скомпилирован, но не имеет никакого смысла. Здесь разработчик забыл написать оператор return. Правильная версия:

if (lhs.size() != rhs.size()) { return false; }

Вот пример из проекта Roslyn (C#).

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

Здесь может возникнуть NullReferenceException. Разработчикам необходимо проверить ссылку otherResourceString вместо ссылки other на наличие неравенства null.

Другие примеры ошибок вы можете найти в статье Зло внутри функций сравнения.

Если я скажу уделять больше внимания функциям сравнения во время проверки кода, я не думаю, что это вас мотивирует. Итак, я отвечу на этот вопрос более практично.

Совет: Вы можете использовать статические анализаторы кода, такие как PVS-Studio, чтобы помочь себе. Такие анализаторы тщательно работают над любым кодом.

Вы можете использовать форматирование кода в виде таблицы.

Подробнее о табличном форматировании кода я писал в своей мини-книге 60 ужасных советов для C++-разработчика — см. Ужасный совет N20. Компактный код». Совет начинается с объяснения, как не надо делать, а затем дается полезный совет. Табличное форматирование не гарантирует отсутствие опечаток, но определенно уменьшает их количество.

Должен отметить, что начиная с C++20 существует способ попросить компилятор сгенерировать код операторов сравнения: равенства, неравенства и отношения. Вам редко придется писать собственные функции сравнения. Это снижает вероятность совершения ошибки. Используйте современные возможности языка C++!

Неправильные функции копирования

Если функции сравнения кажутся разработчикам скучными, то реализация функций копирования провоцирует их на излишний энтузиазм. Иногда я нахожу ошибки в таких функциях. Интересно, что функциональный текст мне кажется слишком сложным. Собственно, эта чрезмерная сложность и является источником ошибок.

Вот специальная реализация функции strdup из проекта Zephyr RTOS (C).

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';
    memcpy(cpy_mntpt, mntpt, strlen(mntpt));
  }
  return cpy_mntpt;
}

Функция memcpy копирует строку, но не может скопировать терминальный нуль. Итак, пишется дополнительный код для копирования терминального нуля:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';

Но это не работает! Код содержит опечатку, из-за которой нулевой терминал копируется в себя. Обратите внимание, что целевой массив — mntpt, а не cpy_mntpt. В результате функция mntpt_prepare возвращает строку, не завершающуюся нулем.

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

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';

Вы можете быть озадачены тем, почему разработчик написал этот код таким беспорядочным и странным образом. Мы можем упростить его до этой версии:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    strcpy(cpy_mntpt, mntpt);
  }
  return cpy_mntpt;
}

Код из проекта LFortran (C) — ошибка конкатенации двух строк в новом буфере.

void _lfortran_strcat(char** s1, char** s2, char** dest)
{
    int cntr = 0;
    char trmn = '\0';
    int s1_len = strlen(*s1);
    int s2_len = strlen(*s2);
    int trmn_size = strlen(&trmn);
    char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);
    for (int i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (int i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = &(dest_char[0]);
}

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

Думаю, все-таки сочту это опечаткой. Разработчик использовал strlen вместо sizeof, чтобы определить размер нулевого значения терминала в байтах.

Объяснение:

char trmn = '\0';
int trmn_size = strlen(&trmn);

Здесь символ trmn интерпретируется как пустая строка нулевой длины. Таким образом, переменная trmn_size, имя которой означает размер нулевого терминала, всегда будет равна 0.

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

int trmn_size = sizeof(trmn);

Мы можем улучшить этот код, но это выходит за рамки данной статьи. О других улучшениях читайте в статье «Красивая ошибка в реализации функции конкатенации строк».

И сейчас мы обсудим последний пример. Я взял его из проекта QuantConnect Lean (C#).

/// <summary>
/// Copy contents of the portfolio collection to a new destination.
/// </summary>
/// <remarks>
/// IDictionary implementation calling the underlying Securities collection
/// </remarks>
/// <param name="array">Destination array</param>
/// <param name="index">Position in array to start copying</param>
public void CopyTo(KeyValuePair<Symbol, SecurityHolding>[] array, int index)
{
  array = new KeyValuePair<Symbol, SecurityHolding>[Securities.Count];
  var i = 0;
  foreach (var asset in Securities)
  {
    if (i >= index)
    {
      array[i] = new KeyValuePair<Symbol,SecurityHolding>(asset.Key,
                                                          asset.Value.Holdings);
    }
    i++;
  }
}

Метод получает коллекцию и немедленно перезаписывает ее значение. Комментарий и название метода подсказывают, что один массив следует скопировать в другой. Однако этого не произойдет, и значение array вне текущего метода останется неизменным.

Дело в том, что аргумент array передается методу по значению, а не по ссылке. Таким образом, после операции присваивания переменная array, доступная внутри метода, будет хранить ссылку на новый объект. Значение переменной внутри метода останется неизменным.

Это версия опечатки, в которой разработчик забыл модификатор out. Вот правильный код:

public void CopyTo(out KeyValuePair<Symbol, SecurityHolding>[] array,
                   int index)
{
  array = new KeyValuePair<Symbol, SecurityHolding>[Securities.Count];
  ....
}

Совет. Напишите модульные тесты для функций копирования.

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

Например, я не упомянул модульные тесты, когда мы обсуждали функции сравнения. Проверять функции сравнения юнит-тестами — дело неблагодарное:

• Писать модульные тесты для функций сравнения скучно. В любом случае, мы уже знаем, что скучный код редко бывает успешным :).
• В модульных тестах вам нужно подумать о многих особых сценариях. Например, вам нужно отдельно проверить корректность сравнения с каждым членом класса. Код модульного теста будет в десятки раз больше кода функций сравнения. Таким образом, использование модульных тестов может оказаться непрактичным с точки зрения соотношения выгод и затрат.

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

Ошибки, связанные с данными и временем

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

Пример из проекта Umbraco (C#) — неправильно вызываются конструкторы класса DateTime.

public static DateTime TruncateTo(this DateTime dt,
                                  DateTruncate truncateTo)
{
  if (truncateTo == DateTruncate.Year)
    return new DateTime(dt.Year, 0, 0);
  if (truncateTo == DateTruncate.Month)
    return new DateTime(dt.Year, dt.Month, 0);
  ....
}

Разработчики допустили опечатку, потому что привыкли нумеровать все с нуля — индексы массивов, например. Но здесь вместо массивов у нас даты. Первый день или месяц имеет номер 1 вместо 0. Вызов конструкторов в приведенном выше коде приводит к ArgumentOutOfRangeException. Правильная версия:

if (truncateTo == DateTruncate.Year)
  return new DateTime(dt.Year, 1, 1);
if (truncateTo == DateTruncate.Month)
  return new DateTime(dt.Year, dt.Month, 1);

Пример из проекта MPC-HC (C++) — здесь можно использовать неинициализированную переменную.

void CSyncAP::RenderThread()
{
  ....
  REFERENCE_TIME rtRefClockTimeNow;
  if (m_pRefClock) {
    m_pRefClock->GetTime(&rtRefClockTimeNow);
  }
  LONG lLastVsyncTime =
    (LONG)((m_llEstVBlankTime - rtRefClockTimeNow) / 10000);
  ....

}

Если условие не выполнено, переменная rtRefClockTimeNow останется неинициализированной. Он по-прежнему будет использоваться в оценках. Разработчики могут забыть инициализировать переменную при объявлении ее значением по умолчанию.

Вот красивая опечатка из проекта Tizen (C). Разработчики ошиблись с представлением данных.

static void preview_down_cb(....)
{
  ....
  int delay = 0.5;
  double fdelay;
  fdelay = ((double)delay / 1000.0f);
  DbgPrint("Long press: %lf\n", fdelay);
  ....
}

Идея записать значение 0.5 в переменную типа int плохая. Задержка фактически исчисляется в миллисекундах. Правильное значение задержки по умолчанию, равное 500 миллисекундам, должно быть записано так:

int delay = 500;

Вот пример некорректного преобразования типов из проекта MSBuild (C#).

internal static void Trace(....)
{
  ....
  long now = DateTime.UtcNow.Ticks;

  float millisecondsSinceLastLog =
    (float)((now - s_lastLoggedTicks)/10000L);
  ....
}

Здесь сначала происходит целочисленное деление, а уже потом выполняется явное преобразование к типу float. Преобразование типов должно выполняться до деления, а не после. Опечатка в том, что разработчик ошибся в скобках. Правильный код:

float millisecondsSinceLastLog = (float)(now - s_lastLoggedTicks)/10000L;

Вот пример забытой запятой из проекта Linux Kernel (C).

static ssize_t lp8788_show_eoc_time(struct device *dev,
        struct device_attribute *attr, char *buf)
{
  struct lp8788_charger *pchg = dev_get_drvdata(dev);
  char *stime[] = { "400ms", "5min", "10min", "15min",
      "20min", "25min", "30min" "No timeout" };
  ....
}

Между предпоследним и последним строковым литералом нет запятой. Итак, они слипаются, и в результате получается «30 минут без тайм-аута».

Я думаю, этого достаточно. Примеры других ошибок вы можете увидеть в статьях 31 февраля и Обработка дат привлекает ошибки или 77 дефектов в Qt 6».

Я показал, насколько разнообразны ошибки, связанные с датами и временем. В этой главе нет шаблона ошибок. По крайней мере я не вижу ни одного.

Совет. Просто имейте в виду, что код, обрабатывающий время и даты, сложен и по каким-то причинам вызывает ошибки. Вам следует уделять больше внимания такому коду во время проверки кода.

И не забывайте про юнит-тесты, они тоже полезны.

Несчастливые числа: 0, 1, 2.

Числа 0, 1, 2 часто используются в программировании как индексы массивов или как часть имен переменных. Наличие таких цифр говорит о том, что код типовой.

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

Начнём с опечаток в индексах

Вот классическая ошибка копирования из проекта LibreOffice (C++).

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;
}

Чтобы не перепечатывать скучную строку, разработчик ее копирует. Они заменяют sdbc на sdbcx в дублированной строке, но забывают исправить индекс массива.

Вот еще одна ошибка копирования-вставки из проекта Quake III Arena (C).

int VL_FindAdjacentSurface(....)
{
  ....
  if (fabs(dir[0]) > test->radius ||
      fabs(dir[1]) > test->radius ||
      fabs(dir[1]) > test->radius)
  {
  ....
}

Вы можете снова увидеть эффект последней строки. Это комбо. Такой код — магнит для ошибок.

Вот еще неожиданная опечатка (индекс массива 2 вместо 1) из проекта OpenCOLLADA (C++).

struct short2
{
  short values[2];
  short2(short s1, short s2)
  {
    values[0] = s1;
    values[2] = s2;
  }
  ....
};

Есть совпадение с ошибкой отклонения на единицу, о которой мы поговорим в конце статьи.

На заметку :). Мы снова видим, как закономерности увеличивают вероятность ошибки. Это интересная тема. Возможно, будет хорошей идеей обсудить это в следующей статье.

Вот еще один пример опечатки из проекта .NET Core SDK (C#).

public bool MatchesXmlType(IList<XPathItem> seq, int indexType)
{
  ....
  typBase = typBase.Prime;
  for (int i = 0; i < seq.Count; i++)
  {
    if (!CreateXmlType(seq[0]).IsSubtypeOf(typBase))
      return false;
  }

  return true;
}

Первый элемент массива seq постоянно обрабатывается внутри цикла. Правильная версия:

if (!CreateXmlType(seq[i]).IsSubtypeOf(typBase))

Давайте посмотрим на опечатки в именованных переменных.

Вот пример из проекта Doom (C++).

uint AltOp::fixedLength()
{
  uint l1 = exp1->fixedLength();
  uint l2 = exp1->fixedLength();

  if (l1 != l2 || l1 == ~0u)
    return ~0;

  return l1;
}

Скорее всего, разработчик скопировал и вставил код и забыл заменить exp1 на exp2 во второй строке.

Вот пример из проекта Vangers (C++).

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

Вот противоположный случай. Правильная версия:

ret = (lang)
  ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1]
  : iJoystickStickSwitch1[vkey - VK_STICK_SWITCH_1];

Вот пример из проекта Boost (C++).

point3D operator/(const point3D &p1, const point3D &p2)
{
  return point3D(p1.x/p2.x, p1.y/p2.y, p1.z/p1.z);
}

Здесь опечатка: p1.z/p1.z.

Другие варианты опечаток

Вот ошибка форматирования из проекта Azure PowerShell (C#).

protected override void ProcessRecordInternal()
{
  ....
  if (this.ShouldProcess(this.Name,
    string.Format("Creating Log Alert Rule '{0}' in resource group {0}",
      this.Name, this.ResourceGroupName)))
  ....
}

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

string.Format("Creating Log Alert Rule '{0}' in resource group {1}",
              this.Name, this.ResourceGroupName))

Думаю, мы рассмотрели достаточно примеров. Если хотите еще большего, предлагаю ознакомиться со статьей Ноль, один, два, Фредди идет за тобой».

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

Давайте еще раз посмотрим на этот код:

point3D operator/(const point3D &p1, const point3D &p2)
{
  return point3D(p1.x/p2.x, p1.y/p2.y, p1.z/p1.z);
}

Нельзя сказать, что код сложный. Наоборот, это слишком просто. Мы не можем быть просто точными при написании такого кода. Вместо этого мы начинаем думать об алгоритмах и о том, как лучше написать тот или иной класс. И вот что мы получаем.

Совет. Будьте более внимательны во время проверки кода, где встречаются несчастливые числа 0, 1, 2.

Статический анализатор кода станет хорошим помощником при проверке кода.

Ошибка отклонения на единицу

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

• Википедия. Ошибка отклонения на единицу.
• Перечень общих слабостей. CWE-193: Ошибка отклонения на единицу.

Наиболее распространенной ошибкой является доступ к элементу массива, прошедшему один конец. Думаю, каждый сталкивался с ними, начиная изучать программирование. Это те самые случаи, когда вас смутило то, что элементы массива нумеруются с нуля, а не с единицы.

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

Вот пример опечатки индекса из проекта Umbraco (C#).

protected virtual string VisitMethodCall(MethodCallExpression m)
{
    ....
    if (m.Arguments.Count == 2)
    {
      var n1 = Visit(m.Arguments[0]);
      var f = m.Arguments[2];
      ....
}

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

var f = m.Arguments[1];

Кстати, этот код можно отнести к части о несчастливых числах 0, 1, 2. Думаю, что взаимное притяжение разных шаблонов ошибок вас не удивляет.

Вот пример опечатки при проверке диапазона значений из проекта CMake (C).

static int64_t
expand(struct archive_read *a, int64_t end)
{
  ....
  if ((lensymbol = read_next_symbol(a, &rar->lengthcode)) < 0)
    goto bad_data;

  if (lensymbol > (int)(sizeof(lengthbases)/sizeof(lengthbases[0])))
    goto bad_data;
  ....
  len = lengthbases[lensymbol] + 2;
  ....
}

При доступе к массиву lengthbases возможно переполнение массива, поскольку разработчики написали оператор › вместо оператора ›= выше. Правильный вариант проверки:

if (lensymbol >= (int)(sizeof(lengthbases)/sizeof(lengthbases[0])))

Вот пример опечатки индекса из проекта ELKI (Java).

public List<double[]> points;

@Override
public double[] computeMean() {
  // Not supported except for singletons.
  return points.size() == 1 ? points.get(1) : null;
}

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

return points.size() == 1 ? points.get(0) : null;

Совет. Несмотря на то, что все знают об ошибках этого типа, они по-прежнему распространены. Будь осторожнее.

Статические и динамические анализаторы кода — хорошие помощники в обнаружении ошибок переполнения массива.

Поделитесь с нами своим опытом!

Спасибо за внимание! Если вы знаете подобные закономерности ошибок, поделитесь ими в комментариях. Во-первых, вы можете помочь предупредить своего товарища по команде о таких закономерностях. Во-вторых, мы можем написать новую диагностику для статического анализатора PVS-Studio, чтобы выявлять подобные ошибки.

Возможно, в будущем я также найду новые закономерности и опишу их. Чтобы не пропустить интересные новости, вы можете подписаться на ежемесячный дайджест.

PVS-Студия

Анализатор PVS-Studio помог обнаружить все ошибки, описанные в этой статье. Этот инструмент поможет вам быстро обнаружить опечатки и другие дефекты в новом коде, тем самым сократив затраты на их исправление. Вы можете бесплатно попробовать пробную версию PVS-Studio: скачать PVS-Studio.

Дополнительные ссылки:

1. Сбор ошибок.
2. Эффект последней строки.
3. Объяснение эффекта последней строки.
4. Самая опасная функция в мире C/C++.
5. Зло внутри функций сравнения.
6. Начинаю коллекцию ошибок, найденных в функциях копирования.
7. Красивая ошибка в реализации функции конкатенации строк.
8. 31 февраля.
9. Обработка данных вызывает ошибки или 77 дефектов в Qt 6.
10. Ноль, раз, два, Фредди придет за тобой.
11. 60 ужасных советов C++-разработчику.