Comments 108
Для начала я проверил небольшую часть операционной системы (3.3%) и выписал около 900 предупреждений, указывающих на настоящие ошибки. Если экстраполировать результаты, то получается, что наша команда способна выявить и устранить в Tizen около 27000 ошибок.
Если честно, странная логика.
Проверив всего 1 строку программы анализатор не нашёл в ней ошибок.
Если экстраполировать результаты, то получается, что программа не содержить ошибок!
Почему то вспомнился анекдот, про жену председателя и доярку, которые в среднем обе б@$ди…
Данная картинка здесь совершенно неуместна. Прочитайте статью (хотя-бы по диагонали :-).
В крупных продуктах код может (и будет) неоднороден, и, в данном случае, на мой взгляд, произведённая вами экстраполяция неверна.
P.S. Мне нравится ваш продукт и ваш подход к его продвижению. И статью я прочитал, как и почти все остальные.
зато у этих ребят огромный опыт в нахождении ошибок в продуктах, разной степени тяжести. Причем не просто проверки, а анализ ложных срабатываний, накопление статистики распределений ошибок (о чем они в том числе писали на хабре) и т.п.
Вы тоже работаете в этой области, или просто теоретизируете?
ps. И экстраполяция по определению не может быть верной. Она может быть допустимой.
Вы тоже работаете в этой области, или просто теоретизируете?
ps. И экстраполяция по определению не может быть верной. Она может быть допустимой.
Я работаю в области написания этих самых продуктов (в том числе крупных).
И бывает, что какую-то часть проекта пишет один человек/команда, а другую часть — другой человек/команда. И качество кода может отличаться если не на порядки, то в разы.
А потому экстраполировать результаты проверки 3.3% проекта на весь проект нельзя. Вполне возможна ситуация, когда в выборку (в статье не указана методика выборки, мы не знаем была ли она репрезентативной) попала только часть кода, написанная условно хорошо, тогда как другая часть написана условно хуже. Или наоброт.
Я использовал метод выборки «пальцем в небо». Очень хороший, честный способ для решаемой задачи.
Вряд ли мне «повезло» взять столько проектов, относящихся к одной команде. Почти наверняка, эти проекты создаются как минимум несколькими командами.
В статье перечислены проекты, которые были взяты
alsa-lib-1.0.28, aspell-0.60.6.1, augeas-1.3.0, bind-9.11.0, efl-1.16.0, enlightenment-0.20.0, ise-engine-anthy-1.0.9, bluetooth-frwk-0.2.157, capi-appfw-application-0.5.5, capi-base-utils-3.0.0, capi-content-media-content-0.3.10, capi-maps-service-0.6.12, capi-media-audio-io-0.3.70, capi-media-codec-0.5.3, capi-media-image-util-0.1.15, capi-media-player-0.3.58, capi-media-screen-mirroring-0.1.78, capi-media-streamrecorder-0.0.10, capi-media-vision-0.3.24, capi-network-bluetooth-0.3.4, capi-network-http-0.0.23, cynara-0.14.10, e-mod-tizen-devicemgr-0.1.69, ise-engine-default-1.0.7, ise-engine-sunpinyin-1.0.10, ise-engine-tables-1.0.10, isf-3.0.186, org.tizen.app-selector-0.1.61, org.tizen.apps-0.3.1, org.tizen.bluetooth-0.1.2, org.tizen.browser-3.2.0, org.tizen.browser-profile_common-1.6.4, org.tizen.classic-watch-0.0.1, org.tizen.d2d-conv-setting-profile_mobile-1.0, org.tizen.d2d-conv-setting-profile_wearable-1.0, org.tizen.download-manager-0.3.21, org.tizen.download-manager-0.3.22, org.tizen.dpm-toolkit-0.1, org.tizen.elm-demo-tizen-common-0.1, org.tizen.indicator-0.2.53, org.tizen.inputdelegator-0.1.170518, org.tizen.menu-screen-1.2.5, org.tizen.myplace-1.0.1, org.tizen.privacy-setting-profile_mobile-1.0.0, org.tizen.privacy-setting-profile_wearable-1.0.0, org.tizen.quickpanel-0.8.0, org.tizen.screen-reader-0.0.8, org.tizen.service-plugin-sample-0.1.6, org.tizen.setting-1.0.1, org.tizen.settings-0.2, org.tizen.settings-adid-0.0.1, org.tizen.telephony-syspopup-0.1.6, org.tizen.voice-control-panel-0.1.1, org.tizen.voice-setting-0.0.1, org.tizen.volume-0.1.149, org.tizen.w-home-0.1.0, org.tizen.w-wifi-1.0.229, org.tizen.watch-setting-0.0.1, security-manager-1.2.17.
Вряд ли мне «повезло» взять столько проектов, относящихся к одной команде. Почти наверняка, эти проекты создаются как минимум несколькими командами.
UFO just landed and posted this here
Прошу прочитать статью и тогда станет понятно, что этот подход достоверен. В данном случае 3.3% кода это 2 400 000 строк кода (без учёта комментариев). Это много, это размер некоторых проектов. Проверив такой размер кода вполне модно проводить расчеты и получить результат близкий к реальности.
Вы раньше сами писали, что плотность ошибок на 1000 строк кода непостоянная, и зависит от проекта.
А здесь вдруг приняли её за константу.
Конечно непостоянная. Но переживать об том есть смысл, если мы проверили 1000, 3000 или 10000 строк кода. Или стоит переживать, если проверялся только один проект написанный одной командой. Когда проверено 2400000 строк кода из множества разных проектов, вполне смело можно считать, что полученное значение плотности является средним и для оставшейся части проекта.
Так навскидку, какой доверительный интервал у этой оценки? Тысяча наберется?
Вообще, молодцы и те и другие — вы, что проверяете, они, что занимаются рефакторингом (пускай и создавая новые баги :D)
ИМХО, выборка была сделана достаточно большая для экстраполяции.
sborisov наверное хотел услышать общие рассуждения о выборе и количестве репрезентативных строк кода…
Интересные ошибки.
Поясните, пожалуйста, одну:
Поясните, пожалуйста, одну:
Указатель на строку argv[2] является небезопасным источником данных, так как это аргумент командной строки. При формировании строки в буфере temp может произойти выход за границу массива.Что argv[2] может быть любым, понятно. Но там же snprintf с правильным размером буфера. Как можно выйти за границу temp?
у temp фиксированная длина, и длина argv[2] может оказаться больше
upd. Ой, невнимательно прочитал, там snprinf. Тогда присоединяюсь к вопросу.
upd. Ой, невнимательно прочитал, там snprinf. Тогда присоединяюсь к вопросу.
Да, какая-то глупость написана в статье. Видимо, выписал какой-то не тот фрагмента кода. Разбираться в причинах уже сил нет, поэтому просто удалил этот фрагмент из статьи и изменил, что находим не 914, а 913 ошибок :).
P.S. Возможно и ещё что-то окажется не ошибкой при тщательном изучении. Но с другой стороны, при тщательном анализе и наоборот выяснится, что я пропускал некоторые ошибки. Например, я поленился изучать V730 (очень трудоёмко). А там наверняка несколько ошибок скрывается.
P.S. Возможно и ещё что-то окажется не ошибкой при тщательном изучении. Но с другой стороны, при тщательном анализе и наоборот выяснится, что я пропускал некоторые ошибки. Например, я поленился изучать V730 (очень трудоёмко). А там наверняка несколько ошибок скрывается.
Зачем этот кактус с ручным выделением памяти и работой с указателями и си-строками на протяжении в 72 миллиона строчек кода? Может лучше сделайте им презентацию про std::unique_ptr, std::srting и std::vector. Из названий переменных видно что даже какой то шттп на голом си у них работает.
Может это не ошибки а фичи :). 27000 фич, хм, звучит круто. Маркетологи будут очень довольны, очень очень.
Функция malloc, если не может выделить память, возвращает NULL. Оператор new при нехватке памяти генерирует исключение std::bad_alloc.У меня на паре личных проектов
new возвращает NULL, чтобы избежать исключений. По стандарту это, конечно, неправильно, но я и не собираюсь писать свою стандартную библиотеку. Просто я выделяю память по-своему, и оператор new выглядит красивее, чем какой-нибудь my_memory_alloc(sizeof(*foo));Учитывая, что вы проверяете ОС — там такая перегрузка операторов очень даже возможна.
PVS Studio не пользовался, но мне кажется, она умеет отличать перегруженные операторы от встроенных и по-разному анализировать их использование.
А почему не используете RAII?
А как это мне поможет? Допустим, мы «отжали» у системы пару ГБ памяти, чтобы более быстро и эффективно поделить ее между своими объектами. RAII происходит в конструкторе, а объекта еще нет. Можно, наверное, как-то извратиться, но это будет ужасное решение. Например, если
Возможно, пример не очень удачный (тут, в лучших традициях ООП, «отжатая» память сама должна являться объектом), но глобальная перегрузка оператора
this не NULL, то объект находится в стеке, а иначе ему нужно выделить место в куче… Если бы это не был личный проект, коллеги бы меня придушили.Возможно, пример не очень удачный (тут, в лучших традициях ООП, «отжатая» память сама должна являться объектом), но глобальная перегрузка оператора
new запросто может встретиться в коде ОС.
Кстати, по стандарту есть new(std::nothrow), который как раз возвращает nullptr. Никогда сам его не использовал, правда, но работает: https://ideone.com/Z43jcF
Об этом написано в посте:
Если требуется, чтобы оператор new возвращал nullptr, следует использовать nothrow версию оператора:Но мне это требуется вообще везде, во всем коде. И с моим выделением памяти это работать не будет.
А зачем так надо? Потому что кажется что если код не рассчитан на то, что вернётся nullptr, то там плохо возвращать nullptr, и уж лучше сразу std::terminate() если исключения нельзя. А если код пишется самостоятельно, то что мешает использовать nothrow-версию?
Во-первых, что значит «что мешает использовать nothrow-версию»? У меня она всего одна, и она не бросает исключения. Конечно, можно дописать это, но зачем, если у меня вообще во всем коде no-throw guarantee.
Во-вторых, я использую
Кстати, практически любую стандартную библиотеку можно собрать без исключений. Например, вот реализация из LLVM libcxx: (обратите внимание на
Но я так не делаю, потому что теряется вся идея переносимости стандартной библиотеки.
Во-вторых, я использую
new, потому что он проще (размер считается автоматически) и короче. Если я начну писать везде (std::nothrow), то в чем будет заключаться выигрыш?Кстати, практически любую стандартную библиотеку можно собрать без исключений. Например, вот реализация из LLVM libcxx: (обратите внимание на
_LIBCPP_NO_EXCEPTIONS)Заголовок спойлера
__attribute__((__weak__, __visibility__("default")))
void *
operator new(std::size_t size)
#if !__has_feature(cxx_noexcept)
throw(std::bad_alloc)
#endif
{
if (size == 0)
size = 1;
void* p;
while ((p = ::malloc(size)) == 0)
{
// If malloc fails and there is a new_handler,
// call it to try free up memory.
std::new_handler nh = std::get_new_handler();
if (nh)
nh();
else
#ifndef _LIBCPP_NO_EXCEPTIONS
throw std::bad_alloc();
#else
break;
#endif
}
return p;
}
__attribute__((__weak__, __visibility__("default")))
void*
operator new(size_t size, const std::nothrow_t&) _NOEXCEPT
{
void* p = 0;
#ifndef _LIBCPP_NO_EXCEPTIONS
try
{
#endif // _LIBCPP_NO_EXCEPTIONS
p = ::operator new(size);
#ifndef _LIBCPP_NO_EXCEPTIONS
}
catch (...)
{
}
#endif // _LIBCPP_NO_EXCEPTIONS
return p;
}Но я так не делаю, потому что теряется вся идея переносимости стандартной библиотеки.
Получается что libcxx без исключений не соблюдает стандарты? В отличие от libstdc++
new_handler handler = std::get_new_handler ();
if (! handler)
_GLIBCXX_THROW_OR_ABORT(bad_alloc());
handler ();Ровно такое поведение я ожидал бы увидеть если exceptions запрещены.
Видимо отклонение от стандарта позволяет писать код быстрее, но потом не надо ожидать что его будет правильно понимать PVS-Studio, когда для семантики return nullptr on fail есть специальный синтаксис, который можно использовать везде вместо обычного new. Так что мне кажется что они в своём праве всегда на это ругаться.
По сравнению с malloc выигрыш в вызове конструктора и (судя по С++ 17) учёту extra type alignment.
И я правильно понимаю что с вашим определением operator new стандартные контейнеры совсем не используются? Потому что вроде они никак нулевой указатель обрабатывать не станут, и действительно лучше было сразу std::terminate делать.
Видимо отклонение от стандарта позволяет писать код быстрее, но потом не надо ожидать что его будет правильно понимать PVS-StudioЕще раз напоминаю, что проверяется код ОС, где можно встретить все, что угодно. Даже чтение-запись по NULL-указателю не должны вас удивлять.
И я правильно понимаю что с вашим определением operator new стандартные контейнеры совсем не используются?С каких это пор стандартные контейнеры стали прибивать гвоздями к operator new? Там же сплошное мета-программирование, вся работа с памятью идет через отдельные абстракции, которые в итоге могут раскрыться во все, что угодно.
действительно лучше было сразу std::terminate делатьКак пользователь, прошу вас, никогда так не делайте! Уже достало, когда нехватка памяти рушит всю программу, вместо отмены запрошенной операции. Например, Tor Browser падает постоянно, хотя было бы достаточно убить пару вкладок. Встречаются графические редакторы, которые также завершаются при попытке импорта больших файлов, хотя достаточно отменить операцию импорта и вывести сообщение об ошибке.
Еще раз напоминаю, что проверяется код ОС, где можно встретить все, что угодно. Даже чтение-запись по NULL-указателю не должны вас удивлять.
Я очень удивлюсь если увижу любую реализацию libstdc++ в которой operator new() возвращает валидный адрес 0x0.
А в остальном: во-первых, спасибо за то что было не лень мне отвечать! Интересно подумать что же на самом деле вроде может работать, а что должно быть очень опасно.
TLDR: либо в коде должны быть написано new (std::nothrow), либо он не никогда не должен вернуться к исполнению после неуспеха выделения памяти в operator new.
Полная версия
Безусловно можно в случае нехватки памяти вместо terminate() попытаться сходить осободить кеши и ресурсы, или отменить текущую операцию. Но первое странно выносить за пределы работы с памятью, то есть в приципе это всё можно сделать внутри нашего operator new(). А для второго надо либо реализовывать какой-то вариант корутин (чтобы можно было выкинуть кусок стека который про это ничего не знает), либо код, который это использует, должен быть готов к этому.
Cтандартные контейнеры не готовы в целом к тому что allocate() вернёт невалидный указатель. Тот же самый std::list обязательно попытается слинковать ноды, записав что-то в память.
Наверное, можно попытаться возвращать в allocate() одинковый для всех, но хотя бы writable и хорошо выравненный кусочек памяти. А для std::vector можно внутри MyAllocator::construct() проверять была ли проблема с выделением памяти и если была то ничего не иницаильнизировать. Но тем самым нарушается всё больше и больше контрактов, и на каком-то этапе это может взорваться.
Лучший способ продемнострировать то, что код готов к невалидному указателю — написать лишние 15 символов на каждый new. И всем сразу всё ясно.
А если часть кода этого не ожидает, то надо прервать выполнение текущего стека. Это можно сделать разными способами, но я не могу придумать примеров когда это нужно делать за пределами operator new(), сначала вернув оттуда невалидный указатель.
Я очень удивлюсь если увижу любую реализацию libstdc++ в которой operator new() возвращает валидный адрес 0x0.Я очень удивлюсь, если ОС будет использовать обычную user-space библиотеку. Для меня
operator new — это, в первую очередь, конструкция языка, и лишь потом одна из реализаций.Безусловно можно в случае нехватки памяти вместо terminate() попытаться сходить осободить кеши и ресурсы <...> Но первое странно выносить за пределы работы с памятью <...>Как раз для этого есть
std::new_handler. Он вызывается, когда памяти перестает хватать.А для второго надо либо реализовывать какой-то вариант корутинОбычный паттерн «команда» + какая-нибудь арена, чтобы разом прибить память, затраченную на операцию.
Cтандартные контейнеры не готовы в целом к тому что allocate() вернёт невалидный указатель.Получается, Google уже давно должен был бы упасть в segmentation fault? Вот как раз кусок из их реализации арены:
pointer allocate(size_type n,
std::allocator<void>::const_pointer /*hint*/ = 0) {
assert(arena_ && "No arena to allocate from!");
return reinterpret_cast<T*>(arena_->AllocAligned(n * sizeof(T),
kAlignment));
}AllocAligned вернет NULL, если что-то пошло не так.
protobuf не выглядит как библиотека которая использует стандартные контейнеры.
Как раз для этого есть std::new_handler. Он вызывается, когда памяти перестает хватать.Ну да, но это происходит в рамках аллокации, пока что управление находится внутри operator new(), то есть невалидный указатель нам никто не возвращает
Обычный паттерн «команда» + какая-нибудь арена, чтобы разом прибить память, затраченную на операцию.
То есть вы сами пишете все строки где вызывается operator new() и можно там везде вызвать именно nothrow-версию?
AllocAligned вернет NULL, если что-то пошло не так.
Это вот этот код?
Любопытный пример.
AllocAligned() вроде бы является по семантике (см комментарии в arena.h) просто malloc, то есть в конце концов он там ссылается на malloc. Но:
- В
AllocNewBlock(цепочкаAllocAligned()->GetMemory()->GetMemoryFallback()->AllocNewBlock()) есть вызов new std::vector и скорее всего это обычный глобальный new, из которого может прийтиbad_alloc - Потом там же есть
вызов aligned_mallocи проверка на то что вернулось ненулевое значение. - И там же есть вызов обычного
malloc, уже безо всяких проверок.
То есть такое впечатление что про это просто не думали c таким ранообразием сценариев обработки этого события. Свидетельством этого же является попытка записать что-то без проверки получилось ли выделить память в классе Gladiator.
Вы уверены что оно там не падает в segmentation fault когда память заканчивается?
https://github.com/kafeman/mne_ne_len
Сейчас нет времени разбираться, но падать не должно. Этот класс, на сколько я знаю, используется также в других проектах Google.
Сейчас нет времени разбираться, но падать не должно. Этот класс, на сколько я знаю, используется также в других проектах Google.
Ну вот стек gdb в момент SIGSEGV
Стек
Program received signal SIGSEGV, Segmentation fault.
0x00000000004044ae in ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena>::construct (this=0x7fffffffda30, p=0x4) at arena-inl.h:109
109 new(reinterpret_cast<void*>(p)) T();
#0 0x00000000004044ae in ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena>::construct (this=0x7fffffffda30, p=0x4) at arena-inl.h:109
#1 0x0000000000404340 in std::allocator_traits<ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena> >::_S_construct<int> (__a=..., __p=0x4)
at /usr/include/c++/5/bits/alloc_traits.h:256
#2 0x000000000040417c in std::allocator_traits<ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena> >::construct<int> (__a=..., __p=0x4)
at /usr/include/c++/5/bits/alloc_traits.h:402
#3 0x0000000000403f41 in std::__uninitialized_default_n_a<int*, unsigned long, ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena> > (__first=0x0,
__n=9999999998, __alloc=...)
at /usr/include/c++/5/bits/stl_uninitialized.h:623
#4 0x0000000000403d13 in std::vector<int, ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena> >::_M_default_append (this=0x7fffffffda30, __n=9999999999)
at /usr/include/c++/5/bits/vector.tcc:565
#5 0x0000000000403ab3 in std::vector<int, ctemplate::ArenaAllocator<int, ctemplate::UnsafeArena> >::resize (this=0x7fffffffda30, __new_size=9999999999)
at /usr/include/c++/5/bits/stl_vector.h:676
#6 0x00000000004035bf in test_vector () at main.cc:27
#7 0x000000000040374c in main () at main.cc:55Что очень логично — resize сразу же начинает конструировать объекты, в данном случае по адресу nullptr.
Наверное у Google очень много памяти) Что более реально — большой набор своих контейнеров.
OK, буду смотреть. Части этого кода, с различными изменениями, используются и в других местах. Например, в Chromium, где также есть STL-контейнеры.
Наверное у Google очень много памяти)Вообще, этот класс был создан, чтобы эффективно выделять очень маленькие куски памяти. Но ведь какая-то другая часть программы может выделить ее столько, что этот маленький кусочек уже не поместится…
КДПВ сделала мой день
Как бывший сотрудник Samsung. Дам вам ребят совет, если хотите продать ваш продукт компании(улучшив качество Tizen таким образом)
Переведите ВСЕ материалы на качественный Корейский. И съездите на какую нибудь конференцию в Сеул(даже самую маленькую) по Tizen с докладом на корейском(желательно чтобы Samsung ее хостил). Вас тогда с руками и ногами оторвут. Все попытки что то сделать на английском и тем более на русском сразу летят в корзину у корейско говорящего менеджера, ибо у него нет времени переводить.
Переведите ВСЕ материалы на качественный Корейский. И съездите на какую нибудь конференцию в Сеул(даже самую маленькую) по Tizen с докладом на корейском(желательно чтобы Samsung ее хостил). Вас тогда с руками и ногами оторвут. Все попытки что то сделать на английском и тем более на русском сразу летят в корзину у корейско говорящего менеджера, ибо у него нет времени переводить.
UFO just landed and posted this here
Не ослик с рогом, а благородный единорог :D
Именно из-за этого блюющего маскота многие серьёзные фирмы наотрез отказываются работать с pvs.
Я думаю, что серьезной фирме, которой нужен софт для проверки софта, глубоко… начхать на логотип компании, чей софт они собираются лицензировать.
Человеческий фактор — он может решать. Года назад были свои терки на хабре (=агрессивные владельцы, отсюда и ниже, про "закрытие статей и школу").
Если сейчас они изменились к лучшему, то проблем быть не должно с тем, чтобы продать продукт, ибо имейдж не страдает.
то второе условие всегда ложно и функция никогда не вернёт значение «es_US»
Так "рыжий" же деньги на строительство стены выделяет. Скоро и по стране es_US будет не найти.
А можно вот такой вопрос:
Вы анализируете разные проекты… А отклик-то есть вообще или это чисто для юзеров хабра?
Интересует например как отреагировали в Qt…
Вы анализируете разные проекты… А отклик-то есть вообще или это чисто для юзеров хабра?
Интересует например как отреагировали в Qt…
У меня общий вопрос, связанный со статич. анализом кода, но не связанный с PVS-Studio.
Как-то прочитав очередную вашу статью (я их почти всегда читаю), я решил, что пора. Поскольку пишу для себя на скриптовых ЯПах, нашел стат. анализатор для JS, загрузил туда свой код… и получил просто тонны ошибок и предупреждений. Буквально на каждую строчку (а то и не по одному разу).
В общем, я так и не смог заставить уйти хотя бы часть этих вещей, потому что при исправлении кода согласно предупреждениям (не все получалось исправить, да), у меня ломалось поведение в браузере.
Вопрос простой: что делать?
Как-то прочитав очередную вашу статью (я их почти всегда читаю), я решил, что пора. Поскольку пишу для себя на скриптовых ЯПах, нашел стат. анализатор для JS, загрузил туда свой код… и получил просто тонны ошибок и предупреждений. Буквально на каждую строчку (а то и не по одному разу).
В общем, я так и не смог заставить уйти хотя бы часть этих вещей, потому что при исправлении кода согласно предупреждениям (не все получалось исправить, да), у меня ломалось поведение в браузере.
Вопрос простой: что делать?
Очевидно, что это не очень хороший анализатор кода. «Шумный».
Многие делают частую ошибку полагая, что чем больше сообщений выдаст анализатор кода, тем лучше. Вот хороший пример, почему это не так.
Многие делают частую ошибку полагая, что чем больше сообщений выдаст анализатор кода, тем лучше. Вот хороший пример, почему это не так.
Это плохой статический анализатор, потому что так много ошибок и предупреждений не должно быть. В хорошем с ложными срабатываниями борятся всеми путями
Да. Как раз недавно писал рассуждения на эту тему: "Философия статического анализатора PVS-Studio".
Только хотел купить телевизор Samsung на Tizen, и тут как тут ваша статья.
Теперь очень крепко подумаю, и скорее всего куплю… хм, LG тормозят, Panasonic российской плохой сборки, ломаются… остаётся Sony. Но их диспели и дизайн что-то не впечатляют. Эх, дилема.
извиняюсь за оффтоп, для меня выбор ТВ это боль.
Анализ неплох, подумаю над тем чтобы прогнать свои проекты через анализатор.
Теперь очень крепко подумаю, и скорее всего куплю… хм, LG тормозят, Panasonic российской плохой сборки, ломаются… остаётся Sony. Но их диспели и дизайн что-то не впечатляют. Эх, дилема.
извиняюсь за оффтоп, для меня выбор ТВ это боль.
Анализ неплох, подумаю над тем чтобы прогнать свои проекты через анализатор.
Анализаторы должны использоваться регулярно и тогда многие, в том числе и критические, ошибки будут обнаруживаться на самом раннем этапе
и
У меня ушло около недели на то, чтобы отобрать предупреждения, которые, по моему мнению, указывают на настоящие ошибки(и это жалкие 3% от общей кодовой базы).
Вот это противоречие слегка мешает внедрять стат. анализ на постоянной основе, особенно для проектов с огромным легаси, для которых порог вхождения высотой с гору.
Не мешает. В инструментах типа PVS-Studio есть возможность легко подавить все старые сообщения об ошибках и получать срабатывания только на новый или модифицированный код.
Один человек за неделю проработал 1/33 огромного проекта, а сколько понадобилось человекочасов написать эти жалкие 3%?
Я думаю для проекта можно проанализировать все, а потом анализировать только изменения (новый/измененный код).
Я думаю для проекта можно проанализировать все, а потом анализировать только изменения (новый/измененный код).
Не мешает. В инструментах типа PVS-Studio есть возможность легко подавить все старые сообщения об ошибках и получать срабатывания только на новый или модифицированный код. Это позволяет получать пользу от анализатора СРАЗУ на проекте любого размера.
UFO just landed and posted this here
27к ошибок — выглядит как пасхалка, как бы намекает на соответствие ISO 27000 :)
Было интересно почитать, спасибо!
Было интересно почитать, спасибо!
> Создаются структуры типа sockaddr_un и sockaddr_in. При этом хранятся и уничтожаются они как структуры типа sockaddr.
> Надо понимать, что рассмотренный код очень опасен и достаточно, чтобы в одном из классов появился конструктор/деструктор или был добавлен член сложного типа (например std::string), как всё сразу сломается окончательно.
Появление конструктора в sockaddr_* это из области невероятной фантастики — это гарантированно плоские структуры данных.
Код может быть тут сколько угодно формально некорректным, исходя из формальности типа «если new, то вероятен конструктор», но сама эта формальность неуместна.
Более того, если бы это случилось, то удаление объекта через указатель на базовый класс с виртуальным деструктором — нормальная ситуация: если B и C — потомки A, а A имеет виртуальный деструктор, то код вида
законен.
Так что тут ваша проверка некорректна, рекомендую исправить. Для семейства sockaddr лучше вообще добавить исключение (работа через sockaddr* — общее правило BSD sockets API), а в общем случае ловить удаление через базовый класс без виртуального конструктора.
А вот что я бы рекомендовал добавить — именно для sockaddr_* (всех адресных семейств) — это или использование calloc(), или заполнение структуры нулевыми байтами перед использованием. Отсутствие такого заполнения может приводить к странным эффектам при переносе между ОС: например, для BSD семейства (включая MacOS) sin_len (отсутствующее в Linux) должно быть или 0, или актуального размера.
Также это касается ряда других интерфейсных структур — с ходу вспоминается sigaction. Можно вообще поставить общим правилом для ядерных структур.
> Для таких случаев есть стандартный макрос M_PI, который вдобавок раскрывается в более точное значение.
M_PI не определён ни в стандарте C, ни в базовом Posix; есть в Posix расширении XSI и в Microsoft SDK. Утверждение, что он стандартен, распространено, но неверно. Эта диагностика также требует исправления.
> snprintf(buf + strlen(buf), sizeof(buf),
Против такого лучший идиоматический приём выглядит примерно так:
Хотя для короткого буфера и двух записей хватит и такого strlen.
> Здесь забыты фигурные скобки.
Вообще я бы рекомендовал настаивать на включении тел if, else, while в фигурные скобки в принудительном порядке везде и всегда. Новые языки (Swift, Go) это уже делают в основах синтаксиса.
> argv2[i] = NULL;
> Не будут обнулены указатели.
> NULL будет записан за границу массива.
Обнуление не обязательно. NULL, скорее всего, не будет записан за границу, потому что массив argv в Unix-стиле передачи аргументов другим процессам всегда заканчивается ещё одним NULL, не указанным в arg_count (argc — для main).
Тут жалоба, я уверен, ложна, хотя определить это по самому коду может быть слишком сложно для анализатора.
> The 'strncat' function call could lead to the 'dd_info->object_uri' buffer overflow.
И опять strlen (см. выше), но strncat это вообще диверсия почти всегда.
В случае Unix мира лучше рекомендовать переходить на strlcat, а в общем случае — на strncat_s.
> Предупреждение PVS-Studio: V591 Non-void function should return a value.
Нормальный компилятор это вообще должен считать за ошибку. Этот код, подозреваю, вообще не компилируют.
> Надо понимать, что рассмотренный код очень опасен и достаточно, чтобы в одном из классов появился конструктор/деструктор или был добавлен член сложного типа (например std::string), как всё сразу сломается окончательно.
Появление конструктора в sockaddr_* это из области невероятной фантастики — это гарантированно плоские структуры данных.
Код может быть тут сколько угодно формально некорректным, исходя из формальности типа «если new, то вероятен конструктор», но сама эта формальность неуместна.
Более того, если бы это случилось, то удаление объекта через указатель на базовый класс с виртуальным деструктором — нормальная ситуация: если B и C — потомки A, а A имеет виртуальный деструктор, то код вида
A* a1 = new B(); A* a2 = new C(); ... delete a1; delete a2;
законен.
Так что тут ваша проверка некорректна, рекомендую исправить. Для семейства sockaddr лучше вообще добавить исключение (работа через sockaddr* — общее правило BSD sockets API), а в общем случае ловить удаление через базовый класс без виртуального конструктора.
А вот что я бы рекомендовал добавить — именно для sockaddr_* (всех адресных семейств) — это или использование calloc(), или заполнение структуры нулевыми байтами перед использованием. Отсутствие такого заполнения может приводить к странным эффектам при переносе между ОС: например, для BSD семейства (включая MacOS) sin_len (отсутствующее в Linux) должно быть или 0, или актуального размера.
Также это касается ряда других интерфейсных структур — с ходу вспоминается sigaction. Можно вообще поставить общим правилом для ядерных структур.
> Для таких случаев есть стандартный макрос M_PI, который вдобавок раскрывается в более точное значение.
M_PI не определён ни в стандарте C, ни в базовом Posix; есть в Posix расширении XSI и в Microsoft SDK. Утверждение, что он стандартен, распространено, но неверно. Эта диагностика также требует исправления.
> snprintf(buf + strlen(buf), sizeof(buf),
Против такого лучший идиоматический приём выглядит примерно так:
// удобно, потому что blimit уже сдвигаться не будет const char *blimit = buf + sizeof buf; // а вот этот едет каждый раз const char *bpos = buf; ... r = snprintf(bpos, blimit - bpos, порция1); // привет Шлемиэлю, но его подвиг мы не повторяем // также компилятор может соптимизировать blimit-bpos в одно вычисление bpos += min(r, blimit - bpos); r = snprintf(bpos, blimit - bpos, порция2); bpos += min(r, blimit - bpos); ...
Хотя для короткого буфера и двух записей хватит и такого strlen.
> Здесь забыты фигурные скобки.
Вообще я бы рекомендовал настаивать на включении тел if, else, while в фигурные скобки в принудительном порядке везде и всегда. Новые языки (Swift, Go) это уже делают в основах синтаксиса.
> argv2[i] = NULL;
> Не будут обнулены указатели.
> NULL будет записан за границу массива.
Обнуление не обязательно. NULL, скорее всего, не будет записан за границу, потому что массив argv в Unix-стиле передачи аргументов другим процессам всегда заканчивается ещё одним NULL, не указанным в arg_count (argc — для main).
Тут жалоба, я уверен, ложна, хотя определить это по самому коду может быть слишком сложно для анализатора.
> The 'strncat' function call could lead to the 'dd_info->object_uri' buffer overflow.
И опять strlen (см. выше), но strncat это вообще диверсия почти всегда.
В случае Unix мира лучше рекомендовать переходить на strlcat, а в общем случае — на strncat_s.
> Предупреждение PVS-Studio: V591 Non-void function should return a value.
Нормальный компилятор это вообще должен считать за ошибку. Этот код, подозреваю, вообще не компилируют.
Примечание. Я постоянно сталкиваюсь с желанием программистов заявить, что это возможно не баг, а фича :). Пример с reddit. Мне это непонятно, зачем пытаться оправдать явно плохой, опасный и ошибочный код? В чем профит? :)
Теперь про разные моменты.
1) sockaddr_
Не важно, появится там конструктор или нет. Код неправильный. Нельзя создавать объект одного размера, а уничтожать как объект другого размера. Да везёт, что пока всё это работает и я не могу продемонстрировать обратное (быть может кто-то поможет). Сбой может произойти по разным причинам. Например, менеджер памяти реализован так, что хранит информацию об объектах разного размера в разных таблицах и использовать разные пулы памяти. И тогда, он может не найти запись об выделенной памяти в соответствующей таблице.
Или менеджер памяти особо-секьюрный и обнуляет освобождаемую память. А тут ему подсовывают объект не того размера.
Так что анализатор ведёт себя правильно и ничего в нём исправлять не надо. Надо исправлять код программы.
Вы приводите пример и говорите об наследовании с виртуальными деструкторами. Это совсем другая ситуация и в подобном случае естественно анализатор промолчит. Я ведь специально отметил в статье, что эти 3 класса никак не связаны между собой.
2) M_PI
Согласен, местами его нет. Но раз в других местах используется M_PI, то не вижу смысла городить велосипед. Первый попавшийся M_PI в C-файле (проект org.tizen.myplace-1.0.1, файл myplace-suggestedview.c):
3) argv2[i] = NULL;
Это не тот argv, который приходит в main. Это нечто самобытное:
4) Non-void function should return a value
Я неприятно удивлю, но компиляторы до сих пор проглатывают такой код. Примеры.
Теперь про разные моменты.
1) sockaddr_
Не важно, появится там конструктор или нет. Код неправильный. Нельзя создавать объект одного размера, а уничтожать как объект другого размера. Да везёт, что пока всё это работает и я не могу продемонстрировать обратное (быть может кто-то поможет). Сбой может произойти по разным причинам. Например, менеджер памяти реализован так, что хранит информацию об объектах разного размера в разных таблицах и использовать разные пулы памяти. И тогда, он может не найти запись об выделенной памяти в соответствующей таблице.
Или менеджер памяти особо-секьюрный и обнуляет освобождаемую память. А тут ему подсовывают объект не того размера.
Так что анализатор ведёт себя правильно и ничего в нём исправлять не надо. Надо исправлять код программы.
Вы приводите пример и говорите об наследовании с виртуальными деструкторами. Это совсем другая ситуация и в подобном случае естественно анализатор промолчит. Я ведь специально отметил в статье, что эти 3 класса никак не связаны между собой.
2) M_PI
Согласен, местами его нет. Но раз в других местах используется M_PI, то не вижу смысла городить велосипед. Первый попавшийся M_PI в C-файле (проект org.tizen.myplace-1.0.1, файл myplace-suggestedview.c):
static inline double __rad_2_deg(double radians)
{
return radians * (180.0 / M_PI);
}3) argv2[i] = NULL;
Это не тот argv, который приходит в main. Это нечто самобытное:
....
argc = malloc(sizeof(int));
argv = malloc(sizeof(gchar *) *max_argc);
argv2 = malloc(sizeof(gchar *) *max_argc);
if (!argc || !argv || !argv2)
goto ERROR;
memset(argv, 0, sizeof(gchar *) *max_argc);
memset(argv2, 0, sizeof(gchar *) *max_argc);
....
4) Non-void function should return a value
Я неприятно удивлю, но компиляторы до сих пор проглатывают такой код. Примеры.
> Например, менеджер памяти реализован так, что хранит информацию об объектах разного размера в разных таблицах и использовать разные пулы памяти.
Да, это аргумент (хотя такой реализации для C/C++ ни разу вживую не видел).
Тогда это надо переделать на malloc/free. Использование же через общий sockaddr* остаётся основным методом.
> Так что анализатор ведёт себя правильно и ничего в нём исправлять не надо.
Но про контроль обнуления структур рекомендую таки подумать.
> Это не тот argv, который приходит в main. Это нечто самобытное:
Я и не говорил, что это входной argv данного процесса. Это, скорее всего, выходной, для передачи в какой-то из exec*(). И он обязан завершаться NULLʼом, иначе дочерний процесс не запустится правильно.
> Я неприятно удивлю, но компиляторы до сих пор проглатывают такой код.
MSVC, да?
Да, это аргумент (хотя такой реализации для C/C++ ни разу вживую не видел).
Тогда это надо переделать на malloc/free. Использование же через общий sockaddr* остаётся основным методом.
> Так что анализатор ведёт себя правильно и ничего в нём исправлять не надо.
Но про контроль обнуления структур рекомендую таки подумать.
> Это не тот argv, который приходит в main. Это нечто самобытное:
Я и не говорил, что это входной argv данного процесса. Это, скорее всего, выходной, для передачи в какой-то из exec*(). И он обязан завершаться NULLʼом, иначе дочерний процесс не запустится правильно.
> Я неприятно удивлю, но компиляторы до сих пор проглатывают такой код.
MSVC, да?
Поддерживает ли C# анализатор у PVS-Studio data-flow-analysis?
Да.
Интересно.
Либо вы не понимаете, что стоит за словами data-flow-analysis, либо просто нагло врете.
Просто ради интереса загрузил с вашего сайта анализатор и декомпилировал первой попавшийся под руку тулзой.
Никакого анализа потока данных у вас не вижу абсолютно.
Ожидал, что хоть хваленный механизм виртуальных значений будет честно строится, но нет…
Все работает крайне наивным способом, разбито на какие-то частные случаи, проходах по AST, и с кучей дублирования кода.
Я не буду тут приводить участки кода, просто скажу, что после увиденного, рекомендовать и уж тем более покупать ваш продукт отпало сразу.
Либо вы не понимаете, что стоит за словами data-flow-analysis, либо просто нагло врете.
Просто ради интереса загрузил с вашего сайта анализатор и декомпилировал первой попавшийся под руку тулзой.
Никакого анализа потока данных у вас не вижу абсолютно.
Ожидал, что хоть хваленный механизм виртуальных значений будет честно строится, но нет…
Все работает крайне наивным способом, разбито на какие-то частные случаи, проходах по AST, и с кучей дублирования кода.
Я не буду тут приводить участки кода, просто скажу, что после увиденного, рекомендовать и уж тем более покупать ваш продукт отпало сразу.
Продолжение. Поговорим о микрооптимизациях на примере кода Tizen.
Что интересно, часть ошибок и подозрительных мест, обнаруживаемых статическими анализаторами, успешно обнаруживают и современные компиляторы с выкрученным на максимум уровнем предупреждений.
Я еще лет двадцать назад взял за правило писать код так, чтобы он компилировался с /Wall (у меня MS VC++). Для этого, как правило, требуется простая аккуратность, иногда приходится отключать отдельные предупреждения локально, а глобально — только самые малополезные, вроде «function not inlined».
Чужой же код в большинстве случаев неспособен компилироваться даже с /W4, не говоря уже о /Wall. Более того, в среде «чистых сишников» преобладает убеждение, что повышенный уровень предупреждений — для новичков или тупиц, а «настоящий сишник» попросту обязан использовать различные трюки, на которые обычно и возбуждается компилятор.
Я еще лет двадцать назад взял за правило писать код так, чтобы он компилировался с /Wall (у меня MS VC++). Для этого, как правило, требуется простая аккуратность, иногда приходится отключать отдельные предупреждения локально, а глобально — только самые малополезные, вроде «function not inlined».
Чужой же код в большинстве случаев неспособен компилироваться даже с /W4, не говоря уже о /Wall. Более того, в среде «чистых сишников» преобладает убеждение, что повышенный уровень предупреждений — для новичков или тупиц, а «настоящий сишник» попросту обязан использовать различные трюки, на которые обычно и возбуждается компилятор.
Что интересно, часть ошибок и подозрительных мест, обнаруживаемых статическими анализаторами, успешно обнаруживают и современные компиляторы с выкрученным на максимум уровнем предупреждений.
Да, это так. Однако помимо того, что статические анализаторы мощнее, есть ещё одно важное отличие. Они больше внимания уделяют борьбе с ложными срабатываниями, пытаясь угадать, используется особый стиль написания кода, или это действительно ошибка.
Поэтому часто бывает, что диагностика вроде одна и та-же, но компилятор даёт так много шума, что проще её выключить. А анализатор на том же самом коде может давать вполне вменяемый результат, который возможно изучить, найти ошибки, а затем донастроить, чтобы ложных срабатывания вообще пропали.
На какие «особые стили» компилятор может давать так много шума, что проще выключить предупреждения, считая стиль более ценным?
На какие «особые стили» компилятор может давать так много шума, что проще выключить предупреждения, считая стиль более ценным?
Да, такие есть. Имя им – легион.
Мне сложно сходу приводить примеры. Первое, что вспоминается, это диагностика C4265 (class has virtual functions, but destructor is not virtual), которая отключена по умолчанию. Можно конечно сказать, что диагностика сделана плохо, но формально она звучит правильно. Однако, если используется паттерн примесь (подмешивание), то эта диагностика мешает. Подробности здесь.
Собственно, можно взять любой большой проект, который не рассчитан на /Wall, скомпилировать его с /Wall и получить массу предупреждений. И вот как только возникнет мысль, что не хочу вот тут править код, а проще выключить предупреждения, так вот это и есть тот самый случай, когда стиль важнее дурацкого предупреждения. :)
Почему диагностика сделана плохо? В такой ситуации действительно существует опасность неправильной обработки объекта. Если программист уверен, что такой опасности нет — он может обернуть определение класса временным отключением предупреждения, а его последователям это явно подскажет наличие ограничений на поведение деструктора.
Когда уже есть проект, не рассчитанный на /Wall, вопросов к его продолжателям нет. :) Вопрос к разработчикам проекта о том, почему в нем слишком много кода, вызывающего предупреждения.
Когда уже есть проект, не рассчитанный на /Wall, вопросов к его продолжателям нет. :) Вопрос к разработчикам проекта о том, почему в нем слишком много кода, вызывающего предупреждения.
Почему диагностика сделана плохо?
Потому, что на практике она даёт столь много бесполезных сообщений, что её выключили.
Я уже указывал на то, что ряд расширенных предупреждений компилятора совпадает с предупреждениями вашего анализатора. Если они столь бесполезны — для чего ваш анализатор их выдает?
Если они столь бесполезны — для чего ваш анализатор их выдает?
Я многократно отмечал в статьях и комментариях, что если мы и делаем диагностики схожие с диагностиками компилятора, то только в том случае, если можем сделать их чем-то лучше/полезнее/удобнее. Просто повторять смысла конечно нет.
Приведённый пример, был как раз одним из них. Прошу познакомиться с ссылкой, которую я привёл ранее. Там разобрано, как работает диагностика Visual C++ и наша.
Диагностика V599 PVS-Studio обладает низким процентом false positives и хорошо находит ошибки, на которые ориентирована. А если хочется посмотреть на все классы без виртуальных деструкторов (но содержавших другие виртуальные функции), то можно включить C4265 и наслаждаться.
Ещё пример.
#define M1 100
#define M2 100
int F(int a)
{
if (a == M1 || a == M2)
a = 42;
return a;
}
Очень частая ситуация. Числа объявлены через макросы и обозначают разные сущности, но по факту имеют одно и тоже числовое представление. Анализатор Visual C++ выдаёт здесь
warning C6287: Redundant code: the left and right sub-expressions are identical.
Что приводит к сильному шуму в некоторых проектах. Мы же учитываем подобные ситуации и специально молчим. Это было одно из первых исключений в диагностике V501 (всего их там сейчас 35).
Если здесь используются дополнительные проверки на частные случаи — вопросов нет. Но предупреждения «Expression is Always True/False», «Use of Uninitialized Variable», «Unreachable Code», как я понял, выдаются в тех же случаях, что и компилятором. Они не создают шума?
Вы путаетесь в показаниях. Если у вас все проекты с /Wall — откуда «лишний шум»? У вас не будет этих сообщений. Ни от компилятора, ни от анализатора.
С чего Вы взяли, что я говорил о своих проектах? У меня сообщений от компилятора как раз нет, и везде стоит /WX (threat warnings as errors).
Андрей выше утверждал, что компиляторы «генерируют шум», а ваш анализатор этого не делает, но как минимум в трех вышеперечисленных случаях он дублирует предупреждения компилятора.
Я ни в коем случае не подвергаю сомнению полезность PVS в плане обнаружения ошибок. Речь о том, что логично сперва включить расширенные предупреждения компилятора и избавиться хотя бы от большинства из них, а затем уже напускать на код специальные анализаторы. Разбираемый здесь код Tizen, судя по всему, компилируется в стандартном режиме с умеренными предупреждениями, отчего PVS и наловил в нем столько ошибок. То есть, разработчики не слишком критично относятся к качеству кода.
Андрей выше утверждал, что компиляторы «генерируют шум», а ваш анализатор этого не делает, но как минимум в трех вышеперечисленных случаях он дублирует предупреждения компилятора.
Я ни в коем случае не подвергаю сомнению полезность PVS в плане обнаружения ошибок. Речь о том, что логично сперва включить расширенные предупреждения компилятора и избавиться хотя бы от большинства из них, а затем уже напускать на код специальные анализаторы. Разбираемый здесь код Tizen, судя по всему, компилируется в стандартном режиме с умеренными предупреждениями, отчего PVS и наловил в нем столько ошибок. То есть, разработчики не слишком критично относятся к качеству кода.
Я не понимаю эти вопросы. Мы просто делаем мощный, эффективный анализатор кода, который намного превосходит возможности компиляторов. Не вижу смысл обсуждать надо делать или не надо делать «Use of Uninitialized Variable». Мы просто тщательно подходим к этому вопросу и делаем хорошую диагностику. И она обнаруживает ошибки.
Компиляторы… Так мы использование неинициализированной переменной и в компиляторе можем найти. Вот, например, мы нашли такую ошибку в Clang:
PVS-Studio: V573 Uninitialized variable 'BytesToDrop' was used. The variable was used to initialize itself. typerecordmapping.cpp 73
Предупреждение «Expression is Always True/False»… Компиляторы… Пфф… :-) Вот, пожалуйста, вновь Clang:
PVS-Studio. V547 Expression is always true. Probably the '&&' operator should be used here. LoopInterchange.cpp 208
Компиляторы… Так мы использование неинициализированной переменной и в компиляторе можем найти. Вот, например, мы нашли такую ошибку в Clang:
static Error mapNameAndUniqueName(....) {
....
size_t BytesLeft = IO.maxFieldLength();
if (HasUniqueName) {
.....
if (BytesNeeded > BytesLeft) {
size_t BytesToDrop = (BytesNeeded - BytesLeft);
size_t DropN = std::min(N.size(), BytesToDrop / 2);
size_t DropU = std::min(U.size(), BytesToDrop - DropN);
....
}
} else {
size_t BytesNeeded = Name.size() + 1;
StringRef N = Name;
if (BytesNeeded > BytesLeft) {
size_t BytesToDrop = std::min(N.size(), BytesToDrop); // <=
N = N.drop_back(BytesToDrop);
}
error(IO.mapStringZ(N));
}
....
}
PVS-Studio: V573 Uninitialized variable 'BytesToDrop' was used. The variable was used to initialize itself. typerecordmapping.cpp 73
Предупреждение «Expression is Always True/False»… Компиляторы… Пфф… :-) Вот, пожалуйста, вновь Clang:
static bool containsNoDependence(CharMatrix &DepMatrix,
unsigned Row,
unsigned Column) {
for (unsigned i = 0; i < Column; ++i) {
if (DepMatrix[Row][i] != '=' || DepMatrix[Row][i] != 'S' ||
DepMatrix[Row][i] != 'I')
return false;
}
return true;
}
PVS-Studio. V547 Expression is always true. Probably the '&&' operator should be used here. LoopInterchange.cpp 208
Так, скорее всего, и очередная версия PVS, будучи натравлена сама на себя, периодически находит что-то подозрительное, и среди этого время от времени обнаруживаются и ошибки. :)
Но я говорил не об этом, а о том, что маловероятно получить от приличного компилятора в режиме, аналогичном /Wall, столь большое количество шума, что «аж руки опускаются», и при этом получить от PVS в основном дельные предупреждения. Если код достаточно хорош, то и компилятор, и PVS выдадут разумное количество предупреждений. Если же количество предупреждений компилятора огромно, то и код, скорее всего, написан халтурно, на «лишь бы работало». PVS в этом случае точно так же завалит разработчика предупреждениями, и ему останется либо махнуть рукой, или таки заняться массовой чисткой, а не отдельными косметическими правками.
Но я говорил не об этом, а о том, что маловероятно получить от приличного компилятора в режиме, аналогичном /Wall, столь большое количество шума, что «аж руки опускаются», и при этом получить от PVS в основном дельные предупреждения. Если код достаточно хорош, то и компилятор, и PVS выдадут разумное количество предупреждений. Если же количество предупреждений компилятора огромно, то и код, скорее всего, написан халтурно, на «лишь бы работало». PVS в этом случае точно так же завалит разработчика предупреждениями, и ему останется либо махнуть рукой, или таки заняться массовой чисткой, а не отдельными косметическими правками.
Например, если в проекте используются только умные указатели, то виртуальные деструкторы уже как бы и не нужны — у умных указателей свои механизмы.
Если не секрет, в чем причина сокрытия цен на лицензии? Предложения в стиле «если хотите купить — свяжитесь с нами» привычно наводят на мысли о стремлении аккуратно прощупать клиента, чтобы раскрутить его на максимальную цену.
Предложения без явных цен вполне нормальны для чего-то необычного, эксклюзивного, изготавливаемого по заказу, у Вас же предлагается готовый продукт.
Предложения без явных цен вполне нормальны для чего-то необычного, эксклюзивного, изготавливаемого по заказу, у Вас же предлагается готовый продукт.
Секрета нет. Мы считаем, что так правильно. Наше мнение разделяют другие компании, продающие решения в сфере статического анализа кода.
А в чем именно правильность?
Если подобную позицию займут другие компании, предлагающие различные готовые продукты — программы, книги, фильмы, музыкальные записи, автомобили и т.п., у Вас, при желании приобрести такой продукт, не возникнет удивления или недовольства?
А если таким образом будут предлагаться продукты в супермаркете? Не совсем массовые, но готовые, не требующие какой-либо доработки под приобретателя?
Если подобную позицию займут другие компании, предлагающие различные готовые продукты — программы, книги, фильмы, музыкальные записи, автомобили и т.п., у Вас, при желании приобрести такой продукт, не возникнет удивления или недовольства?
А если таким образом будут предлагаться продукты в супермаркете? Не совсем массовые, но готовые, не требующие какой-либо доработки под приобретателя?
Мы уже пробовали подход с открытыми ценами, причём очень низкими. Подробности.
Похоже на то, что вы переходите из крайности в крайность. Возможно, не было особого смысла выделять упрощенную версию в отдельный продукт, а просто сделать модифицированный вариант основного продукта с тем же названием. Еще логичнее было бы предлагать отдельные цены на конкретные возможности (объем анализируемого кода, глубина анализа, используемые эвристики и т.п.), чтобы можно было выбрать наиболее подходящую конфигурацию. Ну и, понятное дело, сильно ограничить упрощенные версии в поддержке.
В текущем состоянии продукта дискомфорт вызывает не подразумеваемо высокая цена, а именно ее неопределенность.
В текущем состоянии продукта дискомфорт вызывает не подразумеваемо высокая цена, а именно ее неопределенность.
Касательно самопальных чисел пи. Что говорит ваш анализатор на вот такое?
github.com/mwhagedorn/SoundTouch/blob/master/SoundTouch/AAFilter.cpp#L52
github.com/mwhagedorn/SoundTouch/blob/master/SoundTouch/AAFilter.cpp#L52
в ту же копилку: gitlab.com/xonotic/netradiant/blob/a78573e0e9d7a7d13519dc03d5f00f9e87bedaa1/tools/quake3/q3map2/light.c#L709
Ну и чтоб добить танцем, константа AI_MATH_PI_F из этой либы:
assimp.sourceforge.net/lib_html/defs_8h.html
Ну и чтоб добить танцем, константа AI_MATH_PI_F из этой либы:
assimp.sourceforge.net/lib_html/defs_8h.html
#define PI 3.141592655357989 — не ругаемся, так как константа задана очень точно.
#define ONE_OVER_2PI 0.159154942f — не ругаемся, т.к. анализтор ничего не знает про эту магическую константу. Вот константы, с которыми знаком анализтор: M_E, M_LOG2E, M_LOG10E, M_LN2, M_LN10, M_PI, M_PI_2, M_PI_4, M_1_PI, M_2_PI, M_2_SQRTPI, M_SQRT2, M_SQRT1_2.
#define ONE_OVER_2PI 0.159154942f — не ругаемся, т.к. анализтор ничего не знает про эту магическую константу. Вот константы, с которыми знаком анализтор: M_E, M_LOG2E, M_LOG10E, M_LN2, M_LN10, M_PI, M_PI_2, M_PI_4, M_1_PI, M_2_PI, M_2_SQRTPI, M_SQRT2, M_SQRT1_2.
Первая это константа с ошибкой:
А во второй ошибка в вычислении, см. комментарий.
Я к тому, что если ловить такие ошибки, то стоит проверять их не только по имени, но и по мат.разнице с правильной константой.
То есть в AI_MATH_PI_F последняя цифра должна быть 6 а не 8:
3.141592655357989
3.14159265357989
А во второй ошибка в вычислении, см. комментарий.
Я к тому, что если ловить такие ошибки, то стоит проверять их не только по имени, но и по мат.разнице с правильной константой.
То есть в AI_MATH_PI_F последняя цифра должна быть 6 а не 8:
3.1415926538f
3.1415926536f
Решил загуглить, и эта же опечатка (что в OVER_2PI в комменте) встречается прямо в коде goo.gl/Wchb9E (игрушка Space Engineers вроде)
float ac = N * (float)acos(saturate(VDIR.y)) / 3.141592657f;
// правильно:
float ac = N * (float)acos(saturate(VDIR.y)) / 3.141592654f;
Тем, кто читал эту статью, возможно будет интересно, что я добрался и до Android: Проверили с помощью PVS-Studio исходные коды Android, или никто не идеален.
Sign up to leave a comment.
27000 ошибок в операционной системе Tizen