Docotic.Pdf: ما هي المشاكل التي يكتشفها PVS-Studio في مشروع ناضج؟

الجودة مهمة بالنسبة لنا. وسمعنا عن PVS-Studio. كل هذا أدى إلى الرغبة في التحقق من Docotic.Pdf ومعرفة ما يمكن تحسينه.

Docotic.Pdf هي مكتبة للأغراض العامة للعمل مع ملفات PDF. هو مكتوب في C # ، لا يوجد رمز غير آمن ، لا تبعيات خارجية بخلاف وقت تشغيل .NET. يعمل تحت .NET 4+ و تحت .NET Standard 2+.

المكتبة قيد التطوير لأكثر من 10 سنوات بقليل ولديها 110 ألف سطر من التعليمات البرمجية دون مراعاة الاختبارات والأمثلة وأشياء أخرى. للتحليل الثابت ، نستخدم باستمرار تحليل الكود و StyleCop. عدة آلاف من الاختبارات الآلية تحمينا من الانحدارات. يثق عملاؤنا من مختلف البلدان والصناعات المختلفة بجودة المكتبة.

ما هي المشاكل التي سيكتشفها PVS-Studio؟

التثبيت والانطباع الأول

لقد قمت بتنزيل النسخة التجريبية من موقع PVS-Studio. فوجئت بسرور من حجم صغير المثبت. مثبت مع الإعدادات الافتراضية: محركات التحليل ، بيئة PVS-Studio منفصلة ، الاندماج في Visual Studio 2017.

بعد التثبيت ، لم يبدأ أي شيء ، وتمت إضافة اختصارين بنفس الرموز إلى قائمة ابدأ: Standalone و PVS-Studio. للحظة ، فكرت ما يجب أن أبدأ. تم إطلاق Standalone وفوجئت بالواجهة غير السارة. مقياس 200٪ المعيّن لنظام Windows الخاص بي مدعوم بشكل منحرف. جزء من النص صغير جدًا ، وجزء من النص لا يتناسب مع المساحة المخصصة له. يتم اقتصاص الاسم و Unicorn وقائمة الإجراءات لأي حجم نافذة. حتى مع ملء الشاشة.



حسنًا ، قررت فتح ملف مشروعي. فجأة ، لم تجد قائمة ملف مثل هذه الفرصة. هناك عرضت فقط لفتح الملفات الفردية. شكرا لك ، فكرت ، أنا أفضل تجربة خيار آخر. تم إطلاق PVS-Studio - أظهروا لي نافذة بها نص ضبابي. مقياس 200 ٪ جعل نفسه يشعر مرة أخرى. أبلغ النص: ابحث عني في Three Crowns ، ابحث عن قائمة PVS-Studio في Visual Studio. حسنا ، افتتح الاستوديو.

حل مفتوح. في الواقع ، هناك قائمة PVS-Studio ، ولديها القدرة على التحقق من "المشروع الحالي". قام بعمل المشروع الذي أحتاجه وأطلق شيكًا. ظهرت نافذة في الاستوديو مع نتائج التحليل. ظهرت نافذة في الخلفية مع تقدم الفحص ، لكنني لم أجدها على الفور. في البداية كان هناك شعور بأن الشيك لم يبدأ أو انتهى على الفور.

نتيجة الاختيار الأول

قام المحلل بفحص جميع ملفات المشروع البالغ عددها 1253 في حوالي 9 دقائق و 30 ثانية. في نهاية الفحص ، لم يتغير عداد الملفات بالسرعة التي كانت عليه في البداية. ربما يكون هناك بعض الاعتماد غير الخطي على مدة المسح على عدد الملفات التي تم فحصها.

ظهرت معلومات حول 81 ارتفاع ، 109 متوسط ​​و 175 تحذير منخفض في نافذة النتائج. إذا قمت بحساب التردد ، فستحصل على 0.06 تحذيرات عالية / ملف ، 0.09 تحذيرات / ملف متوسط ​​، و 0.14 تحذيرات منخفضة / ملف. أو
0.74 تحذيرات عالية لكل ألف سطر من التعليمات البرمجية ، 0.99 تحذيرات متوسطة لكل ألف سطر من التعليمات البرمجية ، و 1.59 تحذيرات منخفضة لكل ألف سطر من التعليمات البرمجية.

هنا في هذه المقالة ، يشار إلى أنه في CruiseControl.NET ، مع 256 ألف سطر من التعليمات البرمجية ، وجد المحلل 15 تحذيرًا عاليًا و 151 متوسطًا و 32 منخفضًا.

اتضح أنه من حيث النسبة المئوية لـ Docotic.Pdf تم إصدار المزيد من التحذيرات في كل مجموعة.

ما هو موجود؟

قررت تجاهل التحذيرات المنخفضة في هذه المرحلة.

لقد قمت بفرز التحذيرات حسب عمود الرمز وتبين أن صاحب التسجيل المطلق للتردد كان V3022 "التعبير دائمًا صواب / خطأ" و V3063 "جزء من التعبير الشرطي دائمًا صواب / خطأ إذا تم تقييمه". في رأيي ، هم حول شيء واحد. في المجموع ، يعطي هذان التحذيران 92 حالة من أصل 190. التردد النسبي = 48٪.

منطق الانقسام إلى مرتفع ومتوسط ​​ليس واضحًا تمامًا. كنت أتوقع V3072 "الفئة" A "التي تحتوي على أعضاء IDisposable لا تقوم بنفسها بتطبيق IDisposable" و V3073 "لا يتم التخلص من جميع الأعضاء IDisposible بشكل صحيح. استدعاء "التخلص" عند التخلص من فئة "أ" في المجموعة عالية ، على سبيل المثال. لكن هذا طعم بالطبع.

فوجئت بأن V3095 "تم استخدام الكائن قبل التحقق من صحته. تحقق من الخطوط: N1 ، N2 "تم وضع علامة مرتين على أنها عالية ومرة ​​واحدة متوسطة. خلل؟

ثق ولكن تحقق

حان الوقت للتحقق من مدى معقولية التحذيرات. هل وجدت أخطاء حقيقية؟ هل هناك تحذيرات خاطئة؟

لقد قسمت التحذيرات الموجودة إلى المجموعات أدناه.

تحذيرات مهمة

زاد تصحيحهم الاستقرار ، وحل المشاكل مع تسرب الذاكرة ، إلخ. أخطاء / عيوب حقيقية.

تم إصدار 16 من هذه ، وهو ما يمثل حوالي 8 ٪ من جميع التحذيرات.

سأعطي بعض الأمثلة.

V3019 "ربما تتم مقارنة متغير غير صحيح بالصفر بعد تحويل النوع باستخدام الكلمة الأساسية" as ". تحقق من "لون" المتغيرات ، "مفهرس" »

public override bool IsCompatible(ColorImpl color) { IndexedColorImpl indexed = color as IndexedColorImpl; if (color == null) return false; return indexed.ColorSpace.Equals(this); } 

كما ترى ، بدلاً من الفهرسة ، تتم مقارنة اللون المتغير بالصفر. هذا غير صحيح ويمكن أن يؤدي إلى NRE.

V3080 " إحالة مرجعية فارغة. خذ بعين الاعتبار فحص "cstr_index.tile_index" »

جزء صغير لتوضيح:

 if (cstr_index.tile_index == null) { if (cstr_index.tile_index[0].tp_index == null) { // .. } } 

من الواضح أن الشرط الأول يعني ضمناً! في النموذج الحالي ، سيقوم الرمز برمي NRE مع كل مكالمة.

V3083 "استدعاء غير آمن للحدث" OnProgress "، NullReferenceException ممكن. فكر في تعيين حدث لمتغير محلي قبل استدعاؤه. "

 public void Updated() { if (OnProgress != null) OnProgress(this, new EventArgs()); } 

ساعد التحذير في إصلاح استثناء محتمل. لماذا يمكن أن تنشأ؟ يحتوي Stackoverflow على تفسير جيد .

V3106 "ربما يكون الفهرس خارج الحدود. يشير المؤشر "0" إلى ما وراء حدود "v" »

 var result = new List<FontStringPair>(); for (int i = 0; i < text.Length; ++i) { var v = new List<FontStringPair>(); createPairs(text[i].ToString(CultureInfo.InvariantCulture)); result.Add(v[0]); } 

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

V3117 "معلمة المنشئ " ValidateType "غير مستخدمة

تم إصدار تحذير لرمز مشابه لهذا

 public SomeClass(IDocument document, bool validateType = true) : base(document, true) { m_provider = document; } 

التحذير نفسه لا يبدو مهما. لكن المشكلة أكثر خطورة مما تبدو للوهلة الأولى. في عملية إضافة معلمة ValidateType الاختيارية ، نسوا إصلاح المكالمة إلى مُنشئ الفئة الأساسية.

V3127 „تم العثور على جزأين رمز مشابه. ربما ، هذا هو خطأ مطبعي ويجب استخدام متغير النطاق بدلاً من "المجال" "

 private void fillTransferFunction(PdfStreamImpl function) { // .. PdfArrayImpl domain = new PdfArrayImpl(); domain.AddReal(0); domain.AddReal(1); function.Add(Field.Domain, domain); PdfArrayImpl range = new PdfArrayImpl(); range.AddReal(0); range.AddReal(1); function.Add(Field.Range, domain); // .... } 

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

تحذيرات نظرية / رسمية

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

تم إصدار 57 منها ، وهو ما يمثل حوالي 30 ٪ من جميع التحذيرات. سأعطي أمثلة للحالات التي تستحق الاهتمام.

V3013 "من الغريب أن جسم دالة" BeginText "يكافئ تمامًا نص وظيفة" EndText "(166 ، السطر 171)"

 public override void BeginText() { m_state.ResetTextParameters(); } public override void EndText() { m_state.ResetTextParameters(); } 

كلا وظائف الجسم هي نفسها في الواقع. لكن هذا صحيح. وهل من الغريب حقًا أن تتطابق أجسام الدوال من سطر واحد؟

V3106 "قيمة مؤشر سلبية محتملة. يمكن أن تصل قيمة مؤشر "c1" إلى -1 "

 freq[256] = 1; // .... c1 = -1; v = 1000000000L; for (i = 0; i <= 256; i++) { if (freq[i] != 0 && freq[i] <= v) { v = freq[i]; c1 = i; } } // .... freq[c1] += freq[c2]; 

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

V3107 "التعبير المجاور" المجاور "إلى اليسار وإلى يمين التخصيص المركب"

يحدث التحذير بسبب رمز شائع جدًا:

 neighsum += neighsum; 

نعم ، يمكن إعادة كتابته من خلال الضرب. لكن ليس هناك خطأ.

V3109 التعبير الفرعي "l_cblk.data_current_size" موجود على جانبي عامل التشغيل. التعبير غير صحيح أو يمكن تبسيطه. "

 /* Check possible overflow on size */ if ((l_cblk.data_current_size + l_seg.newlen) < l_cblk.data_current_size) { // ... } 

يوضح التعليق في المدونة النية. مرة أخرى إنذار كاذب.

تحذيرات مبررة

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

تم إصدار 103 منها ، وهو ما يمثل 54 ٪ من جميع التحذيرات.

V3008 "متغير" l_mct_deco_data يتم تخصيص قيم مرتين متتاليتين. ربما هذا خطأ "

 if (m_nb_mct_records == m_nb_max_mct_records) { ResizeMctRecords(); l_mct_deco_data = (OPJ_INT32)m_nb_mct_records; } l_mct_deco_data = (OPJ_INT32)m_nb_mct_records; 

محلل الحقوق: التعيين في الداخل إذا لم يكن ضروريًا.

V3009 "من الغريب أن هذه الطريقة ترجع دائمًا نفس القيمة"

 private static bool opj_dwt_decode_tile(opj_tcd_tilecomp_t tilec, OPJ_UINT32 numres) { if (numres == 1U) return true; // ... return true; } 

بناءً على نصيحة المحلل ، تم تغيير الطريقة ولا تُرجع شيئًا أكثر.

V3022 "Expression '! Add' صحيح دائمًا"

 private void addToFields(PdfDictionaryImpl controlDict, bool add) { // ... if (add) { // .. return; } if (!add) { // ... } // ... } 

في الواقع ، ليس هناك نقطة في الثانية إذا. الشرط سيكون دائما صحيحا.

V3029 "التعبير الشرطي لعبارات" if "الموجودة جنبا إلى جنب متطابقة"

 if (stroke) extGState.OpacityStroke = opacity; if (stroke) state.AddReal(Field.CA, opacity); 

من غير الواضح كيف نشأ هذا الرمز. لكننا أصلحناه الآن.

V3031 "يمكن تبسيط الشيك الزائد. '||' عامل التشغيل محاطًا بالتعابير المعاكسة "

هذه حالة كابوس:

 if (!(cp.m_enc.m_tp_on != 0 && ((!opj_codec_t.OPJ_IS_CINEMA(cp.rsiz) && (t2_mode == J2K_T2_MODE.FINAL_PASS)) || opj_codec_t.OPJ_IS_CINEMA(cp.rsiz)))) { // ... } 

بعد التغييرات أصبحت أفضل بكثير

 if (!(cp.m_enc.m_tp_on != 0 && (opj_codec_t.OPJ_IS_CINEMA(cp.rsiz) || t2_mode == J2K_T2_MODE.FINAL_PASS))) { // ... } 

V3063 "يكون جزء من التعبير الشرطي صحيحًا دائمًا إذا تم تقييمه: x! = Null"
V3022 "التعبير" x! = Null "صحيح دائمًا"

لقد قمت هنا بتضمين تحذيرات بأن التحقق من قيمة null لا معنى له. ما إذا كان من الصحيح القيام بذلك سؤال مثير للجدل. لقد وصفت أدناه بمزيد من التفصيل جوهر القضية.

تحذيرات لا أساس لها

الإيجابيات الكاذبة بسبب أخطاء في تنفيذ اختبار معين أو نوع من خلل المحلل.

تم إصدار 14 منها ، وهو ما يمثل حوالي 7٪ من جميع التحذيرات.

V3081 " العداد " i غير مستخدم داخل حلقة متداخلة. النظر في فحص استخدام عداد "j"

نسخة مبسطة قليلاً من التعليمات البرمجية التي تم إصدار هذا التحذير لها:

 for (uint i = 0; i < initialGlyphsCount - 1; ++i) { for (int j = 0; j < initialGlyphsCount - i - 1; ++j) { // ... } } 

من الواضح ، أنا يستخدم في حلقة متداخلة.

V3125 "تم استخدام الكائن بعد التحقق من صحته"

الرمز الذي صدر تحذير بشأنه:

 private static int Compare_SecondGreater(cmapEncodingRecord er1, cmapEncodingRecord er2) { if (er1 == er2) return 0; if (er1 != null && er2 == null) return 1; if (er1 == null && er2 != null) return -1; return er1.CompareTo(er2); } 

لا يمكن أن يكون er1 فارغًا عند استدعاء CompareTo ().

رمز آخر يصدر هذا التحذير:

 private static void realloc(ref int[][] table, int newSize) { int[][] newTable = new int[newSize][]; int existingSize = 0; if (table != null) existingSize = table.Length; for (int i = 0; i < existingSize; i++) newTable[i] = table[i]; for (int i = existingSize; i < newSize; i++) newTable[i] = new int[4]; table = newTable; } 

لا يمكن أن يكون الجدول فارغًا في حلقة.

V3134 "إزاحة بمقدار [32..255] بت أكبر من حجم نوع التعبير" UInt32 "(uint) 1"

جزء من الشفرة صدر له هذا التحذير:

 byte bitPos = (byte)(numBits & 0x1F); uint mask = (uint)1 << bitPos; 

يمكن ملاحظة أن bitPos يمكن أن يكون له قيمة من النطاق [0..31] ، لكن المحلل يعتقد أنه يمكن أن يكون له قيمة من النطاق [0..31] ، وهو غير صحيح.

لن أعطي حالات أخرى مماثلة ، لأنها متكافئة.

أفكار إضافية حول بعض الشيكات

بدا لي أنه من غير المرغوب فيه أن أحذر من أن 'x! = Null' صحيح دائمًا في الحالات التي تكون فيها x نتيجة لاستدعاء طريقة ما. مثال:

 private X GetX(int y) { if (y > 0) return new X(...); if (y == 0) return new X(...); throw new ArgumentOutOfRangeException(nameof(x)); } private Method() { // ... X x = GetX(..); if (x != null) { // ... } } 

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

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

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

ونتيجة لذلك ، وفقًا لمبدأ YAGNI ، قرروا عدم التمسك بالشيكات وحذفها. تم نقل جميع التحذيرات من نظري / رسمي إلى مبرر.

يسعدني أن أقرأ رأيك في التعليقات.

الاستنتاجات

التحليل الثابت شيء جيد. يسمح لك PVS-Studio بالعثور على أخطاء حقيقية.

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

أخيرا ، بعض الإحصائيات.

أعلى 3 تحذيرات غير معقولة

V3081 " العداد " X غير مستخدم داخل حلقة متداخلة. النظر في فحص استخدام العداد "Y"
1 من أصل 1 وجد غير معقول

V3125 „تم استخدام الكائن بعد التحقق من صحته. التحقق من الخطوط: N1 ، N2 "
9 من أصل 10 أعلن أنها لا أساس لها

V3134 "Shift by N bits أكبر من حجم النوع"
وجدت 4 من أصل 5 لا أساس لها

أعلى 3 تحذيرات مهمة

V3083 "استدعاء غير آمن للحدث ، NullReferenceException ممكن. ضع في اعتبارك تعيين حدث لمتغير محلي قبل استدعاؤه "
5 من 5 اعتبرت مهمة.

V3020 "فاصل / متابعة / عودة / انتقال " غير مشروط ضمن حلقة "
V3080 "احتمال عدم الإشارة إلى قيمة"
تم التعرف على 2 من أصل 2 على أنها مهمة.

V3019 "من الممكن مقارنة متغير غير صحيح بالصفر بعد تحويل النوع باستخدام" ككلمة ""
V3127 „تم العثور على جزأين رمز مشابه. ربما ، هذا خطأ مطبعي ويجب استخدام المتغير "X" بدلاً من "Y" "
اعتبر 1 من أصل 1 مهمًا.