# Компьютерное зрение для кода: что PVS\-Studio разглядел в OpenCV
Что общего у компьютерного зрения и статического анализа? Оба ищут смысл в данных\. OpenCV находит образы среди миллионов пикселей, а PVS\-Studio — ошибки среди тысяч строк кода\. В статье изучим исходники крупнейшей библиотеки компьютерного зрения\.
## Разговоры о важном
[OpenCV](https://opencv.org/) — крупнейшая в мире библиотека компьютерного зрения с открытым исходным кодом, поддерживаемая некоммерческой организацией Open Source Computer Vision Foundation\. Она предлагает обширный набор алгоритмов, охватывающих огромный список задач: от базовых функций обработки изображений до продвинутых методов распознавания объектов и анализа движения\.
[PVS\-Studio](https://pvs-studio.ru/ru/) — это статический анализатор кода для поиска ошибок и уязвимостей, который активно развивается с 2008 года\. За прошедшие 12 лет \(именно столько прошло с [последней проверки OpenCV](https://pvs-studio.ru/ru/blog/posts/cpp/0191/)\) мы значительно усилили анализ, добавив более 300 диагностических правил и улучшив уже существующие\. Эффективность анализатора постоянно повышается также благодаря работе над устранением ложных срабатываний, о которых сообщают пользователи и которые мы находим сами при проверке открытых проектов\.
Проект OpenCV также активно развивается, о чём свидетельствуют 27 тысяч коммитов и около 120 тегов, появившиеся за последние 12 лет\. Библиотека нашла применение в самых разных сферах: от распознавания объектов и промышленной автоматизации до медицинской диагностики, позволяя выявлять аномалии на снимках с высокой точностью\. И именно из\-за такого широкого использования для OpenCV крайне важна тщательная проверка кода\.
В этой статье предлагаю посмотреть, как статический анализ помогает избежать попадания багов в релиз и облегчить жизнь разработчикам\. Стоит отметить, что я рассматриваю не все предупреждения, выданные анализатором, а только те, которые кажутся мне наиболее интересными\.
О проверке
Анализ проекта выполнен для коммита [`12b64b0`](https://github.com/opencv/opencv/tree/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee) с применением [PVS\-Studio 7\.39](https://pvs-studio.ru/ru/whatsnew/)\.
Проект был собран на Windows с помощью CMake под Visual Studio\.
## Ошибочки\!
### N1\. А вы любите играть с неопределённым поведением?
Предупреждение PVS\-Studio: [V1061](https://pvs-studio.ru/ru/docs/warnings/v1061/) Extending the 'std' namespace may result in undefined behavior\. [test\_descriptors\_invariance\.impl\.hpp 195](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/features2d/test/test_descriptors_invariance.impl.hpp#L195)
```cpp
typedef std::function()> DetectorFactory;
typedef tuple
String_FeatureDetector_Float_Float_t;
// ....
namespace std {
using namespace opencv_test;
static inline void PrintTo(
const String_FeatureDetector_DescriptorExtractor_Float_t& v,
std::ostream* os)
{
*os << "(\"" << get<0>(v)
<< "\", " << get<3>(v)
<< ")";
}
} // namespace
```
Расширение пространства имён `std` является нарушением стандарта C\+\+ и в большинстве случаев ведёт к неопределённому поведению\. Стандарт явно запрещает добавление собственных определений в `std`, за исключением специализаций шаблонов для пользовательских типов\.
Фрагмент стандарта C\\\+\\\+11
The behavior of a C\+\+ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std unless otherwise specified\. A program may add a template specialization for any standard library template to namespace std only if the declaration depends on a user\-defined type and the specialization meets the standard library requirements for the original template and is not explicitly prohibited\.
The behavior of a C\+\+ program is undefined if it declares
— an explicit specialization of any member function of a standard library class template, or
— an explicit specialization of any member function template of a standard library class or class template, or
— an explicit or partial specialization of any member class template of a standard library class or class template, or
— a deduction guide for any standard library class template\.
A program may explicitly instantiate a template defined in the standard library only if the declaration depends on the name of a user\-defined type and the instantiation meets the standard library requirements for the original template\.
A translation unit shall not declare namespace std to be an inline namespace \(10\.3\.1\)\.
В коде в пространство `std` добавляется обычная функция `PrintTo`, что недопустимо\. Разработчик, скорее всего, хотел обеспечить видимость этой функции через механизм ADL \([Argument\-Dependent Lookup](https://en.cppreference.com/w/cpp/language/adl.html)\), чтобы она автоматически находилась при работе с соответствующими типами\. Однако такой код опасен и его следует исправить\.
Этот фрагмент кода довольно запутанный, и я с ходу не могу предложить для него решения, поэтому оставляю задачу разработчикам\. Но если вам хочется подробнее узнать о механизме ADL, пишите в комментарии\.
Аналогичные срабатывания
V1061\. Extending the 'std' namespace may result in undefined behavior\. test\_descriptors\_invariance\.impl\.hpp 195
V1061\. Extending the 'std' namespace may result in undefined behavior\. gapi\_fluid\_parallel\_rois\_test\.cpp 292
### N2\. Прощай, надоедливый временный объект\!
Предупреждение PVS\-Studio: [V758](https://pvs-studio.ru/ru/docs/warnings/v758/) The 'graph' reference becomes invalid when smart pointer returned by a function is destroyed\. [utils\.cpp 391](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/calib3d/src/usac/utils.cpp#L391)
```cpp
template
struct Ptr : public std::shared_ptr;
// ....
Ptr FlannNeighborhoodGraph::create(
const Mat &points, int points_size,
int k_nearest_neighbors_, bool get_distances,
int flann_search_params_, int num_kd_trees)
{
return makePtr(points, points_size,
k_nearest_neighbors_, get_distances,
flann_search_params_, num_kd_trees);
}
void Utils::densitySort (const Mat &points, int knn,
Mat &sorted_points, std::vector &sorted_mask)
{
// ....
// get neighbors
FlannNeighborhoodGraph &graph = // <=
*FlannNeighborhoodGraph::create(points, points_size, knn,
true /*get distances */, 6, 1);
std::vector sum_knn_distances (points_size, 0);
for (int p = 0; p < points_size; p++) {
const std::vector &dists = graph.getNeighborsDistances(p);
for (int k = 0; k < knn; k++)
sum_knn_distances[p] += dists[k];
}
// ....
}
```
Ещё немного контекста по Ptr
```cpp
template
struct Ptr : public std::shared_ptr
{
inline Ptr(const std::shared_ptr& o)
CV_NOEXCEPT : std::shared_ptr(o) {}
inline Ptr(std::shared_ptr&& o)
CV_NOEXCEPT : std::shared_ptr(std::move(o)) {}
typename std::add_lvalue_reference::type operator*() const
CV_NOEXCEPT { return *std::shared_ptr::get(); }
// ....
}
template static inline
Ptr<_Tp> makePtr(const A1&... a1)
{
static_assert( !has_custom_delete<_Tp>::value,
"Can't use this makePtr with custom DefaultDeleter");
return (Ptr<_Tp>)std::make_shared<_Tp>(a1...);
}
```
Если вы думаете, что использование умных указателей раз и навсегда решает проблему "висячих" ссылок и доступов к памяти, то здесь всё пошло не так\. Давайте разбираться\. Сейчас код работает следующим образом:
1. Функция `create` создаёт и возвращает умный указатель на тип `FlannNeighborhoodGraphImpl`, и его счётчик ссылок на объект равен единице;
1. Создаётся ссылка `graph` на значение этого умного указателя, при этом счётчик ссылок на объект не изменяется;
1. Указатель является временным объектом, и поэтому после завершения инициализации счётчик ссылок уменьшится до нуля, что приведёт к освобождению управляемого объекта\. Теперь ссылка указывает на разрушенный объект;
1. В цикле `for` происходит обращение к невалидной ссылке\.
В итоге код, который казался правильным, приводит к неопределённому поведению\. Кроме того, эту проблему находит не только PVS\-Studio, но и санитайзер\. [Пруф](https://godbolt.org/z/rxf3xEc37)\.
Чтобы исправить проблему, необходимо сохранить умный указатель, тогда объект типа `FlannNeighborhoodGraph` будет жить до конца блока\. Например, можно сделать [так](https://godbolt.org/z/rxf3xEc37):
```cpp
std::vector sum_knn_distances (points_size, 0);
{
// get neighbors
auto graph = FlannNeighborhoodGraph::create(points, points_size, knn,
true /*get distances */, 6, 1);
for (int p = 0; p < points_size; p++) {
const std::vector &dists = graph->getNeighborsDistances(p);
for (int k = 0; k < knn; k++)
sum_knn_distances[p] += dists[k];
}
}
```
Мы дополнительно ограничили область видимости `graph`, чтобы ресурс освободился после выполнения циклов\.
> Больше других подводных камней, связанных с \[временными переменными\]\(https://pvs\-studio\.ru/ru/blog/posts/cpp/1149/\), можно найти в третьей части книги "\[Путеводитель C\\\+\\\+ программиста по неопределённому поведению\]\(https://pvs\-studio\.ru/ru/blog/posts/cpp/1215/\)"\\\.
### N3\. Как много у вас памяти?
Предупреждение PVS\-Studio: [V1023](https://pvs-studio.ru/ru/docs/warnings/v1023/) A pointer without owner is added to the 'm\_sources' container by the 'emplace\_back' method\. A memory leak will occur in case of an exception\. [queue\_source\.cpp 70](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/gapi/src/streaming/queue_source.cpp#L70)
```cpp
class GAPI_EXPORTS QueueInput {
std::vector > m_sources;
// ....
};
QueueInput::QueueInput(const cv::GMetaArgs &args) {
for (auto &&m : args) {
m_sources.emplace_back(new cv::gapi::wip::QueueSourceBase(m));
}
}
```
В этом коде можно увидеть опасный паттерн: вставка сырого указателя на аллоцированный ресурс в вектор умных указателей с использованием функции\-члена [`emplace_back`](https://en.cppreference.com/w/cpp/container/vector/emplace_back)\. Она представляет собой шаблон функции, которая осуществляет идеальную передачу своих аргументов\. Получается, что тип аргумента `emplace_back` в нашем случае будет `QueueSourceBase *`\.
В исключительной ситуации аллоцированный на куче объект может быть утерян\. Вот как это происходит:
1. В вектор пытаются добавить новый элемент\.
1. Для вектора выполняется следующее предусловие: `capacity() == size()`\. Это означает, что для вставки нового элемента потребуется новый буфер большего размера\.
1. При попытке выделить память на новый буфер происходит ошибка, бросается исключение `std::bad_alloc`\.
1. Исключение улетает наверх по стеку, но для выделенной памяти не был вызван `delete`\.
Чтобы избежать ошибки, нужно обернуть сырой указатель в умный, ну и заодно можно поменять `emplace_back` на `push_back`:
```cpp
m_sources.push_back(std::make_unique(m));
```
Аналогичные срабатывания
* V1023 A pointer without owner is added to the 'tiles' container by the 'emplace\_back' method\. A memory leak will occur in case of an exception\. gfluidbackend\.cpp 1383
### N4\. Неопределённое поведение? Неопределённое поведение\.
Предупреждение PVS\-Studio: [V610](https://pvs-studio.ru/ru/docs/warnings/v610/) Undefined behavior\. Check the shift operator '<<'\. The right operand is negative \('\(aperture\_size \- 1\)' \= \[\-1\.\.2147483646\]\)\. [test\_filter\.cpp 1397](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/imgproc/test/test_filter.cpp#L1397)
```cpp
static void
test_cornerEigenValsVecs(....)
{
int i, j;
Scalar borderValue = _borderValue;
int aperture_size = _aperture_size < 0 ? 3 : _aperture_size;
// ....
double denom = (1 << (aperture_size-1))*block_size;
// ....
```
\}
Анализатор обнаружил опасную операцию сдвига влево на отрицательное число\. Согласно стандарту C\+\+, поведение при сдвиге на отрицательное число не определено\.
Фрагмент стандарта C\\\+\\\+11
The shift operators << and \>\> group left\-to\-right\.
shift\-expression << additive\-expression
shift\-expression \>\> additive\-expression
The operands shall be of integral or unscoped enumeration type and integral promotions are performed\.
1\. The type of the result is that of the promoted left operand\. The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand\.
2\. The value of E1 << E2 is E1 left\-shifted E2 bit positions; vacated bits are zero\-filled\. If E1 has an unsigned type, the value of the result is E1 \* 2^E2, reduced modulo one more than the maximum value representable in the result type\. Otherwise, if E1 has a signed type and non\-negative value, and E1\*2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined\.
3\. The value of E1 \>\> E2 is E1 right\-shifted E2 bit positions\. If E1 has an unsigned type or if E1 has a signed type and a non\-negative value, the value of the result is the integral part of the quotient of E1/2^E2\. If E1 has a signed type and a negative value, the resulting value is implementation\-defined\.
В коде выполняется сдвиг на выражение `aperture_size - 1`\. При инициализации переменной мы видим проверку на отрицательное значение `_aperture_size`, однако в ней не учитывается случай нулевого значения\. А значит, если `_aperture_size` равна нулю, то и переменная `aperture_size` также будет равна нулю, а выражение `aperture_size - 1` примет значение `-1`\.
Чтобы исправить код, надо заменить знак неравенства:
```cpp
int aperture_size = _aperture_size <= 0 ? 3 : _aperture_size;
```
А чтобы узнать больше об особенностях операторов сдвига, предлагаю ознакомиться со статьёй "[Не зная брода, не лезь в воду\. Часть третья](https://pvs-studio.ru/ru/blog/posts/cpp/0142/)"\.
Аналогичные срабатывания
* V610 Undefined behavior\. Check the shift operator '<<'\. The right operand is negative \('\(aperture\_size \- 1\)' \= \[\-1\.\.2147483646\]\)\. test\_goodfeaturetotrack\.cpp 90
### N5\. Привет оптимизациям компилятора\!
Предупреждение PVS\-Studio: [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'heap' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. [zmaxheap\.cpp 87](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/objdetect/src/aruco/apriltag/zmaxheap.cpp#L87)
```cpp
void zmaxheap_destroy(zmaxheap_t *heap)
{
free(heap->values);
free(heap->data);
memset(heap, 0, sizeof(zmaxheap_t));
free(heap);
}
```
Мы видим метод, который очищает и зануляет память после использования указателя `heap`\. Если при работе с конфиденциальными данными этого не сделать, то они останутся в памяти, что может привести к неприятным последствиям\.
К сожалению, такой код может оставить буфер неочищенным\. Компилятор вправе считать вызов `memset` избыточным, поскольку сразу после зануления объект освобождается\.
Вряд ли в этой части OpenCV торчат какие\-либо конфиденциальные данные\. Скорее разработчик просто хотел дополнительно занулить память перед возвратом системе\.
А вот если вы работаете с приватными данными, то нужно найти способ сделать это правильно\. Например, в С23 была добавлена функция [`memset_explicit`](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2897.htm), которая позволяет указать, что область памяти должна быть очищена\. Есть все шансы, что она также попадёт и в C\+\+26 \([P3348](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3348r4.pdf)\)\. А до этих стандартов о гарантированных способах очистки памяти можно почитать [здесь](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1315r7.html)\.
### N6\. Не переживай\. Если что, здесь будет exception
Предупреждение PVS\-Studio: [V668](https://pvs-studio.ru/ru/docs/warnings/v668/) There is no sense in testing the 'm\_file' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. [grfmt\_exr\.cpp 153](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/imgcodecs/src/grfmt_exr.cpp#L154)
```cpp
bool ExrDecoder::readHeader()
{
bool result = false;
m_file = new InputFile( m_filename.c_str() );
if( !m_file ) // probably paranoid
return false;
// ....
}
```
Разработчик попытался перестраховаться от ошибки выделения памяти, добавив проверку указателя `m_file` после вызова `new`\. Однако используемая перегрузка оператора `new` никогда не вернёт `nullptr`, а сгенерирует исключение: или `std::bad_alloc`, или брошенное пользователем в конструкторе\. Таким образом, проверка `if (!m_file)` никогда не сработает: либо память будет выделена успешно, либо выполнение прервётся исключением\. Действительно, проверка вышла параноидальной :\)
Для корректной обработки этой ситуации необходимо обернуть выделение памяти в блок `try`:
```cpp
try
{
m_file = new InputFile( m_filename.c_str() );
}
catch (...)
{
return false;
}
```
### N7\. Будьте осторожны, нулевой указатель разыменовывается
Предупреждения PVS\-Studio:
[V522](https://pvs-studio.ru/ru/docs/warnings/v522/) There might be dereferencing of a potential null pointer 'uf'\. Check lines: 38, 37\. [unionfind\.hpp 38](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/objdetect/src/aruco/apriltag/unionfind.hpp#L38)
[V522](https://pvs-studio.ru/ru/docs/warnings/v522/) There might be dereferencing of a potential null pointer 'uf\-\>data'\. Check lines: 41, 39\. [unionfind\.hpp 41](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/objdetect/src/aruco/apriltag/unionfind.hpp#L39)
```cpp
static inline unionfind_t *unionfind_create(uint32_t maxid){
unionfind_t *uf = (unionfind_t*) calloc(1, sizeof(unionfind_t));
uf->maxid = maxid;
uf->data = (struct ufrec*) malloc((maxid+1) * sizeof(struct ufrec));
for (unsigned int i = 0; i <= maxid; i++) {
uf->data[i].size = 1;
uf->data[i].parent = i;
}
return uf;
}
```
Всего в пяти строках кода скрываются сразу две возможности для неопределённого поведения\. Проблема возникает из\-за того, что функции `calloc` и `malloc` могут вернуть нулевой указатель при невозможности выделения памяти, а здесь отсутствует обработка на случай этой ситуации\.
Чтобы избежать ошибок, необходимо добавить проверку возвращаемых значений и ранний выход из функции в случае неудачи, например так:
```cpp
unionfind_t *uf = (unionfind_t*) calloc(1, sizeof(unionfind_t));
if (!uf) return NULL;
uf->data = (struct ufrec*) malloc((maxid+1) * sizeof(struct ufrec));
if (!uf->data)
{
free(uf);
return NULL;
}
```
Аналогичные срабатывания
* V522 There might be dereferencing of a potential null pointer 'heap'\. Check lines: 73, 72\. zmaxheap\.cpp 73
* V522 There might be dereferencing of a potential null pointer 'im\_max'\. Check lines: 1129, 1109\. apriltag\_quad\_thresh\.cpp 1129
* V522 There might be dereferencing of a potential null pointer 'im\_max\_tmp'\. Check lines: 1160, 1137\. apriltag\_quad\_thresh\.cpp 1160
* V522 There might be dereferencing of a potential null pointer 'im\_min'\. Check lines: 1130, 1110\. apriltag\_quad\_thresh\.cpp 1130
* V522 There might be dereferencing of a potential null pointer 'im\_min\_tmp'\. Check lines: 1161, 1138\. apriltag\_quad\_thresh\.cpp 1161
* V522 There might be dereferencing of a potential null pointer 'za'\. Check lines: 45, 44\. zarray\.hpp 45
### N8\. Бесконечность не предел
Предупреждение PVS\-Studio: [V654](https://pvs-studio.ru/ru/docs/warnings/v654/) The condition '\_row' of loop is always true\. [chessboard\.cpp 2172](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/calib3d/src/chessboard.cpp#L2172)
```cpp
cv::Point2f &Chessboard::Board::getCorner(int _row, int _col)
{
int _rows = int(rowCount());
int _cols = int(colCount());
if(_row >= _rows || _col >= _cols)
CV_Error(Error::StsBadArg,"out of bound");
if(_row == 0)
{
// ....
}
else
{
Cell *row_start = top_left;
int count = 1;
do
{
if(count == _row)
{
PointIter iter(row_start,BOTTOM_LEFT);
int count2 = 0;
do
{
if(count2 == _col)
return *(*iter);
++count2;
}while(iter.right());
}
++count;
row_start = row_start->bottom;
}while(_row); // <=
}
CV_Error(Error::StsInternal,"cannot find corner");
// return *top_left->top_left; // never reached
}
```
Анализатор обнаружил цикл с всегда истинным условием `while (_row)`\. Цикл находится в `else`\-ветке, и в этом контексте `_row != 0`\. При этом в теле цикла переменная не изменяется, что делает условие выхода недостижимым\. Если что\-то пойдёт не так, то сообщение об ошибке не будет выдано, ведь цикл окажется бесконечным\. Предложить решение для этой ошибки я не могу, поэтому оставляю это разработчикам\.
### N9\. Больше ошибок — меньше ошибок\!
Предупреждение PVS\-Studio: [V665](https://pvs-studio.ru/ru/docs/warnings/v665/) Possibly, the usage of '\#pragma warning\(default: X\)' is incorrect in this context\. The '\#pragma warning\(push/pop\)' should be used instead\. Check lines: 20, 187\. [any\.hpp 187](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/gapi/include/opencv2/gapi/util/any.hpp#L187)
```cpp
#if defined(_MSC_VER)
// disable MSVC warning on "multiple copy constructors specified"
# pragma warning(disable: 4521)
#endif
// ....
#if defined(_MSC_VER)
// Enable "multiple copy constructors specified" back
# pragma warning(default: 4521)
#endif
```
Программисты зачастую думают, что после директивы `#pragma warning(default: X)` опять начнут действовать предупреждения, отключённые ранее с помощью `pragma warning(disable: X)`, но это не так\. Эта директива устанавливает состояние по умолчанию, а оно могло отличаться от того, которое было до отключения\.
Чтобы вернуть прежнее состояние, следует воспользоваться директивами `#pragma warning(push)` и `#pragma warning(pop)`\. Исправленный код будет выглядеть следующим образом:
```cpp
#if defined(_MSC_VER)
// disable MSVC warning on "multiple copy constructors specified"
# pragma warning(push)
# pragma warning(disable: 4521)
#endif
// ....
#if defined(_MSC_VER)
// Enable "multiple copy constructors specified" back
# pragma warning(pop)
#endif
```
### N10\. Только один вариант правильный
Предупреждение PVS\-Studio: [V785](https://pvs-studio.ru/ru/docs/warnings/v785/) Constant expression in switch statement\. [approx\.cpp 782](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/imgproc/src/approx.cpp#L782)
```cpp
CV_IMPL CvSeq*
cvApproxPoly( const void* array, int header_size,
CvMemStorage* storage, int method,
double parameter, int parameter2 )
{
// ....
if( method != CV_POLY_APPROX_DP )
CV_Error( cv::Error::StsOutOfRange, "Unknown approximation method" );
while( src_seq != 0 )
{
CvSeq *contour = 0;
switch (method)
{
case CV_POLY_APPROX_DP:
// ....
default:
CV_Error( cv::Error::StsBadArg, "Invalid approximation method" );
}
// ....
}
return dst_seq;
}
```
В этом фрагменте можно увидеть избыточный код: переменная `method` сначала проверяется в условии `if`, где допустимым является только значение `CV_POLY_APPROX_DP` \(в ином случае вызывается `noreturn`\-функция [`cv::error`](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/include/opencv2/core/base.hpp#L362) внутри макроса [`CV_Error`](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/include/opencv2/core/base.hpp#L399)\)\. Затем она попадает в конструкцию `switch`, создающую видимость множественного выбора\.
Фактически свитч теперь бессмысленен: ветка `CV_POLY_APPROX_DP` будет выполняться всегда, а код внутри `default` становится недостижимым, потому что проверка ранее уже отсекла все другие значения\.
Возможно, разработчик планировал добавить другие методы, но не реализовал их\. Или код остался от рефакторинга, когда убрали другие варианты\.
Аналогичные срабатывания
* V785 Constant expression in switch statement\. qrcode\_encoder\.cpp 559
### N11\. Мне не нужен твой ответ, я уже всё решил
Предупреждение PVS\-Studio: [V583](https://pvs-studio.ru/ru/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: 1e\-2\. [test\_fundam\.cpp 626](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/calib3d/test/test_fundam.cpp#L626)
```cpp
double CV_RodriguesTest::get_success_error_level( int /*test_case_idx*/,
int /*i*/, int j )
{
return j == 4 ? 1e-2 : 1e-2;
}
```
Следующие три фрагмента — классический пример ошибки из\-за copy\-paste\. Здесь тернарный оператор проверяет условие `j == 4`, но всегда возвращает одинаковое значение `1e-2`\. Скорее всего, забыли изменить код после копирования\.
Мы уже очень много раз находили разные варианты опечаток и описывали их в разных статьях\. Предлагаю посмотреть одну из них: "[Распространённые паттерны опечаток при программировании](https://pvs-studio.ru/ru/blog/posts/cpp/1064/)"\.
Аналогичные срабатывания
* V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: 0\.15\. test\_model\.cpp 400
### N12\. Несчастливые числа
Предупреждение PVS\-Studio: [V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 629, 643\. [nary\_eltwise\_layers\.cpp 629](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/dnn/src/layers/nary_eltwise_layers.cpp#L629)
```cpp
void ternary_forward_impl(....)
{
// ....
if (nplanes == 1) { // parallelize within the plane
const T *ptr1 = (const T*)data1;
const T *ptr2 = (const T*)data2;
const T *ptr3 = (const T*)data3;
T* ptr = (T*)data;
auto worker = [&](const Range &r) {
if (dp1 == 1 && dp2 == 1 && dp3 == 1 && dp == 1) { // <=
for (int i = r.start; i < r.end; i++) {
ptr[i] = op(ptr1[i], ptr2[i], ptr3[i]);
}
} else if (dp1 == 0 && dp2 == 1 && dp3 == 1 && dp == 1){
T x1 = *ptr1;
for (int i = r.start; i < r.end; i++) {
ptr[i] = op(x1, ptr2[i], ptr3[i]);
}
} else if (dp1 == 1 && dp2 == 0 && dp3 == 1 && dp == 1){
T x2 = *ptr2;
for (int i = r.start; i < r.end; i++) {
ptr[i] = op(ptr1[i], x2, ptr3[i]);
}
} else if (dp1 == 1 && dp2 == 1 && dp3 == 1 && dp == 1) { // <=
T x3 = *ptr3;
for (int i = r.start; i < r.end; i++) {
ptr[i] = op(ptr1[i], ptr2[i], x3);
}
} else {
for(int i = r.start; i < r.end;
i++, ptr1 += dp1, ptr2 += dp2, ptr3 += dp3, ptr += dp) {
*ptr = op(*ptr1, *ptr2, *ptr3);
}
}
};
// ....
}
```
Огромный фрагмент кода, который не захочет ревьюить ни один программист, но анализатору несложно просмотреть его полностью и найти опечатку\.
Здесь PVS\-Studio нашёл два одинаковых условия\. Вероятно, разработчику нужно было перебрать все варианты, но в последнем он забыл изменить одну проверку\. Корректное условие, скорее всего, должно быть таким:
```cpp
} else if (dp1 == 1 && dp2 == 1 && dp3 == 0 && dp == 1) {
```
А ещё больше ошибок с несчастливыми числами, которые часто допускают разработчики, можно найти в статье "[Ноль, один, два, Фредди заберёт тебя](https://pvs-studio.ru/ru/blog/posts/cpp/0713/)"\.
Аналогичные срабатывания
* V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 679, 693\. nary\_eltwise\_layers\.cpp 679
### N13\. Я изменился на все 360\!
Предупреждение PVS\-Studio: [V678](https://pvs-studio.ru/ru/docs/warnings/v678/) An object is used as an argument to its own method\. Consider checking the first actual argument of the 'copyTo' function\. [ocl\_test\.cpp 107](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/ts/src/ocl_test.cpp#L107)
```cpp
double TestUtils::checkRectSimilarity(const Size & sz,
std::vector& ob1, std::vector& ob2)
{
double final_test_result = 0.0;
size_t sz1 = ob1.size();
size_t sz2 = ob2.size();
if (sz1 != sz2)
return sz1 > sz2 ? (double)(sz1 - sz2) : (double)(sz2 - sz1);
else
{
if (sz1 == 0 && sz2 == 0)
return 0;
cv::Mat cpu_result(sz, CV_8UC1);
cpu_result.setTo(0);
for (vector::const_iterator r = ob1.begin(); r != ob1.end(); ++r)
{
cv::Mat cpu_result_roi(cpu_result, *r);
cpu_result_roi.setTo(1);
cpu_result.copyTo(cpu_result); // <=
}
// ....
}
}
```
В этом фрагменте можно увидеть попытку копирования матрицы в себя же\. Хотя метод `copyTo` содержит [ранний выход](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/src/copy.cpp#L497) при совпадении матриц, перед этой проверкой выполняется [значительный объем кода](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/src/copy.cpp#L429-L496): создание матрицы, проверки типов и размеров, инициализация итераторов\.
Эта операция создаёт избыточную нагрузку на систему и, возможно, тоже является опечаткой\. Может быть, разработчик хотел применить копирование матрицы `cpu_result_roi`\.
Аналогичные срабатывания
* V678 An object is used as an argument to its own method\. Consider checking the first actual argument of the 'copyTo' function\. ocl\_test\.cpp 117
### N14\. Иллюзия выбора
Предупреждение PVS\-Studio: [V549](https://pvs-studio.ru/ru/docs/warnings/v549/) The first argument of 'min' function is equal to the second argument\. [test\_goodfeaturetotrack\.cpp 512](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/imgproc/test/test_goodfeaturetotrack.cpp#L512)
```cpp
class CV_GoodFeatureToTTest : public cvtest::ArrayTest
{
protected:
std::vector cornersQuality;
std::vector RefcornersQuality;
// ....
};
int CV_GoodFeatureToTTest::validate_test_results( int test_case_idx )
{
// ....
if (e > eps)
{
EXPECT_LE(e, eps); // never true
ts->set_failed_test_info(cvtest::TS::FAIL_BAD_ACCURACY);
for (int i = 0;
i < (int)std::min((unsigned int)(cornersQuality.size()),
(unsigned int)(cornersQuality.size()));
i++)
{
if ( std::abs(cornersQuality[i] - RefcornersQuality[i])
> eps * std::max(cornersQuality[i], RefcornersQuality[i]))
printf("i = %i Quality %2.6f Quality ref %2.6f\n",
i, cornersQuality[i], RefcornersQuality[i]);
}
}
return BaseTest::validate_test_results(test_case_idx);
}
```
Здесь функция `min` сравнивает один и тот же аргумент `(unsigned int)(cornersQuality.size())`\. Это не только бессмысленно, но и опасно\. Может произойти выход за границу массива `RefcornersQuality`, если его размер меньше массива `cornersQuality`\.
Возможно, опечатка произошла из\-за громоздкого преобразования, которое тяжело воспринимать\. Однако есть простое решение: использовать `size_t` и убрать лишние преобразования, оставив только сравнение размеров массивов\.
```cpp
for (size_t i = 0; i < std::min(cornersQuality.size(),
RefcornersQuality.size()); i++)
```
Этот фрагмент кода мы нашли в тестах\. Да, тесты не застрахованы от багов и их тоже необходимо проверять\. Кстати, этой теме мы даже посвятили отдельную статью: "[Поиск ошибок в юнит\-тестах](https://pvs-studio.ru/ru/blog/posts/cpp/1179/)"\.
## Заключение
Статический анализ выявляет скрытые дефекты даже в больших работающих проектах\. А регулярное использование позволяет находить ошибки на ранних этапах, повышая надёжность кода и снижая затраты на отладку\. [Проверьте свой проект](https://pvs-studio.ru/ru/pvs-studio/try-free/) с помощью PVS\-Studio и избавьте его от проблем уже сейчас\.