Для получения триального ключа
заполните форму ниже
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.

Популярные статьи по теме
Почему в С++ массивы нужно удалять через delete[]

Дата: 27 Июл 2022

Автор: Михаил Гельвих

Заметка рассчитана на начинающих C++ программистов, которым стало интересно, почему везде твердят, что нужно использовать delete[] для массивов, но вместо внятного объяснения – просто прикрываются ма…
Конкурс внимательности: PVS-Studio vs Хакер

Дата: 21 Июл 2022

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

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

Дата: 19 Июл 2022

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

Некоторые предупреждения анализатора или компилятора сложно однозначно классифицировать как ложное срабатывание или указание на настоящую ошибку. Бывает, что формально анализатор/компилятор прав, но …
Заводим трактор: QMake -> CMake

Дата: 18 Июл 2022

Автор: Гость

Заводим трактор: переезжаем с QMake на CMake. По дороге заглянем на улицу "Кросс компиляторщиков", в сквер "Систем сборки" и посидим в баре "Управления зависимостями". Заодно увидим тех, кто использу…
Межмодульный анализ C и C++ проектов в деталях. Часть 2

Дата: 14 Июл 2022

Автор: Олег Лысый

В первой части статьи мы рассматривали основы теории компиляции C и C++ проектов, в частности особое внимание уделили алгоритмам компоновки и оптимизациям. Во второй части мы погрузимся глубже и пока…

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

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