Vérification de l'encapsuleur OpenCvSharp sur OpenCV avec PVS-Studio

Figure 2

OpenCV est une bibliothèque d'algorithmes de vision par ordinateur, de traitement d'image et d'algorithmes numériques à usage général avec open source, familière à de nombreux développeurs C ++. En plus de C ++, OpenCV est également en cours de développement pour Python, Java, Ruby, Matlab, Lua et d'autres langages. Puisque parmi ces langages il n'y a pas mon principal, C #, j'ai décidé de faire attention à la bibliothèque OpenCvSharp - wrapper sous C # et de vérifier ce projet. Ce qui est arrivé de cela peut être trouvé dans cet article.

Présentation


Auparavant, avant de rejoindre PVS-Studio, j'étais engagé dans la robotique lors d'expositions. Mes tâches comprenaient le correctif le plus élémentaire (en cas de panne majeure, le robot était confié à une autre personne), ainsi que le développement d'une grande variété de logiciels et d'utilitaires.

Figure 1

Fatigué et récemment arrivé dans une nouvelle ville, j'ai accompagné un robot KIKI fraîchement déballé.

En parlant de développement. C'était assez drôle. Chaque fois qu'une idée est venue à quelqu'un de l'équipe, quoi d'autre surprendrait les invités des expositions, nous avons soulevé cette question pour une discussion générale et, si l'idée était bonne, nous l'avons prise pour la mise en œuvre. Une fois, l'idée nous est venue de créer une créature qui répond à un visage humain avec un discours de bienvenue.

Après avoir cherché sur Internet une bibliothèque pour mes besoins, je suis tombé sur le site OpenCV, des bibliothèques d'algorithmes de vision par ordinateur. J'ai été bientôt déçu - OpenCV a été implémenté en C ++. Ma connaissance des avantages que j'ai retirés de l'université n'était clairement pas suffisante. Par conséquent, en parcourant brièvement Google, je suis tombé sur OpenCvSharp - wrapper de cette bibliothèque sous C #, mon langage principal. Six mois se sont écoulés depuis lors, le programme a déjà été écrit et utilisé depuis longtemps, et j'ai décidé de me mettre à l'abri d'OpenCvSharp et de vérifier son code source à l'aide de l'analyseur statique PVS-Studio.

Projet audité


OpenCvSharp est un wrapper sur OpenCV pour utiliser la bibliothèque dans des projets C #. La bibliothèque OpenCV, d'ailleurs, nous avons également vérifié . Les avantages d'OpenCvSharp incluent une grande collection d'échantillons de code, multiplateforme (peut fonctionner sur n'importe quelle plate-forme prise en charge par Mono) et facilité d'installation.

L'encapsuleur est un petit projet et contient environ 112 200 lignes de code C #. Parmi ceux-ci, 1,2% sont des commentaires, qui, soit dit en passant, sont en quelque sorte étrangement petits. Mais pour un si petit projet, il y a beaucoup d'erreurs. Dans l'article, j'ai écrit plus de 20 erreurs, mais il y en avait d'autres qui n'étaient pas si intéressantes ou évidentes.

Analyseur de code PVS-Studio


PVS-Studio est un outil pour détecter les erreurs et les vulnérabilités potentielles dans le code source des programmes écrits en C, C ++, C # et Java. Fonctionne sur Windows, Linux et macOS. Outre le code, les erreurs et les fautes de frappe inaccessibles, PVS-Studio peut détecter les vulnérabilités potentielles, comme indiqué ci-dessus. Par conséquent, il peut être considéré comme un outil pour les tests de sécurité des applications statiques (Static Application Security Testing, SAST).

Morceaux de code qui ont retenu l'attention lors de l'examen d'un rapport d'analyseur


La méthode WriteableBitmapConverter attire immédiatement l'attention avec quatre avertissements PVS-Studio du même type:

  • V3005 La variable 'optimumChannels [PixelFormats.Indexed1]' est assignée à elle-même. WriteableBitmapConverter.cs 22
  • V3005 La variable 'optimumChannels [PixelFormats.Indexed8]' est assignée à elle-même. WriteableBitmapConverter.cs 23
  • V3005 La variable 'optimumTypes [PixelFormats.Indexed1]' est assignée à elle-même. WriteableBitmapConverter.cs 50
  • V3005 La variable 'optimumTypes [PixelFormats.Indexed8]' est assignée à elle-même. 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; } .... } 

La classe PixelFormats est définie dans l' espace de noms System.Windows.Media et est une collection de différents formats de pixels. L'analyseur attire l'attention sur le fait que la méthode WriteableBitmapConverter réaffecte les valeurs aux éléments optimumChannels [PixelFormats.Indexed1] et optimumChannels [PixelFormats.Indexed8] , ce qui n'a aucun sens pratique. Il n'est pas clair s'il s'agit d'une simple faute de frappe ou d'autre chose. Soit dit en passant, cette section de code montre clairement les avantages des analyseurs statiques. Lorsque vous voyez un tas de lignes du même type devant vos yeux, vos yeux commencent à "flouter" et votre attention se dissipe - il n'est pas surprenant que même après la révision du code, une faute de frappe se glisse dans le programme. Et l'analyseur statique n'a pas de problèmes d'attention et n'a pas besoin de se reposer, il est donc plus facile pour lui de trouver de telles erreurs.

Figure 5

Ressentez la puissance et la puissance de l'analyse statique.

PVS-Studio Warning : V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «if» est insensée InputArray.cs 394

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

Cette erreur est un peu comme la précédente. Le programmeur a fait une situation où la même condition est vérifiée deux fois. Dans ce cas, cela n'a pas de sens - alors la branche de l' instruction if "dupliquée" ne sera pas exécutée, car:

  • si la première expression conditionnelle est vraie, la méthode se termine;
  • si la première condition est fausse, la seconde sera également fausse, puisque la variable testée - t - ne change pas entre les expressions conditionnelles.

Le développeur doit revérifier ce morceau de code. Il est probable qu'à la place de la deuxième variable Vec2s devrait en être une autre.

PVS-Studio Warning : V3010 La valeur de retour de la fonction 'ToString' doit être utilisée. ImgProcTest.cs 80

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

La méthode RotatedRectangleIntersection renvoie un tableau d'éléments Point2f via le paramètre intersectingRegion . Une fois que le programme a rempli l' intersectingRegion avec des valeurs, la méthode ToString () est appelée sur ce tableau . Avec les éléments du tableau, aucun changement ne se produit et aucun travail utile n'est effectué dans la dernière ligne, il y a donc des raisons de soupçonner le développeur qu'il a simplement oublié de le supprimer.

Avertissements de PVS-Studio :

  • V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «if» est insensée Cv2_calib3d.cs 1370
  • V3022 L'expression 'objectPoints == null' est toujours fausse. Cv2_calib3d.cs 1372

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

Dans ce cas, un fragment de code a été dupliqué, raison pour laquelle deux avertissements sont apparus. Le premier avertissement indique que les deux instructions if ont la même expression conditionnelle. Si cette expression est vraie, la méthode quittera la branche then- de la première instruction if . Pour cette raison, la deuxième condition sera toujours fausse, comme indiqué par l'avertissement suivant. Apparemment, le texte a été copié, mais a oublié de corriger.

Figure 6

Copier-coller mignon.

Autres avertissements similaires de l'analyseur:

  • V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «si» est insensée Cv2_calib3d.cs 1444
  • V3022 L'expression 'objectPoints == null' est toujours fausse. Cv2_calib3d.cs 1446

Avertissement PVS-Studio : l'expression V3022 'label == MarkerValue' est toujours fausse. Labeller.cs 135

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

Dans cette section de code, une variable d' étiquette de zéro est créée. Lorsqu'une certaine condition est remplie, une peut être ajoutée à cette variable. Dans ce cas, dans le code, la valeur de l' étiquette de variable ne change pas vers le bas. Dans la ligne marquée d'une flèche, cette variable est comparée à une constante égale à -1, ce qui n'a aucun sens pratique.

PVS-Studio Warning : V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 124

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

Pour comprendre ce que signifie l'analyseur, regardons les paramètres de la méthode photo_fastNlMeansDenoisingMulti :

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

Simplifiez encore plus pour le rendre très évident. Comparez ces lignes:

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

L'analyseur attire l'attention sur le fait que le développeur a utilisé la variable templateWindowSize deux fois, bien que, très probablement, temporalWindowSize devrait être à la place de la première mention de cette variable. Il est également suspect que la valeur de temporalWindowSize dans la méthode photo_fastNlMeansDenoisingMulti ne soit pas utilisée du tout. Peut-être que cela a été fait délibérément, mais à la place des développeurs, il vaut la peine d'examiner de plus près ce code, y a-t-il une erreur s'y glissant?

Avertissements similaires de l'analyseur:

  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 149
  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 180
  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 205

L'erreur suivante sera un peu similaire à la précédente.

Avertissement PVS-Studio : V3066 Ordre incorrect possible des arguments passés à la méthode 'calib3d_Rodrigues_MatToVec': 'matrixM.CvPtr' et 'vectorM.CvPtr'. Cv2_calib3d.cs 86

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

Regardons les paramètres calib3d_Rodrigues_MatToVec

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

Peut-être, lors de l'appel de la méthode calib3d_Rodrigues_MatToVec , les arguments matrixM.CvPtr et vectorM.CvPtr ont été mélangés. Les développeurs devraient examiner de plus près ce code. Il est possible qu'une erreur s'y soit glissée qui interfère avec les calculs corrects.

PVS-Studio Warning : V3063 Une partie de l'expression conditionnelle est toujours fausse si elle est évaluée: data == null. Mat.cs 3539

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

L'analyseur indique que la deuxième donnée de contrôle == null ne sera jamais vraie , car si dans la première condition les données sont nulles , une exception sera levée et l'exécution du programme n'atteindra pas le deuxième contrôle.

Figure 7

Je comprends que vous êtes déjà fatigué, mais il reste très peu.

Avertissement PVS-Studio : V3127 Deux fragments de code similaires ont été trouvés. Il s'agit peut-être d'une faute de frappe et la variable 'window' devrait être utilisée à la place de 'src2' Cv2_imgproc.cs 1547

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

Et puis l'analyseur a trouvé une faute de frappe. Dans cette section du code, la valeur des variables est vérifiée pour null , et si cette condition est remplie, une exception est levée pour chacune des variables. Cependant, la variable de fenêtre n'est pas si simple. Si cette variable est nulle , une exception est également générée pour elle, mais le texte de cette exception est mal orthographié. La fenêtre variable elle-même n'apparaît pas dans le texte de cette exception; à la place, src2 y est indiqué. Apparemment, la dernière condition devrait être la suivante:

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

Avertissement PVS-Studio : V3142 Code inaccessible détecté. Il est possible qu'une erreur soit présente. MatOfT.cs 873

Maintenant, pour un changement, regardons le cas où l'analyseur a en fait tout à fait raison lorsqu'il signale un code inaccessible, mais il n'y a pas d'erreur. C'est le cas quand on peut dire que l'analyseur génère un avertissement à la fois correct et faux en même temps.

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

L'analyseur prétend que l' instruction de retour est inaccessible ici. Pour vérifier cela, regardez le corps de la méthode SubMat .

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

Comme vous pouvez le voir, la fonction n'a pas encore été ajoutée et lève toujours une exception. Et l'analyseur a raison lorsqu'il signale un code inaccessible. Mais cela ne peut pas être qualifié de véritable erreur.

Les trois erreurs suivantes trouvées par l'analyseur sont du même type, mais elles sont tellement cool que je n'ai pas pu m'empêcher de toutes les écrire.

Avertissement PVS-Studio : l'expression V3022 'String.IsNullOrEmpty ("winName")' est toujours fausse. Cv2_highgui.cs 46

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

Avertissement PVS-Studio : l'expression V3022 'string.IsNullOrEmpty ("fileName")' est toujours fausse. FrameSource.cs 37

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

Avertissement PVS-Studio : l'expression V3022 'string.IsNullOrEmpty ("fileName")' est toujours fausse. FrameSource.cs 53

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

Parfois, derrière l'avertissement de l'analyseur V3022 (l'expression est toujours vraie / fausse), il y a des choses vraiment étranges ou drôles. Dans les trois cas, la même situation est observée. Le code de méthode a un paramètre de type chaîne dont la valeur doit être vérifiée. Cependant, ce n'est pas la valeur de la variable qui est vérifiée, mais le littéral de chaîne avec son nom, c'est-à-dire le nom est en vain cité.

Figure 18

Apparemment, le développeur a scellé une fois et, en utilisant le copier-coller, a propagé cette erreur par code.

Conclusion


Les développeurs d'OpenCvSharp ont fait un travail important et formidable. Et moi, en tant qu'utilisateur de cette bibliothèque, je leur en suis très reconnaissant. Je vous remercie

Cependant, étant dans l'équipe PVS-Studio et regardant le code, je dois admettre que le problème de sa qualité n'a pas été suffisamment résolu. Très probablement, les analyseurs de code statiques ne sont pas utilisés régulièrement dans ce projet. Et de nombreuses erreurs sont corrigées par des méthodes plus coûteuses (tests, selon les avis des utilisateurs, par exemple). Et certains restent généralement à vivre longtemps dans le code, et on ne fait que les retrouver. Cette idée est présentée plus en détail dans une brève note sur le thème de la philosophie de l'utilisation de la méthodologie de l'analyse statique.

Le projet étant ouvert et situé sur GitHub, ses développeurs ont la possibilité de profiter de l' option de licence gratuite PVS-Studio et de commencer à appliquer l'analyse de manière régulière.

Merci de votre attention. Téléchargez et testez vos projets en utilisant la version d'essai de PVS-Studio.



Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Ekaterina Nikiforova. Vérification de OpenCvSharp Wrapper pour OpenCV avec PVS-Studio .

Source: https://habr.com/ru/post/fr473476/


All Articles