to the top
close form
Для получения триального ключа
заполните форму ниже
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Единорог, который смог

Единорог, который смог

22 Июн 2016

Одна из команд разработчиков Microsoft уже использует в работе анализатор PVS-Studio. Это хорошо, но недостаточно. Поэтому я продолжаю демонстрировать, какую пользу может приносить статический анализ кода на примере проектов Microsoft. Три года назад мы проверяли проект Casablanca и не смогли в нём ничего обнаружить. За это проект был отмечен медалью "безбажный код". Прошло время, проект развивался и рос. В свою очередь, анализатор PVS-Studio существенно продвинулся в возможностях анализа кода. И наконец я могу написать статью об ошибках, которые анализатор выявляет в проекте Casablanca (C++ REST SDK). Ошибок мало, но то, что теперь их достаточно для написания статьи, говорит о эффективности PVS-Studio.

0406_The_Little_Unicorn_That_Could_ru/image1.png

Casablanca

Как я уже отметил в аннотации, мы уже проверяли проект Casablanca. Вы можете прочитать про это в статье "Маленькая заметка о проекте Casablanca".

Casablanca (C++ REST SDK) - это небольшой проект, написанный на современном C++. Говоря о современном языке C++, я имею в виду, что в коде активно используется семантика перемещения (move semantics), лямбда-функции, auto и так далее. Новые возможности языка C++ позволяют писать более короткий и надёжный код. Это подтверждается тем, что найти ошибки в этом проекте непростая задача. Хотя обычно мы делаем это крайне легко.

Для заинтересовавшихся, какие ещё проекты Microsoft мы проверяли, привожу список статей, посвященных этим проверкам: Xamarin.Forms, CNTK, Microsoft Edge, CoreCLR, Windows 8 Driver Samples, библиотека Visual C++ 2012 / 2013, CoreFX, Roslyn, Microsoft Code Contracts, и скоро появится статья про проверку WPF Samples.

Итак, проект Casablanca является образцом хорошего, качественного кода. Давайте посмотрим, что же все-таки можно найти в нём с помощью статического анализатора кода PVS-Studio.

Что удалось найти плохого

Фрагмент N1: опечатка

Имеется структура NumericHandValues, содержащая два члена: low и high. Вот объявление этой структуры:

struct NumericHandValues
{
  int low;
  int high;
  int Best() { return (high < 22) ? high : low; }
};

А теперь посмотрим, как в одном месте инициализируется эта структура:

NumericHandValues GetNumericValues()
{
  NumericHandValues res;
  res.low = 0;
  res.low = 0;
  
  ....
}

Предупреждение PVS-Studio: V519 The 'res.low' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 130, 131. BlackJack_Client140 messagetypes.h 131

Как видите, случайно два раза инициализировали член low, но при этом забыли инициализировать high. Глубокомысленный комментарий здесь написать сложно. Просто никто не застрахован от опечаток.

Фрагмент N2: ошибка освобождения памяти

void DealerTable::FillShoe(size_t decks)
{
  std::shared_ptr<int> ss(new int[decks * 52]);
  ....
}

Предупреждение PVS-Studio: V554 Incorrect use of shared_ptr. The memory allocated with 'new []' will be cleaned using 'delete'. BlackJack_Server140 table.cpp 471

По умолчанию умный указатель типа shared_ptr для уничтожения объекта вызовет оператор delete без квадратных скобок []. В данном случае это неправильно.

Корректный вариант кода должен быть таким:

std::shared_ptr<int> ss(new int[decks * 52],
                        std::default_delete<int[]>());

Фрагмент N3: потерянный указатель

Статический член s_server_api представляет собой умный указатель и определён следующим образом:

std::unique_ptr<http_server>
  http_server_api::s_server_api((http_server*)nullptr);

Подозрение вызывает код следующей функции:

void http_server_api::unregister_server_api()
{
  pplx::extensibility::scoped_critical_section_t lock(s_lock);

  if (http_server_api::has_listener())
  {
    throw http_exception(_XPLATSTR("Server API ..... attached"));
  }

  s_server_api.release();
}

Предупреждение PVS-Studio: V530 The return value of function 'release' is required to be utilized. cpprestsdk140 http_server_api.cpp 64

Обратите внимание на "s_server_api.release();". После вызова функции release умный указатель больше не владеет объектом. Указатель на объект "теряется" и объект будет существовать до конца жизни программы.

Скорее всего, мы вновь столкнулись с опечаткой. Я думаю, что хотели вызвать функцию reset, а вовсе не release.

Фрагмент N4: не тот enum

В проекте имеется два перечисления BJHandState и BJHandResult, объявленные следующим образом:

enum BJHandState {
  HR_Empty, HR_BlackJack, HR_Active, HR_Held, HR_Busted
};
enum BJHandResult {
  HR_None, HR_PlayerBlackJack, HR_PlayerWin,
  HR_ComputerWin, HR_Push
};

А теперь посмотрим на фрагмент кода из функции PayUp:

void DealerTable::PayUp(size_t idx)
{
  ....
  if ( player.Hand.insurance > 0 &&
       Players[0].Hand.state == HR_PlayerBlackJack )
  {
    player.Balance += player.Hand.insurance*3;
  }
  ....
}

Предупреждение PVS-Studio: V556 The values of different enum types are compared. Types: BJHandState, BJHandResult. BlackJack_Server140 table.cpp 336

Переменная state имеет тип BJHandState. А это значит, что программист запутался в перечислениях. По всей видимости код должен был выглядеть так:

if ( player.Hand.insurance > 0 &&
     Players[0].Hand.state == HR_BlackJack )

Забавно то, что это ошибка на самом деле пока никак не сказывается на работе программы. Благодаря счастливому стечению обстоятельств, на данный момент константы HR_BlackJack и HR_PlayerBlackJack имеют одинаковое значение, равное 1. Дело в том, что обе эти константы в перечислении находятся на одной позиции в списке. Однако, в процесс развития программы это может измениться, и тогда возникнет странная, непонятная ошибка.

Фрагмент N5: странный break

web::json::value AsJSON() const 
{
  ....
  int idx = 0;
  for (auto iter = cards.begin(); iter != cards.end();)
  {
    jCards[idx++] = iter->AsJSON();
    break;
  }
  ....
}

Предупреждение PVS-Studio: V612 An unconditional 'break' within a loop. BlackJack_Client140 messagetypes.h 213

Оператор break в этом коде выглядит крайне подозрительно. Цикл может совершить максимум одну итерацию. Мне сложно сказать, как должен вести себя этот код, но, скорее всего, сейчас с ним что-то не так.

Прочие мелочи

Помимо фрагментов кода, рассмотренных выше и претендующих на звание ошибок, анализатор обнаружил также несколько неаккуратных мест. Например, использование постинкремента для итераторов.

inline web::json::value
TablesAsJSON(...., std::shared_ptr<BJTable>> &tables)
{
  web::json::value result = web::json::value::array();

  size_t idx = 0;
  for (auto tbl = tables.begin(); tbl != tables.end(); tbl++)
  {
    result[idx++] = tbl->second->AsJSON();
  }
  return result;
}

Предупреждение PVS-Studio: V803 Decreased performance. In case 'tbl' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. BlackJack_Client140 messagetypes.h 356

Это, конечно, не ошибка. Однако, хорошим стилем считается использование преинкремента: ++tbl. Для тех, кто сомневается, что в этом есть смысл, я отсылаю к следующим двум статьям:

В коде библиотеки есть ещё 10 мест, где используется постинкремент итераторов в циклах, но я не вижу смысла приводить их в статье.

Рассмотрим ещё один пример, когда анализатор указывает на неаккуратный код:

struct _acquire_protector
{
  _acquire_protector(....);
  ~_acquire_protector();
  size_t   m_size;
private:
  _acquire_protector& operator=(const _acquire_protector&);
  uint8_t* m_ptr;
  concurrency::streams::streambuf<uint8_t>& m_buffer;
};

Предупреждение PVS-Studio: V690 The '=' operator is declared as private in the '_acquire_protector' class, but the default copy constructor will still be generated by compiler. It is dangerous to use such a class. cpprestsdk140.uwp.staticlib fileio_winrt.cpp 825

Как видим, программист запретил использование оператора копирования. Однако, объект по-прежнему может быть скопирован с помощью конструктора копирования, создаваемого компилятором по умолчанию.

Заключение

Наконец-то анализатор PVS-Studio смог к чему-то придраться. Ошибок нашлось немного, но всё-таки они есть. А это значит, что если применять статический анализ не разово, как я сделал сейчас, а регулярно, то можно предотвращать множество ошибок на самом раннем этапе. Лучше править ошибки сразу после написания кода, а не в процессе тестирования, отладки и тем более после того как о дефекте сообщит один из пользователей.

Дополнительные ссылки

Популярные статьи по теме
Под капотом SAST: как инструменты анализа кода ищут дефекты безопасности

Дата: 26 Янв 2023

Автор: Сергей Васильев

Сегодня речь о том, как SAST-решения ищут дефекты безопасности. Расскажу, как разные подходы к поиску потенциальных уязвимостей дополняют друг друга, зачем нужен каждый из них и как теория ложится на…
Ложные представления программистов о неопределённом поведении

Дата: 17 Янв 2023

Автор: Гость

Неопределённое поведение (UB) – непростая концепция в языках программирования и компиляторах. Я слышал много заблуждений в том, что гарантирует компилятор при наличии UB. Это печально, но неудивитель…
Топ-10 ошибок в C++ проектах за 2022 год

Дата: 29 Дек 2022

Автор: Владислав Столяров

Дело идёт к Новому году, а значит, самое время традиционно вспомнить десять самых интересных срабатываний, которые нашёл PVS-Studio в 2022 году.
PVS-Studio и RPCS3: лучшие предупреждения в один клик

Дата: 12 Дек 2022

Автор: Александр Куренев

Best Warnings — режим анализатора, оставляющий в окне вывода 10 лучших предупреждений. Мы предлагаем вам ознакомиться с обновлённым режимом Best Warnings на примере проверки проекта RPCS3.
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

В этой статье постараюсь затронуть все вещи, которые можно без зазрения совести выкинуть из С++, не потеряв ничего (кроме боли), уменьшить стандарт, нагрузку на создателей компиляторов, студентов, из…

Комментарии (0)

Следующие комментарии next comments
close comment form
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо