# Проверка обёртки OpenCvSharp над OpenCV с помощью PVS\-Studio OpenCV \- библиотека алгоритмов компьютерного зрения, обработки изображений и численных алгоритмов общего назначения с открытым кодом, знакомая многим разработчикам на C\+\+\. Помимо C\+\+, OpenCV также разрабатывается для Python, Java, Ruby, Matlab, Lua и других языков\. Так как среди этих языков нет моего основного, C\#, то я решила обратить внимание на OpenCvSharp \- wrapper библиотеки под C\# и проверить этот проект\. Что же из этого вышло, можно узнать в данной статье\. ![0683_OpenCvSharp_ru/image1.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image1.png) ## Введение Ранее, до прихода в PVS\-Studio, я занималась робототехникой в рамках выставок\. В мои задачи входила самая элементарная починка \(если случалась крупная поломка, то робота отдавали другому человеку\), а также разработка самого разнообразного софта и утилит\. ![0683_OpenCvSharp_ru/image2.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image2.png) _Уставшая и недавно приехавшая в новый город я вместе со свежераспакованным роботом KIKI\._ К слову о разработке\. Это было довольно забавно\. Каждый раз, когда кому\-то из коллектива приходила идея, чем бы еще удивить гостей выставок, мы выносили этот вопрос на общее обсуждение и, если идея была хорошей, то принимались за реализацию\. Однажды так нам пришла идея сделать существо, реагирующее на человеческое лицо приветственной речью\. Поискав в интернете какую\-нибудь библиотеку под мои нужды, я набрела на сайт OpenCV, библиотеки алгоритмов компьютерного зрения\. Очень скоро меня ждало разочарование \- OpenCV реализована на C\+\+\. Моих знаний о плюсах, доставшихся из колледжа, явно не хватало\. Поэтому, путем недолгого гугления, я наткнулась на OpenCvSharp \- wrapper этой библиотеки под C\#, мой основной язык\. С тех пор прошло с полгода, программа уже давно написана и используется, а я решилась залезть "под капот" к OpenCvSharp и проверить её исходный код при помощи статического анализатора PVS\-Studio\. ## Проверяемый проект [OpenCvSharp](https://github.com/shimat/opencvsharp) \- это обёртка над OpenCV для использования библиотеки в C\# проектах\. Библиотеку OpenCV, к слову, [мы тоже проверяли](https://pvs-studio.ru/ru/blog/posts/cpp/0191/)\. К плюсам OpenCvSharp можно отнести большую коллекцию примеров кода, кроссплатформенность \(может работать на любой платформе, которую поддерживает Mono\) и легкость в установке\. Обёртка представляет собой маленький проект и содержит около 112 200 строк кода на языке C\#\. Из них 1,2% \- комментарии, что, кстати, как\-то подозрительно мало\. Зато для такого маленького проекта находится немало ошибок\. В статье я выписала более 20 ошибок, но были и другие не столь интересные или очевидные\. ## Анализатор кода PVS\-Studio [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) \- это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C\+\+, C\# и Java\. Работает на Windows, Linux и macOS\. Помимо недостижимого кода, ошибок и опечаток, PVS\-Studio умеет находить потенциальные уязвимости, как было отмечено выше\. Следовательно, его можно рассматривать как средство статического тестирования безопасности приложений \(Static Application Security Testing, SAST\)\. ## Участки кода, привлекшие внимание при рассмотрении отчета анализатора Метод _WriteableBitmapConverter_ привлекает внимание сразу четырьмя однотипными предупреждениями PVS\-Studio: * 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; } .... } ``` Класс _PixelFormats_ определён в пространстве имен _System\.Windows\.Media_ и представляет из себя коллекцию разнообразных форматов пикселей\. Анализатор обращает внимание на то, что в методе _WriteableBitmapConverter_ происходит повторное присваивание значений элементам _optimumChannels\[PixelFormats\.Indexed1\]_ и _optimumChannels\[PixelFormats\.Indexed8\]_, в чем нет практического смысла\. Непонятно, это простая опечатка или имелось в виду что\-то другое\. К слову, этот участок кода наглядно демонстрирует пользу статических анализаторов\. Когда перед глазами куча однотипных строк, то взгляд начинает "замыливаться", а внимательность рассеиваться \- не удивительно, если даже после code review в программу закрадется какая\-нибудь опечатка\. А у статического анализатора проблем с внимательностью нет и ему не надо отдыхать, поэтому ему проще найти подобного рода ошибки\. ![0683_OpenCvSharp_ru/image3.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image3.png) _Прочувствуйте мощь и силу статического анализа\._ **Предупреждение PVS\-Studio**: [V3021](https://pvs-studio.ru/ru/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; .... } ``` Эта ошибка немного похожа на предыдущую\. Программист допустил ситуацию, когда проверяется одно и то же условие дважды\. В данном случае это не имеет смысла \- then\-ветвь "продублированного" оператора _if_ не будет выполнена, так как: * если истинно первое условное выражение, будет осуществлён выход из метода; * если первое условие ложно, второе также будет ложно, так как проверяемая переменная \- _t_ \- между условными выражениями не изменяется\. Разработчику стоит перепроверить этот участок кода\. Вполне вероятно, что на месте второй переменной _Vec2s_ должна стоять какая\-то другая\. **Предупреждение PVS\-Studio**: [V3010](https://pvs-studio.ru/ru/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(); } ``` Метод _RotatedRectangleIntersection_ возвращает массив элементов типа _Point2f_ через параметр _intersectingRegion_\. После того, как программа заполняет _intersectingRegion _значениями, у этого массива вызывается метод _ToString\(\)\._ С элементами массива от этого никаких изменений не происходит и никакой полезной работы в последней строке не производится, поэтому есть основания подозревать разработчика в том, что он просто забыл это убрать\. **Предупреждения PVS\-Studio**: * 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)); .... } ``` В этом случае был продублирован фрагмент кода, из\-за чего появилось два предупреждения\. Первое предупреждение говорит о том, что оба оператора _if_ имеют одинаковое условное выражение\. В случае истинности этого выражения будет осуществлен выход из метода в _then_\-ветви первого оператора _if_\. По этой причине второе условие всегда будет ложно, о чем говорит следующее предупреждение\. Видимо, текст скопировали, но забыли поправить\. ![0683_OpenCvSharp_ru/image4.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image4.png) _Милый Copy\-Paste\._ Другие подобные предупреждения анализатора: * 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**: [V3022](https://pvs-studio.ru/ru/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(); .... } .... } .... } } } ``` В этом участке кода создается переменная _label_, равная нулю\. При выполнении определенного условия, к этой переменной может прибавляться единица\. При этом в коде ни разу значение у переменной _label_ не изменяется в меньшую сторону\. В отмеченной стрелочкой строке эта переменная сравнивается с константой, равной \-1, что не имеет практического смысла\. **Предупреждение PVS\-Studio**: [V3038](https://pvs-studio.ru/ru/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); .... } ``` Чтобы понять, что имеет в виду анализатор, посмотрим на параметры метода _photo\_fastNlMeansDenoisingMulti_: ```cpp public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize) ``` Упростим ещё больше, чтобы стало совсем очевидно\. Сравните эти строки: ```cpp NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....) ``` Анализатор обращает внимание на то, что разработчик дважды использовал переменную _templateWindowSize_, хотя, скорее всего, на месте первого упоминания этой переменной должна стоять _temporalWindowSize_\. Подозрительным выглядит и то, что значение _temporalWindowSize_ в методе _photo\_fastNlMeansDenoisingMulti_ не используется совсем\. Возможно, это сделано сознательно, но на месте разработчиков стоит приглядеться к этому коду повнимательнее, не закралась ли там ошибка? Похожие предупреждения анализатора: * 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 Следующая ошибка будет немного похожа на предыдущую\. **Предупреждение PVS\-Studio**: [V3066](https://pvs-studio.ru/ru/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); .... } } ``` Посмотрим на параметры _calib3d\_Rodrigues\_MatToVec_ ```cpp public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian) ``` Возможно, при вызове метода _calib3d\_Rodrigues\_MatToVec_ были перепутаны местами аргументы _matrixM\.CvPtr _и _vectorM\.CvPtr_\. Разработчикам стоит повнимательнее посмотреть на этот код\. Есть вероятность, что в него закралась ошибка, мешающая правильным расчётам\. **Предупреждение PVS\-Studio**: [V3063](https://pvs-studio.ru/ru/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) .... } ``` Анализатор указывает на то, что вторая проверка _data \=\= null_ никогда не будет _true_, т\.к\. если в первом условии _data_ имеет значение _null_, то будет сгенерировано исключение, и исполнение программы до второй проверки не дойдет\. ![0683_OpenCvSharp_ru/image5.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image5.png) _Понимаю, что вы уже устали, но осталось совсем немного\._ **Предупреждение PVS\-Studio**: [V3127](https://pvs-studio.ru/ru/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)); // <= .... } ``` А тут анализатор нашел опечатку\. В этом участке кода значение переменных проверяется на _null_, и, если это условие выполняется, то для каждой из переменных генерируется исключение\. Однако с переменной _window_ не все так просто\. Если эта переменная равна _null_, то для нее тоже генерируется исключение, но текст этого исключения записан неправильно\. В тексте этого исключения не фигурирует сама переменная _window_, вместо нее там указана _src2_\. Судя по всему, последнее условие должно быть таким: ```cpp if (window == null) throw new ArgumentNullException(nameof(window)); ``` **Предупреждение PVS\-Studio**: [V3142](https://pvs-studio.ru/ru/docs/warnings/v3142/) Unreacheble code detected\. It is possible that an error is present\. MatOfT\.cs 873 Теперь, для разнообразия, давайте рассмотрим случай, когда фактически анализатор совершенно прав, сообщая о недостижимом коде, но при этом ошибки нет\. Это тот случай, когда можно сказать, что анализатор выдаёт предупреждение, которое и правильно и ложно одновременно\. ```cpp public new Mat SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); } ``` Анализатор утверждает, что недостижим здесь оператор _return_\. Чтобы это проверить, посмотрим на тело метода _SubMat_\. ```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;*/ } ``` Как видите, функция пока не дописана и всегда генерирует исключение\. И анализатор прав, сообщая о недостижимом коде\. Но и настоящей ошибкой это назвать нельзя\. Следующие три ошибки, найденные анализатором, однотипные, но они настолько классные, что я не могла не выписать их все\. **Предупреждение PVS\-Studio**: [V3022](https://pvs-studio.ru/ru/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**: [V3022](https://pvs-studio.ru/ru/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**: [V3022](https://pvs-studio.ru/ru/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")) .... } ``` Иногда за предупреждением анализатора [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) \(выражение всегда истинно/ложно\) скрываются и впрямь странные или забавные вещи\. Во всех трёх случаях наблюдается одна и та же ситуация\. В коде метода имеется параметр типа _string_, значение которого надо проверить\. Однако проверяется не значение переменной, а строковый литерал с её именем, т\.е\. имя зря взято в кавычки\. ![0683_OpenCvSharp_ru/image6.png](https://import.viva64.com/docx/blog/0683_OpenCvSharp_ru/image6.png) Судя по всему, разработчик один раз опечатался и при помощи копипасты размножил эту ошибку по коду\. ## Заключение Разработчики OpenCvSharp проделали важную и большую работу\. И я, как пользователь этой библиотеки, им весьма благодарна\. Спасибо\. Однако оказавшись в команде PVS\-Studio и заглянув в код, я вынуждена констатировать, что вопрос его качества проработан недостаточно\. Скорее всего, в этом проекте на регулярной основе не используются статические анализаторы кода\. И многие ошибки правятся более дорогими методами \(тестированием, по отзывам пользователей, например\)\. А некоторые вообще остаются надолго жить в коде и их\-то мы как раз и обнаруживаем\. Чуть подробнее эта мысль изложена в небольшой [заметке](https://pvs-studio.ru/ru/blog/posts/0534/) на тему философии использования методологии статического анализа\. Поскольку проект открыт и расположен на GitHub, его разработчики имеют возможность воспользоваться [бесплатным вариантом лицензирования](https://pvs-studio.ru/ru/blog/posts/0614/) PVS\-Studio и начать применять анализ на регулярной основе\. Спасибо за внимание\. [Скачайте](https://pvs-studio.ru/ru/pvs-studio/download/) и проверьте ваши проекты с помощью триальной версии PVS\-Studio\.