Не так давно, один из сотрудников покинул наш коллектив и присоединился к компании, занимающийся разработкой программного обеспечения, связанного с встраиваемыми системами. Ничего особенного в этом нет, всегда и везде, кто-то уходит, а кто-то приходит. Всё зависит от количества плюшек, удобства и предпочтений. Интересно другое. Человек искренне переживает за состояние кода на новом месте работы, что в результате и вылилось в эту совместную статью. Тяжело, "просто программировать", когда знаешь, что такое статический анализ кода.



Заповедники

Мне кажется, в мире сложилась интересная ситуация. Что происходит, если отдел программирования, это всего лишь небольшой вспомогательный элемент, не имеющий к основной сфере деятельности организации прямого отношения? Возникает заповедник. Сфера деятельности организации может быть сколь угодно важной и ответственной (медицина, военная техника). Всё равно образуется болотце, где завязают новые идеи и используются технологии 10-летней давности.

Приведу пару штрихов из переписки с одним человеком, работающим в отделе программирования на АЭС:

А он мне отвечает: Зачем нам git? Вот смотри, у меня всё в тетрадке записано.

...

А у вас вообще есть какой-то контроль версий?

2 человека используют git. Остальная контора в лучшем случае нумерованные zip'ы. Хотя насчет зипов, это я только про 1 человека уверен.

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

Хочу подчеркнуть, что застой в больших организациях явление международное. У иностранцев дела обстоят в точности также. Долго искал, но так и не смог найти одну очень подходящую статью. Название тоже не помню. Если кто-то подскажет, добавлю ссылку. В ней программист рассказывает историю, как работал в одном военном ведомстве. Ведомство было естественно жутко секретное и жутко бюрократическое. Настолько секретное и бюрократичное, что в течение нескольких месяцев не могли согласовать, какие права ему выделить для работы с компьютером. В результате, он писал программу в Notepad (не компилируя). А потом его уволили за неэффективность.

Лесничие

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

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

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

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

Предлагаю познакомиться, с подготовленным им документом "Как не надо писать программы". Многие пункты могут показаться написанными в стиле капитана очевидности. Однако, это реальные проблемы, которые он пытается предотвратить.

Как не надо писать программы

Пункт N1

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

Пункт N2

В условии оператора 'if' происходит не проверка значения, а присваивание:



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



Пункт N3

Запись вида "using namespace std;" в заголовочных файлах может приводить к тому, что будет использована эта область видимости во всех файлах, включающих этот заголовочный файл. Это может привести к тому, что будут выбраны не те функции или возникнет конфликт имен.

Пункт N4

Сравнение знаковых и беззнаковых переменных:



Помните, что при смешивании знаковых и беззнаковых переменных:

• может произойти переполнение;
• может возникнуть всегда истинное или ложное условие и как следствие вечный цикл;
• в знаковую переменную может быть помещено значение более INT_MAX (и она будет иметь отрицательное значение);
• переменная типа int при сложении/вычитании/... с переменной unsigned типа, тоже становится unsigned (отрицательные значения превращаются в большие положительные числа);
• прочие неожиданности и радости

Приведённый пример некорректно обрабатывает ситуацию, когда массив 'ba' пуст. Выражение "ba.size() - 1" имеет беззнаковый тип size_t. Если в массиве нет элементов, результат выражения равен 0xFFFFFFFFu.

Пункт N5

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



В данном примере оператор '=' перепутан с оператором '=='. Если бы переменная 'str' была объявлена как константная, то такой код даже не скомпилировался бы.

Пункт N6

Сравниваются не строки, а указатели на строки:



Даже если в переменной TypeValue будет находиться строка "S" такое сравнение всегда будет возвращать 'false'. Корректным будет использовать функции для сравнения строк 'strcmp' или 'strncmp'.

Пункт N7

Выход за границы буфера: 



Такой код может привести к тому, что несколько байт памяти находящейся следом за 'prot.ID' так же будет заполнена нулями.

Не следует путать sizeof() и strlen(). Оператор sizeof() возвращает полный размер объекта в байтах. Функция strlen() возвращает длину строки в символах (без учета терминального нуля).

Пункт N8

Недостаточное заполнение буфера:



В данном случае нулями будет заполнена не вся структура '*ptr', а  только N байт (N - размер указателя в данной платформе). Корректным будет такой код: 



Пункт N9

Некорректное выражение:



С точки зрения компилятора здесь нет ошибки, однако оно не имеет смысла, при выполнении всегда будет получено значение 'true' или 'false' в зависимости от операторов сравнения и граничных условий. Компилятор выдает предупреждение на такую запись. Корректно будет записать этот код так:



Пункт N10



Беззнаковые переменные не могут быть меньше нуля.

Пункт N11

Сравнение переменной со значением, которое оно не может достигнуть ни при каких условиях. Пример:



О таких случаях предупреждает компилятор.

Пункт N12

Выделяем памяти через 'new' или 'malloc' и забываем ее освободить через 'delete'/'free' соответственно. Например, может быть такой код:



Скорее всего, раньше в 'v2' сохранялся указатель на 'std::vector<int>'. Теперь из-за изменения части кода, это не нужно и сохраняются просто значения типа 'int'. При этом мы не освобождаем память, которую выделили под 'v1', поскольку раньше это было не нужно. Для того, что бы сделать код корректным, следует добавить выражение 'delete v1' в конец функции. Или использовать умные указатели.

А ещё лучше довести рефакторинг до конца и сделать 'v1' локальным объектом, раз его больше не надо никуда передавать:



Пункт N13

Выделение памяти через 'new[]', а освобождение через 'delete'. Или наоборот выделение - 'new', а освобождение через 'delete[]'. Почему это плохо, можно почитать здесь: "delete, new[] в C++ и городские легенды об их сочетании".

Пункт N14

Использование неинициализированных переменных:



В Си/Си++ переменная по умолчанию не инициализируется нулём. Иногда может казаться, что этот код работает. Это не так. Это просто везение.

Пункт N15

Возвращение из функции ссылки или указателя на локальные объекты:



После выхода из функции 'FileName' будет указывать на уже освобожденную память, поскольку все локальные объекты создаются на стеке, и дальнейшая корректная работа с ней будет невозможна.

Пункт N16

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

Пункт N17

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

Пункт N18

Жадничаем поставить скобки в математических выражениях. В результате получаем:



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



Здесь ошибка заключается в том, что макрос TYPE забыли окружить круглыми скобками. Поэтому сначала будет выполнено выражение 'type & A', а уже затем зачем '(type & A ) | B'. Результат - условие всегда истинно.

Пункт N19

Выход за границы массива:



Выражение 'mas[3] = 4;' обращается к несуществующему элементу массива, поскольку при объявлении массива 'int mas[N]' индексация его элементов возможна в интервале [0...N-1].

Пункт N20

Перепутаны приоритеты логических операций '&&' и '||'. Оператор '&&' имеет более высокий приоритет.



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

Пункт N21

Результат присваивания не будет иметь эффекта за пределами функции:



Указателю 'a' не может быть присвоено другое значение адреса, для этого следовало бы записать объявление функции в таком виде:



или:





Список рекомендуемой литературы:

1. ВЕРЕВКА ДОСТАТОЧНОЙ ДЛИНЫ, ЧТОБЫ ВЫСТРЕЛИТЬ СЕБЕ В НОГУ. Правила программирования на С и С++. Ален И. Голуб;
2. Стандарты программирования на С++. 101 правило и рекомендация. Герб Саттер, Андрей Александреску;
3. Совершенный код. С. Макконнелл;
4. Скользкие места Си++. Стефан К. Дьюхэрст;
5. Эффективное использование C++. 50 рекомендаций по улучшению ваших программ и проектов. Скот Майерс.

Заключение

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

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

Вот и получается странная картина. Молодой фрилансер может выполнять работу более качественно (благодаря знаниям: TDD, непрерывная интеграция, статический анализ, система контроля версий, ...), чем программист 10 лет проработавший на РЖД/АЭС/(подставить что-то крупное). Слава богу, это далеко не всегда так. Но всё-таки что-то такое есть.

Почему меня это огорчает? Туда бы продавать PVS-Studio. А там даже не догадываются о существовании и пользе таких инструментов. 



Это сообщение отредактировал(а) Thunderbolt - 26.9.2013, 16:41

[quote]Пункт N2

В условии оператора 'if' происходит не проверка значения, а присваивание:

Выделить всёкод C++
1:
if (numb_numbc[i] = -1) { }[quote]А ему не приходило в голову, что замена присваивания сравнением меняет алгоритм? Что имелось ввиду "присвоить -1 и если она к bool приводится как true", а сравнение означало бы "если элемент уже равен -1"? Хотя в случае константы смысла мало, но оэто как раз пример того, почему предупреждения кажутся шагом назад от простой и понятной выдачи с одними ошибками, а статический анализ вообще недоступен пониманию: ни один нормальный прогер не мыслит символами, все думают действиями и конструкциями, двойное же равенство или набирается автоматически, или уровень проги - учебная поделуха, в которой и подсветка то излишня. Не упираться в присваивание надо, а объяснять действительно сильные стороны предупреждалки и анализа.

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

 Как он вообще написан? Для разработки функции мало знать, что ей разрешено и запрещено менять и какого типа её параметры, а надо точно знать её назначение. И если уж меняешь, то надо знать. как именно, что исключает константность. Или уж не надо менять вообще. Флаг константности не достаточно информативен, если полагаешься на него, то там, где надо всё таки менять параметр, не будешь знать, что именно присвоить. Перепутать == и = - то же самое, что перепутать = и := на паскале (распространённейшая, но исключительно ученическая ошибка), LET и PRINT на бейсике (вообще не видал), или вместо  написать .

Добавлено @ 15:51
std::string сравнивает именно строки. Его и надо использовать. А кто сравнивает указатели вместо строк, ничего хорошего и с таким документом не напишет.


Это сообщение отредактировал(а) ТарасАтавин - 2.9.2013, 15:52

ТарасАтавин, что за бред ты несешь? Причем тут паскаль? В паскале нельзя сделать присваивание внутри условия. И нельзя сделать сравнение вне. Так что этот пример вообще не катит. А про константность ты зря. Приведенный пример - лишь условность. Если не ошибаюсь, он даже в таком виде не скомпилируется. Но смысл от этого не меняется (поменяй std::string на int). Константность очень сильно упрощает жизнь.
А потом, тут идет реклама программы для статического анализа кода. А не автоматического исправления. Не путай вещи. В данном случае программист сам принимает решение об исправлении, а pvs-studio лишь указывает что и где ей кажется "странным".

Добавлено через 1 минуту и 8 секунд
Ты это расскажи программистам на С.

За домом наблюдали трое: математик, физик и биолог. Они видели, сколько народу входило, а сколько выходило. Обычно всё сходилось. И вдруг вошло двое, а вышло трое. Физик:
- Это ошибка наблюдения.
Биолог:
- Люди имеют привычку размножаться.
Математик:
- Если сейчас войдёт ещё кто то, то дом будет снова пуст.
За ними наблюдал программист и изрёк мораль:
- Всегда инициируйте переменные.

Это не мастера, а возомнившие себя мастерами. А ошибка ученическая, уровень допускающих её - писать поделухи до 60-ти строк. И не больше. Даже студенты специальности "учитель труда" от этой ошибки избавляются к четвёртому занятию.

Это сообщение отредактировал(а) ТарасАтавин - 3.9.2013, 05:17

Мифы о статическом анализе. Миф второй – профессиональные разработчики не допускают глупых ошибок.

А кто сказал, что это единственная глупая ошибка? Забывают даже о неоднозначности выбора перегруженной функции, приведении типов, неявных вызовах  конструкторов фактических параметров, расстановке signed/unsigned, забывают, что не только нуль-терминальная строка является указателем, но и, например, Сanvas. Когда явно объявленный указатель пытаются сравнивать с кавычками, имея ввиду сравнение строк, то это уже незнание языка. А кто не знает языка, но пишет на нём? Ученики. Но бывает, что sizeof применяют к указательному полю чужого, а иногда и стандартного класса, подразумевая объект. И даже не не понимают, что следует из четырёхбайтного размера поля Canvas. Это тоже глупые ошибки, но для их ловли действительно нужен инструмент. А не способность отличить сравнение от присваивания, заметить в собственном тексте звёздочку перед идентификатором, или вспомнить, что обычные массивы реализуются на указателях - это уже незнание языка. И признак константности здесь уже не поможет.

Это сообщение отредактировал(а) ТарасАтавин - 3.9.2013, 09:03

. Здесь конструктор и оператор приведения типа имеют одно назначение, но могут выполнять фактически разные операции, приводить по разным алгоритмам и иметь разные результаты применения в одной и той же ситуации. Программист, допустивший такое безобразие может быть профессионалом, но забыть о нём при вызове функции с прототипом  и экземпляром A в роли фактического параметра. А потом удивляться, что привелось не правильно. Можно до покраснения доказывать, что один профессионал ни когда не допустит одновременно двух не эквивалентных приведений. А два? Если классы A и B разработаны двумя разными членами команды? Если не профессионально руководство? А может быть это вообще сделано намеренно, а в проекте есть ещё и десятка два, функций приведения, различно решающих ту же самую задачу? При фактическом вызове нельзя полагаться на определённые приоритеты компилятора просто потому, что тогда выбор приведения станет неявным, что отрицательно скажется на читабельности и увеличит вероятность ошибок из-за забывчивости о том, что приведений то два. По закону Мёрфи о том, что всё, что может быть не правильно понято, именно так и будет понято, кто нибудь обязательно прочитает одно приведение, а сработает другое. В таких случаях надо или явно указывать оператор приведения типов, или явно декларировать объект с параметром. И не только в функциях, но и при присваивании экземпляру B экземпляра A. И даже в "математических" выражениях с участием экземпляров обоих классов. Но об этом умудряются забывать. И ещё спорят до покраснения, что конструктор и оператор приведения по определению имеют противоположное назначение и конфликтовать не могут, забывая о том, что они могут принадлежать разным классам в приводимой паре и тогда их назначение совпадёт.

Это сообщение отредактировал(а) ТарасАтавин - 3.9.2013, 09:30

  ТарасАтавин, очень прикольно слушать подобные выступления и нравоучения от человека, который сравнивает символ с строковым литералом, не может строку разбить на подстроки и пр. Может стоит наконец осознать смысл анекдота про чукчу: "... Чукча не читатель, чукча - писатель"?

Я сравнивал результат оператора [], при этом юзал привычные символы, какими выделяется строка, что для экспасквилянта, привыкшего, что паскаль не различает ограничители строк и символов - типичная глупая ошибка. И описанного выше безобразия с дублирующими друг друга оператором и конструктором у меня вообще не может не быть. Остаётся радоваться, что хоть избегаю неявного приведения, но иногда забываю. Точное написание оператора - это совсем другое, это как раз всё равно, что не писать is вместо == и will вместо =.

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

Нынче, это сделать сложно. Мы же в нотепадах пишем. А нормальная IDE отступ не поставит в этом случае.   

Так что же, всё-таки, такое "заземлённые указатели"?