Для получения триального ключа
заполните форму ниже
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Для тех, кто хочет поиграть в детектива…

Для тех, кто хочет поиграть в детектива: найди ошибку в функции из Midnight Commander

07 Фев 2019

Приглашаем попробовать найти ошибку в очень простой функции из проекта GNU Midnight Commander. Зачем? Просто так. Это забавно и интересно. Хотя нет, мы соврали. Мы в очередной раз хотим продемонстрировать ошибку, которую с трудом находит человек в процессе code review, но легко находит статический анализатор кода PVS-Studio.

0610_Detective_and_MC_ru/image1.png

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

static int
EatWhitespace (FILE * InFile)
  /* ----------------------------------------------------------------------- **
   * Scan past whitespace (see ctype(3C)) and return the first non-whitespace
   * character, or newline, or EOF.
   *
   *  Input:  InFile  - Input source.
   *
   *  Output: The next non-whitespace character in the input stream.
   *
   *  Notes:  Because the config files use a line-oriented grammar, we
   *          explicitly exclude the newline character from the list of
   *          whitespace characters.
   *        - Note that both EOF (-1) and the nul character ('\0') are
   *          considered end-of-file markers.
   *
   * ----------------------------------------------------------------------- **
   */
{
    int c;

    for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile))
        ;
    return (c);
}                               /* EatWhitespace */

Как видите, функция EatWhitespace совсем маленькая. Даже комментарий к функции занимает больше места, чем тело самой функции :). Теперь немного деталей.

Описание функции getc:

int getc ( FILE * stream );

Функция возвращает символ, на который указывает внутренний индикатор позиции файла указанного потока. Затем индикатор переходит к следующему символу. Если на момент вызова потока достигнут конец файла, функция возвращает значение EOF и устанавливает индикатор конца файла для данного потока. Если возникает ошибка чтения, функция возвращает значение EOF и устанавливает индикатор ошибки для данного потока (ferror).

Описание функции isspace:

int isspace( int ch );

Функция проверяет, является ли символ пробельным по классификации текущей локали. В стандартной локали следующие символы являются пробельными:

  • пробел (0x20, ' ');
  • смена страницы (0x0c, '\f');
  • перевод строки LF (0x0a, '\n');
  • возврат каретки CR (0x0d, '\r');
  • горизонтальный таб (0x09, '\t');
  • вертикальный таб (0x0b, '\v').

Возвращаемое значение. Ненулевое значение, если символ является пробельным, ноль иначе.

Функция EatWhitespace должна пропустить все символы, считающиеся пробельными, кроме перевода строки '\n'. Ещё одной причиной остановки чтения из файла может стать достижение конца файла (EOF).

А теперь, зная всё это, попробуйте найти ошибку!

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

0610_Detective_and_MC_ru/image2.png

Рисунок 1. Время поискать ошибку. Единороги пока подождут.

Всё равно не видите ошибку?

Всё дело в том, что мы обманули читателей по поводу isspace. Ха-ха! Это вовсе не стандартная функция, а самодельный макрос. Да, мы - бяки и заставили вас запутаться.

0610_Detective_and_MC_ru/image3.png

Рисунок 2. Единорог дарит читателям ложное представление о том, что такое isspace.

На самом деле виноваты, конечно, не мы и не наш единорог. Путаницу внесли авторы проекта GNU Midnight Commander, решив создать собственную реализацию isspace в файле charset.h:

#ifdef isspace
#undef isspace
#endif
....
#define isspace(c) ((c)==' ' || (c) == '\t')

Создав такой макрос, одни разработчики запутали других разработчиков. Код написан из предположения, что isspace — это стандартная функция, считающая возврат каретки (0x0d, '\r') одним из пробельных символов.

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

for (c = getc (InFile);
     ((c)==' ' || (c) == '\t') && ('\n' != c);
     c = getc (InFile))

Подвыражение ('\n' != c) является лишним (избыточным), так как его результатом всегда будет true. Об этом и предупреждает анализатор PVS-Studio, выдавая предупреждение:

V560 A part of conditional expression is always true: ('\n' != c). params.c 136.

Для полной ясности давайте разберём 3 варианта развития событий:

  • Достигнут конец файла. Конец файла (EOF) — это не пробел и не табуляция. Подвыражение ('\n' != c) из-за short-circuit evaluation не вычисляется. Цикл останавливается.
  • Прочитан любой символ, не являющийся пробелом или табуляцией. Подвыражение ('\n' != c) не вычисляется из-за short-circuit evaluation. Цикл останавливается.
  • Прочитан символ пробела или горизонтальная табуляции. Подвыражение ('\n' != c) вычисляется, но его результат всегда будет истинным.

Другими словами, рассмотренный код эквивалентен этому:

for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile))

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

Программист, написавший в теле функции EatWhitespace вызов isspace, рассчитывал, что будет вызвана стандартная функция. Именно поэтому он добавил условие, что перевод строки LF ('\n') не следует считать пробельным символом.

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

Примечательнее, что планировалось пропускать и символ возврат каретки CR (0x0d, '\r'). Этого не происходит и цикл остановится, встретив этот символ. Это приведёт к неприятным неожиданностям, если разделителем строк в файле является последовательность CR+LF, используемая в некоторых не-UNIX системах, таких как Microsoft Windows.

Для тех, кто хочет узнать больше об исторических причинах использования в качестве разделителей строк LF или CR+LF, предлагаем вниманию статью на Wikipedia "Перевод строки".

Функция EatWhitespace по задумке должна одинаково обрабатывать файлы, где в качестве разделителя используется как LF, так и CR+LF. Для случая CR+LF это не так. Другими словами, если ваш файл прибыл из мира Windows, то вам не повезло :).

Возможно, это и не является серьёзной ошибкой, тем более, что GNU Midnight Commander распространён в UNIX-подобных операционных системах, где для перевода строки используется символ LF (0x0a, '\n'). Однако из-за таких мелочей и возникают различные досадные проблемы несовместимости данных, подготавливаемых в Linux и Windows системах.

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

Переопределение стандартных функций является плохой практикой. Кстати, недавно в статье "Любите статический анализ кода" рассматривался похожий случай с макросом #define sprintf std::printf.

Более правильным решением было бы дать макросу уникальное имя, например, is_space_or_tab. Тогда путаница была бы невозможна.

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

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

Последние статьи:

Опрос:

Популярные статьи по теме
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

В этой статье постараюсь затронуть все вещи, которые можно без зазрения совести выкинуть из С++, не потеряв ничего (кроме боли), уменьшить стандарт, нагрузку на создателей компиляторов, студентов, из…
Продление жизни временных значений в С++: рецепты и подводные камни

Дата: 01 Ноя 2022

Автор: Гость

Прочитав эту статью, вы узнаете следующее: способы, которыми можно продлить время жизни временного объекта в С++; рекомендации и подводные камни этого механизма, с которыми может столкнуться С++ прог…
Как мы баг в PVS-Studio искали или 278 Гигабайтов логов

Дата: 28 Окт 2022

Автор: Григорий Семенчев, Сергей Ларин, Филипп Хандельянц

Предлагаем вашему вниманию интересную историю о поиске бага внутри анализатора PVS-Studio. Да, мы тоже допускаем ошибки, но мы готовы засучить рукава и залезть в самую глубину "кроличьей норы".
0, 1, 2, Фредди забрал Blender

Дата: 26 Окт 2022

Автор: Андрей Карпов

Эта статья могла бы получить название "Как PVS-Studio защищает от поспешных правок кода, пример N7". Однако так именовать статьи становится скучновато. Поэтому сейчас вы узнаете, причём здесь Фредди …
Примеры ошибок, которые может обнаружить PVS-Studio в коде LLVM 15.0

Дата: 25 Окт 2022

Автор: Андрей Карпов

Компиляторы развиваются и выдают всё больше предупреждений. Остаются ли преимущества от использования статических анализаторов кода, таких как PVS-Studio? Да, так как анализаторы тоже развиваются. Пе…

Комментарии (0)

Следующие комментарии
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо