Kualitas penting bagi kami. Dan kami mendengar tentang PVS-Studio. Semua ini mengarah pada keinginan untuk memeriksa Docotic.Pdf dan mencari tahu apa lagi yang bisa diperbaiki.
Docotic.Pdf adalah
pustaka serba guna untuk bekerja dengan file PDF. Itu ditulis dalam C #, tidak ada kode tidak aman, tidak ada dependensi eksternal selain .NET runtime. Ia bekerja di bawah. NET 4+ dan di bawah .NET Standard 2+.
Perpustakaan telah dikembangkan selama kurang lebih 10 tahun dan memiliki 110 ribu baris kode tanpa memperhitungkan tes, contoh, dan hal-hal lain. Untuk analisis statis, kami terus menggunakan Analisis Kode dan StyleCop. Beberapa ribu tes otomatis melindungi kita dari regresi. Klien kami dari berbagai negara dan industri berbeda mempercayai kualitas perpustakaan.
Masalah apa yang akan terdeteksi oleh PVS-Studio?
Instalasi dan kesan pertama
Saya mengunduh versi uji coba dari situs web PVS-Studio. Terkejut dengan ukuran kecil installer. Diinstal dengan pengaturan default: mesin analisis, lingkungan PVS-Studio yang terpisah, integrasi ke dalam Visual Studio 2017.
Setelah instalasi, tidak ada yang dimulai, dan dua pintasan dengan ikon yang sama ditambahkan ke menu Start: Standalone dan PVS-Studio. Untuk sesaat, saya berpikir untuk memulai apa. Meluncurkan Standalone dan terkejut dengan antarmuka yang tidak menyenangkan. Skala 200% yang diatur untuk Windows saya didukung dengan bengkok. Sebagian teks terlalu kecil, sebagian teks tidak sesuai dengan ruang yang disediakan untuk itu. Daftar nama, Unicorn, dan Tindakan dipotong untuk ukuran jendela apa pun. Bahkan dengan layar penuh.
Baiklah, oke, saya memutuskan untuk membuka file proyek saya. Tiba-tiba, menu File tidak menemukan kesempatan seperti itu. Di sana saya hanya ditawari untuk membuka file individual. Terima kasih, saya pikir, saya lebih suka mencoba opsi lain. Meluncurkan PVS-Studio - mereka menunjukkan saya jendela dengan teks buram. Skala 200% lagi membuatnya terasa. Teks yang dilaporkan:
lihat saya di Three Crowns, cari menu PVS-Studio di Visual Studio. Oke, buka Studio.
Solusi terbuka. Memang, ada menu PVS-Studio, dan memiliki kemampuan untuk memeriksa "Proyek Saat Ini". Dia membuat proyek yang saya butuhkan saat ini dan meluncurkan cek. Sebuah jendela muncul di Studio dengan hasil analisis. Sebuah jendela muncul di latar belakang dengan kemajuan pemindaian, tetapi saya tidak segera menemukannya. Pada awalnya ada perasaan bahwa cek tidak dimulai atau segera berakhir.
Cek hasil pertama
Alat analisis memeriksa semua file proyek 1253 dalam waktu sekitar 9 menit dan 30 detik. Pada akhir pemeriksaan, penghitung file tidak berubah secepat di awal. Mungkin ada beberapa ketergantungan nonlinear dari durasi pemindaian pada jumlah file yang dipindai.
Informasi tentang 81 peringatan Tinggi, 109 Sedang, dan 175 Rendah muncul di jendela hasil. Jika Anda menghitung frekuensi, Anda mendapatkan 0,06 peringatan tinggi / file, 0,09 peringatan menengah / file, dan 0,14 peringatan rendah / file. Atau
0,74 Peringatan tinggi per seribu baris kode, 0,99 Peringatan menengah per seribu baris kode, dan 1,59 Peringatan rendah per seribu baris kode.
Di sini,
di artikel ini ,
diindikasikan bahwa di CruiseControl.NET, dengan 256 ribu baris kode, penganalisa menemukan 15 peringatan Tinggi, 151 Sedang, dan 32 Rendah.
Ternyata dalam bentuk persentase untuk Docotic.Pdf lebih banyak peringatan dikeluarkan di masing-masing kelompok.
Apa yang ditemukan?
Saya memutuskan untuk mengabaikan peringatan Rendah pada tahap ini.
Saya mengurutkan peringatan berdasarkan kolom Kode dan ternyata pemegang catatan absolut untuk frekuensi adalah
V3022 "Ekspresi selalu benar / salah" dan
V3063 "Sebagian ekspresi kondisional selalu benar / salah jika dievaluasi". Menurut saya, mereka tentang satu hal. Secara total, kedua peringatan ini memberikan 92 dari 190 kasus. Frekuensi relatif = 48%.
Logika membagi menjadi Tinggi dan Menengah tidak sepenuhnya jelas. Saya mengharapkan
V3072 "Kelas 'A' yang berisi anggota IDisposable tidak mengimplementasikan IDisposable sendiri" dan
V3073 "Tidak semua anggota IDisposable dibuang dengan benar. Panggil 'Buang' ketika membuang kelas 'A' di grup High, misalnya. Tapi ini rasa, tentu saja.
Terkejut bahwa
V3095 “Objek itu digunakan sebelum diverifikasi terhadap nol. Periksa baris: N1, N2 ”ditandai dua kali lebih tinggi dan sekali sebagai Sedang. Bug?
Percaya tapi verifikasi
Sudah waktunya untuk memeriksa seberapa masuk akal peringatan itu. Apakah ada kesalahan nyata yang ditemukan? Apakah ada peringatan yang salah?
Saya membagi peringatan yang ditemukan ke dalam grup di bawah ini.
Peringatan Penting
Koreksi mereka meningkatkan stabilitas, memecahkan masalah dengan kebocoran memori, dll. Kesalahan / ketidaksempurnaan nyata.
16 di antaranya dikeluarkan, yaitu sekitar 8% dari semua peringatan.
Saya akan memberikan beberapa contoh.
V3019 "Mungkin variabel yang salah dibandingkan dengan nol setelah konversi jenis menggunakan kata kunci 'sebagai'. Periksa variabel 'warna', 'diindeks' »
public override bool IsCompatible(ColorImpl color) { IndexedColorImpl indexed = color as IndexedColorImpl; if (color == null) return false; return indexed.ColorSpace.Equals(this); }
Seperti yang Anda lihat, alih-alih diindeks, warna variabel dibandingkan dengan nol. Ini tidak benar dan dapat menyebabkan NRE.
V3080 “Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'cstr_index.tile_index' »
Sebuah fragmen kecil untuk menggambarkan:
if (cstr_index.tile_index == null) { if (cstr_index.tile_index[0].tp_index == null) {
Jelas, kondisi pertama tersirat! = Null. Dalam formulir saat ini, kode akan melempar NRE dengan setiap panggilan.
V3083 “Doa yang tidak aman dari acara 'OnProgress', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. "
public void Updated() { if (OnProgress != null) OnProgress(this, new EventArgs()); }
Peringatan membantu memperbaiki kemungkinan pengecualian. Kenapa bisa muncul? Stackoverflow memiliki
penjelasan yang bagus .
V3106 “Kemungkinan indeks keluar dari batas. Indeks '0' menunjuk di luar batas '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]); }
Kesalahannya adalah bahwa hasil createPairs diabaikan, dan sebagai gantinya daftar kosong diakses. Rupanya, awalnya createPairs menerima daftar sebagai parameter, tetapi kesalahan dibuat dalam proses mengubah metode.
V3117 'Parameter konstruktor' validateType 'tidak digunakan
Peringatan dikeluarkan untuk kode yang mirip dengan ini
public SomeClass(IDocument document, bool validateType = true) : base(document, true) { m_provider = document; }
Peringatan itu sendiri tampaknya tidak penting. Tapi masalahnya lebih serius daripada yang terlihat pada pandangan pertama. Dalam proses menambahkan parameter validateType opsional, mereka lupa untuk memperbaiki panggilan ke konstruktor dari kelas dasar.
V3127 "Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'range' harus digunakan alih-alih 'domain' “
private void fillTransferFunction(PdfStreamImpl function) {
Mungkin peringatan tidak akan dikeluarkan jika bagian kode sedikit berbeda. Namun dalam kasus ini, kesalahan terdeteksi saat menggunakan salin-tempel.
Peringatan teoritis / formal
Mereka benar, tetapi koreksi mereka tidak memperbaiki kesalahan spesifik dan tidak mempengaruhi keterbacaan kode. Atau mereka menunjuk ke tempat-tempat di mana mungkin ada kesalahan, tetapi itu tidak ada. Misalnya, urutan parameter sengaja diubah. Untuk peringatan semacam itu, Anda tidak perlu mengubah apa pun dalam program.
Dari jumlah tersebut, 57 dikeluarkan, yaitu sekitar 30% dari semua peringatan. Saya akan memberikan contoh untuk kasus-kasus yang patut mendapat perhatian.
V3013 "Aneh bahwa tubuh fungsi 'BeginText' sepenuhnya setara dengan tubuh fungsi 'EndText' (166, baris 171)"
public override void BeginText() { m_state.ResetTextParameters(); } public override void EndText() { m_state.ResetTextParameters(); }
Kedua fungsi tubuh itu sebenarnya sama. Tapi itu benar. Dan apakah benar-benar aneh jika fungsi tubuh dari satu baris bersamaan?
V3106 "Kemungkinan nilai indeks negatif. Nilai indeks 'c1' bisa mencapai -1 “
freq[256] = 1;
Saya setuju, saya memberikan algoritma yang tidak begitu jelas. Tapi, menurut saya, dalam hal ini, alat analisis khawatir sia-sia.
V3107 “Ungkapan identik 'ungkapan' di sebelah kiri dan di sebelah kanan penugasan majemuk”
Peringatan ini disebabkan oleh kode yang cukup umum:
neighsum += neighsum;
Ya, itu dapat ditulis ulang melalui multiplikasi. Namun tidak ada kesalahan.
V3109 "Sub-ekspresi 'l_cblk.data_current_size' hadir di kedua sisi operator. Ungkapannya salah atau bisa disederhanakan. "
if ((l_cblk.data_current_size + l_seg.newlen) < l_cblk.data_current_size) {
Komentar dalam kode menjelaskan maksudnya. Sekali lagi alarm palsu.
Peringatan yang Dibenarkan
Koreksi mereka secara positif mempengaruhi pembacaan kode. Artinya, itu mengurangi kondisi yang tidak perlu, pemeriksaan, dll. Efeknya pada cara kode bekerja tidak jelas.
Dari jumlah tersebut, 103 dikeluarkan, yaitu sekitar 54% dari semua peringatan.
V3008 „
Variabel 'l_mct_deco_data' diberi nilai dua kali berturut-turut. Mungkin ini kesalahan "
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;
Penganalisa hak: penugasan di dalam jika tidak perlu.
V3009 "Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama"
private static bool opj_dwt_decode_tile(opj_tcd_tilecomp_t tilec, OPJ_UINT32 numres) { if (numres == 1U) return true;
Atas saran penganalisa, metode telah diubah dan tidak menghasilkan apa-apa lagi.
V3022 "Ekspresi '! Tambah' selalu benar"
private void addToFields(PdfDictionaryImpl controlDict, bool add) {
Memang, tidak ada gunanya jika yang kedua. Kondisinya akan selalu benar.
V3029 "Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik"
if (stroke) extGState.OpacityStroke = opacity; if (stroke) state.AddReal(Field.CA, opacity);
Tidak jelas bagaimana kode tersebut berasal. Tapi sekarang kami memperbaikinya.
V3031 „Pemeriksaan berlebihan dapat disederhanakan. '||' Operator dikelilingi oleh ekspresi yang berlawanan “
Ini adalah kondisi mimpi buruk:
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)))) {
Setelah perubahan itu menjadi jauh lebih baik
if (!(cp.m_enc.m_tp_on != 0 && (opj_codec_t.OPJ_IS_CINEMA(cp.rsiz) || t2_mode == J2K_T2_MODE.FINAL_PASS))) {
V3063 “Bagian dari ekspresi kondisional selalu benar jika dievaluasi: x! = Null”
V3022 “Ekspresi 'x! = Null' selalu benar”
Di sini saya menyertakan peringatan bahwa memeriksa null tidak masuk akal. Apakah itu benar untuk dilakukan adalah pertanyaan yang kontroversial. Di bawah ini saya telah menjelaskan esensi masalah ini.
Peringatan tanpa dasar
Positif palsu Karena kesalahan dalam pelaksanaan tes tertentu atau semacam cacat analisa.
Dari jumlah tersebut, 14 dikeluarkan, yaitu sekitar 7% dari semua peringatan.
V3081 “
Penghitung 'i' tidak digunakan di dalam loop bersarang. Pertimbangkan untuk memeriksa penggunaan penghitung 'j' “
Versi kode yang sedikit disederhanakan untuk mana peringatan ini dikeluarkan:
for (uint i = 0; i < initialGlyphsCount - 1; ++i) { for (int j = 0; j < initialGlyphsCount - i - 1; ++j) {
Jelas, saya digunakan dalam loop bersarang.
V3125 "Objek digunakan setelah diverifikasi terhadap nol"
Kode untuk mana peringatan dikeluarkan:
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 tidak boleh nol ketika CompareTo () dipanggil.
Kode lain yang mengeluarkan peringatan ini:
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; }
tabel tidak boleh nol dalam satu lingkaran.
V3134 "Shift by [32..255] bit lebih besar dari ukuran tipe ekspresi 'UInt32' '(uint) 1'"
Sepotong kode untuk mana peringatan ini dikeluarkan:
byte bitPos = (byte)(numBits & 0x1F); uint mask = (uint)1 << bitPos;
Dapat dilihat bahwa bitPos dapat memiliki nilai dari rentang [0..31], tetapi penganalisa percaya bahwa itu dapat memiliki nilai dari kisaran [0..31], yang tidak benar.
Saya tidak akan memberikan kasus serupa lainnya, karena mereka setara.
Pikiran tambahan pada beberapa cek
Sepertinya saya tidak diinginkan untuk memperingatkan bahwa 'x! = Null' selalu benar dalam kasus di mana x adalah hasil dari memanggil beberapa metode. Contoh:
private X GetX(int y) { if (y > 0) return new X(...); if (y == 0) return new X(...); throw new ArgumentOutOfRangeException(nameof(x)); } private Method() {
Ya, secara formal, alat analisisnya benar: x tidak akan selalu menjadi nol, karena GetX akan mengembalikan instance lengkap atau melempar pengecualian. Tetapi apakah kode akan meningkatkan penghapusan cek dengan nol? Bagaimana jika GetX berubah nanti? Apakah Metode harus mengetahui implementasi GetX?
Di dalam tim, pendapat dibagi. Telah disarankan bahwa metode saat ini memiliki kontrak dimana tidak boleh mengembalikan nol. Dan tidak masuk akal untuk menulis kode berlebihan "untuk masa depan" dengan setiap panggilan. Dan jika kontraknya berubah, kode panggilan harus diperbarui.
Untuk mendukung pendapat ini, penilaian berikut dibuat: memeriksa nol adalah seperti membungkus setiap panggilan dalam try / catch jika metode mulai melempar pengecualian di masa depan.
Akibatnya, sesuai dengan prinsip
YAGNI , mereka memutuskan untuk tidak memegang cek dan menghapusnya. Semua peringatan ditransfer dari teoretis / formal ke dibenarkan.
Saya akan senang membaca pendapat Anda di komentar.
Kesimpulan
Analisis statis adalah hal yang baik. PVS-Studio memungkinkan Anda menemukan kesalahan nyata.
Ya, ada peringatan yang tidak masuk akal / salah. Tapi tetap saja PVS-Studio menemukan kesalahan nyata dalam proyek yang sudah menggunakan Analisis Kode. Produk kami tercakup dengan baik oleh pengujian, ini adalah satu atau lain cara diuji oleh pelanggan kami, tetapi
robot melakukannya dengan lebih baik Analisis statis masih menguntungkan.
Akhirnya, beberapa statistik.
3 Peringatan Teratas yang Tidak Beralasan
V3081 „
Penghitung 'X' tidak digunakan di dalam loop bersarang. Pertimbangkan untuk memeriksa penggunaan penghitung 'Y' “
1 dari 1 ditemukan tidak masuk akal
V3125 "Objek digunakan setelah itu diverifikasi terhadap nol. Periksa baris: N1, N2 “
9 dari 10 dinyatakan tidak berdasar
V3134 "Shift by N bits lebih besar dari ukuran tipe"
4 dari 5 ditemukan tidak berdasar
3 Peringatan Penting Top
V3083 “Doa acara yang tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya “
5 dari 5 dianggap penting.
V3020 "'break / continue / return / goto' tanpa syarat dalam satu lingkaran"
V3080 “Kemungkinan null dereference”
2 dari 2 diakui sebagai penting.
V3019 "Ada kemungkinan bahwa variabel yang salah dibandingkan dengan nol setelah konversi jenis menggunakan kata kunci 'sebagai'"
V3127 "Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'X' harus digunakan daripada 'Y' "
1 dari 1 dianggap penting.