Pull to refresh

Comments 35

>> Функция IsAllASCII7() всегда возвращает 'true'.…
А вот это далеко не факт, у многих компиляторов (в том числе и Visual Studio) есть опции, которые делают типа char по умолчанию как unsigned char. Хоть это и костыльный механизм, но такое используется.
Плагин PVS-Studio проверяет наличие этого ключа и передаёт информацию анализатору. В общем, этот момент учтён.
Костыльный механизм?
Вообще, под той же вижуалкой очень широко принято использовать ключ компиляции _UNICODE в настройках проекта, который заставляет компилироваться MFC'шный шаблон CString на базе wchar_t, который не просто unsigned char, а самый что ни на есть WORD.
В контексте данной функции сия ремарка может быть вполне уместна. Просто, возможно, автор поста анализировал ANSI-версию (если, конечно, таковая до сих пор осталась в TortoiseSVN), либо анализатор не учитывает ключей компиляции, либо про них забыли сами разработчики.
UFO just landed and posted this here
Откиньтесь на спинку кресла и отдохните, пока %driver_name% устанавливается на ваш компьютер. Во время установки вы узнаете о новых замечательных возможностях %driver_name% и о некоторых из наиболее важных функций системы. После завершения установки компьютер пригласит вас на более подробный курс ознакомления с %driver_name%.
А как ваша программа справится с более сложным кодом, например с Boost? Может ли она находить ошибки/странности при использования шаблонов. К примеру что-то типа #8702:
template<typename T>
class has_size_type
{
    typedef char no_type;
    struct yes_type { char dummy[2]; };

    template<typename C>
    static yes_type test(BOOST_DEDUCED_TYPENAME C::size_type x);

    template<typename C, typename Arg>
    static no_type test(Arg x); // Ошибка здесь. Должен быть '...' вместо шаблонного параметра
    /*утилита может выдать предупреждение о том, что "ищется строгое соответствие с int. Вы имели в виду "..."?"*/

public:
    static const bool value = sizeof(test<T>(0)) == sizeof(yes_type);
};
Boost мы прожёвываем. Другое дело, что искать ошибки в шаблонах дело очень неблагодарное. Кое-что мы находим, но мало. Например, одна из бед в том, что шаблоны нередко содержат вообще не компилируемый код. Шаблоны приходится «аккуратно трогать палочкой». Ибо неизвестно, что там. Кстати, Visual C++ проглатывает некорректные неиспользуемые функции в шаблонах. При попытке их использовать возникнет ошибка компиляции. Далеко ходить не надо. Такие функции есть даже в заголовочных файлах от Visual C++. См. например файл «Microsoft Visual Studio 8\VC\atlmfc\include\atldb.h»:

template <const GUID* pguidProvider>
class CErrorReporterHelper
public:
  HRESULT PostError(HRESULT hrErr, IID* piid)
  {
    . . .
    errorinfo.hrError  = hrErr;
    errorinfo.iid    = *piid;
    spCrtErrInfo->SetGUID(errorinfo.iid)
    spCrtErrInfo->SetSource(OLESTR("Provider PROGID"));


После SetGUID(errorinfo.iid) нет точки с запятой.

Возвращаясь к приведенному примеру. Нет, не найдем. Слишком новые и тонкие материи. Мы ищем более приземленные вещи. И уверен, правильно, что поступаем так. На одну заковырку приходится с десяток обыкновенных опечаток и просто ляпов. Да, это именно так. :) Ощутите глубину глубин.
Ощутите глубину глубин.

Ощутил. Внушает.

А вы не хотели бы сделать как один из ваших конкурентов (кажется Coverity): выложить ошибки опенсорсных проектов чтобы разработчики могли их поправить. Я после ваших предыдущих постов искал отдельную ссылку ошибок по Boost, но не нашел :(
Проверяя очередной проект, мы пишем статью. Уведомляем авторов проекта о найденных недочетах. Выдаем им на некоторое время анализатор, чтобы они смогли более тщательно проверить проект, чем делаем это мы. То, что находим мы сами, оформляется в виде базы ошибок. Этого вполне достаточно для демонстрации.

Сам Boost мы не проверяли. Там сложная система сборки, в которую непросто интегрироваться. По крайней мере, с наскока проверить Boost не удалось. А стимула заняться библиотекой более плотно у не было. Тот же Coverity проверят open source проекты не из-за любви к искусству, а за гранты от United States Department of Homeland Security (DHS).

Был ещё какой-то энтузиаст, который очень хотел проверить Boost. Утверждал, что сам сделает это. Мы выдали ему на время ключ. Он пропал. Видимо тоже не смог разобраться в системе сборки.

Глубина, глубина, я не твой. (с)
Два вопроса:
1) Возможно ли как-то прикрутить PVS Studio к Эклипсу? Или, может быть, интеграция планируется в будущем?

2) Ругается ли PVS на такие вещи:

uint8_t size;
if(size >= UART_RX_MSG_SIZE_MIN && size <= UART_RX_MSG_SIZE_MAX) ( компилятор выдает ворнинг — comparison is always true, потому что сейчас эти дефайны равны 0 и 255. Но это не обязательно всегда так. Аналогичные ворнинги при сравнении с параметрами шаблонов)
Отвечу про Эклипс. Вот здесь описано как использовать анализатор из командной строки. Если такой вариант устроит (без плагина, без автоматической интеграции), то тогда использовать можно.
Для этого все равно нужен проектный файл VS, так что не очень.
Я ошибся и дал ссылку не туда. Вот правильная ссылка. Где как раз не нужен проектный файл.
Пункт N2.

Да, PVS-Studio тоже предупредит. Как человеку, мне понятно, что тут всё хорошо. С точки зрения анализатора, не понятно на чем основывать исключение из правила. По отдельности, каждая проверка весьма подозрительна. И лучше перестраховаться и предупредить.
Ложное срабатывание не так страшно. В PVS-Studio есть масса методов их подавления. Например, можно использовать специальный комментарий:

if(size >= 0 && size <= 255) //-V547


Ещё вариант — указать, что там где используется макросы UART_RX_MSG_SIZE_MIN и UART_RX_MSG_SIZE_MAX, не стоит выдавать диагностику V547. Для этого в заголовочном файле рядом с макросами можно вписать:

#define UART_RX_MSG_SIZE_MIN 0
#define UART_RX_MSG_SIZE_MAX 255
//-V:UART_RX_MSG_SIZE_MIN:547
//-V:UART_RX_MSG_SIZE_MAX:547


Чтобы убедить, что диагностика V547 ой как полезна, предлагаю посмотреть примеры ошибок, выявляемые ей. Там встретится и сравнение с 0, и с 255.
Подавление комментарием — это способ понятный и хороший.

Но почему бы просто не давить предупреждение, если происходит сравнение с дефайном или параметром шаблона? По ссылке-то сравнения с магическими числами (если только вы дейфайны не убирали самостоятельно)
Макросы часто используются. Вполне может быть ошибка. То что именно такого примера нет, ни о чем не говорит. Зато, например, есть такое:
#define DISKREADBUFFER_SIZE HEX(10000)

typedef unsigned short USHORT, *PUSHORT;

static VOID DetectBiosDisks(....)
{
  USHORT i;
  ....
  Changed = FALSE;
  for (i = 0; ! Changed && i < DISKREADBUFFER_SIZE; i++)
  {
    Changed = ((PUCHAR)DISKREADBUFFER)[i] != 0xcd;
  }
  ....
}
V547 Expression 'i < 0x10000' is always true. The value range of unsigned short type: [0, 65535]. xboxhw.c 358
По пункту один позволю себе сформулировать пожелание:
было бы очень удобно иметь возможность из командной строки проверять директорию целиком, а не отдельные файлы. Cppcheck может, а проверять больше 10 файлов за раз в ручном режиме по одному — достаточно неудобно.
Батники писать разучились?
Нет, но PVS-Studio стала ругаться, что строка с файлами слишком большая :)
Плюс этот батник пришлось бы руками обновлять при добавлении/удалении файлов из проекта.
Мне кажется, Вы что-то не то пытаетесь делать. Командная версия предназначена для интеграции в систему сборки (вызов из makefile и т.п.). Идея в том, чтобы заменить вызов компилятора на вызов PVS-Studio, немного пошаманив. Не понятно, зачем пытаться придумывать командные строки для абстрактного набора файлов.

Напишите нам в поддержку. Попробуем разобраться с Вашей ситуацией.
Я же говорю — разучились.
Функция IsAllASCII7() всегда возвращает 'true'. Условие 's[i] >= 0x80' всегда ложно. Значение переменной типа 'char' не может быть больше или равно 0x80.

Неправда: TortoiseSVN компилируется в unicode, а это значит что s[i] возвращает WCHAR который без проблем может быть больше 0x80.
Да, действительно. Что-то видимо пошло при анализе не так. Потом посмотрю. Вот и верь таким рекламщикам, как я :). В погоне за статьёй уже некогда всматриваться в код.
Кстати, именно поэтому я всегда настаиваю, чтобы авторы сами проверяли проекты. То что я считаю ошибкой, может оказаться ложным срабатыванием и наоборот. Лучше проверить самостоятельно, зная код. И нам написать про явные ляпы.
Ну вот, а я по наивности думал, что никто кроме меня эту тему не заметил =). Особенно с учётом что первый же комментарий к статье был на эту же тему.
Я не представляю, как вы, ребята, вообще используете хоть какой-то софт, зная, что там внутри такие баги. Это всё равно что есть колбасу, зная, из чего она сделана.
Всё просто. Найденные ошибки содержатся в редко используемых фрагментах кода или вероятность их возникновения крайне мала (например, усечение 64-битного указателя). Более часто проявляющие себя ошибки находятся на этапе тестирования и пользователями. Что медленно, печально и дорого. Поэтому я всегда подчеркиваю:

Основная ценность инструментов статического анализа не в том, что они позволяет находить ошибки в коде редко получающего управления. А в нахождении ошибок на самом раннем этапе. Многие из ляпов, которые будут найдены тестированием или пользователями, можно было бы устранить программисту ещё на этапе кодирования.
А вы не пробовали по WRK так пройтись? Там, конечно, лицензия не позволяет публиковать куски кода, но все равно интересно.
Проверяли где-то полтора года назад. На тот момент нашлось всего одна ошибка и пара подозрительных мест. Но в любом случае на заметку не хватит. С тех пор не возвращались к этому проекту, так как непонятно, что дальше делать, даже если найдём ошибки.
Sign up to leave a comment.