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

الصورة 5

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

مقدمة

في أوائل ديسمبر 2018 ، أعلنت Microsoft عن إصدار .NET Core 3 Preview 1. قبل ذلك بقليل (حوالي منتصف أكتوبر) ، بدأ GitHub في الكشف بنشاط عن مصادر نماذج Windows - نظام .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 { .... } .... } 

إذا وآخر إذا كانت كتل تحقق نفس الحالة. يبدو مثل نسخ لصق. هل هو خطأ؟ إذا نظرت إلى إعلان الأسلوب 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; } } .... } 

تشير هذه القطعة إلى "رائحة الكود". لا يوجد خطأ هنا. ولكن تثور أسئلة حول الطريقة التي يتم بها تنظيم حلقة foreach . من الواضح سبب الحاجة إليها هنا: نظرًا للحاجة إلى استخراج عناصر المجموعة ، تم إقرارها كـ 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 فعليًا بقيمة جديدة دون استخدامها بأي طريقة. ولكن ما الذي يؤدي إليه في النهاية؟ أعتقد أنه في مكان ما في رمز الاتصال ، ستبقى الإشارة التي تم تمريرها عبر هذه المعلمة كما هي ، على الرغم من أنه لم يكن القصد منها أن تكون كذلك. حقا ، نقدر التعامل مع المتغيرات الأخرى في هذه الطريقة. حتى اسمه (بادئة "الحصول على") يلمح إلى أن بعض التهيئة سيتم تنفيذها داخل الأسلوب من خلال المعلمات التي تم تمريرها. وهو كذلك. يتم تمرير المعلمتين الأولين ( 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) { .... } 

يشبه الخطأ. نعم ، غالبًا ما يتم تمرير بعض الوسائط بترتيب عكسي ، على سبيل المثال ، لتبادل بعض المتغيرات. لكنني لا أعتقد أن هذا هو الحال. لا يوجد شيء في أساليب الاتصال أو المستدعي يشير إلى نمط الاستخدام هذا. أولاً ، يتم خلط متغيرات نوع bool . ثانياً ، تكون أسماء الطرق منتظمة أيضًا: لا يوجد "تبديل" أو "عكس". الى جانب ذلك ، ليس من الصعب جدا ارتكاب خطأ كهذا. غالبًا ما يرى الناس ترتيب زوج "الصف / العمود" بشكل مختلف. بالنسبة لي ، على سبيل المثال ، هو "الصف / العمود" هو مألوف. ولكن بالنسبة إلى مؤلف الأسلوب المسمى 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) مرتبكًا إذا (comboBox == null) . وهكذا ، سوف نحصل على NullReferenceException آخر .

لقد أخذنا في الاعتبار خطأين V3080 واضحين إلى حد ما حيث يمكنك بصريا تتبع استخدام مرجعي لاغٍ محتمل ضمن طريقة ما. لكن تشخيص V3080 أكثر فاعلية ويمكنه العثور على مثل هذه الأخطاء لسلاسل استدعاء الأسلوب. منذ وقت ليس ببعيد ، قمنا بتحسين آليات تحليل تدفق البيانات و interprocedural بشكل كبير. يمكنك أن تقرأ عن ذلك في المقالة " أنواع المرجع 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(); // <= .... } } 

جميل. ولكن هذا وجهة نظر الباحث الخارجي. بعد كل شيء ، وجد المحلل أكثر من 50 من هذه الأنماط في كود WinForms بجانب هذين V3125 . المطورين لديهم الكثير ليعملوا عليه.

وأخيرا ، هناك خطأ مثير للاهتمام ، في رأيي.

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 . السطر الأخير هناك هو بالضبط ما تفعله طريقة DisposFont في الحجرة الفرعية إذا (هذا ما يشير إليه المحلل). تم تعريف حقل hCurrentFont الذي يحمل نفس الاسم في جزء آخر من الفصل الجزئي في ملف DeviceContext.cs :

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

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

الاستنتاجات

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

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

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

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

كود نظيف للجميع!

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


All Articles