OpenCV ist eine Open-Source-Bibliothek mit Computer Vision- und Bildverarbeitungsalgorithmen sowie universellen numerischen Algorithmen. Die Bibliothek ist unter C ++ - Entwicklern bekannt. Neben C ++ gibt es auch Versionen für Python, Java, Ruby, Matlab, Lua und andere Sprachen. Da C #, die Sprache, auf die ich mich spezialisiert habe, nicht auf dieser Liste steht, habe ich OpenCvSharp, einen C # -Wrapper von OpenCV, ausgewählt, um dies mit PVS-Studio zu überprüfen. Die Ergebnisse dieser Überprüfung werden in diesem Artikel erläutert.
Einleitung
Bevor ich Teil des PVS-Studio-Teams wurde, war ich an der Herstellung von Robotern beteiligt, die auf Ausstellungen präsentiert werden sollten. Zu meinen Aufgaben gehörten die grundlegendsten Reparaturarbeiten (größere Fehler wurden von einer anderen Person behoben) sowie die Entwicklung von Software und Dienstprogrammen aller Art.
Ich, müde und neu in der Stadt, mit einem frisch ausgepackten KIKI-Roboter.Der Entwicklungsteil war übrigens ziemlich lustig. Jedes Mal, wenn einer von uns eine Idee hatte, um die Ausstellungsbesucher zu überraschen, brachten wir sie zur Diskussion, und wenn es allen gefiel, machten wir uns an die Arbeit. Einmal kam uns der Gedanke, einen Roboter zu bauen, der ein menschliches Gesicht erkennen und mit einer Begrüßungsrede antworten konnte.
Ich googelte nach einer Bibliothek für meine Bedürfnisse und stieß auf OpenCV, eine Bibliothek mit Computer-Vision-Algorithmen. Aber ich wurde sehr bald enttäuscht, als ich herausfand, dass OpenCV in C ++ implementiert war. Meine Kenntnisse in C ++, die ich am College studiert hatte, reichten offensichtlich nicht aus. Also habe ich ein bisschen mehr gegoogelt und OpenCvSharp gefunden, einen Wrapper der Bibliothek für C #, die Sprache, auf die ich mich spezialisiert habe. Seitdem ist ungefähr ein halbes Jahr vergangen, das Programm wurde lange geschrieben und verwendet, und jetzt habe ich mich endlich entschlossen, "unter die Haube" von OpenCvSharp zu schauen und seinen Quellcode mit dem statischen Analysator PVS-Studio zu scannen.
Das zu analysierende Projekt
OpenCvSharp ist ein OpenCV-Wrapper zur Verwendung in C # -Projekten. Übrigens haben
wir OpenCV bereits in der Vergangenheit
überprüft . Die Stärken von OpenCvSharp sind die große Sammlung von Codebeispielen, die plattformübergreifende Unterstützung (die auf jeder von Mono unterstützten Plattform ausgeführt werden kann) und die einfache Installation.
Der Wrapper ist ein kleines Projekt mit etwa 112.200 Zeilen C # -Code. 1,2% davon sind Kommentare, die, wie ich sagen sollte, verdächtig wenige sind. Auf der anderen Seite gibt es einige Fehler für ein so kleines Projekt. Ich habe über 20 Beispiele für diesen Artikel ausgewählt, aber der Analysator hat tatsächlich viele weitere gefunden, die nicht so interessant oder offensichtlich sind.
PVS-Studio
PVS-Studio ist ein Tool zum Erkennen von Fehlern und potenziellen Schwachstellen im Quellcode von Programmen, die in C, C ++, C # und Java geschrieben wurden. Es läuft unter Windows, Linux und MacOS. Neben nicht erreichbarem Code, Programmierfehlern und Tippfehlern kann PVS-Studio, wie bereits erwähnt, potenzielle Sicherheitsprobleme erkennen. Daher kann es als SAST-Tool (Static Application Security Testing) angesehen werden.
Die interessantesten Warnungen
Das Besondere an der
WriteableBitmapConverter- Methode ist, dass vier Warnungen desselben Typs gleichzeitig ausgelöst wurden:
- V3005 Die Variable 'optimumChannels [PixelFormats.Indexed1]' wird sich selbst zugewiesen. WriteableBitmapConverter.cs 22
- V3005 Die Variable 'optimumChannels [PixelFormats.Indexed8]' wird sich selbst zugewiesen. WriteableBitmapConverter.cs 23
- V3005 Die Variable 'optimumTypes [PixelFormats.Indexed1]' wird sich selbst zugewiesen. WriteableBitmapConverter.cs 50
- V3005 Die Variable 'optimumTypes [PixelFormats.Indexed8]' wird sich selbst zugewiesen. WriteableBitmapConverter.cs 51
static WriteableBitmapConverter() { optimumChannels = new Dictionary <PixelFormat, int>(); optimumChannels[PixelFormats.Indexed1] =
Die
PixelFormats- Klasse wird im
System.Windows.Media- Namespace definiert und ist eine Sammlung verschiedener Pixelformate. Der Analysator weist darauf hin, dass den Elementen
optimumChannels [PixelFormats.Indexed1] und
optimumChannels [PixelFormats.Indexed8] in der
WriteableBitmapConverter- Methode zum zweiten Mal Werte zugewiesen werden, was keinen Sinn ergibt. Es ist unklar, ob dies nur ein Tippfehler ist oder der Programmierer etwas anderes gemeint hat. Übrigens ist dieses Snippet ein anschauliches Beispiel dafür, wie hilfreich statische Analysegeräte sein können: Wenn Sie sich eine Reihe ähnlicher Zeilen ansehen, werden Sie weniger fokussiert - kein Wunder, dass Tippfehler trotz der Codeüberprüfung unbemerkt bleiben. Statische Analysegeräte haben jedoch keine Probleme, die Aufmerksamkeit aufrechtzuerhalten, und sie benötigen keine Ruhe, sodass sie solche Fehler ohne Anstrengung erkennen können.
Fühle die Macht der statischen Analyse.PVS-Studio-Diagnosemeldung :
V3021 Es gibt zwei if-Anweisungen mit identischen bedingten Ausdrücken. Die erste 'if'-Anweisung enthält die Methodenrückgabe. Dies bedeutet, dass die zweite 'if'-Anweisung sinnlos ist. 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))
Dieser Fehler ähnelt dem vorherigen. Der Entwickler überprüft denselben Zustand zweimal. Es macht hier keinen Sinn, da der then-Zweig der "duplicate"
if- Anweisung niemals ausgeführt wird, weil:
- Wenn die erste Bedingung erfüllt ist, gibt die Methode zurück.
- Wenn die erste Bedingung falsch ist, ist auch die zweite falsch, da sich die zu prüfende Variable t zwischen den beiden Prüfungen nicht ändert.
Dieser Code muss überarbeitet werden. Es ist sehr wahrscheinlich, dass die zweite Kopie von
Vec2s tatsächlich eine andere Variable sein sollte.
PVS-Studio-Diagnosemeldung :
V3010 Der Rückgabewert der Funktion 'ToString' muss verwendet werden. 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(); }
Auf die
RotatedRectangleIntersection- Methode wird über den Parameter
intersectingRegion zugegriffen, und es wird ein Array von Elementen vom Typ
Point2f zurückgegeben . Sobald die
intersectingRegion mit Werten gefüllt wurde, wird die
ToString () -Methode für das Array aufgerufen. Dies wirkt sich in keiner Weise auf die Elemente des Arrays aus, und in der letzten Zeile werden keine nützlichen Arbeiten ausgeführt. Es ist daher fair anzunehmen, dass der Entwickler einfach vergessen hat, dieses Teil zu entfernen.
PVS-Studio-Diagnosemeldungen:- V3021 Es gibt zwei 'if'-Anweisungen mit identischen bedingten Ausdrücken. Die erste 'if'-Anweisung enthält die Methodenrückgabe. Dies bedeutet, dass die zweite 'if'-Anweisung sinnlos ist. Cv2_calib3d.cs 1370
- V3022 Der Ausdruck 'objectPoints == null' ist immer falsch. Cv2_calib3d.cs 1372
public static double CalibrateCamera(....) { if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); .... }
Wir haben hier Code geklont, daher die beiden Warnungen. Der erste besagt, dass beide
if- Anweisungen dieselbe Bedingung prüfen. Wenn diese Bedingung erfüllt ist, wird die Methode in der then-Verzweigung der ersten
if- Anweisung zurückgegeben. Folglich wird die zweite Bedingung immer falsch sein, was uns die zweite Warnung sagt. Es scheint, dass der Programmierer dieses Fragment mit Copy-Paste geklont hat, aber vergessen hat, es zu ändern.
Nettes Kopieren-Einfügen.Andere Warnungen dieses Typs:
- V3021 Es gibt zwei 'if'-Anweisungen mit identischen bedingten Ausdrücken. Die erste 'if'-Anweisung enthält die Methodenrückgabe. Dies bedeutet, dass die zweite 'if'-Anweisung sinnlos ist. Cv2_calib3d.cs 1444
- V3022 Der Ausdruck 'objectPoints == null' ist immer falsch. Cv2_calib3d.cs 1446
PVS-Studio-Diagnosemeldung: V3022 Der Ausdruck 'label == MarkerValue' ist immer falsch. 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;
Eine Variable mit dem Namen
label wird erstellt und auf 0 initialisiert. Wenn eine bestimmte Bedingung erfüllt ist, wird sie um eins erhöht. Darüber hinaus wird diese Variable in diesem Snippet nie dekrementiert. Daher macht es keinen Sinn, die Konstante -1 zu überprüfen, wie in der Zeile, auf die der Analysator zeigt.
PVS-Studio-Diagnosemeldung: V3038 Das Argument wurde mehrmals an die Methode übergeben. Es ist möglich, dass stattdessen ein anderes Argument übergeben wird. Cv2_photo.cs 124
public static void FastNlMeansDenoisingMulti(....) { .... NativeMethods.photo_fastNlMeansDenoisingMulti( srcImgPtrs, srcImgPtrs.Length, dst.CvPtr, imgToDenoiseIndex, templateWindowSize, h, templateWindowSize, searchWindowSize); .... }
Um zu verstehen, was der Analysator uns sagt, werfen wir einen Blick auf die Parameter der
photo_fastNlMeansDenoisingMulti- Methode:
public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize)
Vereinfachen wir es noch weiter, um es ganz einfach zu machen. Vergleichen Sie diese Zeilen:
NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....)
Die Variable
templateWindowSize wird zweimal deklariert, aber das erste Mal, wenn sie erwähnt wird, sollte tatsächlich die Deklaration von
temporalWindowSize sein . Eine andere Sache, die dem Analysator nicht gefallen hat, ist, dass der Wert von
temporalWindowSize in der
photo_fastNlMeansDenoisingMulti- Methode überhaupt nicht verwendet wird. Dies könnte eine bewusste Entscheidung sein, aber ich würde mir diesen Code genauer ansehen, wenn ich der Autor wäre.
Andere Warnungen dieses Typs:
- V3038 Das Argument wurde mehrmals an method übergeben. Es ist möglich, dass stattdessen ein anderes Argument übergeben wird. Cv2_photo.cs 149
- V3038 Das Argument wurde mehrmals an method übergeben. Es ist möglich, dass stattdessen ein anderes Argument übergeben wird. Cv2_photo.cs 180
- V3038 Das Argument wurde mehrmals an method übergeben. Es ist möglich, dass stattdessen ein anderes Argument übergeben wird. Cv2_photo.cs 205
Das nächste Beispiel ist dem vorherigen etwas ähnlich.
PVS-Studio-Diagnosemeldung: V3066 Möglicherweise falsche Reihenfolge der an die Methode 'calib3d_Rodrigues_MatToVec' übergebenen Argumente: 'matrixM.CvPtr' und '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); .... } }
Schauen wir uns die Parameter der
calib3d_Rodrigues_MatToVec- Methode an:
public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian)
Es scheint, dass die Methode
calib3d_Rodrigues_MatToVec aufgerufen wird, wobei die Argumente
matrixM.CvPtr und
vectorM.CvPtr versehentlich ausgetauscht wurden. Die Autoren sollten dieses Snippet überprüfen: Möglicherweise liegt ein Fehler vor, der die korrekte Berechnung behindert.
PVS-Studio-Diagnosemeldung: V3063 Ein Teil des bedingten Ausdrucks ist immer falsch, wenn er ausgewertet wird: 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)
Der Analysator meldet, dass die zweiten Prüfdaten
== null niemals
wahr sind, da eine Ausnahme ausgelöst wird und die Ausführung niemals die zweite Prüfung erreicht, wenn die
Daten in der ersten Bedingung gleich
null sind .
Ich weiß, dass du müde bist, aber wir sind fast fertig.PVS-Studio-Diagnosemeldung: V3127 Es wurden zwei ähnliche Codefragmente gefunden. Möglicherweise ist dies ein Tippfehler, und anstelle von 'src2' Cv2_imgproc.cs 1547 sollte die Variable 'window' verwendet werden
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));
Der Analysator hat in diesem Ausschnitt einen Tippfehler entdeckt. Die Variablen werden auf
null geprüft, und wenn dies der Fall ist, löst jede Prüfung eine Ausnahme aus. Es funktioniert jedoch nicht ganz richtig für die
Fenstervariable . Wenn sein Wert gleich
null ist , wird auch eine entsprechende Ausnahme ausgelöst, jedoch mit dem falschen Text. Es wird kein
Fenster erwähnt ; es wird stattdessen
src2 sein . Die Bedingung sollte anscheinend wie folgt geändert werden:
if (window == null) throw new ArgumentNullException(nameof(window));
PVS-Studio-Diagnosemeldung: V3142 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. MatOfT.cs 873
Schauen wir uns zur Abwechslung einmal den Fall an, in dem der Analysator in Bezug auf nicht erreichbaren Code technisch korrekt ist, aber tatsächlich kein Fehler vorliegt. Es ist eine Warnung, die gleichzeitig als wahr und falsch bezeichnet werden kann.
public new Mat<TElem> SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); }
Der Analysator teilt uns mit, dass die
return- Anweisung nicht erreichbar ist. Schauen wir uns den Hauptteil der
SubMat- Methode an, um
festzustellen , ob der Analysator die Wahrheit sagt.
public Mat SubMat(params Range[] ranges) { throw new NotImplementedException(); }
Wie Sie sehen können, ist die Funktion derzeit unvollständig und löst immer eine Ausnahme aus. Der Analysator weist absolut korrekt auf den nicht erreichbaren Code hin - aber es ist kein echter Fehler.
Die nächsten drei Fehler sind vom gleichen Typ, aber sie sind so cool, dass ich nicht anders konnte, als alle drei einzuschließen.
PVS-Studio-Diagnosemeldung: Der V3022- Ausdruck 'String.IsNullOrEmpty ("winName")' ist immer falsch. Cv2_highgui.cs 46
public static void DestroyWindow(string winName) { if (String.IsNullOrEmpty("winName")) .... }
PVS-Studio-Diagnosemeldung: V3022 Der Ausdruck 'string.IsNullOrEmpty ("fileName")' ist immer falsch. FrameSource.cs 37
public static FrameSource CreateFrameSource_Video(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }
PVS-Studio-Diagnosemeldung: V3022 Der Ausdruck 'string.IsNullOrEmpty ("fileName")' ist immer falsch. FrameSource.cs 53
public static FrameSource CreateFrameSource_Video_CUDA(string fileName) { if (string.IsNullOrEmpty("fileName")) .... }
Manchmal
deuten V3022- Warnungen (über immer wahre / falsche Ausdrücke) auf wirklich seltsame oder lustige Fehler hin. Alle drei obigen Beispiele haben den gleichen Fehler. Die Methode verfügt über einen Parameter vom Typ
string, dessen Wert überprüft werden muss. Stattdessen wird jedoch ein Zeichenfolgenliteral überprüft, dessen Text der Name der Variablen ist, d. H. Der Name der Variablen in Anführungszeichen.
Der Programmierer muss einmal einen fehlerhaften Codeblock geschrieben und ihn dann durch Kopieren und Einfügen geklont haben.
Fazit
Die Entwickler von OpenCvSharp haben einen großen und wichtigen Job gemacht, und als Benutzer ihrer Bibliothek bin ich dafür absolut dankbar. Danke Jungs!
Aber jetzt, da ich Teil des PVS-Studio-Teams geworden bin und den Code der Bibliothek gesehen habe, muss ich sagen, dass dem Qualitätsaspekt nicht die richtige Aufmerksamkeit geschenkt wurde. Das Projekt sieht nicht so aus, als würde eines regelmäßig mit statischen Analysegeräten überprüft, und viele der Fehler werden anscheinend mit teureren Techniken (wie Tests oder Benutzerfeedback) behoben, und einige der Fehler leben einfach weiter im Code und es sind sie das fangen wir mit unserem analyzer. Dieses Thema wird in
diesem kleinen Beitrag zur Philosophie der statischen Analyse ausführlicher behandelt.
Da OpenCvSharp Open Source ist und auf GitHub frei verfügbar ist, können seine Autoren eine der
kostenlosen Lizenzoptionen für PVS-Studio verwenden, um es regelmäßig zu verwenden.
Danke fürs Lesen. Zögern Sie nicht, eine Testversion von PVS-Studio
herunterzuladen , um Ihre eigenen Projekte zu überprüfen.