﻿# Computer vision for code: What PVS\-Studio saw in OpenCV

What do computer vision and static analysis have in common? Both seek meaning in data\. OpenCV finds images among millions of pixels, while PVS\-Studio finds errors among thousands of code lines\. This article delves into the source code of the largest computer vision library\.

![1321_OpenCV/image1.png](https://import.viva64.com/docx/blog/1321_OpenCV/image1.png)

## Important info

[OpenCV](https://opencv.org/) is the world's largest open\-source computer vision library, supported by the non\-profit organization, Open Source Computer Vision Foundation\. It offers a wide range of algorithms that cover a variety of tasks, from basic image processing to advanced object recognition and motion analysis\.

[PVS\-Studio](https://pvs-studio.com/en/) is a static code analyzer for detecting errors and vulnerabilities, which has been actively developed since 2008\. Over the past 12 years \(which is how long it has been since the [last OpenCV check](https://pvs-studio.com/en/blog/posts/cpp/0191/)\), we have significantly enhanced our analysis by adding more than 300 diagnostic rules and improving existing ones\. Thanks to our work on fixing false positives reported by users and found when checking open\-source projects, the analyzer efficiency is constantly growing\.

The OpenCV project keeps evolving at a steady pace—you can see that in its 27,000 commits and roughly 120 tags added over the past 12 years\. The library powers everything from object recognition and industrial automation to medical diagnostics, where it helps accurately detect anomalies in images\. Because OpenCV is used in so many different areas, rigorous code review is critical to ensuring its quality and reliability\.

In this article, I'd like to show how static analysis helps keep bugs out of releases and makes developers' lives easier\. I won't list every warning the analyzer has issued; I'll only cover the ones I find most interesting\.

<details>
   <summary>A few words about the check</summary>

We analyzed the project for the `[12b64b0](https://github.com/opencv/opencv/tree/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee)` commit using PVS\-Studio [7\.39](https://pvs-studio.com/en/whatsnew/)\.

The project was compiled on Windows using CMake under Visual Studio\.


</details>
## Errors\!

### N1\. Do you like playing with undefined behavior?

PVS\-Studio warning: [V1061](https://pvs-studio.com/en/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<cv::Ptr<cv::FeatureDetector>()> DetectorFactory;
typedef tuple<std::string, DetectorFactory, float, float> 
    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
```

Extending the `std` namespace violates the C\+\+ standard and usually results in undefined behavior\. The standard explicitly prohibits adding custom definitions to `std`, except for template specializations for user\-defined types\.

<details>
   <summary>An excerpt from the C\\\+\\\+11 standard</summary>

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\)\.


</details>
In the code, the regular `PrintTo` function is added to the `std` namespace, which is prohibited\. The developer may have intended the function to be visible through ADL \([Argument\-Dependent Lookup](https://en.cppreference.com/w/cpp/language/adl.html)\), so that it would be automatically picked up when working with certain types\. However, this approach is dangerous and needs fixing\.

The code snippet is pretty tangled, and I can't think of a solution on the spot, so I'll leave it to the developers\. If you're interested in learning more about how ADL works, feel free to ask in the comments section\.

<details>
   <summary>Similar warnings</summary>

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


</details>


### N2\. Farewell, annoying temporary object\!

PVS\-Studio warning: [V758](https://pvs-studio.com/en/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<typename T>
struct Ptr : public std::shared_ptr<T>;
// ....
Ptr<FlannNeighborhoodGraph> 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<FlannNeighborhoodGraphImpl>(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<int> &sorted_mask) 
{
  // ....
  // get neighbors
  FlannNeighborhoodGraph &graph =                                  // <=
    *FlannNeighborhoodGraph::create(points, points_size, knn,
                                    true /*get distances */, 6, 1);

  std::vector<double> sum_knn_distances (points_size, 0);
  for (int p = 0; p < points_size; p++) {
    const std::vector<double> &dists = graph.getNeighborsDistances(p);
    for (int k = 0; k < knn; k++)
      sum_knn_distances[p] += dists[k];
  }
  // ....
}
```

<details>
   <summary>A little more context on Ptr</summary>

```cpp
template<typename T>
struct Ptr : public std::shared_ptr<T>
{
  inline Ptr(const std::shared_ptr<T>& o) 
    CV_NOEXCEPT : std::shared_ptr<T>(o) {}
  inline Ptr(std::shared_ptr<T>&& o) 
    CV_NOEXCEPT : std::shared_ptr<T>(std::move(o)) {}
  typename std::add_lvalue_reference<T>::type operator*() const 
    CV_NOEXCEPT { return *std::shared_ptr<T>::get(); }
  // ....
}

template<typename _Tp, typename ... A1> 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...);
}
```


</details>
Using smart pointers doesn't resolve the issues of dangling references and memory access here\. Let's dig into this\. The code works as follows:

1. The `create` function creates and returns a smart pointer to the `FlannNeighborhoodGraphImpl` type, and its object reference count is one;
1. The `graph` reference is created for the value of this smart pointer while the object reference count remains unchanged\.
1. Since the pointer is a temporary object, the reference counter will reach zero after initialization is done, releasing the managed object\. Now, the reference points to a destroyed object\.
1. The `for` loop references an invalid reference\.

As a result, the code that seemed correct leads to undefined behavior\. Moreover, PVS\-Studio isn't the only tool that detects this issue; the sanitizer does too\. Here's the [proof](https://godbolt.org/z/rxf3xEc37)\.

To fix this, we need to save the smart pointer so that the `FlannNeighborhoodGraph` object stays until the end of the block\. For example, we can do [this](https://godbolt.org/z/rxf3xEc37):

```cpp
std::vector<double> 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<double> &dists = graph->getNeighborsDistances(p);
    for (int k = 0; k < knn; k++) 
      sum_knn_distances[p] += dists[k];
  }
}
```

We also limited the `graph` visibility area so that the resource would be freed after the loops were executed\.


> You can find more pitfall examples related to \[temporary variables\]\(https://pvs\-studio\.com/en/blog/posts/cpp/1149/\) in the third part of the "\[C\\\+\\\+ programmer's guide to undefined behavior\]\(https://pvs\-studio\.com/en/blog/posts/cpp/1215/\)\\\."

### N3\. How much memory do you have?

PVS\-Studio warning: [V1023](https://pvs-studio.com/en/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<std::shared_ptr<QueueSourceBase> > m_sources;
  // ....
};

QueueInput::QueueInput(const cv::GMetaArgs &args) {
  for (auto &&m : args) {
    m_sources.emplace_back(new cv::gapi::wip::QueueSourceBase(m));
  }
}
```

This code reveals a dangerous pattern: inserting a raw pointer to an allocated resource into a smart pointer vector using the [`emplace_back`](https://en.cppreference.com/w/cpp/container/vector/emplace_back) member function\. It's a template for a function that perfectly passes its arguments\. In our case, the `emplace_back` argument type is `QueueSourceBase*`\.

In an exceptional situation, an object allocated on a heap may be lost\. Here's how it works:

1. Developers try to add a new element to the vector\.
1. The following precondition applies to the vector: `capacity() == size()`\. So, a new, larger buffer is needed to add a new item\.
1. An error occurs when attempting to allocate memory for a new buffer, and the `std::bad_alloc` exception is thrown\.
1. The exception goes up the stack, but `delete` was not called for the allocated memory\.

To avoid the error, the devs should've wrap the raw pointer in a smart pointer and replace `emplace_back` with `push_back`:

```cpp
m_sources.push_back(std::make_unique<cv::gapi::wip::QueueSourceBase>(m));
```

<details>
   <summary>Similar warnings:</summary>

* 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


</details>


### N4\. Is it UB? Yep, it's UB\.

PVS\-Studio warning: [V610](https://pvs-studio.com/en/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;
  // ....
```

\} 

The analyzer has detected a dangerous left\-shift operation by a negative value\. According to the C\+\+ standard, shifting by a negative number leads to undefined behavior\.

<details>
   <summary>An excerpt from the C\\\+\\\+11 standard </summary>

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\.


</details>
The variable shifts to the `aperture_size - 1` expression\. When initializing the variable, we see a check whether the `_aperture_size` value is negative, but it doesn't consider the case of a zero value\. So, if the `_aperture_size` value is zero, then the `aperture_size` variable will also be zero, and the `aperture_size - 1` expression will be `-1`\.

To fix the code, we need to change the inequality sign:

```cpp
int aperture_size = _aperture_size <= 0 ? 3 : _aperture_size;
```

To learn more about the features of shift operators, I invite you to read the article "[Wade not in unknown waters\. Part three](https://pvs-studio.com/en/blog/posts/cpp/0142/)"\.

<details>
   <summary>Similar warnings:</summary>

* V610 Undefined behavior\. Check the shift operator '<<'\. The right operand is negative \('\(aperture\_size \- 1\)' \= \[\-1\.\.2147483646\]\)\. test\_goodfeaturetotrack\.cpp 90


</details>


### N5\. Hello, compiler optimizations\!

PVS\-Studio warning: [V597](https://pvs-studio.com/en/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);
}
```

Here's a method that clears and zeroes memory after using the `heap` pointer\. Skipping this step when working with sensitive data means the information will stay in memory, which could cause trouble later\.

Unfortunately, such code may not clear the buffer\. The compiler may treat the call to `memset` as redundant, since the object gets freed immediately after being zeroed\.

Nothing here suggests OpenCV handles confidential data in this particular part of the code\. More likely, the developers simply wanted to zero memory as an extra precaution before returning it to the system\.

However, when dealing with private data, it's important to do things the right way\. For example, C23 introduced the [`memset_explicit`](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2897.htm) function, which marks a memory region as one that must be cleared\. There's a good chance C\+\+26 \([P3348](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3348r4.pdf)\) will adopt it as well\. Until then, you can look [here](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1315r7.html) for methods that ensure proper memory erasure\.

### N6\. Don't worry —an exception will take care of it

PVS\-Studio warning: [V668](https://pvs-studio.com/en/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;

  // ....
}
```

The developers attempted to prevent a memory allocation failure by checking `m_file` after calling `new`\. However, this operator `new` overload never returns `nullptr`; instead, it throws either `std::bad_alloc`, or an exception thrown by the user in the constructor\. So, the `if (!m_file)` check will never trigger: either the memory allocation will succeed, or execution will stop with an exception\. Indeed, the check ends up looking rather paranoid :\)

To correctly handle this situation, let's wrap the memory allocation in the `try` block:

```cpp
try
{ 
  m_file = new InputFile( m_filename.c_str() ); 
}
catch (...)
{
  return false;
}
```

### N7\. Caution: Null pointer dereference in progress

PVS\-Studio warnings: 

[V522](https://pvs-studio.com/en/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.com/en/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;
}
```

Just five lines of code hide two possibilities for undefined behavior\. Both `calloc` and `malloc` may return a null pointer when they fail to allocate memory, and nothing here handles this\.

To prevent errors, we'll add a check for the return values and an early exit if allocation fails\. For example:

```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;
}
```

<details>
   <summary>Similar warnings:</summary>

* 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


</details>


### N8\. To infinity and beyond\!

PVS\-Studio warning: [V654](https://pvs-studio.com/en/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
}
```

The analyzer has detected a loop where the `while (_row)` condition is always true\. The loop is in the `else` branch, and in this context, `_row != 0`\. However, the variable doesn't change in the loop body, which makes the exit condition unreachable\. If something goes wrong, an error message will not appear because the loop will become infinite\. I can't think of a fix for this bug, so I'll leave it to the developers\.

### N9\. More errors is less errors\!

PVS\-Studio warning: [V665](https://pvs-studio.com/en/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
```

Developers often assume that `#pragma warning(default: X)` re\-enables warnings previously disabled using `#pragma warning(disable: X)`, but it doesn't work that way\. This directive restores the default state, which may differ from the state before the warning was disabled\.

To restore the warnings to their original state, use `#pragma warning(push)` and `#pragma warning(pop)`\. The fixed code looks like this:

```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\. There's only one correct answer

PVS\-Studio warning: [V785](https://pvs-studio.com/en/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;
}
```

You can spot redundant logic in this snippet: First, the `method` variable is checked, where only `CV_POLY_APPROX_DP` is considered valid\. Otherwise, the `noreturn` function of [`cv::error`](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/include/opencv2/core/base.hpp#L362) is called in the [`CV_Error`](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/include/opencv2/core/base.hpp#L399) macro\. Then, the same variable enters `switch` that makes it look as if multiple options exist\.

In reality, the switch no longer serves any purpose\. The `CV_POLY_APPROX_DP` branch always runs, and the code within`default` can't be reached because the earlier check already filtered out all other values\.

Maybe the developers planned to support additional methods later but never implemented them; or maybe the switch have survived the refactoring process after other branches were removed\.

<details>
   <summary>A similar warning:</summary>

* V785 Constant expression in switch statement\. qrcode\_encoder\.cpp 559


</details>


### N11\. I don't need your answer, I've already made up my mind

PVS\-Studio warning: [V583](https://pvs-studio.com/en/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;
}
```

The following three code snippets are classic examples of copy\-paste errors\. Here, the ternary operator checks the `j == 4` condition but always returns the same `1e-2` value\. Most likely, the developers forgot to change the code after copying it\.

We've encountered all kinds of typos many times before and covered them in several articles\. One example worth checking out is "[Common patterns of typos in programming](https://pvs-studio.com/en/blog/posts/cpp/1064/)\."

<details>
   <summary>A similar warning:</summary>

* V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: 0\.15\. test\_model\.cpp 400


</details>


### N12\. Unlucky numbers

PVS\-Studio warning: [V517](https://pvs-studio.com/en/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);
        }
      }
    };
  // ....
}
```

No programmer would want to review such a huge piece of code, but an analyzer can easily check it thoroughly and find a typo\.

PVS\-Studio detected two identical conditions here\. The developers probably needed to go through all the options but forgot to change one check in the last one\. The correct condition is most likely as follows:

```cpp
} else if (dp1 == 1 && dp2 == 1 && dp3 == 0 && dp == 1) {
```

To learn about more mistakes involving unlucky numbers that developers often make, read the article "[Zero, one, two, Freddy's coming for you](https://pvs-studio.com/en/blog/posts/cpp/0713/)\."

<details>
   <summary>A similar warning:</summary>

* 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


</details>


### N13\. Things took a 360 turn\!

PVS\-Studio warning: [V678](https://pvs-studio.com/en/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<Rect>& ob1, std::vector<Rect>& 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<Rect>::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);          // <=
    }
    // ....
  }
}
```

In this fragment, we can see an attempt to copy the matrix into itself\. Even though the `copyTo` member function has an [early exit](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/src/copy.cpp#L497) when matrices match, a [lot of code](https://github.com/opencv/opencv/blob/12b64b0e2abd91f235cf3cc49df0f0f985aa0fee/modules/core/src/copy.cpp#L429-L496) runs before this check: creating the matrix, checking types and sizes, and initializing iterators\.

This operation overloads the system and may also be a typo\. The developers may have wanted to copy the `cpu_result_roi` matrix\.

<details>
   <summary>A similar warning:</summary>

* 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


</details>


### N14\. The illusion of free choice

PVS\-Studio warning: [V549](https://pvs-studio.com/en/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<float> cornersQuality;
  std::vector<float> 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);
}
```

Here, the `min` function compares the same `(unsigned int)(cornersQuality.size())` argument\. It serves no purpose and can cause issues\. An access out of bounds of the `RefcornersQuality` array may occur if its size is smaller than the `cornersQuality` array\.

The typo may have occurred due to a cumbersome and difficult\-to\-understand conversion\. However, there's a simple solution: Use `size_t` and remove the unnecessary conversions, leaving only the comparison of array sizes\.

```cpp
for (size_t i = 0; i < std::min(cornersQuality.size(), 
                                RefcornersQuality.size()); i++)
```

We found this code fragment in tests\. Yes, tests aren't immune to bugs and need to be checked, too\. By the way, we have an article on this topic, "[Finding errors in unit tests](https://pvs-studio.com/en/blog/posts/cpp/1179/)\."

## Conclusion

Static analysis detects hidden defects even in large and actively developed projects\. You can catch bugs early on by using it regularly\. This makes your code more reliable and reduces debugging costs\. [Check your project](https://pvs-studio.com/en/pvs-studio/try-free/) using PVS\-Studio and fix issues today\.