Pull to refresh

Comments 34

Что хотел сказать этой статьёй? Да в общем то ничего. Немного рекламы и не более того

Знаете, более эффективной и честной рекламы ПО, чем ваша, я, пожалуй, и не видел.

Ну, и на правах надоевшей шутки:
А вы отправили авторам список ошибок? А проверили PVS Studio c помощью PVS Studio?
Если бы вся реклама такой была — цены б ей не было!
Эйй!!! А где единорожка?!?! Почему двурогие? Это с чем-то связано? Вы хотите поговорить об этом? А почему вы спрашиваете?
А у вас такого же, только без крыльев для C# нету?
Так для шарпа вроде ReSharper и CodeRush есть? И еще кажется SonarCube
Вот один из таких случаев, где анализатору не понравился аргумент функции free():

А чем NULL в free не понравился?
Вызов free(NULL); не приводит к какой-то беде. Однако, если Вы скажите, что регулярно используете такую конструкцию в своём коде, я буду удивлён. :) Удивлён и анализатор. Он подозревает, что дело не ладно и возможно мы имеем дело с какой-то опечаткой или чем-то ещё.
//а где-то выше
#define NULL 42
И где-то ниже:

if (ps == NULL)
  ps = malloc(sizeof(struct SomeStruct));
А чем NULL в free не понравился?
Вызов free(NULL); «ничего не делает», а нетривиальная конструкция только для того, чтобы «ничего не делать», выглядит странно.

Вот другой гипотетический код, скорее всего, с ошибкой:

if( !ptr ) free(ptr);

Здесь, можно предположить, хотели проверить обратное условие, а получили код, который всегда вызывает free(), передавая нулевой указатель, т.е. с такими параметрами, что он «ничего не делает», зато в случае ненулевого указателя free() не вызывается, а это, скорее всего, приводит к утечке памяти.

В обоих случаях код содержит заведомо «бессмысленный» вызов free(), а статический анализ, среди прочего, может довольно хорошо находить такие «бессмысленные» вызовы.
Про realloc() хорошо разжевано. Старый добрый CppCheck на нее говорит «common realloc mistake», и так писать действительно не стоит!
А вы Java-машину случайно не собираетесь анализировать? OpenJDK, например. ЕМНИП, оно тоже на C++ написано.
Быть может. Проектов вообще разных кругом много :).
Вы не планируете реализовать статический анализ в виде сервиса? Чтобы не каждый программист у себя запускал, а, например, выделяется отдельный сервер для непрерывного статического анализа, а программистам он доступен в виде web-интерфейса. Тогда и SaaS можно будет отдельно еще продавать.
Это я видел. Я имел ввиду аналог Coverity и Klocwork. Я пробовал еще Polyspace и Frama-C, так проекты, которые предоставляют одну точку входа в виде web-интерфейса, по моему мнению, удобнее. Чтобы в команде, например, каждый мог на себя взять какой-то дефект и исправить его, как это делается в системе багтрекинга.
Мы пока не догоняем как это сделать удобно. Если расскажете чуть более подробно в чем смысл и как пользоваться — буду благодарен.
По сути, это тот же PVS Studio, запущенный на сервере, и передающий отчеты напрямую в json/xml к примеру, для последующей укладки, с помощью скриптового языка, в базу данных. По сути, система автоматически будет создавать баг-трекинговую базу данных, с которой уже можно работать в привычном стиле. Особенно, если показывать соответствующий кусок кода. Можно даже сделать это на базе существующих программ для багтрекинга, через их API, предоставляя им соответствующие данные.
Я уже думал о таком сервисе, и пришел к выводу, что @pvs-studio правы: если сделать это таким образом, статический анализатор будут использовать неправильно.

Он не должен создавать тикеты! Он должен маркировать плохие места в момент коммита, ДО того как плохой код покинет разработчика.

… а для старого кода — когда его потрогают, вот тогда и разбираться с ним.
Большинство крупных анализаторов работают именно таким образом. Интеграция с Code-Review и баг-трекингом. И да, существующую кодовую базу вычищать анализатором очень удобно.
Мы в проекте radare2 применяем комбинацию:
  • статический анализатор Coverity
  • профайлер/valgrind
  • автотесты
  • afl (american fuzzy lop)

Результаты всех утилит вместе получаются впечатляющими. И большинство разработчиков предпочитают именно исправления тикетов CID, нежели сферических репортов, например, от gcc.
Я бы сам пристрелил того спаммера, который на каждое сообщение анализатора создает тикет.

А если вы хотите из интерфейса PVS-Studio создать тикет — то это давно есть, через вызов ExternalTool для конкретного сообщения.

Кроме того, отчет PVS-Studio — это XML, с которым можно делать что угодно. Собственно некоторые клиенты с ним так и работают.
Не обязательно это совмещать с главной системой багрепортов. Она может быть второй, параллельной.
То самое чувство, когда почти ничего не понял, но коровы очень понравились)
Проверьте Telegram :) Вдруг на награду наберёте ошибок.
То есть как обычно, не нашли ничего серьезного. Эти мелкие ошибки ни на что не влияют, программа работает, тесты проходят.
Хреновые тесты — бич современного софтостроения.
Нашли мало, так как это неправильное использование методологии статического анализа. Нужно проверять новый код, а не тот которому много лет и который отлажен. Это позволит устранить массу ошибок на самых ранних этапах. Подробнее эта мысль изложена в статье: "Лев Толстой и статический анализ кода".

Кстати, тесты тоже не повредит проверять: Как статический анализ дополняет TDD.

Кстати, на тему Image Uploader...


V614 Potentially uninitialized pointer 'wnd' used. screencapture.cpp 813

Bitmap* CWindowHandlesRegion::CaptureWithTransparencyUsingDWM()
{
  ....
  HWND wnd;
  if (m_ClearBackground || m_RemoveCorners || m_PreserveShadow)
  {
    wnd = CreateDummyWindow(actualWindowRect);
  }
  if (wnd)
  {
  ....
}

V655 The strings were concatenated but are not utilized. Consider inspecting the 'url + "url"' expression. historytreeview.cpp 518

void CHistoryTreeView::DrawSubItem(....)
{
  ....
  url + "url";
  ....
}

V503 This is a nonsensical comparison: pointer >= 0. historytreecontrol.cpp 569
V503 This is a nonsensical comparison: pointer >= 0. historytreecontrol.cpp 600

bool CHistoryTreeControl::LoadThumbnail(HistoryTreeItem * ItemID)
{
  ....
  if(bm && !bm->GetWidth() && ItemID>=0) 
  ....
  if(ItemID>=0)
  ....
}

V614 Uninitialized variable 'nResult' used. browser.h 404

  LONG GetLeft()
  {
    ATLASSERT(m_pBrowser);
    LONG nResult;
    m_pBrowser->get_Left(&bResult);
    return nResult;
  }

V614 Uninitialized variable 'nResult' used. browser.h 416

  LONG GetTop()
  {
    ATLASSERT(m_pBrowser);
    LONG nResult;
    m_pBrowser->get_Top(&bResult);
    return nResult;
  }

V614 Uninitialized variable 'nResult' used. browser.h 428

  LONG GetWidth()
  {
    ATLASSERT(m_pBrowser);
    LONG nResult;
    m_pBrowser->get_Width(&bResult);
    return nResult;
  }

V614 Uninitialized variable 'nResult' used. browser.h 440

  LONG GetHeight()
  {
    ATLASSERT(m_pBrowser);
    LONG nResult;
    m_pBrowser->get_Height(&bResult);
    return nResult;
  }

V530 The return value of function 'unique' is required to be utilized. imagereuploaderdlg.cpp 381

bool CImageReuploaderDlg::ExtractLinks(....)
  ....
  std::unique(matches.begin(), matches.end());
  ....
}



Ok, хватит. Это всё-таки комментарий, а не статья. :)

Остальные интересные места можно посмотреть, скачав PVS-Studio и попробовав. Нет ничего лучше, чем пощупать вживую.
В «void SAL_CALL Theme::disposing (void)» ещё и утечка, похоже: после maChangeListeners.swap(aListeners) в maChangeListeners будет пусто, и цикл побежит по пустому контейнеру. Там ведь в цикле что-то освобождается наверняка? Тогда он должен бежать по aListeners.
Sign up to leave a comment.