Introduction

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

The project under analysis

PVS-Studio

The most interesting warnings

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

static WriteableBitmapConverter() { optimumChannels = new Dictionary <PixelFormat, int>(); 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 <PixelFormat, MatType>(); 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; } .... }

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. OpenCvSharp is a wrapper of OpenCV for use in C# projects. By the way, we already checked 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 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.What makes themethod special is that it triggered four warnings of the same type at once:Theclass is defined in thenamespace and is a collection of various pixel formats. The analyzer points out that the elementsandare assigned values for a second time in themethod, 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.

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

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.

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(); }

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

public static double CalibrateCamera(....) { if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); .... }

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 394This 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»statement will never execute because:This code needs revising; it's very likely that the second copy ofwas actually meant to be some other variable. V3010 The return value of function 'ToString' is required to be utilized. ImgProcTest.cs 80Themethod is accessed through theparameter and returns an array of elements of type. Once thehas been filled with values, themethod 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.We have cloned code here, hence the two warnings. The first says that bothstatements check the same condition. If that condition is true, the method will return in the thenbranch of the firststatement. 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.

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

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(); .... } .... } .... } } }

public static void FastNlMeansDenoisingMulti(....) { .... NativeMethods.photo_fastNlMeansDenoisingMulti( srcImgPtrs, srcImgPtrs.Length, dst.CvPtr, imgToDenoiseIndex, templateWindowSize, h, templateWindowSize, searchWindowSize); .... }

public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize)

NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....)

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

public static void Rodrigues(double[,] matrix, out double[] vector, out double[,] jacobian) { .... using (var jacobianM = new Mat<double>()) { NativeMethods.calib3d_Rodrigues_MatToVec (matrixM.CvPtr, vectorM.CvPtr, jacobianM.CvPtr); .... } }

public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian)

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

Other warnings of this type: V3022 Expression 'label == MarkerValue' is always false. Labeller.cs 135A variable namedis 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. V3038 The argument was passed to method several times. It is possible that other argument should be passed instead. Cv2_photo.cs 124To understand what the analyzer is telling us, let's take a look at themethod's parameters:Let's simplify it even more to make it completely straightforward. Compare these lines:Thevariable is declared twice, but the first time it's mentioned should actually be the declaration of. Another thing that the analyzer didn't like is that the value ofis not used in themethod 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:The next example is somewhat similar to the previous one. V3066 Possible incorrect order of arguments passed to 'calib3d_Rodrigues_MatToVec' method: 'matrixM.CvPtr' and 'vectorM.CvPtr'. Cv2_calib3d.cs 86Let's look at themethod's parameters:It seems themethod is called with the argumentsandaccidentally swapped. The authors should check this snippet: there might be a mistake that hinders correct computations. V3063 A part of conditional expression is always false if it is evaluated: data == null. Mat.cs 3539The analyzer reports that the second checkwill never bebecause ifis equal toin the first condition, an exception will be raised and execution will never reach the second check.

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)); // <= .... }

if (window == null) throw new ArgumentNullException(nameof(window));

public new Mat<TElem> SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); }

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;*/ }

public static void DestroyWindow(string winName) { if (String.IsNullOrEmpty("winName")) .... }

public static FrameSource CreateFrameSource_Video(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }

public static FrameSource CreateFrameSource_Video_CUDA(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }

V3127 Two similar code fragments were found. Perhaps, this is a typo and 'window' variable should be used instead of 'src2' Cv2_imgproc.cs 1547The analyzer spotted a typo in this snippet. The variables are checked forand, if true, each check throws an exception. However, it doesn't work quite properly for thevariable. If its value is equal to, a corresponding exception is thrown too but with the wrong text. It won't be mentioning; it will beinstead. The condition should apparently be revised as follows: V3142 Unreachable code detected. It is possible that an error is present. MatOfT.cs 873Now, 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.The analyzer tells us that thestatement is unreachable. Let's look at the body of themethod to see if the analyzer is telling the truth.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. V3022 Expression 'String.IsNullOrEmpty(«winName»)' is always false. Cv2_highgui.cs 46 V3022 Expression 'string.IsNullOrEmpty(«fileName»)' is always false. FrameSource.cs 37 V3022 Expression 'string.IsNullOrEmpty(«fileName»)' is always false. FrameSource.cs 53Sometimes 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 typewhose 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.

Conclusion

The programmer must have written a faulty block of code once and then cloned it through copy-paste.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 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 for PVS-Studio to start using it on a regular basis.Thanks for reading. Don't hesitate to download a trial copy of PVS-Studio to check your own projects.