Pull to refresh

Comments 99

Для создания сообщения об ошибке, происходит вызов функции: cond_col->getName(). Этого делать нельзя, так как указатель cond_col будет нулевым.

Он возможно будет нулевым, но не всегда из-за наличия второго условия. Думаю там просто стоит добавить дополнительную проверку перед выбросом исключения.
Когда выполняется «throw Exception» указатель не возможно нулевой, а точно нулевой. Посмотрите повнимательнее на код.

А если getName — static функция?

class IColumn : private boost::noncopyable
{
  ....
  /// Name of a Column. It is used in info messages.
  virtual std::string getName() const = 0;
  ....
};

template <typename T>
class ColumnVector final : public IColumn
{
  ....
  std::string getName() const override;
  ....
};

using ColumnUInt8 = ColumnVector<UInt8>;
я не обладаю плюсами, но разве компилятор не выдаст ошибку компиляции при попытке получить статический метод от экземпляра класса, а не от самого класса?
по моему если код компилируется то getName явно не статичен
#include <iostream>
#include <string>

class Lol 
{         
public:                                                                         
        static std::string  name() { return "lol"; }
};        

int main()
{         
        Lol *ptr = nullptr;

        std::cout <<  ptr->name() << std::endl;

        return 0;
} 

$ g++ -std=c++17 ./testLol.cpp && echo $?
0
$ ./a.out
lol

#include <iostream>
#include <string>

class Lol 
{         
public:                                                                         
        std::string  name() { return "lol"; }
};        

int main()
{         
        Lol *ptr = nullptr;

        std::cout <<  ptr->name() << std::endl;

        return 0;
} 

$ g++ -std=c++17 ./testLol.cpp && echo $?
0
$ ./a.out
lol
я не обладаю плюсами, но разве компилятор не выдаст ошибку компиляции при попытке получить статический метод от экземпляра класса, а не от самого класса?
Не выдаст и, что удивительно, экземпляр класса по-прежнему должен существовать для того, чтобы программа гарантированно работала (хотя на конкретных компиляторах и статические и нестатические фенкции могут отрабатывать при вызове для nullptr).
С другой стороны, видно, что этот код корректен и просто написан с «запасом надёжности».

Как раз наоборот: этот код так и манит, добавляя/удаляя очередную версию формата, исправить одну из проверок, и забыть исправить вторую.

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

Кажется, в ваших обзорах ещё не попадалась диагностика V789. Редкий вид (с) :)

У этого проекта достаточно свежий и современный код. Такое не очень часто встречается в проверяемых нами проектах. Обычно у проекта уже есть какое-нибудь наследие, и разрабокта продолжается в том же стиле.
В данном случае, думаю еще влияет то, что диагностика V789 весьма свежая и появилась недавно в PVS-Studio 6.16 (28 июня, 2017).
UFO just landed and posted this here
UFO just landed and posted this here
UFO just landed and posted this here

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


Хотя если тесты гонять перед релизом, то гемор будет меньше, и можно будет выцедить нормальное соотношение время/ошибки, так что хз.


Дисклеймер: пвс я не пробовал.

Шикарный комментарий. Спасибо. Я возьму его в рамочку и буду использовать как собирательный образ заблуждений, касающихся статического анализа кода. И дискутировать на эту тему в статьях и конференциях.
P.S. Смайлик ставить не буду, так как я совершенно серьезен. И даже напишу на тему этого комментария маленькую статью-ответ.
Не забудьте ещё про ваших реальных конкурентов (не де-юре, а де-факто): динамические анализаторы (ASAN, MSAN, TSAN, UBSAN). У них, конечно, есть беда: срабатывают не так быстро и ресурсов требуют больше. Но зато у них есть и преимущество: если уж *SAN чего-то «такое» у вас при фаззинге нашёл, то отбиваться бесполезно — это ошибка, надо чинить. А со статическим анализатором… всякое бывает.

Так что вам нужно влезть в нишу между встроенным в clang/gcc/msvc анализатором (бесплатно и всегда в наличии) и *SAN'ом (сложно и дорого, но зато ложных срабатываний — почти нуль, любую ошибку можно сразу в багтрекер заносить). Так ли велика эта ниша?
Ну сфера статических анализаторов, как бы не бедствует: SonarSource, Synopsys (бывший Coverity), Gimpel Software, Rogue Wave Software так далее :).

И собственно непонятно, прочему речь идёт про «нишу между». clang/gcc/msvc это своего рода статические анализаторы. С этим все просто. Они работают вот так. Мы лучше. С динамическими анализаторами всё тоже просто. Здесь нет конкуренции, а есть взаимное дополнение. В чем-то лучше динамические анализаторы, в чем-то статические. Пример: находим ошибки в Valgrind.
С этим все просто.
С этим всё совершенно непросто.

Они работают вот так. Мы лучше.
Ну если бы вы работали хуже, то в вас бы вообще смысла не было. Но тут важно заметить что анализ с помощью clang/gcc/msvc у вас уже есть — всегда, по умолчанию. А вот всё остальное — нужно устанавливать/настраивать отдельно. А если у вас в проекте контрибуторы из кучи других компаний? Сказать им «включите -Wall -Werror» — это одно, а отлавливать ошибки с помощью PVS-Studio (который они и запустить-то не могут!) — совсем другое.

С динамическими анализаторами всё тоже просто. Здесь нет конкуренции, а есть взаимное дополнение.
Это всё «халва». От неё слаще во рту не становится, опять-таки.

Как я уже сказал — если динамический анализатор чего-то нашёл, то «отмазаться» не получится: «на спор» я могу сдеать код, в котором не будет ошибки, но который будет вопить на анализаторе — но на практике я такого ни разу не видел.

А у статического анализатора (любого) есть ложные срабатывания. А если ты ещё и локально код проверить не можешь… В общем — так ли велик выигрыш?

Пример: находим ошибки в Valgrind.
Странный пример. Valgrind под Valgrind'ом запустить нельзя, так что понятно, что в нём самом могут остаться ошибки, которые PVS-Studio может найти. Но это не такому большому количеству народу нужно…

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

Вот Вы вроде и логично пишете (так может подумать сторонний наблюдатель). Но нет.

Статический анализ — это ответ на вопрос «Что ЕЩЕ я могу сделать для того, чтобы улучшить качество кода?». Если компания зарабатывает с помощью своего программного кода, то она стремиться всеми силами улучшить его качество. Качество — это расширяемость, масштабируемость, простота поддержки и др. Так вот, «что ЕЩЕ я могу сделать, чтобы улучшить качество кода?». Внедрить статический анализ! Не надо думать, что это ВМЕСТО чего-то. Вместо тестов, вместо код ревью, вместо динамических анализаторов… Это «что ЕЩЕ».

«А вот в компании XXX нах… не сдался ваш PVS-Studio!» И такое бывает конечно. Далеко не все компании, который пишут программный код, зарабатывают на нем деньги. Например, российская оборонка программный код пишет, но деньги они зарабатывают не на нем. Поэтому в оборонных предприятиях России PVS-Studio не используют.

А компания Epic Games зарабатывает на своем коде — движке Unreal Engine. Поэтому когда встал вопрос «Что ЕЩЕ мы можем сделать, чтобы код Unreal Engine стал качественнее?», они внедрили PVS-Studio.
Не надо думать, что это ВМЕСТО чего-то. Вместо тестов, вместо код ревью, вместо динамических анализаторов… Это «что ЕЩЕ».
Так не бывает. Если код вы не пишите для участия в каком-либо конкурсе, где качество — критерий успеха, то вы всегда должны выбирать между улучшением качества кода и чем-то ещё. Можно написать ещё пару тестов, а можно — новую фичу дополнить… что выбрать?

Если компания зарабатывает с помощью своего программного кода, то она стремиться всеми силами улучшить его качество.
Вот только «количество ошибок» — далеко не единственный критерий. Могут быть много других. И уменьшения количества ошибок можно достигать разными средствами — можно вложиться в статический анализатор, а можно — в рефакторинг или более подробный style guide. Что принесёт большую отдачу? Результат — сходу не очевиден, вот совсем не очевиден.

Поэтому когда встал вопрос «Что ЕЩЕ мы можем сделать, чтобы код Unreal Engine стал качественнее?», они внедрили PVS-Studio.
А могли бы вместо этого вложиться в фаззинг — но предпочли PVS-Studio. Вопрос — почему и почему Google поступил иначе?
Так не бывает.

То есть вы ЛИБО пишите тесты, ЛИБО делаете код ревью? И сразу же увольняете того разработчика, который просит сделать код ревью для его тестов? Ведь он тратит время!

Вопрос — почему и почему Google поступил иначе?

Google сам разрабатывает статический анализ. В clang. Мы знаем людей, которые делают это.

Можно написать ещё пару тестов, а можно — новую фичу дополнить… что выбрать?

То есть вы ЛИБО пишите тесты, ЛИБО делаете код ревью?

Вы намеренно делаете ложный вывод, доводя до абсурда аргумент khim, или просто некому проревьюить ваш комментарий перед публикацией?
Вы намеренно делаете ложный вывод, доводя до абсурда аргумент khim

Зачем доводить? Аргумент khim «Так не бывает» и так достаточно абсурден.
То есть вы ЛИБО пишите тесты, ЛИБО делаете код ревью?
Представьте себе. Я не умею одновременно делать и то и другое.

То что я пишу и тесты и делаю код ревью (но в разное время) просто обозначает что и то и другое посчитали достаточно ценной деятельностью — и потому оставили. Если бы выяснилось что написание тестов в 100 раз эффективнее код ревью (или наоборот) — от одной из этих двух вещей отказались бы.

Google сам разрабатывает статический анализ. В clang. Мы знаем людей, которые делают это.
А это-то вообще тут причём? Да, разрабытывает — потому что автоматический, всегда присутствующий, встроенный в компилятор статический анализ — это хорошо. А вот хорошо ли иметь ещё и отдельных анализатор — это большой вопрос.
Можно написать ещё пару тестов, а можно — новую фичу дополнить… что выбрать?

По сути статанализатор это некая замена (условно, конечно) тем самым паре тестов сверху.
А могли бы вместо этого вложиться в фаззинг — но предпочли PVS-Studio. Вопрос — почему и почему Google поступил иначе?
Я думаю, это очевидно.

У Гугла приоритет — сетевые сервисы, там ошибка, приводящая к DoS или RCE это очень серьёзно.

Для UE такие ошибки некритичные (если падает при загрузке какой-то кривой модельки, никто и не заметит, потому что модельки загружают туда прямые).

Главное в UE — чтобы графика правильно рисовалась и физика моделировалась. Тут фаззинг бессилен. Ну скормишь рандомные данные в движок, получишь какой-то результат. А как проверить, что результат верный?
Сертификационные лаборатории активно пользуются статическими (как и динамическими) анализаторами, так что утверждение не совсем корректно.
Давайте не будем про сертификационные лаборатории? Дай им всем Бог здоровья и процветания!
Пишу на яве, может у вас лучше, я честно написал. :)

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

В общем, пользуйтесь, рад что повеселил, хоть смайлик и не поставили. :D
А в юнит тестах не бывает ошибок?
Бывает и много. Из статей обычно исключаются.

Бывает ещё хуже: как-то исправление настоящей ошибки по нашему отчёту привело к завалу сотни тестов в одном проекте. Отличные тесты были, не правда ли?
Тесты — это просто проверка соответствия фактического поведения программы ожидаемому. Они вполне могли быть отличными.
Вопрос не в цене анализатора. Вопрос в цене внедрения и поддержки. Ложные срабатывания стоят денег, интеграция стоит времени и т.д. и т.п.

Не думаю что проблема в цене лицензии, Яндекс точно может себе её позволить.

Ну-ну, написал я им, внедрил в линукс билды по триальному ключу, интегрировал, написал маны для соратников. А когда дошёл разговор до платных лицензий, они &₽?!@"&!/₽-&!₽ Ой, чуть NDA не сдержал.

То, что у Яндекса есть деньги — вовсе не значит, что они готовы им разбрасываться.

Говорить о цене стоило до всего остального. Ибо вполне может оказаться, что у них, например, в билд-ферме 10000 машин — и что, для каждой лицензию покупать?

Триальная лицензия — это уж точно не для интеграции и для написания манов для сотрудников…

Да, вы правы. Договариваться стоило на берегу. С другой стороны, хотелось убедить своё начальство аргументами "смотрите, как круто это работает на нашем проекте! Вот нашло! И вот нашло!", но ₽&@?!'-/@&₽):;@/"₽&@₽

&₽?!@"&!/₽-&!₽


Не смог расшифровать. Мы что-то сделали не так? Как мы можем это исправить?

Сделайте исключение сорцов per project, а не глобально, с поддержкой раскрытия переменных окружения из VS-проектов для формирования полных путей вместо масок.

Зачем? Был вопрос, что можно исправить, я написал.

Я не уверен, что я могу обсуждать денежные вопросы прилюдно без вашего разрешения. Если позволите, я объясню, что вы делаете не так, и откуда берутся "&₽?!@"&!/₽-&!₽".

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

Знаете, я писал длинное письмо, но потом передумал. Кратко: не предлагайте отделу 20 лицензий, если отдел запрашивает 8. Отделу и 1 на билд сервер хватило бы, но отдел попросил честные 8, т.к. в команде 7 плюсовиков. Нельзя быть таким жадным, Евгений.

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

Да смысла нет.


Ребята из Я, я вам написал маны по настройке Coverity, извините, но ссылок дать не могу, так как не в компании. Реально хорошо находит баги, умеет делать рассылки и прочие кошерные вещи. И проблем с закупкой нет :)

А, вы просто хотите Coverity в Яндекс продать, а мы вам мешаем, понятно…

Жесть какая, маны он написал… Вот подробнейшая документация по PVS-Studio, и не надо никакие левые маны брать.

:D ну когда я был в Я и у меня был выбор между вашим продуктом и Coverity, то я сделал выбор не в вашу пользу. Видимо, не зря.


  • проснулась моя девушка и попросила удалить слово недоговно
А откуда у вас такая информация? Возможно они coverity используют или еще что-нибудь?
Приблизительно раз в полгода нам пишет кто-то из сотрудников компа
нии Yandex, интересуется лицензированием PVS-Studio, качает триал и пропадает.

Видимо у них в плане работ стоит проверка анализатором каждые полгода.

Разработчикам PVS-Studio: очень понимаю сотрудников Yandex — с ценовой политикой на продление лицензий вида "а давайте увеличим стоимость продления лицензии PVS-Studio на рандомную существенную величину" уже не первый год подряд и как результат проблематичностью бюджетирования, также начинаем поглядывать на альтернативы.

Не понимаю о чем речь. Ценовая политика в плане продлений не менялась много лет. 80% от цены новой лицензии.
А новая цена тоже не менялась много лет?
Конечно менялась! У нас появился C# анализатор, у нас появился C++ анализатор под Linux, поддержка SonarQube, интеграция с CI и многое другое. Сотни диагностик, сотни страниц документации.

Нет никакой возможности продавать по цене пятиледней давности.

Однако если вы не просто так в комментах пообсуждать, а по делу, то напишите мне и я расскажу о способах справиться с указанной бедой.
А почему бы цену продления не делать фиксированной в момент покупки?

Но я так, просто в комментах пообсуждать. Хоть я бы и очень хотел купить ваш продукт себе и на своей работе — возможности такой нет. Для собственного использования слишком жирно, а на работе сейчас тоже бюджета нет.
Так что я вас без коммерческого потенциала отвлекаю.
А почему бы цену продления не делать фиксированной в момент покупки?


Но я так, просто в комментах пообсуждать.


Извините, Хабр — технический ресурс и если мы будем обсуждать нюансы бизнеса, то это будет не интересно аудитории.
Извините, Хабр — технический ресурс и если мы будем обсуждать нюансы бизнеса, то это будет не интересно аудитории.

Друг мой, аудитория годами читает Milfgard'a и нюансы бизнеса МосИгры (это как яркий пример, есть еще вернувшийся Мегамозг).

Возвращаясь же к топ-комментарию:
Конечно менялась! У нас появился C# анализатор, у нас появился C++ анализатор под Linux, поддержка SonarQube, интеграция с CI и многое другое. Сотни диагностик, сотни страниц документации.

Эта фраза может произвести двоякое впечатление. Можно возразить что-то вроде "с чего мне платить за SonarQube и С++ анализаторы под Linux? У меня .NET проекты!!!111".
Слушайте, ну мы все взрослые люди и понимаем, что у вас есть определенный бизнес процесс и определенные причины не обсуждать ценовую политику публично.
Но, попытка это прикрыть интересами сообщества выглядит толстым таким лицемерием и воспринимается крайне негативно.
У нас появился C# анализатор, у нас появился C++ анализатор под Linux, поддержка SonarQube, интеграция с CI и многое другое.

«С этого года во всех наших автомобилях установлены крылья и ракетные двигатели, при приезде на ТО мы в обязательном порядке их ставим и вы обязаны за них заплатить даже если они вам не нужны».

Примерно так это звучит. Зачем компании, которая пишет на плюсах под винду версия под Linux и C# анализатор? Сделайте это отдельной лицензируемой функциональностью, кому надо — тот купит.
а вы код эрланга (виртуальной машины) не проверяли?
А у вас есть диагностика на использование значения, которое переместили?
Что-то вроде этого:
    std::vector<int> a = ...;
    ...
    auto b = std::move(a);
    ...
    auto s = a.size(); // ???
std::move ничего не перемещает.
Это я к тому, что a остался вполне себе валидным объектом.
Но это только стандартная библиотека гарантирует «valid but unspecified state». Для прочих объектов (согласно стандарту) валидность не требуется.
Валидный останется объект или не валидный — не очень важно.
Главное, семантика перемещения. Объект а после перемещения хранит непонятно что (если move реализован через swap, то в а лежит содержимое b), и вроде нет никаких причин работать с а, кроме как присвоить ему новое значение.
Мне кажется это главным изъяном операции перемещения введеной в С++11 — появление объектов с непонятным состоянием, которые по идее должны закончить свою область видимости после перемещения.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод

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

Что делать? Подскажите как улучшить ситуацию.

Выучить английский; нанять того, кто знает английский. Масса вариантов.

Можете привести примеры конкретных проблем?

Анализатор отличная вещь, однако, он не заменит голову программиста, и, прежде чем начинать им пользоваться, хорошо бы писать код так, чтобы компилятор не выдавал стандартных ворнингов (а где вы видели это в больших проектах?).


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


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

хорошо бы писать код так, чтобы компилятор не выдавал стандартных ворнингов (а где вы видели это в больших проектах?).

Большие проекты — это сколько по-вашему?

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

Точно так же как и автоматическая коробка передач на атомобиле, стиральна машина-автомат и тому подобное. Низводят людишек до сосотояния операторов.
На предыдущей работе писал на чистом «C» среда разработки VisualDSP++, после очередной статьи с вашими хвалебными речами о PVS-Studio решил попробовать протестировать свои проекты. А вот хрен. Не работает PVS-Studio в связке с VisualDSP++. Ну думаю ладно, надо на чем то еще попробовать. Нашел код написанный под AVR опять же на чистом «C». Код писался в «блокноте» и собирался в AVR GCC — то же облом. Ну думаю, дай попробую проекты, которые писал под STM32 на «С» и собирал в IAR. Но и тут какая то лажа.
Вы выкатили список нестандартных тулчейнов. Понятное дело, что у подобного может не быть заявленной поддержки, и Вы испортили впечатление о продукте на пустом месте. Подобную информацию следует изучать в документации и поддержке заранее.
Ок. Идем на сайт PVS-Studio https://www.viva64.com/ru/t/0046/
Читаем:
Другие преимущества статического анализа кода:

2. Статический анализ не зависит от используемого компилятора и среды, в которой будет выполняться скомпилированная программа. Это позволяет находить скрытые ошибки, которые могут проявить себя только через несколько лет. Например, это ошибки неопределенного поведения. Такие ошибки могут проявить себя при смене версии компилятора или при ...
Там объёмный текст о преимуществах и недостатках статического анализа кода как методологии. Вы даже заголовок процитировали. Опрометчивая претензия.
www.viva64.com/ru/pvs-studio
Быстрый старт в Windows и Linux

Отметим только, что в PVS-Studio для Windows и Linux предусмотрены специальные утилиты, собирающие информацию о запусках компилятора. Эти инструменты позволяют быстро выполнить анализ проекта, собираемого любым способом. Вы можете быстро познакомиться с возможностями анализатора, не тратя время на его интеграцию с makefile или сборочным скриптом. См. описание утилиты Standalone (Windows) и pvs-studio-analyzer (Linux).

Это тоже опрометчивая претензия?
Отчасти. Я же не знаю на сколько давно Вы там работали. Эти механизмы достаточно новые, сайт обновляется. Уже поздно копать в эту строну, но я рад, что Вы всё-таки решили ознакомиться с описанием и документацией.
Первый раз пробовал PVS-Studio примерно год назад (на DSP++ и GCC), второй раз (на IAR-е) сегодня. Поэтому и пишу, что ничего не изменилось.
В новой версии анализатора PVS-Studio 6.22 наша команда доработала инфраструктуру для проверки проектов следующего типа:
  1. Появилась поддержка ARM Compiler 5 и ARM Compiler 6 в составе среды Keil uVision 5.
  2. Компиляторов ARM Compiler 5 и ARM Compiler 6 в составе среды Keil DS-MDK.
  3. Мы поддерживаем IAR C/C++ Compiler for ARM в составе среды IAR Embedded Workbench.
Интересно, а когда-нибудь проверяли исходники PVS-Studio при помощи самой PVS-Studio?
Интересно, а не было попыток анализировать с помощью PVS-Studio проекты и код на LISP/Clojure/Racket/Scheme/...?
Это не имеет практического смысла.
Спасибо за статью! Скажите, появится ли в PVS-Studio Java?
Но зачем?
Есть же FindBugs и инспекции в IDEA.
Это другой вопрос. Для C#/С++/С тоже есть инструменты статического анализа, это не мешает PVS-Studio делать это.
Появится. Но пока это только в абстрактном плане.
Sign up to leave a comment.