# Checking the OpenCvSharp wrapper for OpenCV with PVS\-Studio OpenCV is an open\-source library of computer vision and image processing algorithms and general\-purpose numerical algorithms\. The library is well known among C\+\+ developers\. Besides C\+\+, there are also versions for Python, Java, Ruby, Matlab, Lua, and other languages\. Since C\#, which is the language I specialize in, is not on that list, I chose OpenCvSharp, a C\# wrapper of OpenCV, to check it with PVS\-Studio\. The results of that check are discussed in this article\. ![0683_OpenCvSharp/image1.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image1.png) ## Introduction Before I became part of the PVS\-Studio team, I had been involved in the making of robots to present at exhibitions\. My duties included the most basic repair work \(major failures were handled by another person\) as well as development of software and utilities of every kind\. ![0683_OpenCvSharp/image2.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image2.png) _Me, tired and new to town, with a freshly unpacked KIKI robot\._ By the way, the development part was pretty funny\. Each time one of us had an idea about some new way to surprise exhibition visitors, we brought it up for discussion and if everyone liked it, we'd get down to work\. Once it occurred to us to make a robot that could recognize a human face and respond with a welcome speech\. I googled for some library for my needs and stumbled upon OpenCV, a computer\-vision algorithms library\. But I got disappointed very soon as I figured out that OpenCV was implemented in C\+\+\. My knowledge of C\+\+, which I had studied at college, was obviously not enough\. So I googled a bit more and found OpenCvSharp, a wrapper of the library for C\#, which is the language I specialize in\. It's been about half a year since then, the program long written and in use, and now I finally decided to peek "under the hood" of OpenCvSharp and scan its source code with the PVS\-Studio static analyzer\. ## The project under analysis [OpenCvSharp](https://github.com/shimat/opencvsharp) is a wrapper of OpenCV for use in C\# projects\. By the way, [we already checked](https://pvs-studio.com/en/blog/posts/cpp/0191/) OpenCV in the past\. The strong points of OpenCvSharp are the large collection of code samples, cross\-platform support \(it runs on any platform supported by Mono\), and easy installation\. The wrapper is a small project about 112,200 lines of C\# code long\. 1,2% of these are comments, which, I should say, is suspiciously few\. On the other hand, there are quite a few bugs for a project that small\. I picked over 20 examples for this article, but the analyzer actually found many more, which aren't that interesting or obvious\. ## PVS\-Studio [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) is a tool for detecting bugs and potential vulnerabilities in the source code of programs written in C, C\+\+, C\#, and Java\. It runs on Windows, Linux, and macOS\. In addition to unreachable code, programming mistakes, and typos, PVS\-Studio, as was already mentioned, is capable of detecting potential security issues\. Therefore, it can be viewed as a Static Application Security Testing \(SAST\) tool\. ## The most interesting warnings What makes the _WriteableBitmapConverter_ method special is that it triggered four warnings of the same type at once: * V3005 The 'optimumChannels\[PixelFormats\.Indexed1\]' variable is assigned to itself\. WriteableBitmapConverter\.cs 22 * V3005 The 'optimumChannels\[PixelFormats\.Indexed8\]' variable is assigned to itself\. WriteableBitmapConverter\.cs 23 * V3005 The 'optimumTypes\[PixelFormats\.Indexed1\]' variable is assigned to itself\. WriteableBitmapConverter\.cs 50 * V3005 The 'optimumTypes\[PixelFormats\.Indexed8\]' variable is assigned to itself\. WriteableBitmapConverter\.cs 51 ```cpp static WriteableBitmapConverter() { optimumChannels = new Dictionary (); optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed8] = // <= optimumChannels[PixelFormats.Gray2] = optimumChannels[PixelFormats.Gray4] = optimumChannels[PixelFormats.Gray8] = optimumChannels[PixelFormats.Gray16] = optimumChannels[PixelFormats.Gray32Float] = optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed2] = optimumChannels[PixelFormats.Indexed4] = optimumChannels[PixelFormats.Indexed8] = // <= .... optimumTypes = new Dictionary (); optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.Gray2] = optimumTypes[PixelFormats.Gray4] = optimumTypes[PixelFormats.Gray8] = optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed2] = optimumTypes[PixelFormats.Indexed4] = optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.BlackWhite] = .... } .... public static class PixelFormats { .... public static PixelFormat Indexed8 { get; } .... public static PixelFormat Indexed1 { get; } .... } ``` The _PixelFormats_ class is defined in the _System\.Windows\.Media_ namespace and is a collection of various pixel formats\. The analyzer points out that the elements _optimumChannels\[PixelFormats\.Indexed1\]_ and _optimumChannels\[PixelFormats\.Indexed8\] _are assigned values for a second time in the _WriteableBitmapConverter_ method, which doesn't make any sense\. It's unclear whether this is just a typo or the programmer meant something else\. By the way, this snippet is a vivid example of how static analyzers can be helpful: looking at a bunch of similar lines makes you less focused – no wonder typos stay unnoticed despite the code review\. Static analyzers, though, don't have trouble maintaining attention and they don't need rest, so they can catch bugs like that with no effort\. ![0683_OpenCvSharp/image3.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image3.png) _Feel the might of static analysis\._ **PVS\-Studio diagnostic message:** [V3021](https://pvs-studio.com/en/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless InputArray\.cs 394 ```cpp private static MatType EstimateType(Type t) { .... if (t == typeof(Vec2b)) return MatType.CV_8UC2; if (t == typeof(Vec3b)) return MatType.CV_8UC3; if (t == typeof(Vec4b)) return MatType.CV_8UC4; if (t == typeof(Vec6b)) return MatType.CV_8UC(6); if (t == typeof(Vec2s)) // <= return MatType.CV_16SC2; .... if (t == typeof(Vec2s)) // <= return MatType.CV_32SC2; .... } ``` This bug is somewhat similar to the previous one\. The developer is checking the same condition twice\. It doesn't make sense here as the then branch of the "duplicate" _if_ statement will never execute because: * if the first condition is true, the method will return; * if the first condition is false, the second will be false too because the variable being checked, _t_, doesn't change between the two checks\. This code needs revising; it's very likely that the second copy of _Vec2s_ was actually meant to be some other variable\. **PVS\-Studio diagnostic message**: [V3010](https://pvs-studio.com/en/docs/warnings/v3010/) The return value of function 'ToString' is required to be utilized\. ImgProcTest\.cs 80 ```cpp public static RectanglesIntersectTypes RotatedRectangleIntersection(RotatedRect rect1, RotatedRect rect2, out Point2f[] intersectingRegion) { using (var intersectingRegionVec = new VectorOfPoint2f()) { int ret = NativeMethods .imgproc_rotatedRectangleIntersection_vector( rect1, rect2, intersectingRegionVec.CvPtr); intersectingRegion = intersectingRegionVec.ToArray(); return (RectanglesIntersectTypes) ret; } } public void RotatedRectangleIntersectionVector() { var rr1 = new RotatedRect(new Point2f(100, 100), new Size2f(100, 100), 45); var rr2 = new RotatedRect(new Point2f(130, 100), new Size2f(100, 100), 0); Cv2.RotatedRectangleIntersection(rr1, rr2, out var intersectingRegion); .... intersectingRegion.ToString(); } ``` The _RotatedRectangleIntersection_ method is accessed through the _intersectingRegion_ parameter and returns an array of elements of type _Point2f_\. Once the _intersectingRegion _has been filled with values, the _ToString\(\)_ method is called on the array\. This doesn't affect the array's elements in any way and no useful work is performed in the last line, so it would be fair to assume that the developer simply forgot to remove that piece\. **PVS\-Studio diagnostic messages:** * V3021 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless Cv2\_calib3d\.cs 1370 * V3022 Expression 'objectPoints \=\= null' is always false\. Cv2\_calib3d\.cs 1372 ```cpp public static double CalibrateCamera(....) { if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); .... } ``` We have cloned code here, hence the two warnings\. The first says that both _if_ statements check the same condition\. If that condition is true, the method will return in the then_ _branch of the first _if_ statement\. Consequently, the second condition will always be false, which is what the second warning is telling us\. It seems the programmer cloned that fragment using copy\-paste but forgot to change it\. ![0683_OpenCvSharp/image4.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image4.png) _Cute Copy\-Paste\._ Other warnings of this type: * V3021 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless Cv2\_calib3d\.cs 1444 * V3022 Expression 'objectPoints \=\= null' is always false\. Cv2\_calib3d\.cs 1446 **PVS\-Studio diagnostic message:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'label \=\= MarkerValue' is always false\. Labeller\.cs 135 ```cpp internal static class Labeller { .... private const int MarkerValue = -1; public static int Perform(Mat img, CvBlobs blobs) { .... int label = 0; int lastLabel = 0; CvBlob lastBlob = null; for (int y = 0; y < h; y++) { for (int x = 0; x < w; x++) { if (imgIn[x + y * step] == 0) continue; bool labeled = labels[y, x] != 0; if (....) { labeled = true; // Label contour. label++; if (label == MarkerValue) // <= throw new Exception(); .... } .... } .... } } } ``` A variable named _label_ is created and initialized to 0\. If a certain condition is true, it will get incremented by one\. What's more, this variable never gets decremented in this snippet\. Therefore, checking it for the constant \-1, as in the line pointed at by the analyzer, doesn't make any sense\. **PVS\-Studio diagnostic message:** [V3038](https://pvs-studio.com/en/docs/warnings/v3038/) The argument was passed to method several times\. It is possible that other argument should be passed instead\. Cv2\_photo\.cs 124 ```cpp public static void FastNlMeansDenoisingMulti(....) { .... NativeMethods.photo_fastNlMeansDenoisingMulti( srcImgPtrs, srcImgPtrs.Length, dst.CvPtr, imgToDenoiseIndex, templateWindowSize, h, templateWindowSize, searchWindowSize); .... } ``` To understand what the analyzer is telling us, let's take a look at the _photo\_fastNlMeansDenoisingMulti_ method's parameters: ```cpp public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize) ``` Let's simplify it even more to make it completely straightforward\. Compare these lines: ```cpp NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....) ``` The _templateWindowSize_ variable is declared twice, but the first time it's mentioned should actually be the declaration of _temporalWindowSize_\. Another thing that the analyzer didn't like is that the value of _temporalWindowSize_ is not used in the _photo\_fastNlMeansDenoisingMulti_ method at all\. This could be a conscious decision, but I'd take a closer look at this code if I were the author\. Other warnings of this type: * V3038 The argument was passed to method several times\. It is possible that other argument should be passed instead\. Cv2\_photo\.cs 149 * V3038 The argument was passed to method several times\. It is possible that other argument should be passed instead\. Cv2\_photo\.cs 180 * V3038 The argument was passed to method several times\. It is possible that other argument should be passed instead\. Cv2\_photo\.cs 205 The next example is somewhat similar to the previous one\. **PVS\-Studio diagnostic message:** [V3066](https://pvs-studio.com/en/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'calib3d\_Rodrigues\_MatToVec' method: 'matrixM\.CvPtr' and 'vectorM\.CvPtr'\. Cv2\_calib3d\.cs 86 ```cpp public static void Rodrigues(double[,] matrix, out double[] vector, out double[,] jacobian) { .... using (var jacobianM = new Mat()) { NativeMethods.calib3d_Rodrigues_MatToVec (matrixM.CvPtr, vectorM.CvPtr, jacobianM.CvPtr); .... } } ``` Let's look at the _calib3d\_Rodrigues\_MatToVec_ method's parameters: ```cpp public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian) ``` It seems the _calib3d\_Rodrigues\_MatToVec_ method is called with the arguments _matrixM\.CvPtr _and _vectorM\.CvPtr_ accidentally swapped\. The authors should check this snippet: there might be a mistake that hinders correct computations\. **PVS\-Studio diagnostic message:** [V3063](https://pvs-studio.com/en/docs/warnings/v3063/) A part of conditional expression is always false if it is evaluated: data \=\= null\. Mat\.cs 3539 ```cpp private void CheckArgumentsForConvert(....) { .... if (data == null) throw new ArgumentNullException(nameof(data)); MatType t = Type(); if (data == null || (data.Length * dataDimension) // <= (data.Length * dataDimension) % t.Channels != 0) .... } ``` The analyzer reports that the second check _data \=\= null_ will never be _true_ because if _data_ is equal to _null_ in the first condition, an exception will be raised and execution will never reach the second check\. ![0683_OpenCvSharp/image5.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image5.png) _I know you're tired, but we're almost done\._ **PVS\-Studio diagnostic message:** [V3127](https://pvs-studio.com/en/docs/warnings/v3127/) Two similar code fragments were found\. Perhaps, this is a typo and 'window' variable should be used instead of 'src2' Cv2\_imgproc\.cs 1547 ```cpp public static Point2d PhaseCorrelateRes(....) { if (src1 == null) throw new ArgumentNullException(nameof(src1)); if (src2 == null) throw new ArgumentNullException(nameof(src2)); if (window == null) throw new ArgumentNullException(nameof(src2)); // <= .... } ``` The analyzer spotted a typo in this snippet\. The variables are checked for _null_ and, if true, each check throws an exception\. However, it doesn't work quite properly for the _window_ variable\. If its value is equal to _null_, a corresponding exception is thrown too but with the wrong text\. It won't be mentioning _window_; it will be _src2_ instead\. The condition should apparently be revised as follows: ```cpp if (window == null) throw new ArgumentNullException(nameof(window)); ``` **PVS\-Studio diagnostic message:** [V3142](https://pvs-studio.com/en/docs/warnings/v3142/) Unreacheble code detected\. It is possible that an error is present\. MatOfT\.cs 873 Now, just for a change, let's take a look at the case where the analyzer is technically correct about unreachable code, but there's actually no error\. It's a warning that can be called both true and false at the same time\. ```cpp public new Mat SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); } ``` The analyzer tells us that the _return_ statement is unreachable\. Let's look at the body of the _SubMat_ method to see if the analyzer is telling the truth\. ```cpp public Mat SubMat(params Range[] ranges) { throw new NotImplementedException(); /* if (ranges == null) throw new ArgumentNullException(); ThrowIfDisposed(); CvSlice[] slices = new CvSlice[ranges.Length]; for (int i = 0; i < ranges.Length; i++) { slices[i] = ranges[i]; } IntPtr retPtr = NativeMethods.core_Mat_subMat1(ptr, ranges.Length, ranges); Mat retVal = new Mat(retPtr); return retVal;*/ } ``` As you can see, the function is currently incomplete and will always throw an exception\. The analyzer is absolutely correct pointing out the unreachable code – but it's not a genuine bug\. The next three defects are of the same type, but they are so cool I couldn't help including all the three\. **PVS\-Studio diagnostic message:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'String\.IsNullOrEmpty\("winName"\)' is always false\. Cv2\_highgui\.cs 46 ```cpp public static void DestroyWindow(string winName) { if (String.IsNullOrEmpty("winName")) .... } ``` **PVS\-Studio diagnostic message:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'string\.IsNullOrEmpty\("fileName"\)' is always false\. FrameSource\.cs 37 ```cpp public static FrameSource CreateFrameSource_Video(string fileName) { if (string.IsNullOrEmpty("fileName")) .... } ``` **PVS\-Studio diagnostic message:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'string\.IsNullOrEmpty\("fileName"\)' is always false\. FrameSource\.cs 53 ```cpp public static FrameSource CreateFrameSource_Video_CUDA(string fileName) { if (string.IsNullOrEmpty("fileName")) .... } ``` Sometimes [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) warnings \(about always true/false expressions\) point at really strange or funny bugs\. All the three examples above have the same mistake in them\. The method has a parameter of type _string_ whose value must be checked\. What is checked instead, though, is a string literal whose text is the variable's name, i\.e\. the variable's name enclosed in quotation marks\. ![0683_OpenCvSharp/image6.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp/image6.png) The programmer must have written a faulty block of code once and then cloned it through copy\-paste\. ## Conclusion The developers of OpenCvSharp have done a big and important job, and, as the user of their library, I am totally thankful for that\. Thank you guys\! But now that I have become part of the PVS\-Studio team and seen the library's code, I have to say that the quality aspect wasn't given proper attention\. The project doesn't look like one being regularly checked with static analyzers, and many of the bugs are apparently fixed using more expensive techniques \(such as testing or user feedback\), and some of the bugs just keep living inside the code and it's them that we catch with our analyzer\. This subject is discussed in more detail in [this small post](https://pvs-studio.com/en/blog/posts/0534/) on the static analysis philosophy\. Since OpenCvSharp is open\-source and freely available on GitHub, its authors can use one of the [free licensing options](https://pvs-studio.com/en/blog/posts/0614/) for PVS\-Studio to start using it on a regular basis\. Thanks for reading\. Don't hesitate to [download](https://pvs-studio.com/en/pvs-studio/download/) a trial copy of PVS\-Studio to check your own projects\.