Emby تحليل شفرة المصدر مع محلل PVS-Studio

الصورة 3

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

PVS-Studio محلل الكود


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

مشروع مدقق


Emby هو خادم وسائط يمكن العثور على كود المصدر الخاص به على GitHub . إنها تتيح لك تنظيم عمليات البث وتمكين الوصول إلى محتوى الوسائط (الفيديو والصوت والصور) من أي جهاز. الضغط من موقع 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 + "))"; } .... } 

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

تحذير 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 تحذيرًا من محلل يتم إصدارها في 13 سطرًا من التعليمات البرمجية (أكثر بقليل من كل سطر ثان). لذلك ، أعتقد أن هذا الرمز يستحق أن يدرج في المقال. بعد ذلك ، سأكتب ما يقسم المحلل على وجه التحديد.

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

الصورة 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 على صواب ثم تحقق من هذا المتغير على الفور في عبارة if؟

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

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

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

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

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

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

تحذير 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 سطرًا من التعليمات البرمجية ، 4.6٪ - تعليقات). إنها رائعة حقًا. ومع ذلك ، حاول المحلل أيضًا من خلال إصدار تحذيرات من المستويات العليا - 113 ، والمتوسطة - 213 ، والمنخفضة - 112. بعضها إيجابي ، لكن معظم الأخطاء لم تظهر في المقالة لأنها تبدو متشابهة.

على سبيل المثال ، سجلت التحذيرات V3022 وحدها (التعبير دائماً غير صحيح / صواب) 106 قطعة. يمكنك ، بالطبع ، التحقق من أي جزء منها كان إيجابيات كاذبة ، وإضافة الباقي إلى المقال ، ولكن هذا سيؤدي إلى نص ممل للغاية لن يقرأه أحد.

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



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

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


All Articles