WinForms: أخطاء ، هولمز

الصورة 5

نحن نحب البحث عن الأخطاء في مشاريع مايكروسوفت. لماذا؟ الأمر بسيط: عادة ما يكون من السهل التحقق من مشروعاتهم (يمكن القيام بالعمل على الفور في بيئة Visual Studio ، التي يحتوي عليها برنامج PVS-Studio مكونًا إضافيًا مناسبًا) ويحتوي على أخطاء قليلة. لذلك ، فإن خوارزمية العمل المعتادة هي كما يلي: البحث عن مشروع مفتوح وتنزيله من MS ؛ التحقق من ذلك. اختيار أخطاء مثيرة للاهتمام. تأكد من أنها قليلة ؛ كتابة مقال دون أن ننسى الثناء على المطورين. عظيم! الفوز - الفوز: استغرق الأمر بعض الوقت ، ويسر الدليل لرؤية مواد جديدة على المدونة ، والكرمة في حالة ممتازة. ولكن هذه المرة حدث خطأ ما. دعونا نرى ما هو مثير للاهتمام تم العثور عليه في التعليمات البرمجية المصدر لـ Windows Forms وما إذا كان ينبغي الإشادة بـ Microsoft هذه المرة.

مقدمة

في أوائل ديسمبر 2018 ، أعلنت Microsoft عن إصدار .NET Core 3 Preview 1. وقبل ذلك بقليل (تقريبًا من منتصف أكتوبر) ، بدأ GitHub العمل النشط على نشر التعليمات البرمجية المصدر لنظام Windows Forms ، نظام .NET Core UI لإنشاء تطبيقات سطح مكتب Windows. تستطيع أن ترى في إحصاءات الالتزام هنا . الآن يمكن لأي شخص تنزيل التعليمات البرمجية المصدر WinForms للمراجعة.

لقد قمت أيضًا بتنزيل المصادر للبحث عن الأخطاء هناك باستخدام PVS-Studio. تحقق لم يسبب صعوبات. مطلوب: Visual Studio 2019 ، .NET Core 3.0 SDK Preview ، PVS-Studio. والآن يتم تلقي سجل تحذير محلل.

بعد تلقي سجل PVS-Studio ، أقوم عادةً بترتيبه بترتيب تصاعدي للأرقام التشخيصية (تحتوي النافذة التي تحتوي على سجل رسائل PVS-Studio في Visual Studio على العديد من الخيارات لفرز القائمة وتصفيتها). يتيح لك ذلك العمل مع مجموعات من الأخطاء من نفس النوع ، مما يبسط إلى حد كبير تحليل الشفرة المصدرية. أقوم بوضع علامة على الأخطاء المهمة في القائمة باستخدام علامة النجمة ، وبعد ذلك فقط ، وبعد تحليل السجل بالكامل ، أكتب أجزاء التعليمات البرمجية وأعد وصفًا. ولأنه عادةً ما توجد أخطاء قليلة ، فإنني أخلطها ، وأحاول أن أضع أكثرها إثارة في بداية المقال ونهايته. لكن هذه المرة ، تبين أن الأخطاء كانت كبيرة بعض الشيء (أي ، لم يكن من الممكن حفظ المؤامرات لفترة طويلة) ، وسأحضرها حسب أرقام التشخيص.

ماذا وجد؟ تم إصدار 833 تحذيرات من مستوى الثقة العالية والمتوسطة (249 و 584 على التوالي) لـ 540،000 سطر من التعليمات البرمجية (باستثناء الخطوط الفارغة) في ملفات 1670 cs. ونعم ، وفقًا للتقاليد ، لم أفحص الاختبارات ولم أفكر في تحذيرات مستوى الثقة المنخفضة (تم إصدار 215). وفقًا لملاحظاتي السابقة ، هناك تحذيرات كثيرة للمشروع من MS. ولكن ليست كل التحذيرات أخطاء.

بالنسبة لهذا المشروع ، كان عدد الإيجابيات الخاطئة حوالي 30 ٪. في حوالي 20٪ من الحالات ، لم أستطع الوصول إلى نتيجة دقيقة حول ما إذا كان هذا خطأ أم لا ، حيث أنني لست على دراية جيدة بالكود. حسنًا ، 20٪ على الأقل من الأخطاء التي فاتني يمكن أن تعزى إلى العامل البشري: التسرع ، التعب ، إلخ. بالمناسبة ، فإن التأثير المعاكس ممكن أيضًا: نظرت إلى بعض من نفس النوع من الإثارة ، الذي يمكن أن يصل عددها إلى 70-80 ، من خلال واحدة يمكن أن تزيد أحيانًا من عدد الأخطاء التي اعتبرتها حقيقية.

في أي حال ، تشير 30٪ من التحذيرات إلى وجود أخطاء حقيقية ، وهي نسبة كبيرة جدًا ، بالنظر إلى أن المحلل لم يتم تكوينه مسبقًا.

لذلك ، كان عدد الأخطاء التي تمكنت من اكتشافها حوالي 240 ، وهو ما يقع ضمن حدود الإحصائيات. أكرر ، في رأيي ، لمشروع من MS هذه ليست النتيجة الأكثر بروزًا (على الرغم من أن هذا سيكون 0.44 فقط أخطاء لكل 1000 سطر من التعليمات البرمجية) ، وربما هناك أخطاء أكثر واقعية في رمز WinForms. أقترح التحدث عن الأسباب في نهاية المقال ، ولكن الآن دعونا نرى الأخطاء الأكثر إثارة للاهتمام.

أخطاء

PVS-Studio: V3003 استخدام النموذج "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط التحقق: 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 { .... } .... } 

في if و if if blocks ، يتم التحقق من نفس الحالة. يبدو مثل نسخ لصق. هل هذا خطأ؟ إذا نظرت إلى إعلان الأسلوب IsHighContrastHighlighted ، فهناك شك:

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

ربما ترجع الطريقة قيمًا مختلفة في المكالمات المتتالية. وما يجري في طريقة الاتصال يبدو غريباً ، بالطبع ، لكن له الحق في الحياة. ومع ذلك ، فإنني أنصح المؤلفين بإلقاء نظرة على هذه الشفرة. فقط في القضية. ومع ذلك ، هذا مثال جيد على مدى صعوبة استخلاص النتائج عند تحليل الشفرة غير المألوفة.

PVS-Studio: V3004 عبارة "then" مكافئة لبيان "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; // <= } .... } .... } 

وهنا يتم إجراء خطأ نسخ اللصق بالتأكيد. بغض النظر عن الشرط ، سيحصل متغير selCharOffset دائمًا على نفس القيمة.

كان هناك اثنين من الأخطاء مماثلة في رمز WinForms:
  • V3004 عبارة "ثم" مكافئة لبيان "آخر". SplitContainer.cs 1700
  • V3004 عبارة "ثم" مكافئة لبيان "آخر". ToolstripProfessionalRenderer.cs 371

PVS-Studio: V3008 يتم تعيين قيم المتغير مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 681 ، 680. ProfessionalColorTable.cs 681

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

الأسلوب بملء القاموس rgbTable . أشار المحلل إلى جزء من الشفرة حيث تتم كتابة قيمتين مختلفتين بالتتابع على نفس المفتاح. سيكون كل شيء على ما يرام ، ولكن كان هناك 16 مكانًا آخر في هذه الطريقة ، لم يعد هذا يبدو خطأً واحدًا. ولكن لماذا هذا ، بالنسبة لي لا يزال لغزا. لم أجد علامات على رمز التوليد التلقائي. في المحرر ، يبدو كما يلي:

الصورة 3

سأقدم العمليات العشر الأولى مع قائمة:

  1. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 785 ، 784. ProfessionalColorTable.cs 785
  2. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 787 ، 786. ProfessionalColorTable.cs 787
  3. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 789 ، 788. ProfessionalColorTable.cs 789
  4. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 791 ، 790. ProfessionalColorTable.cs 791
  5. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 797 ، 796. ProfessionalColorTable.cs 797
  6. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 799 ، 798. ProfessionalColorTable.cs 799
  7. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 807 ، 806. ProfessionalColorTable.cs 807
  8. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 815 ، 814. ProfessionalColorTable.cs 815
  9. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 817 ، 816. ProfessionalColorTable.cs 817
  10. V3008 يتم تخصيص القيم مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 823 ، 822. ProfessionalColorTable.cs 823

PVS-Studio: V3011 تمت مصادفة شرطين متعارضين . الشرط الثاني هو دائما كاذبة. خطوط التحقق: 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; } } 

لن يتم تنفيذ بيان الإرجاع . على الأرجح ، الشرط myGridTable! = لاغ في الخارج إذا تمت إضافة كتلة في وقت لاحق أثناء إعادة التجهيز. والآن التحقق من myGridTable == خالية من العبث. لتحسين جودة الكود ، أزل هذا الفحص.

PVS-Studio: V3019 ربما تتم مقارنة متغير غير صحيح بالقيمة الخالية بعد التحويل باستخدام كلمة "ككلمة". تحقق من المتغيرات "يسار" ، "cscLeft". TypeCodeDomSerializer.cs 611

PVS-Studio: V3019 ربما تتم مقارنة متغير غير صحيح بالقيمة الخالية بعد التحويل باستخدام كلمة "ككلمة". تحقق من المتغيرات "الصحيحة" و "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; // <= } 

أصدر المحلل على الفور تحذيرين لطريقة المقارنة . ما هي المشكلة؟ هو أن قيم cscLeft و cscRight لا يتم التحقق من وجود المساواة خالية . يمكنهم الحصول على هذه القيمة بعد صب غير ناجحة إلى النوع OrderedCodeStatementCollection . ثم سيتم طرح استثناء في بيان الإرجاع الأخير. يكون مثل هذا الموقف ممكنًا عندما تتحقق جميع عمليات المرور من اليسار واليمين ولا تؤدي إلى خروج أولي من الطريقة.

لإصلاح الكود ، يجب استخدام cscLeft / cscRight في كل مكان بدلاً من اليسار / اليمين .

PVS-Studio: V3020 "استراحة" غير مشروطة داخل حلقة. 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; } } .... } 

تشير هذه القطعة ، بدلاً من ذلك ، إلى رمز "مع الرائحة". لا يوجد خطأ هنا. لكن الأسئلة تثور على الطريقة التي يتم بها تنظيم دورة التوقع . لماذا هو مطلوب هنا - فمن الواضح: بسبب الحاجة إلى استخراج عناصر المجموعة ، مرت على أنها ICollection . ولكن لماذا في الحلقة ، المصممة أصلاً لتكرار واحد (شرط مسبق هو وجود عنصر واحد في مجموعة المكونات ) ، كانت هناك حاجة إلى شبكة أمان إضافية في شكل فاصل ؟ ربما ، يمكن اعتبار الإجابة: "لقد تطورت تاريخيا". رمز تبدو قبيحة.

PVS-Studio: V3022 Expression 'ocxState! = Null' صحيح دائمًا. 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; } .... } } 

بسبب خطأ منطقي ، تم تشكيل "رمز ميت" في هذه القطعة. لن يتم تنفيذ التعبيرات في الكتلة الأخرى .

PVS-Studio: V3027 تم استخدام المتغير 'e' في التعبير المنطقي قبل أن يتم التحقق منه مقابل لاغٍ في التعبير المنطقي نفسه. ImageEditor.cs 99

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

يتم استخدام المتغير e في الشرط لأول مرة ، ثم يتم التحقق من عدم المساواة الفارغة . مرحبًا ، NullReferenceException .

خطأ مشابه آخر:

PVS-Studio: V3027 تم استخدام المتغير 'dropDownItem' في التعبير المنطقي قبل أن يتم التحقق منه مقابل لاغٍ في نفس التعبير المنطقي. 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) { .... } .... } 

موقف مشابه للحالة السابقة ، فقط مع المتغير dropDownItem . أعتقد أن مثل هذه الأخطاء تظهر نتيجة عدم الانتباه أثناء إعادة التوطين. ربما تمت إضافة جزء من الشرط (DropDownItem.Owner هو ToolStripDropDownMenu) إلى الرمز لاحقًا.

PVS-Studio: V3030 الاختيار المتكرر. تم التحقق بالفعل من شرط "columnCount> 0" في السطر 3900. 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); } .... } .... } 

خطأ قد يبدو غير ضار. في الواقع ، حسنا ، يتم إجراء فحص إضافي ، والذي لا يؤثر على منطق العمل. وأحيانًا يقومون بذلك عندما تحتاج إلى إعادة التحقق من حالة المكون المرئي ، على سبيل المثال ، من خلال الحصول على عدد الإدخالات في القائمة. في هذه الحالة فقط ، يقومون بالتحقق من العمود المتغير المحلي . هذا مشبوه جدا. إما أنهم يريدون التحقق من متغير آخر ، أو في أحد الاختبارات ، يستخدمون الشرط الخاطئ.

PVS-Studio: تتم إعادة كتابة المعلمة V3061 "lprcClipRect" دائمًا في نصوص الأسلوب قبل استخدامها. 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; } 

خطأ غير واضح. نعم ، يتم بالفعل تهيئة المعلمة lprcClipRect بقيمة جديدة ، دون استخدامها بأي طريقة. لكن ما الذي ينتج عنه هذا؟ أعتقد أنه في مكان ما في رمز الاتصال ، سيظل الارتباط الذي تم تمريره من خلال هذه المعلمة بدون تغيير ، على الرغم من أنه كان مخصصًا بشكل مختلف. في الواقع ، انظر إلى العمل مع المتغيرات الأخرى في هذه الطريقة. حتى اسمها (بادئة "Get") تشير إلى أن بعض التهيئة ستتم داخل الطريقة من خلال المعلمات التي تم تمريرها. وهذا هو الحال كذلك. يتم تمرير المعلمتين الأولين ( ppFrame و ppDoc ) مع معدل الخروج والحصول على قيم جديدة. يتم استخدام الارتباطات lprcPosRect و lpFrameInfo للوصول إلى حقول الفئة وتهيئتها . وفقط lprcClipRect يخرج من القائمة العامة. على الأرجح ، مطلوب معدل الخروج أو المرجع لهذه المعلمة.

PVS-Studio: V3066 تم تمرير ترتيب غير صحيح للوسيطات إلى الأسلوب "AdjustCellBorderStyle": 'isFirstDisplayedRow' و 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

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

محلل يشتبه في أن الحجج الأخيرة كانت مختلطة. دعونا نلقي نظرة على إعلان طريقة AdjustCellBorderStyle :

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

يشبه الخطأ. نعم ، غالبًا ما يتم تمرير بعض الوسيطات بترتيب عكسي ، على سبيل المثال ، من أجل مبادلة بعض المتغيرات. لكنني لا أعتقد أن هذا هو الحال بالضبط. لا يوجد شيء في أساليب الاتصال أو الاتصال يقول مثل هذا النمط من الاستخدام. أولاً ، يتم خلط متغيرات نوع منطقي . ثانياً ، أسماء الطرق شائعة أيضًا: لا يوجد "تبديل" أو "عكس". علاوة على ذلك ، ليس من الصعب للغاية ارتكاب خطأ كهذا. غالبًا ما ينظر الأشخاص إلى ترتيب زوج الصف / العمود بشكل مختلف. بالنسبة لي ، على سبيل المثال ، مجرد "صف / عمود" مألوف. لكن بالنسبة إلى مؤلف طريقة AdjustCellBorderStyle التي يطلق عليها ، من الواضح أن الترتيب الأكثر دراية هو "العمود / الصف".

PVS-Studio: V3070 متغير غير مهيأ 'LANG_USER_DEFAULT' يستخدم عند تهيئة المتغير 'LOCALE_USER_DEFAULT'. 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); .... } 

خطأ نادر. يتم ترتيب ترتيب تهيئة حقول الفصل. لحساب قيمة الحقل LOCALE_USER_DEFAULT ، استخدم الحقل LANG_USER_DEFAULT ، والذي لم تتم تهيئته بعد وله قيمة 0. بالمناسبة ، لا يتم استخدام متغير LANG_USER_DEFAULT في أي مكان آخر في التعليمات البرمجية. لم أكن كسولًا جدًا وكتبت برنامج وحدة تحكم صغير يحاكي الموقف. بدلاً من قيم بعض الثوابت المستخدمة في رمز WinForms ، استبدلت قيمها الفعلية:

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

نتيجة للإطلاق ، سيتم عرض ما يلي على وحدة التحكم: 0. نبدل الآن إعلان حقلي LOCALE_USER_DEFAULT و LANG_USER_DEFAULT . نتيجة البرنامج في هذا النموذج: 1024. أعتقد أنه لا يوجد شيء آخر للتعليق عليه هنا.

PVS-Studio: V3080 ممكن dereference فارغة. النظر في تفتيش "المجال الاقتصادي الموحد". CodeDomSerializerBase.cs 562

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

الكود الذي يجب أن "يسقط" يكون مستقرًا بدرجة كافية ، لأنه يمكنك الدخول إلى الفرع الآخر عندما تكون الإشارة ces فارغة .

مثال آخر مماثل:

PVS-Studio: V3080 ممكن dereference فارغة. النظر في تفتيش "comboBox". ComboBox.cs 6610

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

الكود المتناقض. على ما يبدو ، اختلطوا الاختبار عن طريق الكتابة إذا (comboBox! = Null) بدلاً من if (comboBox == null) . وهكذا - سوف نتلقى NullReferenceException التالي.

درسنا خطأين واضحين إلى حد ما V3080 ، حيث يمكنك بصريًا تتبع حالة الاستخدام المحتمل للمراجع الفارغة في هذه الطريقة. لكن تشخيصات V3080 أكثر ذكاءً ويمكنها البحث عن أخطاء مماثلة لسلاسل استدعاءات الطريقة. منذ وقت ليس ببعيد ، قمنا بتعزيز آليات تحليل تدفق البيانات وتحليلها بشكل كبير. يمكنك أن تقرأ عن ذلك في المقالة " أنواع المرجع Nullable في C # 8.0 والتحليل الثابت ". وإليك خطأ مشابه موجود في WinForms:

PVS-Studio: V3080 ممكن dereference فارغة داخل الأسلوب في "reader.NameTable". النظر في فحص الوسيطة الأولى: 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 { .... } .... } 

انظر إلى ما يحدث مع contentReader المتغير في نص الأسلوب. بعد التهيئة بمرجع لاغٍ ، نتيجة لإحدى عمليات الفحص ، سيتم تهيئة الرابط. ومع ذلك ، لا تنتهي سلسلة من الاختبارات مع كتلة أخرى. هذا يعني أنه في بعض الحالات النادرة (أو بسبب إعادة البناء في المستقبل) ، يمكن أن يظل الارتباط صفراً. بعد ذلك ، سيتم تمريره إلى أسلوب SetupNameTable ، حيث يتم استخدامه دون أي تحقق:

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

هذا رمز يحتمل أن يكون غير آمن.

وخطأ آخر ، حيث كان على المحلل أن يمر عبر سلسلة من المكالمات لتحديد المشكلة:

PVS-Studio: V3080 ممكن dereference فارغة. النظر في تفتيش "تخطيط". DockAndAnchorLayout.cs 156

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

يدعي المحلل أنه من الممكن الحصول على مرجع خالي من أسلوب GetAnchorInfo ، والذي سوف يلقي استثناءً عند حساب القيمة اليسرى . دعنا نذهب عبر سلسلة كاملة من المكالمات وتحقق مما إذا كان هذا هو الحال:

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

في الواقع ، في بعض الحالات ، سيعود أسلوب GetObject الذي يغلق سلسلة الاتصال بإلغاء ، والتي دون أي اختبارات إضافية سيتم تمريرها إلى طريقة الاستدعاء. ربما ، يجب أن يوفر أسلوب GetAnchorDestination لمثل هذا الموقف.

في كود WinForms ، كان هناك عدد غير قليل من هذه الأخطاء ، أكثر من 70 . إنها كلها متشابهة ولن أقدم وصفها في المقال.

PVS-Studio: V3091 التحليل التجريبي. من الممكن وجود خطأ مطبعي داخل السلسلة الحرفية: "ShowCheckMargin". كلمة "ShowCheckMargin" مشبوهة. PropertyNames.cs 136

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

مثال جيد على خطأ ليس من السهل اكتشافه. عند تهيئة حقول الفصل ، فإنها تستخدم نفس القيمة ، على الرغم من أن مؤلف الكود لا يعتقد ذلك بوضوح (نسخ اللوم هو المسؤول). قدم المحلل هذا الاستنتاج من خلال مقارنة أسماء المتغيرات وقيم الأوتار المعينة. لقد قدمت خطوط الخطأ فقط ، لكن انظر إلى كيفية ظهورها في محرر الشفرة:

الصورة 2

إن اكتشاف مثل هذه الأخطاء هو الذي يوضح قوة أدوات التحليل الثابتة ورعايتها.

PVS-Studio: V3095 تم استخدام كائن 'currentForm' قبل أن يتم التحقق منه ضد قيمة خالية. خطوط التحقق: 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 && ....) .... } 

الكلاسيكية. يستخدم المتغير الحالي دون أي اختبارات . ولكن أبعد من ذلك في الكود هناك التحقق من عدم المساواة. في هذه الحالة ، يمكنني أن أنصحك أن تكون أكثر حذراً عند العمل مع أنواع المراجع ، وكذلك استخدام أجهزة التحليل الثابتة :).

خطأ مشابه آخر:

PVS-Studio: V3095 تم استخدام كائن 'backgroundBrush' قبل أن يتم التحقق منه ضد قيمة خالية. خطوط التحقق: 2331 ، 2334. DataGrid.cs 2331

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

في رمز WinForms ، واجهت أكثر من 60 مثل هذه الأخطاء. في رأيي ، كلها مهمة للغاية وتتطلب انتباه المطورين. لكن في المقال ، ليس من المثير للاهتمام التحدث معهم ، لذا سأقتصر على الاثنين اللذين تمت مناقشتهما أعلاه.

PVS-Studio: V3125 تم استخدام كائن '_propInfo' وتم التحقق منه ضد قيمة خالية في فروع التنفيذ المختلفة. خطوط التحقق: 996 ، 982. Binding.cs 996

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

لإكمال الصورة - أيضًا نوع من الأخطاء الكلاسيكية V3125 . الوضع العكسي. في البداية ، يتم استخدام مرجع يُحتمل أن يكون لاغياً بأمان ، والتحقق من أنه لاغٍ ، ولكنهم لم يفعلوا ذلك في الكود بعد الآن.

وخطأ مماثل آخر:

PVS-Studio: V3125 تم استخدام كائن "المالك" بعد أن تم التحقق منه مقابل لاغٍ. خطوط التحقق: 64 ، 60. FlatButtonAppearance.cs 64

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

الجمال. لكن هذا من وجهة نظر باحث خارجي. في الواقع ، بالإضافة إلى هذين V3125s ، وجد المحلل أكثر من 50 نمطًا مشابهًا في كود WinForms. المطورين لديهم عمل للقيام به.

وأخيرا - خطأ مثير للاهتمام إلى حد ما ، في رأيي.

PVS-Studio: V3137 يتم تعيين متغير "hCurrentFont" ولكن لا يتم استخدامه في نهاية الوظيفة. 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; } } .... } 

دعونا نرى ما نبهه المحلل ، ولماذا تشير حقيقة أن المتغير يتم تعيين قيمة ، ولكن لا يتم استخدامه في الطريقة بشكل أكبر ، قد يشير إلى وجود مشكلة.

يتم تعريف فئة جزئية في الملف DeviceContext2.cs . يتم استخدام الأسلوب DisposeFont لتحرير الموارد بعد العمل مع الرسومات: سياق الجهاز والخطوط. لفهم أفضل ، لقد قدمت طريقة DisposFont بأكملها. انتبه إلى المتغير المحلي hCurrentFont . المشكلة هي أن التصريح عن هذا المتغير بطريقة يخفي حقل الفئة الذي يحمل نفس الاسم. لقد وجدت طريقتين لفئة DeviceContext حيث يتم استخدام حقل يسمى hCurrentFont :

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

ألقِ نظرة على طريقة ResetFont . السطر الأخير هناك هو بالضبط ما تفعله طريقة DisposeFont في العنصر المتداخل إذا كانت الكتلة (يشير المحلل إلى هذا المكان). وتم الإعلان عن حقل hCurrentFont الذي يحمل نفس الاسم في جزء آخر من الفصل الجزئي في ملف DeviceContext.cs :

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

وبالتالي ، تم ارتكاب خطأ واضح. سؤال آخر هو أهميته. الآن ، كنتيجة لطريقة DisposFont التي تعمل في القسم المحدد بالتعليق "حدد الخط الأولي مرة أخرى" ، لن يتم تعيين حقل hCurrentFont بعض القيمة الأولية. أعتقد أن مؤلفي الشفرة هم وحدهم الذين يمكنهم إصدار حكم دقيق.

النتائج

لذلك ، هذه المرة لا بد لي من "تأنيب" MS قليلا. في WinForms ، كان هناك الكثير من الأخطاء التي تتطلب عن كثب اهتمام المطورين. ربما يكون هذا بسبب بعض الاندفاع الذي يعمل به MS على .NET Core 3 ومكوناته ، بما في ذلك WinForms. في رأيي ، لا يزال رمز WinForms "رطبًا" ، لكنني آمل أن يتغير الوضع قريبًا للأفضل.

السبب الثاني لعدد كبير من الأخطاء قد يكون أن محللنا أصبح أفضل للبحث عنها :).

بالمناسبة ، سيتم نشر مقال بقلم زميلي سيرجي فاسيلييف قريبًا يبحث فيه ويجد بعض المشكلات في كود مكتبات .NET Core. آمل أن يسهم عمله أيضًا في تحسين أداء النظام الأساسي .NET ، لأننا نحاول دائمًا توصيل نتائج تحليل مشاريعهم إلى المطورين.

حسنًا ، بالنسبة لأولئك الذين يرغبون في تحسين منتجاتهم بأنفسهم أو إجراء البحوث للعثور على أخطاء في مشاريع الآخرين ، أقترح تنزيل وتجربة PVS-Studio.

كل رمز نظيفة!


إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بترجمة: سيرجي خرينوف. WinForms: أخطاء ، هولمز

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


All Articles