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

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
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.

Популярные статьи по теме
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Эффект последней строки

Дата: 31 Май 2014

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

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

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

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
PVS-Studio ROI

Дата: 30 Янв 2019

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

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.

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

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