Как PVS-Studio защищает от поспешных правок кода, пример N4

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

Думал сделать перерыв в серии маленьких заметок про то, как анализатор PVS-Studio оперативно находит ошибки в новом коде. Но, увидев в почте отчёт от системы автоматической проверки проекта Blender, не выдержал. Предлагаю вашему вниманию очередную простую, но именно из-за этого красивую ошибку.

Итак, жил-был вот такой код для обработки вектора значений. Суть — не дать значениям выходить за определённый диапазон.

#define CLAMP(a, b, c) \   { \     if ((a) < (b)) { \       (a) = (b); \     } \     else if ((a) > (c)) { \       (a) = (c); \     } \   } \   (void)0  template <typename T> inline T clamp(const T &a, const bT &min_v, const bT &max_v) {   T result = a;   for (int i = 0; i < T::type_length; i++) {     CLAMP(result[i], min_v, max_v);   }   return result; }

И было всё хорошо. А потом программист решил, что нет смысла использовать самодельный макрос CLAMP, а лучше воспользоваться стандартной функцией std::clamp. И в коммите, призванном сделать мир лучше, код стал таким:

template <typename T, int Size> inline vec_base<T, Size>   clamp(const vec_base<T, Size> &a, const T &min, const T &max) {   vec_base<T, Size> result = a;   for (int i = 0; i < Size; i++) {     std::clamp(result[i], min, max);   }   return result; }

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

Зато дотошный статический анализатор кода PVS-Studio тут как тут с сообщением:

[CWE-252] V530: The return value of function ‘clamp’ is required to be utilized. BLI_math_vector.hh 88

Дело в том, что функция std::clamp не меняет значение элемента в контейнере:

template <class T> constexpr const T& clamp( const T& v, const T& lo, const T& hi );

Макрос CLAMP раньше изменял значение, а стандартная функция нет. Поэтому теперь код сломан и ждёт, когда кто-то заметит проявление ошибки и пойдёт искать её причину. Такую ошибку можно было бы заметить и исправить ещё на этапе написания кода, если бы использовался PVS-Studio. Не будьте как разработчики Blender :). Используйте статический анализ кода на регулярной основе. И вы сэкономите свои силы и время.

Примечание. Кстати, рядом в коде есть ещё одно неправильное использование std::clamp.

Правильный вариант кода:

template <typename T, int Size> inline vec_base<T, Size> clamp(const vec_base<T, Size> &a, const T &min, const T &max) {   vec_base<T, Size> result = a;   for (int i = 0; i < Size; i++) {     result[i] = std::clamp(result[i], min, max);   }   return result; }

Спасибо за внимание. И, если ещё не видели, заглядывайте почитать про топ-10 ошибок в C++ открытых проектах, которые мы нашли за 2021 год.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. How PVS-Studio prevents rash code changes, example N4.