Geekly articles weekly

التحقق من Emby مع PVS-Studio

الصورة 3

Emby هو خادم وسائط مشهور إلى جانب Plex و Kodi. في هذه المقالة ، سنناقش الأخطاء الموجودة في الكود المصدري مع المحلل الثابت PVS-Studio. إن الملاحظة "Built with ReSharper" على الموقع الرسمي للمشروع تجعل التحليل أكثر إثارة للاهتمام.

PVS استوديو


يعمل PVS-Studio على أنظمة Windows و Linux و macOS 64 بت. يمكنه اكتشاف الأخطاء في الكود المصدري للبرنامج المكتوب بلغات C و C ++ و C # و Java.

المشروع قيد التحليل


Emby هو خادم الوسائط. شفرة المصدر الخاصة به متاحة على جيثب . يسمح للمستخدم بدفق محتوى الوسائط والوصول إليه (فيديو ، صوت ، صور) على أي جهاز. فيما يلي ملخص موجز لميزات Emby وفقًا لموقع الويب الرسمي للمشروع:

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

مقتطفات الشفرة الأكثر إثارة للاهتمام التي أبلغ عنها المحلل


رسالة تشخيص PVS-Studio: V3001 هناك تعبيرات فرعية متطابقة 'c! =' <'' إلى اليسار وإلى يمين المشغل '&&'. HttpListenerRequest.Managed.cs 49

internal void SetRequestLine(string req) { .... if ((ic >= 'A' && ic <= 'Z') || (ic > 32 && c < 127 && c != '(' && c != ')' && c != '<' && // <= c != '<' && c != '>' && c != '@' && c != ',' && c != ';' && // <= c != ':' && c != '\\' && c != '"' && c != '/' && c != '[' && c != ']' && c != '?' && c != '=' && c != '{' && c != '}')) continue; .... }

اكتشف المحلل وجود تعبير مكرر c! = '<' . أحد التفسيرات هو أن هذا خطأ في البرمجة وأن المطور يهدف إلى كتابة شيء آخر بدلاً من "<" . التفسير الآخر الأكثر ترجيحًا هو أن التعبير الفرعي الثاني لم يكن يقصد به أن يكون هناك على الإطلاق ويجب إزالته.

رسالة تشخيص PVS-Studio: V3001 هناك تعبيرات فرعية مماثلة 'SmbConstants.AttrHidden' إلى اليسار وإلى يمين '|' المشغل. SmbComDelete.cs 29

 internal SmbComDelete(string fileName) { Path = fileName; Command = SmbComDelete; _searchAttributes = SmbConstants.AttrHidden | SmbConstants.AttrHidden | SmbConstants.AttrSystem; }

خطأ مطبعي آخر له علاقة بمصطلحات فرعية مكررة. كملاحظة جانبية ، هناك العديد من المشكلات مثل تلك الموجودة في التعليمات البرمجية المصدر لـ Emby - الأخطاء الناتجة عن عدم الانتباه. أنا لا ألوم المطورين. يمكن أن نكون جميعًا متغيبين في بعض الأحيان ( أمثلة ، أمثلة ، أمثلة ) ، وهذا هو بالضبط سبب وجود التحليل الثابت - لحمايتنا من أخطائنا.

رسالة تشخيص PVS-Studio: V3004 عبارة "then" مكافئة لبيان "else". SqliteItemRepository.cs 5648

 private QueryResult<Tuple<BaseItem, ItemCounts>> GetItemValues(....) { .... if (typesToCount.Length == 0) { whereText += " And CleanName In (Select CleanValue from ItemValues where " + typeClause + " AND ItemId in (select guid from TypedBaseItems" + innerWhereText + "))"; } else { //whereText += " And itemTypes not null"; whereText += " And CleanName In (Select CleanValue from ItemValues where " + typeClause + " AND ItemId in (select guid from TypedBaseItems" + innerWhereText + "))"; } .... }

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

رسالة تشخيص PVS-Studio: V3005 يتم تعيين متغير "_validProviderIds" إلى نفسه. BaseNfoParser.cs 77

 private Dictionary<string, string> _validProviderIds; .... public void Fetch(....) { .... _validProviderIds = _validProviderIds = new Dictionary<....>(....); .... }

خطأ مطبعي آخر ، مما يؤدي إلى تعيين متغير قيمته الخاصة. هذا الرمز يحتاج إلى مراجعة.

رسالة تشخيص PVS-Studio: V3008 يتم تعيين قيم "الفصول" مرتين على التوالي. ربما هذا خطأ. خطوط الفحص: 29 ، 28. Title.cs 29

 public Title(uint titleNum) { ProgramChains = new List<ProgramChain>(); Chapters = new List<Chapter>(); Chapters = new List<Chapter>(); TitleNumber = titleNum; }

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

رسالة تشخيص PVS-Studio: V3013 من الغريب أن يكون نص وظيفة "القراءة" مساوياً تمامًا لجسم وظيفة "الكتابة" (407 ، السطر 415). BaseSqliteRepository.cs 407

 public static IDisposable Read(this ReaderWriterLockSlim obj) { return new WriteLockToken(obj); } public static IDisposable Write(this ReaderWriterLockSlim obj) { return new WriteLockToken(obj); } private sealed class WriteLockToken : IDisposable { private ReaderWriterLockSlim _sync; public WriteLockToken(ReaderWriterLockSlim sync) { _sync = sync; sync.EnterWriteLock(); } public void Dispose() { if (_sync != null) { _sync.ExitWriteLock(); _sync = null; } } }

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

يجب على المطورين التحقق من هذا الرمز لأنه من المحتمل جدًا احتوائه على خطأ - كل هذا بسبب وجود فئة غير مستخدمة موجودة في الكود:

 private sealed class ReadLockToken : IDisposable { private ReaderWriterLockSlim _sync; public ReadLockToken(ReaderWriterLockSlim sync) { _sync = sync; sync.EnterReadLock(); } public void Dispose() { if (_sync != null) { _sync.ExitReadLock(); _sync = null; } } }

رسالة تشخيص PVS-Studio: V3021 هناك بيانان "if" مع تعبيرات شرطية متطابقة. تحتوي العبارة "if" الأولى على طريقة إرجاع. هذا يعني أن العبارة "if" الثانية لا معنى لها SkiaEncoder.cs 537

 public string EncodeImage(string inputPath, DateTime dateModified, string outputPath, bool autoOrient, ImageOrientation? orientation, int quality, ImageProcessingOptions options, ImageFormat selectedOutputFormat) { if (string.IsNullOrWhiteSpace(inputPath)) { throw new ArgumentNullException("inputPath"); } if (string.IsNullOrWhiteSpace(inputPath)) { throw new ArgumentNullException("outputPath"); } .... }

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

رسائل تشخيص PVS-Studio:

  • V3022 Expression 'processUnsupportedFrame (إطار ، CloseStatusCode.PolicyViolation ، لاغ) هو دائما خاطئ. WebSocket.cs 462
  • V3022 Expression 'processCloseFrame (frame)' غير صحيح دائمًا. WebSocket.cs 461
  • V3022 Expression 'frame.IsClose؟ processCloseFrame (frame): processUnsupportedFrame (frame، CloseStatusCode.PolicyViolation، null) 'false دائماً. WebSocket.cs 460
  • إن تعبير V3022 'processPongFrame (frame)' صحيح دائمًا. WebSocket.cs 459
  • إن تعبير V3022 'processPingFrame (frame)' صحيح دائمًا. WebSocket.cs 457
  • تعبير V3022 'processDataFrame (frame)' صحيح دائمًا. WebSocket.cs 455
  • تعبير V3022 غير صحيح دائمًا. WebSocket.cs 448

 private bool processWebSocketFrame(WebSocketFrame frame) { return frame.IsCompressed && _compression == CompressionMethod.None ? processUnsupportedFrame(....) : frame.IsFragmented ? processFragmentedFrame(frame) : frame.IsData ? processDataFrame(frame) : frame.IsPing ? processPingFrame(frame) : frame.IsPong ? processPongFrame(frame) : frame.IsClose ? processCloseFrame(frame) : processUnsupportedFrame(....); } private bool processUnsupportedFrame(....) { processException(....); return false; } private bool processDataFrame(WebSocketFrame frame) { .... return true; } private bool processPingFrame(WebSocketFrame frame) { var mask = Mask.Unmask; return true; } private bool processPongFrame(WebSocketFrame frame) { _receivePong.Set(); return true; } private bool processCloseFrame(WebSocketFrame frame) { var payload = frame.PayloadData; close(payload, !payload.ContainsReservedCloseStatusCode, false); return false; }

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

  1. إطار التعبير. Isspressed && _compression == يتم تقييم CompressionMethod.None أولاً. إذا كان هذا صحيحًا ، فسيتم تنفيذ طريقة processUnsupportedFrame وإرجاع false في أي حال (هذا هو التحذير الأول). إذا كان التحقق خاطئًا ، فننتقل إلى الاختبار التالي.
  2. يتم التحقق من قيمة الإطار . لا توجد مشاكل هنا.
  3. يتم التحقق من قيمة الإطار . إذا كان هذا صحيحًا ، فستعود طريقة processDataFrame إلى أي حال. هذا هو التحذير الثاني.
  4. يتم التحقق من قيمة الإطار . إذا كان هذا صحيحًا ، فسترجع طريقة processPingFrame بشكل صحيح . هذا هو التحذير الثالث.
  5. يتم التحقق من قيمة الإطار . نفس السابق.
  6. آخر واحد: الإطار . processCloseFrame و processUsupportedFrame إرجاع false في أي حال.

الصورة 1

آمل أنه لم يكن مملا للغاية لمتابعة. الأمثلة المتبقية ليست معقدة.

رسالة تشخيص PVS-Studio: V3085 اسم حقل "RtpHeaderBytes" في نوع متداخل غامض. النوع الخارجي يحتوي على حقل ثابت مع اسم متطابق. HdHomerunUdpStream.cs 200

 public class HdHomerunUdpStream : LiveStream, IDirectStreamProvider { .... private static int RtpHeaderBytes = 12; public class UdpClientStream : Stream { private static int RtpHeaderBytes = 12; private static int PacketSize = 1316; private readonly MediaBrowser.Model.Net.ISocket _udpClient; bool disposed; .... } .... }

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

رسائل تشخيص PVS-Studio:

  • V3090 تأمين غير آمن على نوع. جميع مثيلات النوع سيكون لها نفس كائن "الكتابة". Lmhosts.cs 49
  • V3090 تأمين غير آمن على نوع. جميع مثيلات النوع سيكون لها نفس كائن "الكتابة". Lmhosts.cs 57

 public class Lmhosts { public static NbtAddress GetByName(string host) { lock (typeof(Lmhosts)) { return GetByName(new Name(host, 0x20, null)); } } internal static NbtAddress GetByName(Name name) { lock (typeof(Lmhosts)) { .... } } }

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

من الناحية المثالية ، يجب عليك استخدام حقل خاص للقفل ، على سبيل المثال:

 private Object locker = new Object(); public static NbtAddress GetByName(string host) { lock (locker) { return GetByName(new Name(host, 0x20, null)); } }

رسالة تشخيص PVS-Studio: تم اكتشاف رمز غير قابل للوصول V3142 . من الممكن وجود خطأ. HdHomerunHost.cs 621

 protected override async Task<ILiveStream> GetChannelStream(....) { .... var enableHttpStream = true; if (enableHttpStream) { mediaSource.Protocol = MediaProtocol.Http; var httpUrl = channelInfo.Path; // If raw was used, the tuner doesn't support params if (!string.IsNullOrWhiteSpace(profile) && !string.Equals(profile, "native", StringComparison.OrdinalIgnoreCase)) { httpUrl += "?transcode=" + profile; } mediaSource.Path = httpUrl; return new SharedHttpStream(....); } return new HdHomerunUdpStream(....); }

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

ربما هذا الكود هو ببساطة زائدة عن الحاجة ، لكنه يحتاج إلى مراجعة على أي حال.

رسالة تشخيص PVS-Studio: V3083 الاحتجاج غير الآمن للحدث "RefreshStarted" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. ProviderManager.cs 943

 public void OnRefreshStart(BaseItem item) { .... if (RefreshStarted != null) { RefreshStarted(this, new GenericEventArgs<BaseItem>(item)); } }

يحذرنا المحلل من استدعاء غير آمن محتمل لمعالج الأحداث RefreshStarted .

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

 RefreshStarted(this, new GenericEventArgs<BaseItem>(item));

سيؤدي هذا إلى رمي NullReferenceException .

رسالة تشخيص PVS-Studio: V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 142 ، 152. LocalImageProvider.cs 142

 private void PopulateImages(....) { .... // Logo if (!isEpisode && !isSong && !isPerson) { added = AddImage(....); if (!added) { added = AddImage(....); } } // Art if (!isEpisode && !isSong && !isPerson) { AddImage(....); } .... }

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

رسالة تشخيص PVS-Studio: V3041 تم نقل التعبير ضمنيًا من النوع "int" إلى النوع "double". النظر في استخدام يلقي نوع واضح لتجنب فقدان جزء كسور. مثال: double A = (double) (X) / Y؛ LiveTvManager.cs 1085

 private async Task RefreshChannelsInternal(....) { .... double progressPerService = _services.Length == 0 ? 0 : 1 / _services.Length; .... }

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

في الواقع ، سيكون للمتقدم progressPerService القيمة 1.0 فقط إذا _services.Length = 1 . مع أي قيمة أخرى _services.Length ، ستكون النتيجة 0.0 .

أعتقد أن ما يجب كتابته بدلاً من ذلك هو ما يلي:

 double progressPerService = _services.Length == 0 ? 0 : (double)1 / _services.Length;

رسالة تشخيص PVS-Studio: V3050 ربما HTML غير صحيح. تمت مصادفة علامة الإغلاق </u> ، بينما كانت العلامة </i> متوقعة. SrtParserTests.cs 64

 public void TestParse() { var expectedSubs = new SubtitleTrackInfo { TrackEvents = new SubtitleTrackEvent[] { .... new SubtitleTrackEvent { Id = "6", StartPositionTicks = 330000000, EndPositionTicks = 339990000, Text = "This contains nested <b>bold, <i>italic, <u>underline</u> and <s>strike-through</s></u></i></b> HTML tags" }, .... } }; }

لاحظ هذا السطر:

 <u>underline</u>

لديها بالفعل علامة إغلاق </u> .
ثم نرى النص التالي:
 </s></u></i></b> HTML tags"

هناك علامة إغلاق إضافية </ u> هنا ، وهو ما يشير إليه المحلل.

رسالة تشخيص PVS-Studio: V3051 فحص نوع زائد . الكائن بالفعل من نوع "الاستثناء". SmbFileInputStream.cs 107

 protected internal virtual IOException SeToIoe(SmbException se) { IOException ioe = se; Exception root = se.GetRootCause(); if (root is TransportException) { ioe = (TransportException)root; root = ((TransportException)ioe).GetRootCause(); } if (root is Exception) { ioe = new IOException(root.Message); ioe.InitCause(root); } return ioe; }

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

استنتاج


قام مطورو Emby بعمل رائع بكل طريقة (المشروع يبلغ طوله 215.539 LOC ، 4.6٪ منها عبارة عن تعليقات). لقد قاموا بعمل جيد ، يعني ذلك. لكن برنامج PVS-Studio يستحق الثناء أيضًا: لقد أنتج 113 تحذيرات عالية المستوى و 213 مستوى متوسط ​​و 112 تحذيرًا منخفض المستوى. كان البعض منهم إيجابيات كاذبة ، ولكن لم يتم ذكر معظم الأخطاء هنا لأنها كانت متشابهة إلى حد كبير. على سبيل المثال ، تم تشغيل تشخيص V3022 (دائمًا الحالة الخاطئة / الحقيقية) بمفرده 106 مرات. بالطبع ، كان بإمكاني تصفية المرشحات الإيجابية الخاطئة وأدرجت البقية في المقالة ، لكن كان يمكن أن يكون من الممل جدًا قراءتها.

آمل أن أتمكن من إظهار كيف يساعد التحليل الثابت في تطوير البرمجيات. من الواضح أن الشيكات لمرة واحدة ليست كافية ؛ يجب عليك استخدام التحليل الثابت على أساس منتظم. تمت مناقشة هذا الموضوع بمزيد من التفصيل في مقالة " Godot: On Regular Use of Static Analyzers ".

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

More articles:


All Articles