4. Выравнивайте в коде всё, что возможно

Выравнивание кода уменьшает вероятность допустить опечатку или ошибки при Copy-Paste. Если ошибка все же будет сделана, то найти её в процессе обзора кода будет в несколько раз проще. Рассмотрим пример кода.

V537 Consider reviewing the correctness of 'maxX' item's usage. clist_modern modern_skinengine.cpp 2898



Монолитный фрагмент кода, читать который совершенно не интересно. Отформатируем его:



Это не самый показательный пример, но согласитесь, теперь стало намного проще заметить, что два раза используется переменная maxX.

Мою рекомендацию по выравниванию не стоит понимать буквально и везде строить столбцы кода. Во-первых, это требует дополнительного времени при написании и правке кода. Во-вторых, это может привести к другим ошибкам. В следующем примере, как раз желание сделать красивый столбик привело к возникновению ошибки в коде Miranda IM.

V536 Be advised that the utilized constant value is represented by an octal form. Oct: 037, Dec: 31. msn msn_mime.cpp 192



Желая сделать красивую колонку чисел, легко увлечься и вписать в начале '0', сделав константу восьмеричной.

Уточняю рекомендацию. Выравнивайте в коде всё, что возможно. Но не выравнивайте числа, дописывая нули.

5. Не размножайте строку более, чем один раз

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

V525 The code containing the collection of similar blocks. Check items '1316', '1319', '1318', '1323', '1323', '1317', '1321' in lines 954, 955, 956, 957, 958, 959, 960. clist_modern modern_clcopts.cpp 954



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

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

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

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

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

Рассмотрим некоторые примеры ошибок, которые могут быть оперативно выявлены анализаторами исходного кода:

V560 A part of conditional expression is always true: 0x01000. tabsrmm tools.cpp 1023



Допущена опечатка. Вместо оператора '&' используется оператор '&&'. Как здесь подстраховаться при написании кода, я не знаю. Корректный вариант условия:



Следующий пример.

V528  It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *str != '\0'.  clist_modern  modern_skinbutton.cpp  282

V528  It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *endstr != '\0'.  clist_modern  modern_skinbutton.cpp  283



В этом коде всего лишь забыты две звездочки '*' для разыменования указателей. Результат может быть фатальным. Этот код предрасположен к  access violation. Корректный вариант кода:



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

Следующий пример.

V514  Dividing sizeof a pointer 'sizeof (text)' by another value. There is a probability of logical error presence.  clist_modern  modern_cachefuncs.cpp  567





На первый взгляд все хорошо. В функцию передается текст и его длина, посчитанная с помощью макроса SIZEOF. На самом деле макрос следует назвать COUNT_OF, но не в этом дело. Беда в том, что мы пытаемся посчитать количество символов в указателе. Здесь вычисляется "sizeof(LPTSTR) / sizeof(TCHAR)". Человек такие подозрительные места замечает плохо, а вот компилятор и статический анализатор хорошо. Исправленный вариант кода:



Следующий пример

V560 A part of conditional expression is always true: 0x29. icqoscar8 fam_03buddy.cpp 632



Здесь уместна рекомендация писать константу в условии на первом месте. Вот такой код просто не скомпилируется:



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

Если программист отказывается от такого стиля сравнений, то ему остается или полагаться на компилятор/анализатор, или рисковать.

Кстати, несмотря на то, что про эту ошибку все знают, она не самая редкая. Ещё три примера из Miranda IM, где анализатор PVS-Studio выдал предупреждение V559:



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

V542 Consider inspecting an odd type cast: 'char *' to 'char'.  clist_modern  modern_toolbar.cpp  586



Фактически мы проверяем, что адрес строки не кратен 256. Мне не понятно, что на самом деле хотели написать в условии. Возможно, это даже правильно, но сильно сомнительно.

Анализом кода можно найти немало некорректных условий. Пример:

V501 There are identical sub-expressions 'user->statusMessage' to the left and to the right of the '&&' operator. jabber jabber_chat.cpp 214



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

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

Статический анализ представляет больший интерес в процессе разработки программы, а не в качестве разовых проверок. Множество ошибок и опечаток находится в процессе тестирования и создания юнит-тестов. Но если часть из этих ошибок можно найти ещё на этапе написания кода, то это будет колоссальный выигрыш времени и сил. Обидно два часа отлаживать программу, чтобы потом заметить лишнюю точку с запятой ';' после оператора 'for'. Такую ошибку можно часто обезвредить, потратив 10 минут на статический анализ измененных в процессе работы файлов. 

Заключение

В этой статье я поделился только некоторыми мыслями по поводу того, как допускать меньше ошибок при программировании на Си++. Зреют и другие мысли, о которых я постараюсь написать в следующих статьях и заметках. 

P.S.

Уже стало традицией, что после подобной статьи, кто-то спрашивает, а сообщили ли вы разработчикам программы/библиотеки о найденных ошибках. Заранее отвечу на вопрос, отправили ли мы баг репорт по проекту Miranda IM.

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

Ещё стоит пояснить, почему я часто не берусь судить ошибка тот или иной участок кода или нет. Пример неоднозначного кода:

V523 The 'then' statement is equivalent to the 'else' statement.  scriver  msglog.c  695



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