Perhatian kami baru-baru ini tertarik oleh repositori Electronic Arts di GitHub. Ini kecil, dan dari dua puluh tiga proyek yang tersedia di sana, hanya beberapa perpustakaan C ++ yang tampak menarik: EASTL, EAStdC, EABase, EAThread, EATest, EAMain, dan EAAssert. Proyek-proyek itu sendiri kecil juga (masing-masing sekitar 10 file), jadi bug hanya ditemukan di proyek "terbesar" dari 20 file: D Tapi kami menemukannya, dan mereka memang terlihat menarik! Ketika saya menulis posting ini, kami juga mengadakan diskusi yang bersemangat tentang game EA dan kebijakan perusahaan: D
Pendahuluan
Electronic Arts (EA) adalah perusahaan video game Amerika. Ia memiliki
repositori kecil di GitHub dan beberapa proyek C ++, yaitu pustaka C ++: EASTL, EAStdC, EABase, EAThread, EATest, EAMain, dan EAAssert. Mereka kecil, dan penganalisa
PVS-Studio berhasil menemukan bug sama sekali hanya dalam proyek "terbesar",
EAStdC (20 file). Dengan ukuran seperti itu, Anda
tidak dapat menilai kualitas kode secara
andal , jadi lihat saja lima peringatan berikut 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 membebani operator shift yang berlebihan, programmer membuat kesalahan ketik di salah satu dari mereka dengan menulis << bukan >>. Ini sangat mirip kesalahan salin-tempel.
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, jadi indeks elemen terakhir yang valid adalah
15 . Dalam bentuknya saat ini, fungsi
OVprintfCore akhirnya menambah indeks
nFormatLength ke
16 jika memiliki indeks setinggi mungkin, mis.
15 . Setelah itu, kesalahan array-out-of-bounds akan terjadi dalam pernyataan
switch .
Fragmen ini disalin dua kali lagi:
- 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> = 0 kondisi selalu benar karena variabel
hasil tidak diubah di mana pun di loop. Kode sama sekali tidak terlihat benar, dan pasti ada kesalahan di dalamnya.
Fragmen ini disalin dua kali lagi:
- 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 tidak harus berupa bug, tetapi penulis harus diingatkan bahwa hanya elemen pertama array
spanArgOrder yang diinisialisasi ke
-1 , sedangkan sisanya akan ditetapkan ke 0.
Fragmen ini disalin dua kali lagi:
- 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(); } .... }
Saya memformat contoh ini untuk kejelasan, tetapi dalam bentuk aslinya, kondisi ini sangat panjang dan sulit dibaca. Tetapi kita dapat membuatnya lebih baik dengan menyederhanakan ekspresi kondisional, seperti yang dianalisa oleh penganalisa:
if( bDividendNegative != bDivisorNegative) { quotient.Negate(); }
Kode jauh lebih pendek sekarang, yang membuat logika kondisi jauh lebih mudah dipahami.
Kesimpulan
Seperti yang mungkin Anda perhatikan, sebagian besar peringatan memiliki dua duplikat lagi, dan semua duplikat itu ditemukan dalam file yang sama. Duplikasi kode adalah anti-pola yang sangat buruk karena banyak mempersulit pemeliharaan program. Ketika bug menyusup ke dalam kode seperti itu, stabilitas program turun drastis karena mereka menyebar ke seluruh kode.
Mudah-mudahan, EA akan mengunggah beberapa proyek menarik lainnya dan kami akan mengunjungi repositori mereka sekali lagi :). Sementara itu, selamat datang untuk mengunduh
PVS-Studio dan mencobanya di proyek Anda sendiri.