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

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

![0610_Detective_and_MC_ru/image1.png](https://import.viva64.com/docx/blog/0610_Detective_and_MC_ru/image1.png)

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

```cpp
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_:

```cpp
int getc ( FILE * stream );
```

Функция возвращает символ, на который указывает внутренний индикатор позиции файла указанного потока\. Затем индикатор переходит к следующему символу\. Если на момент вызова потока достигнут конец файла, функция возвращает значение [EOF](http://www.cplusplus.com/reference/cstdio/eof/) и устанавливает индикатор конца файла для данного потока\. Если возникает ошибка чтения, функция возвращает значение EOF и устанавливает индикатор ошибки для данного потока \(ferror\)\.

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

```cpp
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](https://import.viva64.com/docx/blog/0610_Detective_and_MC_ru/image2.png)

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

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

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

![0610_Detective_and_MC_ru/image3.png](https://import.viva64.com/docx/blog/0610_Detective_and_MC_ru/image3.png)

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

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

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

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

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

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

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

[V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: \('\\n' \!\= c\)\. params\.c 136\.

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

* Достигнут конец файла\. Конец файла \(EOF\) — это не пробел и не табуляция\. Подвыражение \('\\n' \!\= c\) из\-за [short\-circuit evaluation](https://en.wikipedia.org/wiki/Short-circuit_evaluation) не вычисляется\. Цикл останавливается\. 
* Прочитан любой символ, не являющийся пробелом или табуляцией\. Подвыражение \('\\n' \!\= c\) не вычисляется из\-за short\-circuit evaluation\. Цикл останавливается\.
* Прочитан символ пробела или горизонтальная табуляции\. Подвыражение \('\\n' \!\= c\) вычисляется, но его результат всегда будет истинным\.

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

```cpp
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 "[Перевод строки](https://ru.wikipedia.org/wiki/%D0%9F%D0%B5%D1%80%D0%B5%D0%B2%D0%BE%D0%B4_%D1%81%D1%82%D1%80%D0%BE%D0%BA%D0%B8)"\.

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

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

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

Переопределение стандартных функций является плохой практикой\. Кстати, недавно в статье "[Любите статический анализ кода](https://pvs-studio.ru/ru/blog/posts/cpp/0535/)" рассматривался похожий случай с макросом _\#define sprintf std::printf_\.

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

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

Спасибо за внимание\. Приглашаем [скачать](https://pvs-studio.ru/ru/pvs-studio/download/) и попробовать анализатор PVS\-Studio для проверки своих проектов\. Плюс напоминаем, что недавно в анализаторе появилась поддержка языка Java\.