Pull to refresh

Comments 102

Судя по их репозиторию пофиксили практически все замечания из предыдущей статьи и некоторые из этой. За последние три дня в коде, где встречались ошибки, был проведен рефакторинг, но даже при рефакторинге видел как добавили новый баг(themes.cpp 1227), но через время исправили. Странно, но первый коммит который был с багом был помечен как «require for review» и скорее всего должен был проверен другим мейнтенером…
UFO just landed and posted this here
Но при всем этом Миранда — просто божественный клиент для своего времени. Даже лет 8 назад всякие там Qip и прочее просто смешны были, Миранда имела плагином, наверное, даже полноценную ОС, игры и текстовый редактор =) Такое вряд ли кому снилось. Но, увы, сейчас все пошли по пути упрощения и рулят Adium и прочие. Эх, Миранда!
ОС, игры и текстовый редактор. В мессенджере. Оооок.
Говорят, Nero и ACDSee тоже этим путём пошли, но к успеху это их почему-то не привело.
Нет, тут несколько иная схема.

Они ничего не встраивали, основное ядро было «только чатица» было легкое и грамотное, вроде бы оно даже icq протокол (Oscar) не умело и его надо было доставлять. А все что хочешь сверху — пожалуйста, на плагинс.миранда.орг, а там уже фантазия ничем не ограничивалась.

А что до неро и асидиси — они в жизни не дадут отключить эту блоатварную вакханалию, это неюзабильное ПО =)
UFO just landed and posted this here
Я до сих пор ею пользуюсь (хотя и гораздо реже, чем когда-то).
Могу подтвердить, что программа действительно не самая надежная и стабильная.
Но все альтернативы тогда чем-то не устроили, а сейчас уже по инерции.
Вот читаешь и думаешь, а как оно вообще работает с такими ошибками?!
Мне кажется, в закрытом ПО их в сотни раз больше. Так что совершенно не стоит ужасаться странному коду :) Вот чисто по ощущениям, когда сам пишешь код, который будет читать кто-то кроме тебя, уже совершенно иначе голова работает))))
С чего вы решили, что закрытый код == никто, кроме тебя, не читает???
Это зависит очень сильно от кучи факторов:
  • Один программист в компании (бесконечно частая ситуация)
  • Компания в целом не ориентированная на качество (такое — сплошь и рядом)
  • Компания со средним низким уровнем программистов (править и корректировать некому)
  • Один программист по конкретно данному ЯП

Поэтому даже работая в огромной компании с кучей народу имеющего доступ к коду вероятность, что его кто-то прочтет — не такая уж и высокая. Кроме того, очень сильно зависит от компании — можно ли сказать John'у, что его код — ГОВНО. Или же Вася настолько ценный сотрудник, что любая критика расценивание как заявление по собственному?

А вот репо на GitHub добавленный еще и в свой профиль — это уже вызов, что мол смотрите на мой код. P.S. за часть моего кода на GitHub мне-таки стыдно, да, но я работают над искоренением того, за что стыдно =)
Как будто в закрытом ПО кто-то пишет код, который никто не увидит. Ну если это конечно не shareware утилита, делаемая одиночным разработчиком. Есть коллеги, начальники, есть кто сопровождает код или делает review.
Я анализировал код не очень большого количества закрытых проектов. Open-source было больше. Тем не менее мое ощущение такое: одинаково.
По тому, что видел я, вывод такой: плотность ошибок в закрытом и открытом программном обеспечении приблизительно одинакова.
Андрей, безусловно, но я указал много случаев, когда критику слушать не будут и поэтому чисто психологически программисты ее не бояться. Я это говорю сугубо из своей практике, когда обнаруживал «перлы» программистов уже постфактум и очень сильно матерился, что в свое время дал свободу и не лез со своей критикой. А открытый проект — это challenge и твое лицо. Если про курсовую написанную копипастом забудут (и слава великому Столлману!), то даже средне-популярный открытый проект написанный с использованием худших практик программирования выпилить из Интернета и памяти людей — почти нереальная задача :)
UFO just landed and posted this here
Сейчас ковыряюсь со статическим анализом опенсорсных Java-проектов. Бывает ещё:
5. Поведение рабочее, хотя и не такое, как имел в виду программист. Например, программист подразумевал в определённом случае к поисковому запросу автоматом подписывать слева звёздочку, но реализовал это неверно. В результате в этом случае ищется подстрока только с начала, а не с произвольного места. Пользователь может и не подумать, что это баг.
6. Очень много ошибок в путях обработки ошибки, потому что они редко тестируются. То есть, если всё хорошо, то будет всё хорошо, но если что-то упало, то, например, мы увидим не внятное исключение в логе или красивое сообщение об ошибке, а какое-нибудь совсем другое неадекватное исключение из другого места.
7. Попадаются ошибки, которые проявятся на некоторых системах. Например, (Java):
void processFilePath(String filePath) {
  String[] components = filePath.split(File.pathSeparator);
  ...
}

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

Или вот менее тривиальный пример:
static Map<String, Integer> protocolPriority = new HashMap<>();
static {
    protocolPriority.put("icq", 1);
    protocolPriority.put("jabber", 2);
}

public static Integer getPriority(String protocol) {
    return protocolPriority.get(protocol.toLowerCase());
}

public static void main(String... args) {
    System.out.println(getPriority("ICQ"));
}

Программа распечатает 1 в большинстве стран, но в Турции напечатает null (можете запустить с -Duser.language=tr). Думаю, туркам вообще несладко живётся — куча софта у них глючить должна.
А можете пояснить особенность поведения для Турции?
У них есть i с точкой и без точки (пары İi и Iı).
У них есть i с точкой и без точки (пары İi и Iı).
В предпросмотре у меня выглядело, как положено… Шрифты, однако.
Скрытый текст

Спасибо. Век учись, как говорится.
А под другие языки делать анализатор не хотите? В faq не нашел)
Не хотим. Нами даже рынок C++ ещё не освоен.
Плюс С++ просто шикарное поле деятельности для таких инструментов. :)
А жаль. Кмк ту же Java намного проще анализиовать — в нй ведь почти нет возможностей для разных хаков…
А Java не нужно так анализировать просто. Там 2/3 этих ошибок просто не сделать.
С java точно такая же история, в одном нашем проекте 10500 классов, во втором 4500 классов (суммарно 1,2 млн строк кода на java, на всех языках вместе 3,7 млн. строк по версии программы cloc). Я тоже был когда-то под впечатлением и ожидал серебряной пули и пробежался несколькими популярными анализаторами. Основная масса предупреждений — ложная либо практически бесполезная(always true, always false, дублирующиеся условия, копипаста и тп), их количество — огромно, времени на разбор руками надо очень-очень много, при этом риск внести новую ошибку(нет, не банальную опечатку) при исправлении старого когда тоже существует, код писался десятками людей и логика не всегда очевидна. При этом приоритетные баги остаются необнаруженными. В итоге от затеи я отказался.
А вы проверяли код уже протестированный или сразу из под программиста?
Проверялся весь проект сразу, но проблема в том, что быстро и безопасно этого не сделать. Лучший вариант это проверка перед коммитом или во время редактирования, примерно как это реализовано в идее. Надо еще понимать, что если приложение стабильно работает, от статического анализа толку будет не очень много, скорее всего все что вы исправите на самом деле ни на что не влияло, т.к. функционал оттестирован по пользовательским сценариям.
Для этого же приложения заказывали платный код-ревью, и насколько я понял он свелся к прогону статическим анализатором и заглядыванием в пару классов, по результатам все ок.
Статический анализатор как раз нужно использовать сразу в инкрементально-интерактивном режиме. К примеру, FindBugs в Java можно настроить, чтобы выполнялся автоматически после сохранения файла. Вы допустили глупую ошибку, анализатор вам сразу её подчеркнул. Вот вы и сэкономили пару минут на цикле «запустить, нажать на нужные кнопки в UI, убедиться, что ничего не работает, в отладчике посмотреть пару переменных, найти ошибку». Если вы запустите статический анализатор после, он вам конечно меньше найдёт, но время вы уже потеряли.

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

Имеет смысл проверять до ручного тестирования инкрементально. То есть начать с того, что поместить в игнор все что существует сейчас, а потом вставить его где-то между компилятором и тестером, лучше ближе к компилятору.
Для Java есть FindBugs.
Плюс нормальные среды разработки умеют сами много чего находить.
В этом плане полезно поставить sonar и фигачить в него исходниками (например, через хук репозитория, maven, gradle etc). Можно локально, можно на сервере. Раньше там был findbugs, pmd, checkstyle и т. п. Какое сейчас состояние по плагинам — не знаю.
Если интересует Perl, то есть Perl Critic (а также крутая книга — Perl Best Practics). Там очень крутой анализатор, он по части стиля и приемов кодирования, но в случае Perl — это решает половину возможных проблем просто запретом ряда фич =)
UFO just landed and posted this here
UFO just landed and posted this here
The first argument of 'memcpy' function is equal to the second argument.
Это вообще UB согласно стандарту, потому что он требует, чтобы источник и приемник у memcpy не пересекались. Хотя на практике будет работать, конечно.
Хотя на практике будет работать, конечно.

Закон Мёрфи тут тоже действует: lwn.net/Articles/414467/
Случай одинаковых аргументов тем изменением затронут не был…
Два одинаковых аргумента тем более указывают на пересекающиеся области памяти.
Да, но ошибка при этом «чисто случайно» не возникает
Вообще, странно, что такие ошибки в коде присутствуют. Даже встроенные в IDE анализаторы кода часть таких ошибок детектируют и выводят предупреждения. Видать, совсем глаза закрывают на предупреждения — знакомая ситуация, признаться. Компилируется — вот главный показатель! :)
V547 Expression 'tszValue.GetBuffer()[0] == TCHAR(9835)' is always false. The value range of char type: [-128, 127]. VKontakte vk_thread.cpp 354
V547 Expression 'StatusText.GetBuffer()[0] != TCHAR(9835)' is always true. The value range of char type: [-128, 127]. VKontakte vk_thread.cpp 1055

Я не стану комментировать никакие другие детекты из этой статьи, но этот код написан мной, так что не могу пройти мимо. Как мне кажется, здесь речь идет, как минимум о неточном, а как максимум – о ложном срабатывании. В обоих случаях сравниваются TCHAR значения. В моем случае проект однозначно не ANSI и никогда, я подчеркиваю, никогда им не будет, а, следовательно, TCHAR разворачивается только в wchar_t и никогда в char. Конечно, можно порассуждать о том, что теоретически такое все-таки может случится, но в любом случае говорить о том, что «Expression TCHAR(9835)' is always true» нельзя, ведь PVS же проверяет при анализе определено _WIN32 или нет. Можно лишь говорить, что выражение может быть при определенных условиях, всегда истинным.
Теперь что касается вообще. Я лично не считаю статические анализаторы бесполезными, потому вчера скачал PVS-студию и прогнал ей свой проект (VKontakte). Из 37 детектов (это L1 и L2, только GA) 2 оказались полезными, остальные, в том числе и “проверка на NULL после использования”, оказались ложными. Мой проект сравнительно небольшой, я знаю логику его работы, он не заброшен и активно развивается, потому я могу потратить полчаса на анализ PVS-студией. Заплатил бы я за нее деньги (ведь цель этих двух статей – привлечь покупателей, в первую очередь)? С таким процентом полезного выхода – однозначно нет, учитывая, что разрабатываемый проект бесплатный.
Будь у меня более двух миллионов строк кода, причем, в значительной массе своей, не моего кода, кода в проектах, которые заброшены/редко используются и взяты в общий транк часто “за компанию” и адаптированы в NG на скорую руку, при том, что большинство таких багов сработают в экзотичных условиях, я бы, возможно, как это сделал Хазан, тоже сравнил бы такую помощь с бесплатной футболкой, за которой надо ехать в соседний город.
Тем не менее, ругать и опускать PVS студию мне не за что, поскольку моему проекту она пользу принесла. Считаю необходимым и правильным поблагодарить Вас за проделанную работу. Так же, пользуясь случаем, хочу поблагодарить и Ivan_83 за его исправления. В любом случае, считаю, что эти две статьи принесли проекту Miranda NG пользу.
Теперь что касается вообще. Я лично не считаю статические анализаторы бесполезными, потому вчера скачал PVS-студию и прогнал ей свой проект (VKontakte). Из 37 детектов (это L1 и L2, только GA) 2 оказались полезными, остальные, в том числе и “проверка на NULL после использования”, оказались ложными. Мой проект сравнительно небольшой, я знаю логику его работы, он не заброшен и активно развивается, потому я могу потратить полчаса на анализ PVS-студией. Заплатил бы я за нее деньги (ведь цель этих двух статей – привлечь покупателей, в первую очередь)? С таким процентом полезного выхода – однозначно нет, учитывая, что разрабатываемый проект бесплатный.


Прокомментирую эту часть. Хоть лично Вы это 100% понимаете и написали ниже, для других хочу донести и нашу позицию. Однозначно статический анализ полезен прежде всего на средних и больших проектах. Если код проекта может просмотреть за вечер один человек, то так и надо делать (время от времени). И если разрабатывается проект одним человеком — тоже ему никто не предлагает тратить кучу денег на статический анализ. Может ли быть польза для «малышей» (маленьких проектов) от статического анализа? Да может. И обучение, и поиск ошибок все-же. Стоит ли тратить «малышам» (маленьким проектам) деньги на статический анализ? Скорее всего нет.

И спасибо за подробный комментарий и оценку нашей работы.
ОК ей, наш проект большой:

6 миллионов строк. Возраст кода до 8 лет. Авторов 3/4 кода давно в проекте нет. Документация вида «код — лучшая документация». В проекте огромное число вот таких вот ошибок зачастую в OutOfCoverage. Куча макросов из соображений «надоело писать одинаковый код». Некоторые файлы тысяч на 80 строк. В принципе не существует людей которые бы знали весь код. Никто не сможет гарантировать как одно повлияет на другое. Ну и прочие прелести проприетарного промышленного кода с огромным багажом.

Но при этом качество работы продукта заказчика устраивает. Более того 24/7.

Поробовал PVS — упал. Убрав особо-изощренные файлы PVS отработал. Среди найденного количества ошибок (оно в демке вроде ограниченно было сотней) самое страшное что нашлось — присвоение в if в том месте которое не используется вообще. Остальное разный мусор из серии депрекатов.
Вывод: да что-то откровенное оно нашло, но только не в полезном коде. Тратить время разработчиков на разбор ложноположительного — глупо, а его там откровенно много. Риск исправлением внести новый дефект — очень высокий. Зачастую «это жжж тут не спроста».

Качество продукта устраивает, при этом существуют известные дефекты, которые имеют конкретный сценарий.

Предположим я куплю продукт и он найдет мне 1000 косяков, мы потратим X человекочасов разработчиков на исправление. Потом X*K человекочасов тестеров на полную перепроверку ВСЕГО функционала ибо эти косяки будут размазаны ровным слоем по всему коду. Итого: потраченные деньги, потраченное время, сомнительная выгода по качеству продукта.

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

Статические анализаторы — для поиска свежих ошибок, а не залежавшихся.
Предположим я куплю продукт и он найдет мне 1000 косяков, мы потратим X человекочасов разработчиков на исправление. Потом X*K человекочасов тестеров на полную перепроверку ВСЕГО функционала ибо эти косяки будут размазаны ровным слоем по всему коду. Итого: потраченные деньги, потраченное время, сомнительная выгода по качеству продукта.
Работа с возражениями: статический анализ будет отнимать часть рабочего времени.
Не могу пройти мимо TCHAR… Зачем вообще его использовать, если проект никогда не уйдет с Юникода? Особенно в таких местах, где кроме как с Юникодом работать невозможно. Было бы куда правильнее использовать тип CMStringW вместо CMString, и сравнивать с числом без приведения к TCHAR.
Тут дело скорее вкуса и привычки. Ну и есть еще такая вещь, как доставшийся по наследству код, в котором для юникода уже повсеместно используется TCHAR/ptrT/CMString.
Допустим, этот пример был бы написан правильно:
#ifdef _UNICODE
#define MUSIC_CHAR 9835
#else
#define MUSIC_CHAR 0x0E
#endif
// ...
if (tszValue.GetBuffer()[0] == TCHAR(MUSIC_CHAR))

для PVS-студии что нибудь изменилось бы?
Нет. Это недоработка в PVS-Studio. Со временем исправится. :)
Мне кажется, что вы не совсем правы, утверждая, что «Из 37 детектов (это L1 и L2, только GA) 2 оказались полезными».
Польза детектов может быть не только в обнаружении ошибок, но и в обнаружении мест, которые, не смотря на то, что работают идеально правильно, и более того, так и задумывались, могут на самом деле быть «плохими».

Проект ведь OpenSource. Любой человек может придти и начать работать с кодом. Вот что я подумаю, когда увижу TCHAR? Вряд-ли именно то, что «проект однозначно не ANSI и никогда, я подчеркиваю, никогда им не будет». Как раз таки обратное, TCHAR ведь. И я уже введён в заблуждение(как и анализатор). И возможно, из-за своих заблуждений(ведь там ещё было, по вашим же словам 35 «бесполезных» срабатываний) я внесу баг, который попротит много нервов как разработчикам, так и пользователям.

Пусть анализатор и не показывает реальных ошибок в тех местах, но он как минимум показывает множество не самых хороших решений, которые автору кода, возможно, казались удачными. И исправлениепереписывание которых поможет новым(а может и старым, но работавшим над другой частью проекта) разработчикам допускать меньше ошибок.
Другими словами PVS-Studio выдает не список ошибок, а список мест, которые стоит посмотреть разработчику и возможно что-то улучшить/поправить там.

Да, конечно лучше бы анализатор выдавал список ошибок. Тогда бы его можно было продавать в десятки раз дороже :-). К сожалению, это невозможно.
Да, это ложное срабатывание. Видимо анализатор подзапутался с шаблонными типом переменной tszValue. Неприятно, но не страшно. Если такое возникает у пользователя или человека, желающего стать пользователем, мы устраняем такие ошибки. Правки, доработки, и поддержка являются составляющей частью приобретённой лицензии. Многим нравится наша поддержка и они вновь и вновь продлевают лицензию. Кстати, в анализаторе уже достаточно много специфичных настроек для различных проектов. Они скрыты от глаз, так как предназначаются для конкретных проектов клиентов.

Хочу немного поговорить на тему полезного выхода (процента ложных срабатываний).
Я не планирую оправдывать инструмент. Ложные срабатывания — это зло, и мы боремся с ними как можем. Но следует учитывать, что есть несколько факторов, узнав про которые люди начинают по-другому смотреть на этот вопрос. Напомню их для читателей.
  • Основная ценность статического анализа в его регулярном применении. Аналогия — предупреждения компилятора. Первый запуск статического анализа для работающего проекта всегда несколько бестолков. Большинство ошибок, который мог бы при разработке проекта найти статический анализ уже исправлены более дорогими методами тестирования. Если бы это было не так, проект бы не работал. Подробнее на эту тему есть хорошая заметка: "Лев Толстой и статический анализ кода".
  • Небольшие проекты дают немного искажённое представление о пользе статического анализа. Дело в том, что в них ниже плотность ошибок. Proof: "Ощущения, которые подтвердились числами".
  • Статический анализатор требует хотя-бы минимальной настройки. Мы стремимся, чтобы даже первый запуск был полезен (вывод не был забит ложными срабатываниям). Но далеко не всегда так получается. Поэтому в PVS-Studio есть масса инструментов для борьбы с ложными срабатываниями. И часто можно очень быстро отсеять большинство из них. Статья на эту тему: "Работа с ложными срабатываниями в PVS-Studio и CppCat".

UFO just landed and posted this here
Запоминает в случае использования std::make_shared, верно?
UFO just landed and posted this here
Главное при этом указатель правильного типа в конструктор (конструктор — шаблонный) передать, вот так будет работать:
std::shared_ptr<Base> a(new Derived);
а вот так уже, разумеется, нет:
Base* b = new Derived;
std::shared_ptr<Base> c(b);
UFO just landed and posted this here
UFO just landed and posted this here
Да, ясно. Впрочем, тут тоже есть где использовать умные указатели — но выгода от них не настолько большая.
Эээ… А как это работает? Откуда в первом случае берётся информация о типе? Мне казалось, эти две конструкции эквивалентны.
Из шаблонного конструктора:
template <T> class shared_ptr {
  //...
  template <Q> shared_ptr(Q* ptr);
};

Примерно так оно выглядит внутри. Соответственно, несмотря на то, что создается умный указатель на T — дестурктор запоминается для типа Q.
У std::shared_ptr конструктор шаблонный и он создает deleter на основе типа переданного объекта, а не на основе типа самого умного указателя, и только после этого происходит конвертация полученного указателя в тип умного указтеля.
Кстати, есть ещё подобная красивая «магия» при биндинге временной переменной на ссылку:
const Base& base_ref = Derived(); // Или любая функция, возвращающая Derived по значению
По окончании времени жизни ссылки будет вызван деструктор Derived, а не Base.
UFO just landed and posted this here
Все просто: использование наиболее общего класса из возможных считается хорошей практикой.
UFO just landed and posted this here
Я тоже не знаю, где это можно применить. Насколько я понимаю, это побочный эффект того, что правила инициализации для ссылок-локальных переменных и ссылок-параметров функций одинаковы. А, скажем, для функции вида void f(const Base&) вызов f( Derived() ) вполне осмысленен.
Анализатор PVS-Studio (в отличии, например, от C4265 в Visual C++) ругается не на все классы без виртуального конструктора. Он негодует только в том случае, если явно вызывается оператор delete для базового класса. Подробнее: Описание V599.
То есть на вот такой код он ругаться не будет?
// У класса Base нет виртуального деструктора
Base *base = new Derived();
std::shared_ptr<Base> ptr = base;
К сожалению нет. Кстати, спасибо за пример. Выписал.
UFO just landed and posted this here
Начнёт. Не хочу вдаваться в подробности. Главное не забывать, что это статический, а не динамический анализ. У него меньше знаний о том, что это за указатель, откуда он пришёл и т.д. www.viva64.com/ru/b/0248/
UFO just landed and posted this here
UFO just landed and posted this here
Тут проблема в ++i, вместо него должно быть (i + 1), и выражение обретает нормальный вид, не считая, конечно, явно впечатанной константы 50.
Чего-то я поспешил, согласно новому стандарту (C++11) здесь нету неопределенного поведения, вот если ++i заменить на i++, то появится. Старый стандарт (С++03) имел более строгие правила, согласно ему это UB.
Так и было предупреждение для старого стандарта (видно из наличия термина «sequence points»).
Я сейчас ощутил, что анализатор PVS-Studio становится стареньким. :) В том смысле, что он поддерживает ещё VS2005 и так далее. А значит должен для разных версий языка иногда выдавать разные предупреждения или не выдавать. Как в этом примере. Конкретно, в этом случае думаю ничего не надо изменять в анализаторе. Пусть лучше ругается. Все равно код не очень хорош и его стоит поправить.
Выражение while — не самый лучший выход, потому что изменить переменную i внизу цикла легко позабыть. Конечно, это совсем не значит, что нужно всякий раз писать что-то вроде for (int i=0, j=1; i<n; i=j-i, j=1-j) — но операция инкремента по модулю, как мне видится, все же довольно простая для понимания.
Лично я за 4 года использования clang понял, что статический анализ кода на сях/плюсах мне не нужен. Я не говорю, что он не нужен никому, я говорю, что он не нужен лично мне. Я пишу на C/C++ уже более 10 лет, причем пишу на нем в основном только low-level структуры данных/алгоритмы, драйвера какие-нибудь и т.д. Учитывая специфику решаемых задач, почти всё время уходит на мыслительный процесс, а не на написание кода. Ошибки в коде конечно же бывают, но для того, чтобы их найти, нужен глубокий человеческий анализ и понимание алгоритма. Статические анализаторы находят только совсем очевидные вещи, учитывая, с какой скоростью я пишу код и как обдумываю каждую строчку, у меня таких ошибок просто не возникает. Статический анализ полезен только в проектах, где пишут мало осмысленный код с огромной скоростью (в основном это десктопные приложения на плюсах, вроде той же миранды).
Приглашаю незамедлительно скачать PVS-Studio и попробовать его на своём проекте.

У вас на сайте вообще нигде (кроме страницы скачивания) не написано, что это всего лишь плагин к Visual Studio, хотя используется громкое название «статический анализатор PVS-Studio». Нужена как минимум standalone-версия, чтобы можно было по-быстрому проверить проект и не ставить ради одной проверки Visual Studio. А ещё неплохо бы иметь не только кросс-IDE версию, но и кросс-платформенную, чтобы под Linux и OSX можно было проверять.
Ок, standalone есть. Теперь бы заставить эту standalone-версию работать под OSX и Linux, было бы замечательно.
Ну, чтобы называться статическим анализатором, standalone версии под винду вполне достаточно :)
Об этом можно и написать на сайте — так и так, есть плагин к VS и standalone, etc… А то сейчас страница скачивания вводит в заблуждение. Когда я последний раз пробовал PVS, этого не было.
> у меня таких ошибок просто не возникает.
Даже гроссмейстеры могут получить детский мат.
Могут, но не нужно забывать, что описываемый анализатор платный и его покупка должна быть целесообразна. В моем случае это не целесообразно. А возможность такой ошибки в своем коде я не отрицаю. Возможно судить обо всех статических анализаторах по clang неправильно, но тем не менее, он то бесплатный (и встроен в компилятор), и я могу его позволить как «дополнительный» уровень защиты от всякого рода ошибок, даже если он мне не помогает. Другое дело PVS Studio, даже если он нашел бы ошибку там, где не справился clang, много ли было бы таких случаев? Вряд ли. Потому что почти все ошибки связаны с логикой работы, а такие ошибки не поддаются статическому анализу.
Гляньте вот эту стаьтью: habrahabr.ru/company/pvs-studio/blog/233179/
PVS Studio и clang находят совершенно разные классы ошибок. В этом блоге неоднократно были отчеты о проверках опенсорсных проектов, проверяемых регулярно другими анализаторами — но PVS все равно что-то да находила.
Говорю же, ошибки, которые обнаруживаются статическим анализом — это очень маленькое подмножество возможных ошибок. Не важно, что конкретно ищет анализатор, даже идеального анализатора будет мало чтобы окупить его стоимость. Ни компилятор, ни анализатор не знают, что за алгоритм вы пишите и как он должен себя вести. А вот хорошие юнит-тесты очень даже помогают.
Мне кажется основное преимущество — это очень раннее обнаружение ошибок. Допустим вы сделали какую-то глупую опечатку (скобку не там поставили, например), а она не привела к ошибке компиляции. Конечно же у вас ничего не заработает, и вы с помощью отладки или внимательно прочитав несколько раз код эту опечатку в итоге найдете. Но если анализатор ткнет вас в неё носом прям в момент компиляции, вы сэкономите время на поиск такой ерунды.
В идеале анализатор должен на лету подчёркивать опечатки и ошибки, как это делает R#
Время-то он конечно экономит, вот только парадокс в том, что сам по себе статический анализ замедляет сборку (а в случае PVS довольно сильно) и не все запускают его прям после каждой правки в коде :) Каждый должен сам для себя решить, если такое происходит постоянно, то может быть анализатор и стоит покупки.
Потому что почти все ошибки связаны с логикой работы, а такие ошибки не поддаются статическому анализу.

Да, но лучше найти хоть что-то, чем ничего. Это что-то может ещё как пить кровь. Хороший пример: habrahabr.ru/post/198836/
Лично я за 4 года использования clang понял, что статический анализ кода на сях/плюсах мне не нужен.

А я понял, что нужен. Хотя я бы не сказал, что мы во всю гов*кодим :). Заметка на эту тему: "Проверка PVS-Studio с помощью Clang. С тех пор, мы используем для регулярного анализа не только PVS-Studio, но и Clang (запускается по ночам вместе с другими разными проверками).

Я бы не был столь критичен в отрицании простых ошибок. Вот я их допускаю и мне не стыдно про это сказать. Почему — потому, что их допускают все. Очень рекомендую к прочтению: Мифы о статическом анализе. Миф второй – профессиональные разработчики не допускают глупых ошибок.

Статический анализ полезен только в проектах, где пишут мало осмысленный код с огромной скоростью (в основном это десктопные приложения на плюсах, вроде той же миранды).
А вот кстати и список этих бредовых проектов: www.viva64.com/ru/a/0084/
Ошибка в том, что забыли разменивать указатель
Вам не хватает статического анализа смысла предложений которые вы пишете :)
Разыменовать, разыменование!
Разрабатываем новые диагностики. В связи с этим попалось вот такое новое:

V718 Suspicious type conversion: HRESULT -> BOOL. Clist_modern modern_xptheme.cpp 180

#define THEMEAPI          EXTERN_C DECLSPEC_IMPORT HRESULT STDAPICALLTYPE

THEMEAPI
EnableThemeDialogTexture(
    __in HWND hwnd,
    __in DWORD dwFlags
    );

BOOL xpt_EnableThemeDialogTexture(HWND hwnd, DWORD flags)
{
	BOOL res = FALSE;
	xptlock();
	res = EnableThemeDialogTexture(hwnd, flags);
	xptunlock();
	return res;
}

BOOL и HRESULT очень разные сущности. Их значения надо проверять по разному. Следовательно, смешивать их нельзя. Дело в том, что:

#define S_OK   ((HRESULT)0L)
#define S_FALSE  ((HRESULT)1L)

#define FALSE  0
#define TRUE   1


Ещё из странного:

V717 It is strange to cast object of base class 'avatarCacheEntry' to derived class 'CacheNode'. AVS cache.cpp 83


typedef struct avatarCacheEntry { .... };

struct CacheNode : public avatarCacheEntry, public MZeroedObject
{
	CacheNode();
	~CacheNode();

	BOOL   loaded;
	DWORD  dwFlags;
	int    pa_format;

	void   wipeInfo();
};

avatarCacheEntry tmp;
....
CacheNode *cc = arCache.find((CacheNode*)&tmp);


Аналогично:

AVS cache.cpp 173

AVS cache.cpp 199
Sign up to leave a comment.