WinForms: Fehler, Holmes

Bild 5

Wir suchen gerne nach Fehlern in Microsoft-Projekten. Warum? Es ist ganz einfach: Ihre Projekte sind normalerweise leicht zu überprüfen (Sie können in einer Visual Studio-Umgebung arbeiten, für die PVS-Studio ein praktisches Plugin hat) und sie enthalten nur wenige Fehler. Aus diesem Grund lautet der übliche Arbeitsalgorithmus wie folgt: Suchen und Herunterladen eines Open Source-Projekts von MS; überprüfe es; wähle interessante Fehler; Stellen Sie sicher, dass es nur wenige davon gibt. Schreiben Sie einen Artikel, ohne zu vergessen, die Entwickler zu loben. Großartig! Win-Win-Win: Es hat ein wenig gedauert, die Chefs sind froh, neue Materialien im Blog zu sehen, und Karma ist in Ordnung. Aber diesmal "ging etwas schief". Mal sehen, was wir im Quellcode von Windows Forms gefunden haben und ob wir diesmal hoch über Microsoft sprechen sollten.

Einführung

Anfang Dezember 2018 kündigte Microsoft die Veröffentlichung von .NET Core 3 Preview 1 an. Etwas früher (etwa Mitte Oktober) begann GitHub, die Quellen von Windows Forms - der .NET Core UI-Plattform zum Erstellen von Windows-Desktopanwendungen - aktiv offenzulegen . Sie können die Commit-Statistiken hier sehen . Jetzt kann jeder den WinForms-Quellcode zur Überprüfung herunterladen.

Ich habe auch die Quellen heruntergeladen, um dort mit PVS-Studio nach Fehlern zu suchen. Die Überprüfung verursachte keine Schwierigkeiten. Wir brauchten: Visual Studio 2019, .NET Core 3.0 SDK-Vorschau, PVS-Studio. Und hier haben wir das Protokoll der Warnungen des Analysators.

Nachdem ich den PVS-Studio-Bericht erhalten habe, sortiere ich ihn normalerweise nach Diagnosenummern in aufsteigender Reihenfolge (das Fenster mit dem PVS-Studio-Nachrichtenprotokoll in der Visual Studio-Umgebung bietet verschiedene Optionen zum Sortieren und Filtern der Liste). Sie können mit Gruppen ähnlicher Fehler arbeiten, was die Quellcode-Analyse erheblich vereinfacht. Ich markiere interessante Fehler in der Liste mit einem "Stern" und erst dann, nachdem ich das gesamte Protokoll analysiert habe, schreibe ich Codefragmente aus und beschreibe sie. Da es normalerweise nur wenige Fehler gibt, "rühre" ich sie und versuche, die interessantesten am Anfang und Ende des Artikels zu platzieren. Aber diesmal stellte sich heraus, dass es viele Fehler gab (eh, die Intrige wurde schon lange nicht mehr gespeichert), und ich werde sie in der Reihenfolge der Anzahl der Diagnosen zitieren.

Was haben wir gefunden? 833 hohe und mittlere Warnungen (249 bzw. 584) wurden für 540.000 Codezeilen (ohne leere) in 1670 cs-Dateien ausgegeben. Und ja, traditionell habe ich die Tests nicht überprüft und die niedrigen Warnungen nicht berücksichtigt (es gab 215 davon). Nach meinen früheren Beobachtungen sind die Warnungen für das MS-Projekt zu viele. Aber nicht alle Warnungen sind Fehler.

Für dieses Projekt betrug die Anzahl der Fehlalarme etwa 30%. In etwa 20% der Fälle konnte ich einfach keine genaue Schlussfolgerung ziehen, ob es sich um einen Fehler handelte oder nicht, da ich mit dem Code nicht gut genug vertraut war. Und mindestens 20% der Fehler, die ich verpasst habe, können als "menschlicher Faktor" abgeschrieben werden: Eile, Müdigkeit usw. Übrigens ist auch der gegenteilige Effekt möglich: Bei einigen Triggern des gleichen Typs, deren Anzahl 70-80 erreichen könnte, habe ich "vorletzter" gesucht, was manchmal die Anzahl der Fehler erhöhen könnte, die ich für real hielt.

Auf jeden Fall weisen 30% der Warnungen auf echte Fehler hin. Dies ist ein ziemlich großer Prozentsatz, wenn Sie berücksichtigen, dass der Analysator nicht vorkonfiguriert war.

Die Anzahl der Fehler, die ich gefunden habe, betrug ungefähr 240, was im Bereich der angegebenen Statistiken liegt. Meiner Meinung nach ist dies nicht das herausragendste Ergebnis für ein MS-Projekt (obwohl es nur 0,44 Fehler pro 1000 Codezeilen macht), und es gibt wahrscheinlich auch mehr echte Fehler im WinForms-Code. Ich schlage vor, die Gründe am Ende des Artikels zu berücksichtigen und nun die interessantesten Fehler zu sehen.

Fehler

PVS-Studio: V3003 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Überprüfen Sie die Zeilen: 213, 224. ButtonStandardAdapter.cs 213

void PaintWorker(PaintEventArgs e, bool up, CheckState state) { up = up && state == CheckState.Unchecked; .... if (up & IsHighContrastHighlighted()) { .... } else if (up & IsHighContrastHighlighted()) { .... } else { .... } .... } 

Wenn und sonst wenn Blöcke den gleichen Zustand prüfen. Es sieht aus wie Kopieren und Einfügen. Ist es ein Fehler? Wenn Sie sich die Deklaration der IsHighContrastHighlighted- Methode ansehen , können Sie dies bezweifeln:

 protected bool IsHighContrastHighlighted() { return SystemInformation.HighContrast && Application.RenderWithVisualStyles && (Control.Focused || Control.MouseIsOver || (Control.IsDefault && Control.Enabled)); } 

Die Methode kann wahrscheinlich unterschiedliche Werte für sequentielle Aufrufe zurückgeben. Und was in der Aufrufermethode passiert, sieht natürlich seltsam aus, hat aber das Recht zu existieren. Ich würde den Autoren jedoch raten, sich dieses Codefragment anzusehen. Nur für den Fall. Es ist auch ein gutes Beispiel dafür, wie schwierig es ist, bei der Analyse von unbekanntem Code Schlussfolgerungen zu ziehen.

PVS-Studio: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. RichTextBox.cs 1018

 public int SelectionCharOffset { get { int selCharOffset = 0; .... NativeMethods.CHARFORMATA cf = GetCharFormat(true); // if the effects member contains valid info if ((cf.dwMask & RichTextBoxConstants.CFM_OFFSET) != 0) { selCharOffset = cf.yOffset; // <= } else { // The selection contains characters of different offsets, // so we just return the offset of the first character. selCharOffset = cf.yOffset; // <= } .... } .... } 

Und hier gibt es definitiv einen Fehler beim Kopieren und Einfügen. Unabhängig von der Bedingung erhält die Variable selCharOffset immer den gleichen Wert.

Es gibt zwei weitere solche Fehler im WinForms-Code:
  • V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. SplitContainer.cs 1700
  • V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. ToolstripProfessionalRenderer.cs 371

PVS-Studio: V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 681, 680. ProfessionalColorTable.cs 681

 internal void InitSystemColors(ref Dictionary<KnownColors, Color> rgbTable) { .... rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonFace; rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonShadow; .... } 

Die Methode füllt das rgbTable- Wörterbuch. Der Analysator zeigte auf ein Codefragment, bei dem verschiedene Werte nacheinander zweimal auf denselben Schlüssel geschrieben werden. Die Dinge wären in Ordnung, aber es gibt immer noch 16 solcher Fragmente in dieser Methode. Es sieht nicht mehr nach einem einzigartigen Fehler aus. Aber warum sie das tun, bleibt mir ein Rätsel. Ich habe keine Anzeichen von automatisch generiertem Code gefunden. Im Editor sieht es so aus:

Bild 3

Ich gebe Ihnen die ersten zehn Warnungen auf der Liste:

  1. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 785, 784. ProfessionalColorTable.cs 785
  2. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 787, 786. ProfessionalColorTable.cs 787
  3. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 789, 788. ProfessionalColorTable.cs 789
  4. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 791, 790. ProfessionalColorTable.cs 791
  5. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 797, 796. ProfessionalColorTable.cs 797
  6. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 799, 798. ProfessionalColorTable.cs 799
  7. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 807, 806. ProfessionalColorTable.cs 807
  8. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 815, 814. ProfessionalColorTable.cs 815
  9. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 817, 816. ProfessionalColorTable.cs 817
  10. V3008 Der Variablen werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 823, 822. ProfessionalColorTable.cs 823

PVS-Studio: V3011 Es wurden zwei entgegengesetzte Bedingungen festgestellt . Die zweite Bedingung ist immer falsch. Überprüfen Sie die Zeilen: 5242, 5240. DataGrid.cs 5242

 private void CheckHierarchyState() { if (checkHierarchy && listManager != null && myGridTable != null) { if (myGridTable == null) // <= { // there was nothing to check return; } for (int j = 0; j < myGridTable.GridColumnStyles.Count; j++) { DataGridColumnStyle gridColumn = myGridTable.GridColumnStyles[j]; } checkHierarchy = false; } } 

Der Rückgabeoperator wird niemals ausgeführt. Höchstwahrscheinlich ist die Bedingung myGridTable! = Null im externen if- Block später während des Refactorings hinzugefügt worden. Und jetzt ist die Überprüfung von myGridTable == null bedeutungslos. Um die Codequalität zu verbessern, sollten Sie diese Prüfung entfernen.

PVS-Studio: V3019 Möglicherweise wird eine falsche Variable nach der Typkonvertierung mit dem Schlüsselwort 'as' mit null verglichen. Überprüfen Sie die Variablen 'left', 'cscLeft'. TypeCodeDomSerializer.cs 611

PVS-Studio: V3019 Möglicherweise wird eine falsche Variable nach der Typkonvertierung mit dem Schlüsselwort 'as' mit null verglichen. Überprüfen Sie die Variablen 'right', 'cscRight'. TypeCodeDomSerializer.cs 615

 public int Compare(object left, object right) { OrderedCodeStatementCollection cscLeft = left as OrderedCodeStatementCollection; OrderedCodeStatementCollection cscRight = right as OrderedCodeStatementCollection; if (left == null) { return 1; } else if (right == null) { return -1; } else if (right == left) { return 0; } return cscLeft.Order - cscRight.Order; // <= } 

Der Analysator hat gleichzeitig zwei Warnungen für die Vergleichsmethode generiert. Was ist das Problem? Es ist so, dass die Werte für cscLeft und cscRight überhaupt nicht auf Null geprüft werden. Sie erhalten diesen Wert möglicherweise nach erfolglosem Umwandeln in den OrderedCodeStatementCollection- Typ. Dann wird im letzten Rückgabeausdruck eine Ausnahme ausgelöst. Diese Situation ist möglich, wenn alle Prüfungen für links und rechts bestanden sind und nicht zu einem vorläufigen Verlassen der Methode führen.

Um den Code zu korrigieren , sollten Sie überall cscLeft / cscRight anstelle von left / right verwenden .

PVS-Studio: V3020 Eine bedingungslose Unterbrechung innerhalb einer Schleife. SelectionService.cs 421

 void ISelectionService.SetSelectedComponents( ICollection components, SelectionTypes selectionType) { .... // Handle the click case object requestedPrimary = null; int primaryIndex; if (fPrimary && 1 == components.Count) { foreach (object o in components) { requestedPrimary = o; if (o == null) { throw new ArgumentNullException(nameof(components)); } break; } } .... } 

Dieses Fragment bezieht sich eher auf den "Code-Geruch". Hier liegt kein Fehler vor. Es stellen sich jedoch Fragen zur Organisation der foreach- Schleife. Es ist klar, warum es hier benötigt wird: weil Elemente der Sammlung extrahiert werden müssen, die als ICollection übergeben wurden . Aber warum benötigte die Schleife, die ursprünglich für eine einzelne Iteration konzipiert war (Voraussetzung ist das Vorhandensein eines einzelnen Elements in den Auflistungskomponenten), zusätzliche Unterstützung, z. B. break ? Wahrscheinlich kann die Antwort wie folgt betrachtet werden: "Historisch gesehen ist dies geschehen." Der Code sieht hässlich aus.

PVS-Studio: V3022 Ausdruck 'ocxState! = Null' ist immer wahr. AxHost.cs 2186

 public State OcxState { .... set { .... if (value == null) { return; } .... ocxState = value; if (ocxState != null) // <= { axState[manualUpdate] = ocxState._GetManualUpdate(); licenseKey = ocxState._GetLicenseKey(); } else { axState[manualUpdate] = false; licenseKey = null; } .... } } 

Aufgrund eines logischen Fehlers ist in diesem Fragment "toter Code" aufgetreten. Ausdrücke im else- Block werden niemals ausgeführt.

PVS-Studio: V3027 Die Variable 'e' wurde im logischen Ausdruck verwendet, bevor sie im gleichen logischen Ausdruck gegen null verifiziert wurde. ImageEditor.cs 99

 public override object EditValue(....) { .... ImageEditor e = ....; Type myClass = GetType(); if (!myClass.Equals(e.GetType()) && e != null && myClass.IsInstanceOfType(e)) { .... } .... } 

Die Variable e in der Bedingung wird zuerst verwendet und dann gegen Null geprüft. Hallo, NullReferenceException .

Noch ein solcher Fehler:

PVS-Studio: V3027 Die Variable 'dropDownItem' wurde im logischen Ausdruck verwendet, bevor sie im selben logischen Ausdruck gegen null verifiziert wurde. ToolStripMenuItemDesigner.cs 1351

 internal void EnterInSituEdit(ToolStripItem toolItem) { .... ToolStripDropDownItem dropDownItem = toolItem as ToolStripDropDownItem; if (!(dropDownItem.Owner is ToolStripDropDownMenu) && dropDownItem != null && dropDownItem.Bounds.Width < commitedEditorNode.Bounds.Width) { .... } .... } 

Die Situation ist ähnlich wie in der vorherigen, jedoch mit der Variablen dropDownItem . Ich denke, dass solche Fehler auf unachtsames Refactoring zurückzuführen sind. Wahrscheinlich wurde später ein Teil der Bedingung (DropDownItem.Owner ist ToolStripDropDownMenu) in den Code eingefügt.

PVS-Studio: V3030 Wiederkehrende Prüfung. Die Bedingung 'columnCount> 0' wurde bereits in Zeile 3900 überprüft. ListView.cs 3903

 internal ColumnHeader InsertColumn( int index, ColumnHeader ch, bool refreshSubItems) { .... // Add the column to our internal array int columnCount = (columnHeaders == null ? 0 : columnHeaders.Length); if (columnCount > 0) { ColumnHeader[] newHeaders = new ColumnHeader[columnCount + 1]; if (columnCount > 0) { System.Array.Copy(columnHeaders, 0, newHeaders, 0, columnCount); } .... } .... } 

Ein Fehler, der harmlos erscheinen mag. In der Tat wird eine unnötige Prüfung durchgeführt, die die Betriebslogik nicht beeinflusst. Und manchmal geschieht dies sogar, wenn Sie den Status einer visuellen Komponente erneut überprüfen müssen, z. B. um die Anzahl der Einträge in der Liste abzurufen. In diesem Fall wird die lokale Variable columnCount jedoch zweimal überprüft. Es ist sehr verdächtig. Entweder wollten sie eine andere Variable prüfen oder sie haben bei einer der Prüfungen eine falsche Bedingung verwendet.

PVS-Studio: V3061 Der Parameter 'lprcClipRect' wird vor seiner Verwendung immer im Methodenkörper neu geschrieben. WebBrowserSiteBase.cs 281

 int UnsafeNativeMethods.IOleInPlaceSite.GetWindowContext( out UnsafeNativeMethods.IOleInPlaceFrame ppFrame, out UnsafeNativeMethods.IOleInPlaceUIWindow ppDoc, NativeMethods.COMRECT lprcPosRect, NativeMethods.COMRECT lprcClipRect, NativeMethods.tagOIFI lpFrameInfo) { ppDoc = null; ppFrame = Host.GetParentContainer(); lprcPosRect.left = Host.Bounds.X; lprcPosRect.top = Host.Bounds.Y; .... lprcClipRect = WebBrowserHelper.GetClipRect(); // <= if (lpFrameInfo != null) { lpFrameInfo.cb = Marshal.SizeOf<NativeMethods.tagOIFI>(); lpFrameInfo.fMDIApp = false; .... } return NativeMethods.S_OK; } 

Ein offensichtlicher Fehler. Ja, der Parameter lprcClipRect wird tatsächlich mit einem neuen Wert initialisiert, ohne ihn in irgendeiner Weise zu verwenden. Aber wozu führt es am Ende? Ich denke, dass irgendwo im aufrufenden Code die durch diesen Parameter übergebene Referenz unverändert bleibt, obwohl dies nicht beabsichtigt war. Schätzen Sie wirklich den Umgang mit anderen Variablen in dieser Methode. Sogar der Name (Präfix "Get") weist darauf hin, dass eine Initialisierung innerhalb der Methode über übergebene Parameter durchgeführt wird. Und so ist es auch. Die ersten beiden Parameter ( ppFrame und ppDoc ) werden mit dem Modifikator out übergeben und erhalten neue Werte. Die Referenzen lprcPosRect und lpFrameInfo werden verwendet, um auf Klassenfelder zuzugreifen und diese zu initialisieren. Nur lprcClipRect fällt auf. Wahrscheinlich ist für diesen Parameter der Modifikator out oder ref erforderlich.

PVS-Studio: V3066 Möglicherweise falsche Reihenfolge der Argumente, die an die Methode 'AdjustCellBorderStyle' übergeben wurden: 'isFirstDisplayedRow' und 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

 protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { .... dgvabsEffective = AdjustCellBorderStyle( DataGridView.AdvancedCellBorderStyle, dgvabsPlaceholder, singleVerticalBorderAdded, singleHorizontalBorderAdded, isFirstDisplayedRow, // <= isFirstDisplayedColumn); // <= .... } 

Der Analysator vermutete, dass die letzten beiden Argumente verwechselt wurden. Schauen wir uns die Deklaration der AdjustCellBorderStyle- Methode an:

 public virtual DataGridViewAdvancedBorderStyle AdjustCellBorderStyle( DataGridViewAdvancedBorderStyledataGridViewAdvancedBorderStyleInput, DataGridViewAdvancedBorderStyle dataGridViewAdvancedBorderStylePlaceholder, bool singleVerticalBorderAdded, bool singleHorizontalBorderAdded, bool isFirstDisplayedColumn, bool isFirstDisplayedRow) { .... } 

Sieht aus wie ein Fehler. Ja, einige Argumente werden häufig in umgekehrter Reihenfolge übergeben, um beispielsweise einige Variablen auszutauschen. Aber ich denke nicht, dass dies der Fall ist. Nichts in den Anrufer- oder Angerufenen-Methoden weist auf dieses Verwendungsmuster hin. Zunächst werden Variablen vom Typ bool verwechselt . Zweitens sind die Namen der Methoden ebenfalls regelmäßig: kein "Swap" oder "Reverse". Außerdem ist es nicht so schwer, so einen Fehler zu machen. Menschen nehmen die Reihenfolge des Paares "Zeile / Spalte" oft unterschiedlich wahr. Für mich ist zum Beispiel die "Zeile / Spalte" bekannt. Für den Autor der Methode namens AdjustCellBorderStyle ist die üblichere Reihenfolge natürlich "Spalte / Zeile".

PVS-Studio: V3070 Nicht initialisierte Variable 'LANG_USER_DEFAULT' wird beim Initialisieren der Variablen 'LOCALE_USER_DEFAULT' verwendet. NativeMethods.cs 890

 internal static class NativeMethods { .... public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT); .... } 

Seltener Fehler. Die Initialisierungsreihenfolge von Klassenfeldern ist verwechselt. Zur Berechnung des Wertes des Feldes LOCALE_USER_DEFAULT wird das Feld LANG_USER_DEFAULT verwendet, das noch nicht initialisiert ist und den Wert 0 hat. Die Variable LANG_USER_DEFAULT wird übrigens an keiner anderen Stelle im Code verwendet. Ich ging eine Extrameile und schrieb ein kleines Konsolenprogramm, das die Situation simuliert. Ich habe einige im WinForms-Code verwendete Konstanten durch ihre tatsächlichen Werte ersetzt:

 internal static class NativeMethods { public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(0x00, 0x01); public static int MAKELANGID(int primary, int sub) { return ((((ushort)(sub)) << 10) | (ushort)(primary)); } public static int MAKELCID(int lgid) { return MAKELCID(lgid, 0x0); } public static int MAKELCID(int lgid, int sort) { return ((0xFFFF & lgid) | (((0x000f) & sort) << 16)); } } class Program { static void Main() { System.Console.WriteLine(NativeMethods.LOCALE_USER_DEFAULT); } } 

Infolgedessen zeigt die Konsole Folgendes an : 0. Tauschen wir nun die Deklarationen der Felder LOCALE_USER_DEFAULT und LANG_USER_DEFAULT aus . Das Ergebnis der Programmausführung lautet wie folgt: 1024. Ich denke, hier gibt es nichts mehr zu kommentieren.

PVS-Studio: V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Inspektion von "ces". CodeDomSerializerBase.cs 562

 protected void DeserializeStatement( IDesignerSerializationManager manager, CodeStatement statement) { .... CodeExpressionStatement ces = statement as CodeExpressionStatement; if (ces != null) { .... } else { .... DeserializeExpression(manager, null, ces.Expression); // <= .... } .... } 

Der Code, der ziemlich regelmäßig "abstürzen" sollte, da Sie in den else- Zweig gelangen können, wenn die ces- Referenz gleich null ist .

Ein weiteres ähnliches Beispiel:

PVS-Studio: V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die 'comboBox'. ComboBox.cs 6610

 public void ValidateOwnerDrawRegions(ComboBox comboBox, ....) { .... if (comboBox != null) { return; } Rectangle topOwnerDrawArea = new Rectangle(0, 0, comboBox.Width, innerBorder.Top); .... } 

Der paradoxe Code. Anscheinend wurde die Prüfung if (comboBox! = Null) mit if (comboBox == null) verwechselt. Und so erhalten wir eine weitere NullReferenceException.

Wir haben zwei ziemlich offensichtliche V3080- Fehler betrachtet, bei denen Sie eine mögliche Verwendung von Nullreferenzen innerhalb einer Methode visuell verfolgen können. Die V3080- Diagnose ist jedoch viel effizienter und kann solche Fehler für Methodenaufrufketten finden. Vor nicht allzu langer Zeit haben wir den Datenfluss und die interprozeduralen Analysemechanismen erheblich verbessert. Sie können dies im Artikel " Nullable Referenztypen in C # 8.0 und statische Analyse " lesen. In WinForms wird jedoch ein solcher Fehler festgestellt:

PVS-Studio: V3080 Mögliche Null-Dereferenzierung innerhalb der Methode bei 'reader.NameTable'. Überprüfen Sie das erste Argument: contentReader. ResXResourceReader.cs 267

 private void EnsureResData() { .... XmlTextReader contentReader = null; try { if (fileContents != null) { contentReader = new XmlTextReader(....); } else if (reader != null) { contentReader = new XmlTextReader(....); } else if (fileName != null || stream != null) { .... contentReader = new XmlTextReader(....); } SetupNameTable(contentReader); // <= .... } finally { .... } .... } 

Schauen Sie, was mit der Variablen contentReader im Methodenkörper passiert. Nach der Initialisierung mit null wird es bei einer der Prüfungen erneut initialisiert. Die Reihe der Prüfungen endet jedoch nicht mit dem else- Block. Dies bedeutet, dass in einigen seltenen Fällen (oder aufgrund zukünftiger Umgestaltungen) die Referenz möglicherweise immer noch null bleibt. Anschließend wird es an die SetupNameTable- Methode übergeben, wo es ohne Überprüfung verwendet wird:

 private void SetupNameTable(XmlReader reader) { reader.NameTable.Add(ResXResourceWriter.TypeStr); reader.NameTable.Add(ResXResourceWriter.NameStr); .... } 

Dies ist möglicherweise unsicherer Code.

Und noch ein Fehler, bei dem der Analysator die Anrufkette durchlaufen musste, um das Problem zu erkennen:

PVS-Studio: V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie das Layout. DockAndAnchorLayout.cs 156

 private static Rectangle GetAnchorDestination( IArrangedElement element, Rectangle displayRect, bool measureOnly) { .... AnchorInfo layout = GetAnchorInfo(element); int left = layout.Left + displayRect.X; .... } 

Der Analysator behauptet, dass es möglich ist, eine Nullreferenz von der GetAnchorInfo- Methode abzurufen , was bei der Berechnung des linken Werts eine Ausnahme verursacht. Lassen Sie uns die gesamte Anrufkette durchgehen und prüfen, ob sie wahr ist:

 private static AnchorInfo GetAnchorInfo(IArrangedElement element) { return (AnchorInfo)element.Properties.GetObject(s_layoutInfoProperty); } public object GetObject(int key) => GetObject(key, out _); public object GetObject(int key, out bool found) { short keyIndex = SplitKey(key, out short element); if (!LocateObjectEntry(keyIndex, out int index)) { found = false; return null; } // We have found the relevant entry. See if // the bitmask indicates the value is used. if (((1 << element) & s_objEntries[index].Mask) == 0) { found = false; return null; } found = true; switch (element) { case 0: return s_objEntries[index].Value1; .... default: Debug.Fail("Invalid element obtained from LocateObjectEntry"); return null; } } 

In einigen Fällen gibt die GetObject- Methode, die die Aufrufkette beendet, null zurück , die ohne zusätzliche Überprüfungen an die Aufrufermethode übergeben wird. Wahrscheinlich ist es notwendig, eine solche Situation in der GetAnchorDestination- Methode abzudecken .

Es gibt ziemlich viele solcher Fehler im WinForms-Code, mehr als 70 . Sie sehen alle gleich aus und ich werde sie im Artikel nicht beschreiben.

PVS-Studio: V3091 Empirische Analyse. Möglicherweise ist im Zeichenfolgenliteral ein Tippfehler vorhanden: "ShowCheckMargin". Das Wort 'ShowCheckMargin' ist verdächtig. PropertyNames.cs 136

 internal class PropertyNames { .... public static readonly string ShowImageMargin = "ShowCheckMargin"; ... public static readonly string ShowCheckMargin = "ShowCheckMargin"; .... } 

Ein gutes Beispiel für einen Fehler, der nicht so leicht zu finden ist. Bei der Initialisierung der Klassenfelder wird derselbe Wert verwendet, obwohl der Autor des Codes dies offensichtlich nicht beabsichtigt hat (Kopieren und Einfügen ist schuld). Der Analysator kam zu diesem Schluss, indem er die Namen der Variablen und die Werte der zugewiesenen Zeichenfolgen verglich. Ich habe nur fehlerhafte Zeilen angegeben, aber Sie sollten überprüfen, wie es im Code-Editor aussieht:

Bild 2

Die Erkennung solcher Fehler zeigt die Leistungsfähigkeit und die unendliche Aufmerksamkeitsspanne statischer Analysewerkzeuge.

PVS-Studio: V3095 Das Objekt 'currentForm' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 3386, 3404. Application.cs 3386

 private void RunMessageLoopInner(int reason, ApplicationContext context) { .... hwndOwner = new HandleRef( null, UnsafeNativeMethods.GetWindowLong( new HandleRef(currentForm, currentForm.Handle), // <= NativeMethods.GWL_HWNDPARENT)); .... if (currentForm != null && ....) .... } 

Das ist klassisch. Die Variable currentForm wird ohne Prüfung verwendet. Aber dann wird im Code auf Null geprüft. In diesem Fall kann ich Ihnen raten, beim Arbeiten mit Referenztypen aufmerksamer zu sein und auch statische Analysegeräte zu verwenden :).

Noch ein solcher Fehler:

PVS-Studio: V3095 Das Objekt 'backgroundBrush' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 2331, 2334. DataGrid.cs 2331

 public Color BackgroundColor { .... set { .... if (!value.Equals(backgroundBrush.Color)) // <= { if (backgroundBrush != null && BackgroundBrush != DefaultBackgroundBrush) .... } } } 

Im WinForms-Code sind mehr als 60 solcher Fehler aufgetreten. Meiner Meinung nach sind alle eher kritisch und erfordern die Aufmerksamkeit der Entwickler. Aber es ist nicht mehr so ​​interessant, im Artikel darüber zu erzählen, deshalb werde ich mich auf die beiden oben genannten beschränken.

PVS-Studio: V3125 Das Objekt '_propInfo' wurde verwendet und in verschiedenen Ausführungszweigen gegen Null verifiziert. Überprüfen Sie die Zeilen: 996, 982. Binding.cs 996

 private void SetPropValue(object value) { .... if (....) { if .... else if (_propInfo != null) .... } else { _propInfo.SetValue(_control, value); } .... } 

Der Vollständigkeit halber - auch eine Art Klassiker, Fehler V3125 . Die gegenteilige Situation. Zunächst verwendet der Entwickler eine potenziell Null- Referenz sicher, nachdem er sie mit Null verglichen hat, hört jedoch auf, sie im Code weiter auszuführen.

Und noch ein solcher Fehler:

PVS-Studio: V3125 Das Objekt 'owner' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 64, 60. FlatButtonAppearance.cs 64

 public int BorderSize { .... set { .... if (owner != null && owner.ParentInternal != null) { LayoutTransaction.DoLayoutIf(....); } owner.Invalidate(); // <= .... } } 

Schön Dies ist jedoch der Standpunkt eines externen Forschers. Immerhin hat der Analysator neben diesen beiden V3125 mehr als 50 solcher Muster im WinForms-Code gefunden . Entwickler haben viel zu tun.

Und schließlich gibt es meiner Meinung nach einen interessanten Fehler.

PVS-Studio: V3137 Die Variable 'hCurrentFont' wird zugewiesen, aber am Ende der Funktion nicht verwendet. DeviceContext2.cs 241

 sealed partial class DeviceContext : .... { WindowsFont selectedFont; .... internal void DisposeFont(bool disposing) { if (disposing) { DeviceContexts.RemoveDeviceContext(this); } if (selectedFont != null && selectedFont.Hfont != IntPtr.Zero) { IntPtr hCurrentFont = IntUnsafeNativeMethods.GetCurrentObject( new HandleRef(this, hDC), IntNativeMethods.OBJ_FONT); if (hCurrentFont == selectedFont.Hfont) { // select initial font back in IntUnsafeNativeMethods.SelectObject(new HandleRef(this, Hdc), new HandleRef(null, hInitialFont)); hCurrentFont = hInitialFont; // <= } selectedFont.Dispose(disposing); selectedFont = null; } } .... } 

Mal sehen, was den Analysator alarmiert hat und warum er möglicherweise auf ein Problem hinweist, dass einer Variablen ein Wert zugewiesen wurde, der jedoch nie im Code verwendet wurde.

Die Datei DeviceContext2.cs enthält eine Teilklasse . Die DisposeFont- Methode wird verwendet, um Ressourcen nach der Arbeit mit Grafiken freizugeben : Gerätekontext und Schriftarten. Zum besseren Verständnis habe ich die gesamte DisposeFont- Methode angegeben. Achten Sie auf die lokale Variable hCurrentFont . Das Problem ist, dass die Deklaration dieser Variablen in der Methode das gleichnamige Klassenfeld verbirgt. Ich habe zwei Methoden der DeviceContext- Klasse gefunden, bei denen das Feld mit dem Namen hCurrentFont verwendet wird:

 public IntPtr SelectFont(WindowsFont font) { .... hCurrentFont = font.Hfont; .... } public void ResetFont() { .... hCurrentFont = hInitialFont; } 

Schauen Sie sich die ResetFont- Methode an. In der letzten Zeile steht genau das, was die DisposeFont- Methode im Unterblock tut, wenn (darauf zeigt der Analysator). Dieses gleichnamige hCurrentFont- Feld wird in einem anderen Teil der Teilklasse in der Datei DeviceContext.cs deklariert :

 sealed partial class DeviceContext : .... { .... IntPtr hInitialFont; .... IntPtr hCurrentFont; // <= .... } 

Somit wurde ein offensichtlicher Fehler gemacht. Eine andere Frage liegt in ihrer Bedeutung. Aufgrund der Arbeit der DisposeFont- Methode in dem Abschnitt mit dem Kommentar " Wählen Sie die ursprüngliche Schriftart wieder aus" wird das Feld " hCurrentFont " nicht initialisiert. Ich denke, nur die Autoren des Codes können ein genaues Urteil abgeben.

Schlussfolgerungen

Dieses Mal muss ich MS ein wenig kritisieren. In WinForms gibt es viele Fehler, die die Aufmerksamkeit der Entwickler erfordern. Vielleicht liegt es an der Eile, mit der MS an .NET Core 3 und Komponenten, einschließlich WinForms, arbeitet. Meiner Meinung nach ist der WinForms-Code immer noch "roh", aber ich hoffe, dass sich die Situation bald zum Besseren ändert.

Der zweite Grund für die große Anzahl von Fehlern kann sein, dass unser Analysator einfach besser darin geworden ist, nach ihnen zu suchen :).

Übrigens wird bald ein Artikel meines Kollegen Sergey Vasiliev veröffentlicht, in dem er viele Probleme im Code von .NET Core-Bibliotheken sucht und findet. Ich hoffe, dass seine Arbeit auch zur Verbesserung der Eigenschaften der .NET-Plattform beiträgt, da wir immer versuchen, die Entwickler über die Ergebnisse der Analyse ihrer Projekte zu informieren.

Und für diejenigen, die ihre Produkte selbst verbessern oder nach Fehlern in Projekten anderer suchen möchten, empfehle ich, PVS-Studio herunterzuladen und auszuprobieren .

Sauberer Code für alle!

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


All Articles