Форум программистов, компьютерный форум, киберфорум
el_programmer
Войти
Регистрация
Восстановить пароль
Блоги Сообщество Поиск  
PVS-Studio - это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#.

PVS-Studio выполняет статический анализ кода и генерирует отчёт, помогающий программисту находить и устранять ошибки. PVS-Studio выполняет широкий спектр проверок кода, но наиболее силён в поисках опечаток и последствий неудачного Copy-Paste. Показательные примеры таких ошибок: V501, V517, V522, V523, V3001.

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

Главный вопрос программирования, рефакторинга и всего такого. Часть 4

Запись от el_programmer размещена 29.04.2016 в 15:02
Показов 2097 Комментарии 0

Часть 1: https://www.cyberforum.ru/blog... g4221.html
Часть 2: https://www.cyberforum.ru/blog... g4222.html
Часть 3: https://www.cyberforum.ru/blog... g4223.html
Полная версия в ПДФ формате: https://yadi.sk/i/LKkWupFjr5WzR
Полная версия в ПДФ формате английский вариант: https://yadi.sk/i/zKHIOS84r87nk
Содержание

36. Если на вашем компьютере происходят магические события, проверьте память
37. Бойтесь оператора continue внутри do { ... } while(...)
38. С сегодняшнего дня используйте nullptr вместо NULL
39. Почему некорректный код иногда работает
40. Внедрите статический анализ кода
41. Сопротивляйтесь добавлению в проект новых библиотек
42. Не давайте функциям название "empty"
Заключение


35. Добавляя в enum новую константу, не забываем поправить операторы switch

Фрагмент взят из проекта Appleseed. Код содержит ошибку, которую анализатор PVS-Studio диагностирует следующим образом: V719 The switch statement does not cover all values of the 'InputFormat' enum: InputFormatEntity.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
enum InputFormat
{
    InputFormatScalar,
    InputFormatSpectralReflectance,
    InputFormatSpectralIlluminance,
    InputFormatSpectralReflectanceWithAlpha,
    InputFormatSpectralIlluminanceWithAlpha,
    InputFormatEntity
};
 
switch (m_format)
{
  case InputFormatScalar:
    ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
    ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
    ....
}
Разъяснение

Нередко бывает необходимо добавить новую именованную константу в перечисление (enum). Делать это надо очень аккуратно. Здесь подстерегает распространенный паттерн ошибки: забываем где-то добавить case внутрь switch или поправить цепочку операторов if. Подобную ситуацию можно наблюдать в приведённом коде.

В какой-то момент в перечисление InputFormat была добавлена константа InputFormatEntity. Это предположение я делаю на основании того, что эта константа находится в конце. Как правило, программисты добавляют новые константы именно в конец enum.

А вот оператор switch исправить забыли. В результате случай, когда "m_format==InputFormatEntity" никак не обрабатывается.

Корректный код

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
switch (m_format)
{
  case InputFormatScalar:
  ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
  ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
  ....
  case InputFormatEntity:
  ....
}
Рекомендация

Давайте, подумаем, как можно предотвратить такие ошибки, производя рефакторинг кода.

Самое простое, но не очень удачное решение, это делать везде "default:", который будет уведомлять об ошибке. Например, так:

C++
1
2
3
4
5
6
7
8
9
switch (m_format)
{
  case InputFormatScalar:
  ....
  ....
  default:
    assert(false);
    throw "В этом месте рассмотрены не все варианты";
}
Теперь если переменная m_format будет равна InputFormatEntity, мы заметим ошибку. У этого подхода есть два больших минуса:

1. Ошибку мы можем заметить только на этапе выполнения программы. При чем есть опасность, что ошибка не будет обнаружена на этапе тестирования. Если при выполнении тестов m_format всегда неравна InputFormatEntity, то эта ошибка попадёт в релизную версию продукта. Будет неприятно, когда пользователи начнут сообщать о проблемах.

2. Раз мы считаем, что попасть в default это ошибка, то придётся писать case для всех именованных констант. Это неудобно, особенно если в перечислении этих констант много. Иногда действительно удобно обрабатывать многие ситуации одинаково в ветке default.

Я предлагаю решать эту проблему организационным образом. Он тоже не идеален, но хоть что-то.

Когда в коде вы проверяете значения переменной типа enum, оставляйте комментарий специального вида. Можно использовать какое-то ключевое слово и имя перечисления. Пример:

C++
1
2
3
4
5
6
7
8
9
10
11
12
enum InputFormat
{
  InputFormatScalar,
  ....
  InputFormatEntity
  //Если добавляешь новую константу, ищи ENUM:InputFormat.
};
 
switch (m_format) //ENUM:InputFormat
{
  ....
}
Теперь, когда Вы будете изменять enum, вы должны поискать "ENUM:InputFormat". Это даёт гарантию, что вы не пропустите какое-то важное место.

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


36. Если на вашем компьютере происходят магические события, проверьте память

Я думаю вы устали от бесконечного разбора паттернов программистских ошибок. Немного отдохнём и отвлечемся

Итак, типовая ситуация - ваша программа работает неправильно. Но вы не можете дать объяснение происходящему.

В таких ситуациях я всегда призываю не спешить перекладывать вину на кого-то ещё, а сосредоточиться на коде вашей программы. В 99.99% случаев причиной неправильно работы программы, является именно ошибка, которую допустил кто-то из разработчиков вашей команды. Причем, часто эта ошибка весьма глупа и банальна. Вот и ищите её!

То, что ошибка, проявляется нерегулярно, ничего не означает. Просто у вас завёлся Heisenbug.

И упаси вас боже, обвинять компилятор, что он неправильно собирает вашу программу. Такое конечно бывает, но крайне редко. Сами же потом будете глупо выглядеть, когда выяснится, что вы просто не умеете правильно обращаться, скажем, с оператором sizeof(). У меня есть интересная заметка в блоге на эту тему: Во всём виноват компилятор.

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

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

У меня отказывался правильно вести себя простой тестовый проект, который я готовил для демонстрации работы анализатора Viva64 (это предшественник PVS-Studio).

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


Нажмите на изображение для увеличения
Название: image3.png
Просмотров: 1106
Размер:	115.6 Кб
ID:	3770

После изменения памяти, отладчик считывает значения для показа в окне. И показывает число 2:


Нажмите на изображение для увеличения
Название: image5.png
Просмотров: 934
Размер:	113.9 Кб
ID:	3771

Видите, там 0x02. Хотя я вводил значение 3.

Младший бит всегда равен нулю.

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

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

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

Рекомендация

Всегда ищите ошибку в своём коде. Не старайтесь переложить ответственность.

Однако, если ошибка вот уже неделю повторяется только на вашем компьютере, это повод заподозрить неладное.

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


37. Бойтесь оператора continue внутри do { ... } while(...)

Фрагмент взят из проекта Haiku (преемница операционной системы BeOS). Код содержит ошибку, которую анализатор PVS-Studio диагностирует следующим образом: V696 The 'continue' operator will terminate 'do { ... } while (FALSE)' loop because the condition is always false.

C++
1
2
3
4
5
6
7
8
9
10
11
12
do {
  ....
  if (appType.InitCheck() == B_OK
    && appType.GetAppHint(&hintRef) == B_OK
    && appRef == hintRef)
  {
    appType.SetAppHint(NULL);
    // try again
    continue;
  }
  ....
} while (false);
Разъяснение

Оператор continue ведет себя внутри конструкции do { } while () не так, как ожидают некоторые программисты. Когда вызывается оператор continue, в любом случае выполняется проверка условия окончания цикла. Попробую пояснить эти тему подробнее.

Предположим, что программист пишет кода вида:

C++
1
2
3
4
5
6
for (int i = 0; i < n; i++)
{
  if (blabla(i))
    continue;
  foo();
}
Или такой:

C++
1
2
3
4
5
6
while (i < n)
{
  if (blabla(i++))
    continue;
  foo();
}
Программист на интуитивном уровне понимает, что, когда сработает оператор continue, управление будет передано выше. Проверится условие (i < n). Если оно истинно, начнётся очередная итерация цикла.

Когда же программист пишет код:

C++
1
2
3
4
5
6
do
{
  if (blabla(i++))
    continue;
  foo();
} while (i < n);
То интуиция его часто подводит. Вверху он не видит условия и ему кажется, что, вызывая оператор continue, он немедленно запускает очередную итерацию цикла. Это не так. Оператор continue в данном случае передаёт управление к while(i < n) и начинается проверка.

Приведёт непонимание как работает continue к ошибке или нет, зависит от везения. Однако, ошибка точно произойдёт, если условие цикла всегда ложно, как в коде, показанном в самом начале.

Программист планировал выполнять определённые действия, начиная новые итерации с помощью оператора continue. Об этом намерении свидетельствует присутствующий в коде комментарий "//try again". Однако, никакого "again" не будет, так как используется условие (false), после вызова continue цикл будет остановлен.

Фактически получается, что в конструкции do { ... } while (false); оператор continue эквивалентен по своему действию оператору break.

Корректный код

Вариантов написать корректный код много. Один из них - создать вечный цикл. Для его возобновления использовать continue, а для остановки - оператор break.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
for (;;) {
  ....
  if (appType.InitCheck() == B_OK
    && appType.GetAppHint(&hintRef) == B_OK
    && appRef == hintRef)
  {
    appType.SetAppHint(NULL);
    // try again
    continue;
  }
  ....
  break;
};
Рекомендация

Всеми способами старайтесь избегать оператор continue внутри do { ... } while (...);. Уж очень эта конструкция обманчива.

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


38. С сегодняшнего дня используйте nullptr вместо NULL

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

Одним из таких нововведений является ключевое слово nullptr, которое призвано заменить макрос NULL.

Напомню, NULL в С++ это ни что иное, как просто 0.

Может показаться, что это синтаксический сахар и не более того. Ну какая разница, пишем мы nullptr или NULL? А разница есть! Использование nullptr реально позволяет избежать разнообразных ошибок.

Я продемонстрирую это на примерах.

Представим, что имеется 2 перегруженных функции:

C++
1
2
void Foo(int x, int y, const char *name);
void Foo(int x, int y, int ResourceID);
Программист может написать следующий вызов:

C++
1
Foo(1, 2, NULL);
И будет уверен, что тем самым вызовет первую функцию. Но это не так. NULL это не что иное, как 0. А ноль как известно имеет тип int. Поэтому будет вызвана вторая, а не первая функция.

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

Ещё можно написать вот такой код:

C++
1
2
if (unknownError)
  throw NULL;
На мой взгляд, генерировать исключение, передавая при этом указатель - подозрительно. Тем не менее, иногда так делают, видимо, разработчикам так было нужно. Впрочем, хорошо или плохо так делать, выходит за рамки обсуждения nullptr.

Важно то, что программист решил в случае неизвестной ошибки сгенерировать исключение и "отправить" во внешний мир нулевой указатель.

На самом деле это не указатель, а int. В результате обработка исключения пойдет не так, как ожидал программист.

Код "throw nullptr;" спасает нас от недоразумения, но это вовсе не значит, что я считаю подобный код нормальным и хорошим.

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

Предположим, что какая-то WinApi функция возвращает тип HRESULT. Тип HRESULT не имеет ничего общего с указателем. Однако, вполне можно написать бессмысленный код вида:

C++
1
if (WinApiFoo(a, b, c) != NULL)
Этот код скомпилируется, так как NULL это 0 типа int, а HRESULT это тип long. Вполне можно сравнивать значения типа int и long. Если использовать nullptr, то следующий код не скомпилируется:

C++
1
if (WinApiFoo(a, b, c) != nullptr)
Тем самым, ошибка будет сразу замечена и исправлена.

Я думаю, что вы поняли идею. Таких примеров можно приводить много, но всё это синтетические примеры, а это всегда не очень убедительно. Есть ли какие-то реальные примеры? Да, есть. Вот один из них, только он не такой красивый, короткий и простой.

Код взят из проекта MTASA.

В природе существует RtlFillMemory(). Это может быть настоящая функция или просто макрос, но это не важно. Это аналог функции memset(), но местами поменян 2 и 3 аргумент. Вот как может быть объявлен этот макрос:

C++
1
2
#define RtlFillMemory(Destination,Length,Fill) \
  memset((Destination),(Fill),(Length))
Ещё существует FillMemory(), который есть не что иное, как RtlFillMemory():

C++
1
#define FillMemory RtlFillMemory
Да, всё длинно и сложно. Но что делать, терпите. Вы ведь наверняка хотели увидеть пример реального кода .

А вот собственно и код, использующий макрос FillMemory.

C++
1
2
3
4
5
6
7
LPCTSTR __stdcall GetFaultReason ( EXCEPTION_POINTERS * pExPtrs )
{
  ....
  PIMAGEHLP_SYMBOL pSym = (PIMAGEHLP_SYMBOL)&g_stSymbol ;
  FillMemory ( pSym , NULL , SYM_BUFF_SIZE ) ;
  ....
}
Это не код, а какая-то бессмысленность. Как минимум здесь перепутан 2 и 3 аргумент, поэтому анализатор PVS-Studio выдаёт 2 предупреждения V575:
  • V575 The 'memset' function processes value '512'. Inspect the second argument. crashhandler.cpp 499
  • V575 The 'memset' function processes '0' elements. Inspect the third argument. crashhandler.cpp 499

Код скомпилировался из-за того, что NULL это 0; в результате заполняется 0 элементов массива.

На самом деле, ошибка не только в этом - NULL здесь вообще не уместен. Функция memset() работает с байтами, поэтому нет смысла просить её заполнить память значениями NULL, это какая-то абракадабра. Правильный код должен быть таким:

C++
1
FillMemory(pSym, SYM_BUFF_SIZE, 0);
Или таким:

C++
1
ZeroMemory(pSym, SYM_BUFF_SIZE);
Но главное не это. Этот бессмысленный код успешно компилируется. Если бы программист написал:

C++
1
FillMemory(pSym, nullptr, SYM_BUFF_SIZE);
Компилятор выдал бы ошибку. Тогда человек понял бы, что поспешил и сделал что-то не то и отнёсся бы к коду внимательней.

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

Рекомендация

Начните использовать nullptr и внесите соответствующий пункт в стандарт кодирования вашей компании. Прямо сейчас.

Использование nullptr позволит избегать некоторых глупых ошибок и тем самым немного ускорит процесс разработки приложения.


39. Почему некорректный код иногда работает

Фрагмент взят из проекта Miranda NG. Код содержит ошибку, которую анализатор PVS-Studio диагностирует следующим образом: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator..

C++
1
2
3
4
5
6
7
8
#define MF_BYCOMMAND 0x00000000L
void CMenuBar::updateState(const HMENU hMenu) const
{
  ....
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
    MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED);
  ....
}
Разъяснение

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

Итак, надо вызвать функцию CheckMenuItem() с определённым набором флагов.

Если значение переменной bShowAvatar истинно, то нужен флаг MF_BYCOMMAND и MF_CHECKED.

Иначе: MF_BYCOMMAND и MF_UNCHECKED.

Для этого написано выражение с использование злосчастного тернарного оператора:

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

Дело в том, что приоритет оператора | выше приоритета оператора ?: (см. Приоритет операций в языке Си/Си++). В результате имеют место сразу 2 ошибки.

Первая ошибка: изменилось условие. Условием является не переменная "dat->bShowAvatar", а выражение "MF_BYCOMMAND | dat->bShowAvatar".

Вторая ошибка: выбирается только один из двух флагов: MF_CHECKED или MF_UNCHECKED. Флаг MF_BYCOMMAND "потерялся".

При всём при этом, код работает совершенно правильно! Причина - счастливое стечение обстоятельств и везение программиста. Ему повезло в том, что флаг MF_BYCOMMAND равен 0x00000000L.

Так как флаг MF_BYCOMMAND равен 0, он не оказывает никакого воздействия. Опытные программисты уже всё поняли, но для новичков разберу этот момент поподробнее.

Посмотрим в начале на правильное выражение с дополнительными круглыми скобками:

MF_BYCOMMAND | (dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED)

Подставим вместо макросов числовые значения:

0x00000000L | (dat->bShowAvatar ? 0x00000008L : 0x00000000L)

Если один из операндов оператора | является 0, то выражение можно упростить:

dat->bShowAvatar ? 0x00000008L : 0x00000000L

Теперь рассмотрим некорректный вариант кода:

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

Подставим вместо макросов числовые значения:

0x00000000L | dat->bShowAvatar ? 0x00000008L : 0x00000000L

В подвыражении "0x00000000L | dat->bShowAvatar" один из операндов оператора | является 0. Сократим выражение:

dat->bShowAvatar ? 0x00000008L : 0x00000000L

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

Корректный код

Код можно поправить по-разному - можно добавить круглые скобки; можно добавить промежуточную переменную, возможно, неплохим вариантом будет использовать старый добрый оператор if:

C++
1
2
3
4
5
6
if (dat->bShowAvatar)
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR, 
                  MF_BYCOMMAND | MF_CHECKED);
else
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
                  MF_BYCOMMAND | MF_UNCHECKED);
Однако, я не настаиваю на таком способе исправить код. Этот код легко прочитать, но он несколько длинноват.

Рекомендация

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

Как уже говорилось ранее в главе N4, оператор ?: очень опасен. Легко забыть, что он имеет очень низкий приоритет и легко составить некорректное выражение. Как правило, ?: используют там, где хотят уместить побольше операторов в одну строчку кода, не делайте так.


40. Внедрите статический анализ кода

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

Фрагмент взят из проекта Haiku (преемница операционной системы BeOS). Код содержит ошибку, которую анализатор PVS-Studio диагностирует следующим образом: V501 There are identical sub-expressions to the left and to the right of the '<' operator: lJack->m_jackType < lJack->m_jackType

C++
1
2
3
4
5
6
7
8
9
10
11
int compareTypeAndID(....)
{
  ....
  if (lJack && rJack)
  {
    if (lJack->m_jackType < lJack->m_jackType)
    {
      return -1;
    }
    ....
}
Разъяснение

Простая опечатка: в правой части вместо rJack случайно вновь написали lJack.

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

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

Итак, проблема существует, и относится она вовсе не к лабораторным работам студентам.

Корректный код

C++
1
if (lJack->m_jackType < rJack->m_jackType)
Рекомендация

В начале о подходах, которые бессильны:
  • Надо быть аккуратным при программировании и не допускать ошибок (красивые слова и не более того);
  • Использовать хороший стиль кодирования (ни один стиль программирование не защитит от ошибки в названии переменной).

Что может помочь:
  • Обзор кода;
  • Юнит-тесты (TDD);
  • Статический анализ кода.

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

Обзоры кода (code review) позволяют выявить множество разнообразнейших ошибок, а заодно улучшить код в плане читаемости. К сожалению, тщательный совместный обзор кода весьма дорог и утомителен, и при том всё равно не даёт гарантию. Очень сложно сохранить внимание и найти опечатку, рассматривая выражения вида:

C++
1
2
3
4
qreal l = (orig->x1 - orig->x2)*(orig->x1 - orig->x2) +
          (orig->y1 - orig->y2)*(orig->y1 - orig->y1) *
          (orig->x3 - orig->x4)*(orig->x3 - orig->x4) +
          (orig->y3 - orig->y4)*(orig->y3 - orig->y4);
Теоретически спасти нас могут юнит тесты, но это только теоретически. На практике нереально проверить все возможные ветвления программы, да и в самих тестах могут быть ошибки.

Статический анализаторы кода - это просто программы, а не искусственный интеллект. Анализатор не замечает многие ошибки и наоборот, часто ругается на корректный код; но при всех этих недостатках это крайне полезный инструмент. Он может выявить множество ошибок на самом раннем этапе.

Статический анализ кода можно рассматривать как более дешёвую альтернативу Code Review. Программа вместо человека быстро изучает код и предлагает более внимательно проверить определённые фрагменты кода.

Естественно, я предлагаю начать использовать разрабатываемый нами анализатор PVS-Studio. Хотя конечно, свет клином на нём не сошелся, и есть множество других платных и бесплатных инструментов. Например, можно начать знакомство с методологией статического анализа с бесплатного открытого анализатора Cppcheck. Множество инструментов перечислено на странице Wikipedia: List of tools for static code analysis.

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

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

Напоследок рекомендую ещё вот эту статью от Джона Кармака: Статический анализ кода.


41. Сопротивляйтесь добавлению в проект новых библиотек

Итак, вам понадобилось реализовать в проекте функциональность X. Теоретики разработки программного обеспечения в этот момент говорят, что для этого нужно взять уже существующую библиотеку Y и использовать её для реализации необходимых вам вещей. Собственно, это классический подход в разработке программного обеспечения - повторное использование своих или чужих наработок (сторонних библиотек). Именно этим путём движется большинство программистов.

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

Я рекомендую всячески сопротивляться добавлению в проект каждой новой библиотеки. Прошу понять меня правильно - я вовсе не говорю, что не надо использовать библиотеки и писать всё самостоятельно. Это просто-напросто глупо. Дело в том, что часто новая библиотека добавляется в проект по прихоти одного разработчика с целью использовать в ней какую-то маленькую "фитюльку". Добавить новую библиотеку несложно, вот только потом всей команде много лет придётся нести груз её поддержки.

Наблюдая за развитием некоторых больших проектов, я могу перечислить ряд проблем из-за наличия большого количества сторонних библиотек. Наверное, я перечислю далеко не все проблемы, но даже следующий список должен побудить вас задуматься:
  1. Добавление новых библиотек быстро увеличивает размер проекта. В нашу эпоху быстрого интернета и больших SSD дисков это не является существенно проблемой. Но когда проект начинает скачиваться из системы контроля версий не за 1 минуту, а за 10, это уже неприятно.
  2. Даже если вы используете 1% от возможностей библиотеки, как правило в проект она будет включена целиком. В результате, если библиотеки используется в виде готовых модулей (например, DLL), то очень быстро растёт размер дистрибутива. Если вы используете библиотеки в виде исходного кода, то существенно увеличивается время компиляции.
  3. Усложняется инфраструктура, связанная с компиляцией проекта. Некоторым библиотекам требуются дополнительные компоненты. Простой пример: для сборки требуется наличие Python. В результате через некоторое время для сборки проекта нужно в начале вырастить на компьютере целый сад вспомогательных программ. Возрастает вероятность, что где-то что-то перестанет работать; объяснить это сложно, это надо прочувствовать. В больших проектах постоянно "отваливается" то одно то другое, и нужно постоянно прилагать усилия, чтобы всё работало и компилировалось.
  4. Если вы заботитесь об уязвимостях, вы должны регулярно обновлять сторонние библиотеки. Злоумышленникам выгодно изучать код библиотек с целью поиска уязвимостей. Во-первых, многие библиотеки открыты, а во-вторых, найдя дыру в одной из библиотек, можно получить отмычку сразу ко многим приложениям, где эта библиотека используется.
  5. Одна из используемых библиотек неожиданно может сменить тип лицензии. Во-первых, вам нужно про это помнить и отслеживать изменения. Во-вторых, не понятно, что делать если это произошло. Например, в один момент распространённая библиотека softfloat перешла с "самодельного" соглашение на BSD.
  6. У вас будут проблемы при переходе на новую версию компилятора. Обязательно будет несколько библиотек, которые не будут торопиться адаптироваться под новый компилятор, и вы будете вынуждены ждать или самим вносить какие-то правки в библиотеки.
  7. У вас будут проблемы при переходе на другой компилятор. Например, вы используете Visual C++, а хотите использовать Intel C++. Стопроцентно найдется пара библиотек, с которыми что-то не заладится.
  8. У вас будут проблемы при переходе на другую платформу. Не обязательно даже на "сильно другую платформу". Достаточно захотеть превратить Win32 приложение в Win64. У вас будут все те-же проблемы - несколько библиотек к этому окажутся не готовы и будет непонятно, что с ними делать. Особенно неприятна ситуация, когда библиотека заброшена и более не развивается.
  9. Рано или поздно, если вы используете множество С-библиотек, где типы не лежат в namespace, у вас начинают пересекаться имена. Это приводит к ошибкам компиляции или к скрытым ошибкам. Например, начинает использоваться константа не из того enum, из которого вы планировали.
  10. Если в проекте используется много библиотек, добавление ещё одной не выглядит чем-то вредным. Можно провести аналогию с теорией разбитых окон. В результате разрастание проекта приобретает неконтролируемый характер.
  11. Есть масса других негативных моментов, о которых я не помню и не знаю. Но в любом случае, дополнительные библиотеки очень быстро увеличивают сложность поддержки проекта. Эта сложность может проявляться в самых неожиданных местах.

Ещё раз подчеркну: я не призываю вас отказаться от использования сторонних библиотек. Если в программе вам понадобилось работать с изображениями в формате PNG, то вам надо взять библиотеку LibPNG и не изобретать велосипед.

Но даже работая с PNG, надо остановиться и подумать. А нужна ли библиотека? Какие операции нужно выполнять с изображениями? Быть может, если вся задача сводится к тому, чтобы сохранить какое-то изображение в *.png - файл, можно обойтись системными функциями. Например, если у вас Windows приложение, то вам поможет WIC. А если вы уже используете библиотеку MFC, то вообще не надо усложнять код, ведь есть класс CImagе (см. обсуждение на сайте StackOverflow). Минус одна библиотека - отлично!

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

Можно было-бы "прикрутить" какую-то из существующих библиотек. Было понятно, что все они будут избыточны, но ведь регулярные выражения все равно нужны и нужно было принять какое-то решение.

Совершенно случайно, именно в тот момент я читал книгу "Beautiful Code" (ISBN 9780596510046). Эта книга о простых и изящных решениях; в ней я повстречал крайне простую реализацию регулярных выражений. Буквально несколько десятков строк. И всё!

Я взял из книги эту реализацию и начал использовать в PVS-Studio. И знаете, что? До сих пор возможностей этой реализации нам хватает. Какие-то сложные регулярные выражения нам просто не нужны.

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

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

Возможно, читателям будет интересно узнать, что же это за такой код для поиска по регулярным выражениям. Перепечатаю его из книги - посмотрите, как элегантно. Это код был мной немного изменён при интеграции в PVS-Studio, но его суть не изменилась. Итак, код из книги:

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
// Формат регулярного выражения.
// c     Соответсвует любой букве "с"
// .(точка)  Соответсвует любому одному символу
// ^     Соответсвует началу входящей строки
// $     Соответствует концу входящей строки
// *     Соответствует появлению предыдущего символа от нуля до
//       нескольких раз
 
int matchhere(char *regexp, char *text);
int matchstar(int c, char *regexp, char *text);
 
// match: поиск соответствий регулярному выражению по всему тексту
int match(char *regexp, char *text)
{
  if (regexp[0] == '^')
    return matchhere(regexp+1, text);
  do { /* нужно посмотреть даже пустую строку */
   if (matchhere(regexp, text))
     return 1;
  } while (*text++ != '\0');
  return 0;
}
 
// matchhere: поиск соответствий регулярному выражению в начале текста
int matchhere(char *regexp, char *text)
{
   if (regexp[0] == '\0')
     return 1;
   if (regexp[1] == '*')
     return matchstar(regexp[0], regexp+2, text);
 
   if (regexp[0] == '$' && regexp[1] == '\0')
     return *text == '\0';
   if (*text!='\0' && (regexp[0]=='.' || regexp[0]==*text))
     return matchhere(regexp+1, text+1);
   return 0;
}
 
// matchstar: поиск регулярного выражения вида с* с начала текста
int matchstar(int c, char *regexp, char *text)
{
  do {   /* символ * соответствует нулю или
            большему количеству появлений */
    if (matchhere(regexp, text))
      return 1;
  } while (*text != '\0' && (*text++ == c || c == '.'));
  return 0;
}
Да, этот вариант крайне прост. Да, он мало что может, но вот уже много лет ничего боле сложного не понадобилось, и думаю, не понадобится. Это хороший пример, когда простое решение оказалось лучше полноценного.

Рекомендация

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

Вот некоторые возможные манёвры:
  1. Быть может, нужную функциональность уже предоставляет API вашей системы или одна из уже используемых библиотек, исследуйте этот вопрос.
  2. Если вы планируете использовать совсем маленький кусочек функциональности из библиотеки, то есть смысл реализовать его самостоятельно. Аргумент "лучше подключить библиотеку, вдруг потом ещё что-то понадобится" никуда не годится. Почти всегда из этой библиотеки в будущем больше ничего использоваться не будет. Программисты слишком тяготеют к универсальности, которая на самом деле не нужна.
  3. Если для решения задачи есть несколько библиотек, то выбирайте самую простую, которая удовлетворяет требованиям. Как я писал выше, гоните прочь мысли "на всякий случай взять библиотеку покруче".
  4. Прежде чем начать добавлять библиотеку, просто подождите и подумайте. Попейте чаю, отвлекитесь, обсудите задачу с коллегами. Возможно, в процессе выяснится, что можно решить задачу совсем иным путём, не прибегая к помощи сторонних библиотек.

P.S. Многим рассказанное здесь придется не по душе. Например, то, что я рекомендую использовать не переносимую универсальную библиотеку, а допустим WinAPI. На это будут возражения, основанные на том, что тем самым мы привязываем проект к одной операционной системе. И потом будет очень сложно сделать программу переносимой. Я с этим не согласен. Часто идея "потом перенесем на другу операционную систему" живет только в голове разработчика. На самом деле такая задача вообще может быть никогда не поставлена руководством. Или проект "загнётся" из-за излишней сложности и универсальности, ещё до момента популярности и необходимости портирования. Плюс не забывайте пункт (8) в списке проблем, приведенный выше.


42. Не давайте функциям название "empty"

Фрагмент взят из проекта WinMerge. Код содержит ошибку, которую анализатор PVS-Studio диагностирует следующим образом: V530 The return value of function 'empty' is required to be utilized.

C++
1
2
3
4
5
6
7
8
9
10
11
void CDirView::GetItemFileNames(
  int sel, String& strLeft, String& strRight) const
{
  UINT_PTR diffpos = GetItemKey(sel);
  if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
  {
    strLeft.empty();
    strRight.empty();
  }
  ....
}
Разъяснение

Программист хотел очистить строки strLeft и strRight. Строки имеют тип String, который представляет собой не что иное, как std::wstring.

Для очистки он вызвал функцию empty(). Это неправильно - функция empty() не изменяет объект, а только возвращает информацию о том, является строка пустой или нет.

Корректный код

Чтобы исправить ошибку, следует заменить функцию empty() на clear() или erase(). Разработчики WinMerge предпочли erase() и сейчас код выглядит так:

C++
1
2
3
4
5
if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
{
  strLeft.erase();
  strRight.erase();
}
Рекомендация

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

В одних библиотеках функция empty() очищает объект; в других - возвращает информацию о том, является ли объект пустым.

Слово "empty" плохое. Каждый понимает его по-своему: кто-то считает его "действием", кто-то считает его "запросом информации". Отсюда вся эта путаница и ошибки.

Выход только один - не делайте в своих классах функцию с именем "empty".
  • Называйте функцию, которая очищает объект "erase" или "clear". Хотя все-таки лучше "erase", имя "clear" тоже является неоднозначным.
  • Функцию, получающую информацию, можно назвать, например, "isEmpty".

Это весьма распространенный паттерн ошибки. Конечно, менять такие классы как std::string уже поздно, но давайте хотя бы дальше не умножать зло!


Заключение

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

Пользуясь случаем приглашаю всех желающих последовать за мной в Twitter: @Code_Analysis.

Желаю всем безбажных программ.

С уважением, Андрей Карпов.
Размещено в Без категории
Надоела реклама? Зарегистрируйтесь и она исчезнет полностью.
Всего комментариев 0
Комментарии
 
Новые блоги и статьи
Был там один разговор по поводу свободы в материальном мире.
kumehtar 19.08.2026
Суть: рассматривается живое существо, оказавшееся внутри довольно странной системы (этого мира) и пытающееся обустроить в ней свой кусок пространства. Жизнь действительно предъявляет каждому. . .
Когда логика программы не спасает от человеческих ошибок
Maks 18.08.2026
В последнее время всё чаще и чаще сталкиваюсь с таким явлением, как абсолютная невнимательность (или глупость) пользователей. Проявляется это чаще всего на работе в коллективе. Допустим, человек с. . .
Лето уходит
kumehtar 17.08.2026
Мысли в слух
kumehtar 17.08.2026
Забавно, насколько сейчас стала доступна информация. Например о магии, духовном развитии, медитациях, и других подобных направлениях, ранее зачастую тайных, передаваемых от учителя к ученику. Хотя. . .
Перемещение строк из ТЧ в другой документ с учетом текущего пробега
Maks 17.08.2026
Реализация из решения ниже выполнена на примере нетипового документа "Автозапчасти", с ТЧ "Шины". За основу взят алгоритм отсюда: https:/ / www. cyberforum. ru/ blogs/ 359708/ 10838. html Задача: . . .
Саморегулирующийся социальный контракт для сервера cross-section.
Hrethgir 14.08.2026
С кодом конечно таких глубоких размышлений пока не было, впрочем я уже привык к алгоритмизации. Суть предмета записи: снова в диалоге с нейросетью (я взял пока себе ник для учётки админа - Rector). . . .
Часы электронные
Uhbif79 12.08.2026
Выкладываю программу часов. Программа позволяет: 1. Использовать системное время и дату, 2. Есть возможность вводить время и дату вручную. 3. Реализованы 2 будильника: начало и конец рабочего дня. . . .
Часы с будильником на основе класса QLCDNumber
Uhbif79 12.08.2026
Всем добрый день, выкладываю программу часов с будильником на основе класса QLCDNumber. Здесь я пробовал самостоятельно создавал классы, впервые столкнулся с видимостью переменной одного класса из. . .
КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin
Copyright ©2000 - 2026, CyberForum.ru