Geekly articles weekly

Analisis kode sumber Emby dengan penganalisa PVS-Studio

Gambar 3

Emby adalah server media yang cukup terkenal, bersama dengan Plex dan Kodi. Pada artikel ini, kami akan mencoba memverifikasi kode sumbernya menggunakan penganalisa statis PVS-Studio. Di situs web resmi Emby ada tanda kecil "Dibangun dengan ReSharper". Yah, ini bahkan lebih menarik.

Penganalisa kode PVS-Studio


PVS-Studio berjalan pada sistem 64-bit pada Windows, Linux dan macOS. Dia tahu bagaimana mencari kesalahan dalam kode sumber program yang ditulis dalam C, C ++, C # dan Java.

Proyek yang Diaudit


Emby adalah server media yang kode sumbernya dapat ditemukan di GitHub . Ini memungkinkan Anda untuk mengatur siaran dan memungkinkan untuk mengakses konten media (video, audio, foto) dari perangkat apa pun. Peras dari situs web resmi Emby :

  • Emby secara otomatis mengonversi media Anda dengan cepat untuk diputar di perangkat apa pun;
  • Kontrol orangtua yang luas memudahkan untuk mengontrol akses ke konten, yang akan relevan untuk keluarga dengan anak kecil;
  • Emby mengatur media Anda menjadi presentasi yang sederhana dan elegan. Mereka tidak akan pernah terlihat sama;
  • Streaming data dengan sinkronisasi di cloud;
  • Bagikan file media Anda dengan mudah ke teman dan keluarga;
  • Dan masih banyak lagi.

Potongan kode yang menarik perhatian saat memeriksa laporan penganalisa


Peringatan PVS-Studio: V3001 Ada sub-ekspresi yang identik 'c! =' <'' Di sebelah kiri dan di sebelah kanan operator '&&'. 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; .... }

Penganalisa mendeteksi duplikat subekspresi c! = '<' . Mungkin kesalahan merayap ke dalam kode ini, dan sesuatu yang lain seharusnya ada di '<' . Atau, lebih mungkin, subekspresi kedua tidak perlu dan Anda hanya perlu menghapusnya.

PVS-Studio Peringatan: V3001 Ada sub-ekspresi identik 'SmbConstants.AttrHidden' ke kiri dan ke kanan '|' operator. SmbComDelete.cs 29

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

Penganalisa lagi menemukan kesalahan ketik yang terkait dengan subekspresi berulang. Ke depan, saya akan mengatakan bahwa dalam proyek ini ada sedikit kesalahan serupa yang dibuat oleh kecerobohan. Saya tidak menyalahkan pencipta Emby sama sekali, kita kadang-kadang tersebar dengan cara ini ( contoh , contoh , contoh ), dan untuk aman dari kesalahan seperti itu, hanya ada analisis statis.

PVS-Studio Warning: V3004 Pernyataan 'then' setara dengan pernyataan '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 + "))"; } .... }

Dan ini sangat mirip dengan masalah yang muncul karena copy-paste, karena jika dan yang lain memblokir tubuh benar-benar identik. Mengapa Anda perlu memeriksa ukuran array typesToCount jika, terlepas dari jumlah elemen yang ditulis di dalamnya, kode yang sama akan dieksekusi - hanya diketahui oleh pengembang.

PVS-Studio Warning: V3005 Variabel '_validProviderIds' ditugaskan untuk dirinya sendiri. BaseNfoParser.cs 77

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

Sekali lagi, salah ketik, yang mensyaratkan penugasan variabel untuk dirinya sendiri. Perlu memeriksa ulang bagian kode ini.

PVS-Studio Warning: V3008 Variabel 'Chapters' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 29, 28. Judul.cs 29

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

Kurang perhatian dan kesalahan ketik, sekali lagi ... Bab variabel diberi nilai dua kali. Jelas bahwa kode tambahan ini tidak mengancam hal buruk, tetapi tetap saja. Tidak masuk akal untuk berlama-lama pada contoh ini, mari kita lanjutkan.

Peringatan PVS-Studio: V3013 Aneh bahwa tubuh fungsi 'Baca' sepenuhnya setara dengan tubuh fungsi 'Tulis' (407, baris 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; } } }

Badan fungsi Baca dan Tulis benar-benar identik satu sama lain, sebagaimana dibuktikan oleh teks peringatan penganalisa. Metode EnterWriteLock diperlukan untuk memasukkan kunci dalam mode tulis. Untuk mode membaca, ada baiknya menggunakan metode EnterReadLock , yang memungkinkan kemampuan membaca sumber daya secara bersamaan di beberapa utas.

Pengembang harus melihat lebih dekat pada bagian kode ini, mungkin ada kesalahan. Selain itu, kelas berikut ini ditemukan dalam kode, yang tidak digunakan:

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

Peringatan PVS-Studio: V3021 Ada dua pernyataan 'jika' dengan ekspresi kondisional yang identik. Pernyataan 'jika' pertama berisi pengembalian metode. Ini berarti bahwa pernyataan 'jika' yang kedua adalah SkiaEncoder.cs 537 yang tidak masuk akal

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

Di sini, tampaknya, pengembang menyalin empat baris pertama, tetapi lupa untuk mengubah variabel yang sedang diuji dari inputPath ke outputPath . Ada beberapa baris dalam kode yang menggunakan outputPath tanpa memeriksa nol , yang dapat menyebabkan pengecualian dilemparkan.

Peringatan PVS-Studio:

  • V3022 Expression 'processUnsupportedFrame (frame, CloseStatusCode.PolicyViolation, null)' selalu salah. WebSocket.cs 462
  • Ekspresi V3022 'processCloseFrame (frame)' selalu salah. WebSocket.cs 461
  • V3022 Expression 'frame.IsClose? processCloseFrame (frame): processUnsupportedFrame (frame, CloseStatusCode.PolicyViolation, null) 'selalu salah. WebSocket.cs 460
  • Ekspresi V3022 'processPongFrame (frame)' selalu benar. WebSocket.cs 459
  • Ekspresi V3022 'processPingFrame (frame)' selalu benar. WebSocket.cs 457
  • Ekspresi V3022 'processDataFrame (frame)' selalu benar. WebSocket.cs 455
  • Ekspresi V3022 selalu salah. 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; }

Di akun saya, tidak banyak, dibandingkan dengan tim PVS-Studio lainnya, diverifikasi oleh penganalisa proyek, dan mungkin ini entah bagaimana terhubung dengan fakta bahwa saya pertama kali melihat bahwa sebanyak 13 peringatan penganalisa dikeluarkan pada 13 baris kode (sedikit lebih dari setiap baris kedua). Karena itu, saya percaya bahwa kode ini layak untuk dimasukkan dalam artikel. Selanjutnya, saya akan menuliskan apa yang disumpah oleh penganalisa secara spesifik.

  1. Pertama, bingkai ekspresi.IsCompressed && _compression == CompressionMethod.Tidak ada yang dipertimbangkan dan, jika ekspresi itu benar, metode processUnsupportedFrame akan dieksekusi, yang dalam kasus apa pun mengembalikan false (karenanya peringatan pertama dari penganalisa). Jika itu salah, maka beralihlah ke poin berikutnya.
  2. Selanjutnya, nilai frame.IsFragmented diperiksa . Tidak ada masalah, jadi mari kita lanjutkan.
  3. Di sini nilai frame.IsData diperiksa . Jika benar, maka metode processDataFrame akan kembali benar pula. Oleh karena itu peringatan penganalisa kedua.
  4. Kemudian kita periksa frame . Benar - processPingFrame akan mengembalikan true . Ini adalah peringatan penganalisa ketiga.
  5. Kami melihat bingkai . Semuanya mirip dengan yang sebelumnya.
  6. Dan yang terakhir: frame.IsClose . processCloseFrame dan processUnsupportedFrame dalam kasus apa pun mengembalikan false .

Gambar 1

Saya harap saya tidak membuat Anda bosan. Ini akan lebih sederhana lagi.

PVS-Studio Warning: V3085 Nama bidang 'RtpHeaderBytes' dalam tipe bersarang adalah ambigu. Tipe luar berisi bidang statis dengan nama yang identik. 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; .... } .... }

Kelas bersarang UdpClientStream menggunakan bidang dengan nama yang mirip dengan yang digunakan di kelas eksternal HdHomerunUdpStream . Ini bukan kesalahan, tetapi merupakan alasan yang bagus untuk memeriksa apakah semuanya baik-baik saja dengan kode ini. Masalahnya adalah bahwa ada kemungkinan untuk membingungkan kedua variabel ini, dan kode tidak akan berfungsi sebagaimana dimaksud oleh pengembang, meskipun tidak akan menimbulkan keluhan dari kompiler.

Peringatan PVS-Studio:

  • V3090 Tidak aman mengunci suatu jenis. Semua instance dari suatu tipe akan memiliki objek 'Type' yang sama. Lmhosts.cs 49
  • V3090 Tidak aman mengunci suatu jenis. Semua instance dari suatu tipe akan memiliki objek 'Type' yang sama. 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)) { .... } } }

Penganalisis memperingatkan bahwa kunci tidak aman digunakan dalam fragmen kode ini. Menggunakan kunci tidak diinginkan karena objek yang digunakan untuk mengunci dibagikan dan dapat digunakan untuk mengunci di tempat lain tanpa sepengetahuan pengembang yang menggunakan objek ini untuk pertama kalinya. Hasilnya akan menjadi jalan buntu pada objek yang sama.

Idealnya, Anda harus menggunakan bidang pribadi untuk mengunci, misalnya, seperti ini:

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

Peringatan PVS-Studio: V3142 Kode unreacheble terdeteksi. Mungkin saja ada kesalahan. 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(....); }

Penganalisa menunjukkan bahwa baris terakhir dalam fragmen kode ini tidak akan pernah dieksekusi. Dan mengapa pengembang menyatakan dan mengatur variabel enableHttpStream menjadi true dan kemudian segera memeriksa variabel ini dalam pernyataan if?

Mungkin kode ini hanya mubazir. Bagaimanapun, perlu dicek kembali.

Peringatan PVS-Studio: V3083 Doa yang tidak aman dari acara 'RefreshStarted', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. ProviderManager.cs 943

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

Metode ini menggunakan panggilan yang berpotensi tidak aman ke event handler RefreshStarted , seperti yang ditunjukkan oleh penganalisa.

Sekarang saya akan menjelaskan mengapa penganalisa menganggap panggilan ini RefreshStarted dalam kode tidak aman. Misalkan, pada saat antara memeriksa null dan langsung memanggil event handler di badan if , berhenti berlangganan dibuat dari acara di utas lain. Dan, jika tidak ada lagi pelanggan, RefreshStarted akan menjadi nol . Tetapi di utas lain, di mana pemeriksaan null telah terjadi, baris berikut akan dieksekusi:
 RefreshStarted(this, new GenericEventArgs<BaseItem>(item));

Ini akan melempar NullReferenceException .

PVS-Studio Warning: V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak bersebelahan adalah identik. Periksa baris: 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(....); } .... }

Dalam potongan kode ini di sebelah satu sama lain adalah dua pernyataan if dengan kondisi yang sama. Badan pernyataan if berbeda. Sulit mengatakan apakah ini kesalahan atau kode yang berlebihan. Mungkin semuanya baik-baik saja dan penulis hanya ingin secara logis memisahkan dua tindakan yang berbeda, salah satunya dikaitkan dengan "Logo" tertentu, dan yang kedua dengan "Seni" tertentu.

Peringatan PVS-Studio: V3041 Ungkapan itu secara implisit dilemparkan dari tipe 'int' ke tipe 'double'. Pertimbangkan untuk menggunakan pemeran tipe eksplisit untuk menghindari hilangnya bagian fraksional. Contoh: double A = (double) (X) / Y; LiveTvManager.cs 1085

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

Kode ini berisi operasi membagi tipe data integer. Nilai yang dihasilkan secara implisit dikonversi ke tipe floating-point, yang sangat mencurigakan.

Sebenarnya, progressPerService variabel akan sama dengan 1.0 hanya jika _services.Length = 1 . Untuk nilai-nilai _services lainnya. Panjang , hasilnya adalah 0,0 .

Rupanya, itu harus ditulis di sini:

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


Peringatan PVS-Studio: V3050 Mungkin salah HTML. Tag penutup </u> ditemukan, sedangkan tag </i> diharapkan. 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" }, .... } }; }

Perhatikan garisnya

 <u>underline</u>

Itu sudah memiliki tag penutup </u> . Selanjutnya kita melihat teks ini:
 </s></u></i></b> HTML tags"

Di sini tag penutup </u> berlebihan, seperti yang ditunjukkan oleh penganalisa.
Peringatan PVS-Studio: V3051 Pemeriksaan tipe berlebihan. Objek sudah dari tipe 'Pengecualian'. 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; }

Dan di sini, terus terang, tidak sepenuhnya jelas apa yang ingin dicapai pengembang pada umumnya. Penganalisa memperingatkan bahwa pernyataan if kedua memeriksa objek root untuk kompatibilitas dengan tipenya sendiri. Kemungkinan besar, ini hanya kode yang berlebihan, tetapi terlihat mencurigakan, pengembang harus melihatnya lebih dekat.

Kesimpulan


Dalam segala hal, para pengembang melakukan pekerjaan besar menciptakan Emby (proyek ini memiliki 215 539 baris kode, 4,6% - komentar). Mereka hebat, sungguh. Namun, penganalisa juga mencoba dengan mengeluarkan peringatan tingkat Tinggi - 113, Menengah - 213, dan Rendah - 112. Beberapa dari mereka adalah positif palsu, tetapi sebagian besar kesalahan tidak muncul dalam artikel karena mereka mirip.

Misalnya, peringatan V3022 saja (ekspresi selalu salah / benar) mencetak 106 buah. Anda dapat, tentu saja, memeriksa bagian mana dari mereka yang positif palsu, dan menambahkan sisanya ke artikel, tetapi ini akan menghasilkan teks yang sangat membosankan sehingga tidak ada yang akan membaca.

Saya harap saya berhasil menunjukkan kegunaan analisa statis dalam pengembangan. Tentu saja, pemeriksaan satu kali saja tidak cukup, dan penganalisa statis harus digunakan secara berkelanjutan. Anda dapat membaca lebih lanjut tentang ini di artikel Godot: Tentang Penggunaan Reguler dari Kode Analisis Statis .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Ekaterina Nikiforova. Memeriksa Emby dengan PVS-Studio .

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

More articles:


All Articles