Perhatian kami tertuju pada repositori Electronic Arts di GitHub. Ini sangat kecil dan dari dua puluh tiga proyek kami hanya tertarik pada beberapa perpustakaan C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain dan EAAssert. Proyek juga sangat kecil (sekitar 10 file), jadi kami menemukan kesalahan hanya di "terbesar" dari 20 file: D Tapi kami menemukan yang menarik juga! Saat menulis catatan, saya dan kolega-kolega saya juga dengan bersemangat mendiskusikan permainan EA dan strateginya: D
Pendahuluan
Electronic Arts (EA) adalah perusahaan Amerika yang mendistribusikan video game. Di situs web GitHub, ia memiliki
repositori kecil dan beberapa proyek C ++. Ini adalah pustaka C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain dan EAAssert. Mereka sangat kecil dan kami menemukan kesalahan menarik menggunakan penganalisis
PVS-Studio hanya di "terbesar" dari mereka -
EAStdC (20 file). Dengan volume seperti
itu ,
sulit untuk berbicara tentang kualitas kode secara keseluruhan. Cukup beri peringkat 5 peringatan dan putuskan sendiri.
Peringatan 1
V524 Aneh bahwa tubuh fungsi '>>' sepenuhnya setara dengan tubuh fungsi '<<'. EAFixedPoint.h 287
template <class T, int upShiftInt, int downShiftInt, int upMulInt, int downDivInt> struct FPTemplate { .... FPTemplate operator<<(int numBits) const { return value << numBits; } FPTemplate operator>>(int numBits) const { return value << numBits; } FPTemplate& operator<<=(int numBits) { value <<= numBits; return *this;} FPTemplate& operator>>=(int numBits) { value >>= numBits; return *this;} .... }
Ketika operator shift overloading, programmer membuat kesalahan ketik di salah satu dari mereka, membingungkan operator << dan >>. Kemungkinan besar, ini adalah hasil dari Copy-Paste-programming.
Peringatan 2
V557 Array overrun dimungkinkan. Nilai indeks 'nFormatLength' bisa mencapai 16. EASprintfOrdered.cpp 246
static const int kSpanFormatCapacity = 16; struct Span8 { .... char mFormat[kSpanFormatCapacity]; .... }; static int OVprintfCore(....) { .... EA_ASSERT(nFormatLength < kSpanFormatCapacity); if(nFormatLength < kSpanFormatCapacity) spans[spanIndex].mFormat[nFormatLength++] = *p;
Rentang
[spanIndex] .mFormat array terdiri dari
16 elemen, mis. item terakhir yang valid memiliki indeks
15 . Sekarang
kode fungsi
OVprintfCore ditulis sehingga jika indeks
nFormatLength memiliki indeks maksimum yang mungkin -
15 , maka peningkatan hingga
16 akan terjadi. Selanjutnya, dalam
pernyataan switch, dimungkinkan untuk melampaui batas array.
Fragmen kode ini disalin ke 2 tempat lain:
- V557 Array overrun dimungkinkan. Nilai indeks 'nFormatLength' bisa mencapai 16. EASprintfOrdered.cpp 614
- V557 Array overrun dimungkinkan. Nilai indeks 'nFormatLength' bisa mencapai 16. EASprintfOrdered.cpp 977
Peringatan 3
V560 Bagian dari ekspresi kondisional selalu benar: (hasil> = 0). EASprintfOrdered.cpp 489
static int OVprintfCore(....) { .... for(result = 1; (result >= 0) && (p < pEnd); ++p) { if(pWriteFunction8(p, 1, pWriteFunctionContext8, kWFSIntermediate) < 0) return -1; nWriteCountSum += result; } .... }
Hasil kondisi
> = 0 selalu benar, karena variabel
hasil tidak berubah di loop. Kode ini terlihat sangat mencurigakan dan kemungkinan besar ada kesalahan dalam kode ini.
Fragmen kode ini disalin ke 2 tempat lain:
- V560 Bagian dari ekspresi kondisional selalu benar: (hasil> = 0). EASprintfOrdered.cpp 852
- V560 Bagian dari ekspresi kondisional selalu benar: (hasil> = 0). EASprintfOrdered.cpp 1215
Peringatan 4
V1009 Periksa inisialisasi array. Hanya elemen pertama yang diinisialisasi secara eksplisit. Elemen lainnya diinisialisasi dengan nol. EASprintfOrdered.cpp 151
static int OVprintfCore(....) { .... int spanArgOrder[kArgCapacity] = { -1 }; .... }
Ini mungkin bukan kesalahan, tetapi pengembang harus diingatkan bahwa hanya elemen pertama array
spanArgOrder yang diinisialisasi dengan nilai
-1 , dan semua yang lain akan memiliki nilai 0.
Fragmen kode ini disalin ke 2 tempat lain:
- V1009 Periksa inisialisasi array. Hanya elemen pertama yang diinisialisasi secara eksplisit. Elemen lainnya diinisialisasi dengan nol. EASprintfOrdered.cpp 518
- V1009 Periksa inisialisasi array. Hanya elemen pertama yang diinisialisasi secara eksplisit. Elemen lainnya diinisialisasi dengan nol. EASprintfOrdered.cpp 881
Peringatan 5
V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. int128.h 1242
inline void int128_t::Modulus(....) const { .... bool bDividendNegative = false; bool bDivisorNegative = false; .... if( (bDividendNegative && !bDivisorNegative) || (!bDividendNegative && bDivisorNegative)) { quotient.Negate(); } .... }
Fragmen kode ini diformat untuk kenyamanan, tetapi dalam bahasa aslinya itu adalah kondisi yang sangat panjang yang sulit dibaca. Hal lain adalah jika kita menyederhanakan ekspresi kondisional, seperti yang disarankan oleh penganalisa:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
Kode menjadi lebih pendek, yang sangat menyederhanakan pemahaman logika ekspresi kondisional.
Kesimpulan
Seperti yang mungkin Anda perhatikan, sebagian besar peringatan mencurigakan digandakan di tiga tempat, dan di dalam file yang sama. Duplikasi kode adalah praktik pengembangan yang sangat buruk mempersulit dukungan kode. Dan jika kesalahan terjadi pada kode tersebut, maka stabilitas program menurun tajam karena penyebaran kesalahan di seluruh kode.
Mari berharap sesuatu yang menarik akan dipublikasikan dan kami akan kembali ke repositori ini lagi :). Sementara itu, saya menyarankan mereka yang ingin mengunduh
PVS-Studio dan mencoba memeriksa proyek mereka sendiri.
Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov.
Perpustakaan Hampir Sempurna oleh Seni Elektronik .