WinForms: Fehler, Holmes

Wir lieben es, in Microsoft-Projekten nach Fehlern zu suchen. Warum? Es ist ganz einfach: Ihre Projekte sind normalerweise leicht zu überprüfen (die Arbeit kann sofort in der Visual Studio-Umgebung ausgeführt werden, für die PVS-Studio über ein praktisches Plug-In verfügt) und sie enthalten nur wenige Fehler. Daher lautet der übliche Betriebsalgorithmus wie folgt: Suchen und Herunterladen eines offenen Projekts von MS; schau es dir an; wähle interessante Fehler; Stellen Sie sicher, dass es nur wenige sind. Schreiben Sie einen Artikel, ohne zu vergessen, die Entwickler zu loben. Großartig! Win-Win-Win: Es hat ein wenig gedauert, das Handbuch freut sich, neue Materialien auf dem Blog zu sehen, und das Karma ist in perfekter Ordnung. Aber diesmal ging etwas schief. Mal sehen, was im Quellcode von Windows Forms interessant war und ob Microsoft diesmal gelobt werden sollte.

Einführung

Anfang Dezember 2018 kündigte Microsoft die Veröffentlichung von .NET Core 3 Preview 1 an. Etwas früher (ungefähr ab Mitte Oktober) begann GitHub aktiv mit der Veröffentlichung des Quellcodes für Windows Forms , der .NET Core UI-Plattform zum Erstellen von Windows-Desktopanwendungen. 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. Überprüfung verursachte keine Schwierigkeiten. Erforderlich: Visual Studio 2019, .NET Core 3.0 SDK-Vorschau, PVS-Studio. Und jetzt wird das Warnprotokoll des Analysators empfangen.

Nachdem ich das PVS-Studio-Protokoll erhalten habe, sortiere ich es normalerweise in aufsteigender Reihenfolge der Diagnosenummern (das Fenster mit dem PVS-Studio-Nachrichtenprotokoll in Visual Studio bietet verschiedene Optionen zum Sortieren und Filtern der Liste). Auf diese Weise können Sie mit Fehlergruppen desselben Typs arbeiten, was die Analyse des Quellcodes erheblich vereinfacht. Ich markiere interessante Fehler in der Liste mit einem Sternchen und erst dann, nachdem ich das gesamte Protokoll analysiert habe, schreibe ich Codefragmente aus und mache eine Beschreibung. Und da es normalerweise nur wenige Fehler gibt, vermische ich sie und versuche, die interessantesten am Anfang und Ende des Artikels zu platzieren. Aber diesmal stellte sich heraus, dass die Fehler ein bisschen groß waren (eh, es war nicht möglich, die Intrige für eine lange Zeit zu retten), und ich werde sie in der Reihenfolge der Diagnosenummern bringen.

Was wurde gefunden? 833 Warnungen des hohen und mittleren Konfidenzniveaus (249 bzw. 584) wurden für 540.000 Codezeilen (ausgenommen leere) in 1670 cs-Dateien ausgegeben. Und ja, der Tradition nach habe ich die Tests nicht überprüft und die Warnungen des niedrigen Konfidenzniveaus (215 wurden ausgegeben) nicht berücksichtigt. Nach meinen früheren Beobachtungen gibt es zu viele Warnungen von MS für das Projekt. Aber nicht alle Warnungen sind Fehler.

Für dieses Projekt betrug die Anzahl der falsch positiven Ergebnisse etwa 30%. In etwa 20% der Fälle konnte ich einfach keine genaue Schlussfolgerung darüber ziehen, ob dies ein Fehler ist oder nicht, da ich mit dem Code nicht gut vertraut bin. Nun, mindestens 20% der Fehler, die ich verpasst habe, sind auf den menschlichen Faktor zurückzuführen: Eile, Müdigkeit usw. Der gegenteilige Effekt ist auch möglich: Übrigens habe ich einige der gleichen Arten von Auslösungen durchgesehen, deren Anzahl 70-80 erreichen könnte, durch eine, die gelegentlich die Anzahl der Fehler erhöhen könnte, die ich für real hielt.

In jedem Fall weisen 30% der Warnungen auf echte Fehler hin, was einen sehr großen Prozentsatz darstellt, da der Analysator nicht vorkonfiguriert wurde.

Die Anzahl der Fehler, die ich feststellen konnte, betrug ungefähr 240, was innerhalb der Grenzen der Statistik liegt. Ich wiederhole meiner Meinung nach, dass dies für ein Projekt von MS nicht das herausragendste Ergebnis ist (obwohl dies nur 0,44 Fehler pro 1000 Codezeilen sind), und es gibt wahrscheinlich mehr echte Fehler im WinForms-Code. Ich schlage vor, am Ende des Artikels über die Gründe zu sprechen, aber jetzt sehen wir uns die interessantesten Fehler an.

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

In if- und else if- Blöcken wird dieselbe Bedingung überprüft. Es sieht aus wie Kopieren und Einfügen. Ist das ein Fehler? Wenn Sie sich die Deklaration der IsHighContrastHighlighted- Methode ansehen , gibt es Zweifel:

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

Die Methode kann wahrscheinlich bei aufeinanderfolgenden Aufrufen unterschiedliche Werte zurückgeben. Und was in der aufrufenden Methode gemacht wird, sieht natürlich seltsam aus, hat aber das Recht auf Leben. Ich würde den Autoren jedoch raten, sich diesen Code anzusehen. Nur für den Fall. Dies ist jedoch 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 ist der Fehler beim Kopieren und Einfügen definitiv gemacht. Unabhängig von der Bedingung erhält die Variable selCharOffset immer den gleichen Wert.

Es gab zwei weitere ähnliche 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 einen Code, in dem zwei verschiedene Werte nacheinander auf denselben Schlüssel geschrieben werden. Und alles wäre in Ordnung, aber es gab 16 weitere solcher Stellen in dieser Methode. Dies sieht nicht mehr wie ein einzelner Fehler aus. Aber warum das tun, für mich bleibt es ein Rätsel. Ich habe keine Anzeichen von automatisch generiertem Code gefunden. Im Editor sieht es so aus:

Ich werde die ersten zehn Operationen mit einer Liste geben:

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

Die return-Anweisung wird niemals ausgeführt. Höchstwahrscheinlich ist die Bedingung myGridTable! = Null im externen if- Block, der später während des Refactorings hinzugefügt wurde. Und jetzt ist es sinnlos, myGridTable == null zu überprüfen. Entfernen Sie diese Prüfung, um die Qualität des Codes zu verbessern.

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 gab sofort zwei Warnungen für die Vergleichsmethode aus . Was ist das Problem? Es ist so, dass die Werte von cscLeft und cscRight nicht auf Gleichheit null geprüft werden. Sie können diesen Wert nach erfolglosem Casting in den Typ OrderedCodeStatementCollection erhalten . Dann wird in der letzten return-Anweisung eine Ausnahme ausgelöst. Eine solche Situation ist möglich, wenn alle Prüfungen für links und rechts bestanden werden und nicht zu einem vorläufigen Verlassen der Methode führen.

Um den Code zu korrigieren, sollte cscLeft / cscRight überall anstelle von links / rechts verwendet werden .

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 vielmehr auf den Code "mit Geruch". Hier gibt es keinen Fehler. Es stellen sich jedoch Fragen zur Organisation des Foreach- Zyklus. Warum wird es hier benötigt - es ist klar: aufgrund der Notwendigkeit, die Elemente der Sammlung zu extrahieren, die als ICollection übergeben wurden . Aber warum war in der Schleife, die ursprünglich für eine einzelne Iteration ausgelegt war (Voraussetzung ist das Vorhandensein eines einzelnen Elements in der Komponentensammlung ), ein zusätzliches Sicherheitsnetz in Form einer Unterbrechung erforderlich? Wahrscheinlich kann die Antwort in Betracht gezogen werden: "Es hat sich historisch entwickelt." 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 wurde in diesem Fragment ein "toter Code" gebildet. 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 auf die Nullungleichung überprüft. Hallo, NullReferenceException .

Ein weiterer ähnlicher 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) { .... } .... } 

Eine Situation ähnlich der vorherigen, nur mit der Variablen dropDownItem . Ich denke, dass solche Fehler auf Unaufmerksamkeit beim Refactoring zurückzuführen sind. Wahrscheinlich ein Teil der Bedingung ! (DropDownItem.Owner ist ToolStripDropDownMenu) wurde später zum Code hinzugefü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 zusätzliche Prüfung durchgeführt, die die Logik der Arbeit nicht beeinflusst. Und manchmal tun sie dies sogar, wenn Sie den Status einer visuellen Komponente erneut überprüfen müssen, indem Sie beispielsweise die Anzahl der Einträge in der Liste abrufen. Nur in diesem Fall überprüfen sie die lokale Variable columnCount . Das ist sehr verdächtig. Entweder wollten sie eine andere Variable überprüfen, oder bei einer der Überprüfungen verwenden sie die falsche Bedingung.

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

Unverständlicher Fehler. Ja, der Parameter lprcClipRect wird wirklich mit einem neuen Wert initialisiert, ohne ihn in irgendeiner Weise zu verwenden. Aber was bringt das? Ich denke, dass irgendwo im aufrufenden Code der durch diesen Parameter übergebene Link unverändert bleibt, obwohl er anders gedacht war. Schauen Sie sich in der Tat die Arbeit mit anderen Variablen in dieser Methode an. Sogar der Name (das Präfix „Get“) weist darauf hin, dass innerhalb der Methode über die übergebenen Parameter eine Initialisierung vorgenommen wird. Und so ist es auch. Die ersten beiden Parameter ( ppFrame und ppDoc ) werden mit dem Modifikator out übergeben und erhalten neue Werte. Die Links lprcPosRect und lpFrameInfo werden verwendet, um auf die Felder der Klasse zuzugreifen und sie zu initialisieren. Und nur lprcClipRect wird aus der allgemeinen Liste entfernt. Höchstwahrscheinlich 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, häufig werden einige Argumente absichtlich in umgekehrter Reihenfolge übergeben, um beispielsweise einige Variablen auszutauschen. Aber ich denke nicht, dass dies genau der Fall ist. Nichts in den aufrufenden oder aufgerufenen Methoden sagt ein solches Verwendungsmuster aus. Erstens werden Variablen vom Typ Bool verwirrt. Zweitens sind auch die Namen der Methoden gebräuchlich: kein "Swap" oder "Reverse". Darüber hinaus ist es nicht so schwierig, einen solchen Fehler zu machen. Menschen nehmen die Reihenfolge des Zeilen- / Spaltenpaars oft unterschiedlich wahr. Für mich ist zum Beispiel nur "Zeile / Spalte" bekannt. Für den Autor der aufgerufenen AdjustCellBorderStyle- Methode ist die bekanntere 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); .... } 

Ein seltener Fehler. Die Reihenfolge der Initialisierung der Klassenfelder ist verwechselt. Verwenden Sie zum Berechnen des Werts des Felds LOCALE_USER_DEFAULT das Feld LANG_USER_DEFAULT , 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 war nicht zu faul und schrieb ein kleines Konsolenprogramm, das die Situation simuliert. Anstelle der Werte einiger Konstanten, die im WinForms-Code verwendet werden, habe ich 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); } } 

Als Ergebnis des Starts wird auf der Konsole Folgendes angezeigt: 0. Jetzt tauschen wir die Deklaration der Felder LOCALE_USER_DEFAULT und LANG_USER_DEFAULT aus . Das Ergebnis des Programms in dieser Form: 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 "fallen" sollte, ist stabil genug, da Sie nur dann in den else- Zweig gelangen können, wenn die ces- Referenz 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 haben sie den Test verwechselt, indem sie if (comboBox! = Null) anstelle von if (comboBox == null) geschrieben haben . Und so erhalten wir die nächste NullReferenceException .

Wir haben zwei ziemlich offensichtliche Fehler V3080 untersucht , bei denen Sie die Situation der möglichen Verwendung von Nullreferenzen innerhalb der Methode visuell verfolgen können. Die V3080- Diagnose ist jedoch viel intelligenter und kann nach ähnlichen Fehlern für Ketten von Methodenaufrufen suchen. Vor nicht allzu langer Zeit haben wir die Mechanismen des Datenflusses und der interprozeduralen Analyse erheblich gestärkt. Sie können dies im Artikel " Nullable Reference Types in C # 8.0 und Static Analysis " lesen. Und hier ist ein ähnlicher Fehler in WinForms:

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

Sehen Sie sich an, was mit der Variablen contentReader im Hauptteil der Methode passiert. Nach der Initialisierung mit einer Nullreferenz wird der Link als Ergebnis einer der Überprüfungen initialisiert. Eine Reihe von Überprüfungen endet jedoch nicht mit einem else- Block. Dies bedeutet, dass in einigen seltenen Fällen (oder aufgrund zukünftiger Umgestaltungen) die Verknüpfung immer noch Null bleiben kann. Als nächstes 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 eine Reihe von Aufrufen durchlaufen musste, um ein Problem zu identifizieren:

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 , die bei der Berechnung des linken Werts eine Ausnahme auslöst . Lassen Sie uns die gesamte Anrufkette durchgehen und prüfen, ob dies der Fall 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 schließt, null zurück , die ohne zusätzliche Überprüfungen an die aufrufende Methode übergeben wird. Wahrscheinlich sollte die GetAnchorDestination- Methode eine solche Situation vorsehen.

Im WinForms-Code gab es einige solcher Fehler, mehr als 70 . Sie sind alle ähnlich und ich werde ihre Beschreibung im Artikel nicht geben.

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 erkennen ist. Beim Initialisieren von Klassenfeldern verwenden sie denselben Wert, obwohl der Autor des Codes dies offensichtlich nicht gedacht 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 die Fehlerzeilen angegeben, aber sehen Sie sich an, wie es im Code-Editor aussieht:

Es ist die Erkennung solcher Fehler, die die Leistungsfähigkeit und unendliche Sorgfalt statischer Analysewerkzeuge demonstriert.

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

Klassisch Die Variable currentForm wird ohne Prüfung verwendet. Aber weiter im Code gibt es seine Prüfung auf Nullgleichheit . In diesem Fall kann ich Ihnen raten, beim Arbeiten mit Referenztypen vorsichtiger zu sein und statische Analysegeräte zu verwenden :).

Ein weiterer ähnlicher 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 sehr kritisch und erfordern die Aufmerksamkeit der Entwickler. Aber in dem Artikel ist es nicht so interessant, darüber zu sprechen, deshalb beschränke ich mich auf die beiden oben diskutierten.

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

Um das Bild zu vervollständigen - auch eine Art Klassiker, Bug V3125 . Die umgekehrte Situation. Zuerst wird eine potenziell Null-Referenz sicher verwendet, um nach Null zu suchen , aber dann tun sie dies im Code nicht mehr.

Und noch ein ähnlicher 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önheit Dies ist jedoch aus der Sicht eines externen Forschers. Zusätzlich zu diesen beiden V3125 fand der Analysator mehr als 50 ähnliche Muster im WinForms-Code. Entwickler haben Arbeit zu erledigen.

Und schließlich - meiner Meinung nach ein ziemlich interessanter 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 der Analysator alarmiert hat und warum die Tatsache, dass einer Variablen ein Wert zugewiesen wurde, dieser jedoch in der Methode nicht weiter verwendet wird, auf ein Problem hinweisen kann.

Eine Teilklasse wird in der Datei DeviceContext2.cs deklariert . 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 bereitgestellt . Achten Sie auf die lokale Variable hCurrentFont . Das Problem ist, dass durch das Deklarieren dieser Variablen in einer Methode das gleichnamige Klassenfeld ausgeblendet wird. Ich habe zwei Methoden der DeviceContext- Klasse gefunden, bei denen ein Feld namens hCurrentFont verwendet wird :

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

Schauen Sie sich die ResetFont- Methode an. Die letzte Zeile dort ist genau das, was die DisposeFont- Methode im verschachtelten if- Block tut (der Analysator zeigt auf diese Stelle). Und dieses gleichnamige hCurrentFont- Feld wurde 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 ist seine Kritikalität. Aufgrund der DisposeFont- Methode, die in dem mit dem Kommentar „ Wählen Sie die ursprüngliche Schriftart wieder ein“ gekennzeichneten Abschnitt arbeitet , wird dem Feld hCurrentFont kein Anfangswert zugewiesen. Ich denke, dass nur Code-Autoren ein genaues Urteil abgeben können.

Schlussfolgerungen

Also muss ich diesmal MS ein bisschen "schelten". In WinForms gab es viele Fehler, die die Aufmerksamkeit der Entwickler erfordern. Möglicherweise liegt dies 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 "feucht", aber ich hoffe, dass sich die Situation bald zum Besseren ändert.

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

Übrigens wird in Kürze ein Artikel meines Kollegen Sergey Vasiliev veröffentlicht, in dem er einige Probleme im Code von .NET Core-Bibliotheken sucht und findet. Ich hoffe, dass seine Arbeit auch zur Verbesserung der Leistung der .NET-Plattform beiträgt, da wir immer versuchen, den Entwicklern die Ergebnisse der Analyse ihrer Projekte mitzuteilen.

Für diejenigen, die ihre Produkte selbst verbessern oder Nachforschungen anstellen möchten, um Fehler in den Projekten anderer Leute zu finden, empfehle ich, PVS-Studio herunterzuladen und auszuprobieren.

Alles sauberer Code!

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Sergey Khrenov. WinForms: Fehler, Holmes