Jika Anda membaca teks ini, Anda mungkin berpikir ada sesuatu yang salah dengan tajuk utama atau Anda telah melihat nama permainan komputer yang akrab. VVVVVV adalah game platformer indie yang telah mencuri hati banyak pemain dengan kesederhanaan eksternal yang menyenangkan dan kompleksitas internal yang tidak kalah menyenangkan. Beberapa hari yang lalu, VVVVVV berusia 10 tahun, dan penulis permainan - Terry Cavanagh - merayakan liburan ini dengan menerbitkan kode sumbernya. Hal-hal menakjubkan apa yang disembunyikannya? Baca jawabannya di artikel ini.
Pendahuluan
Oh, VVVVVVV ... Saya ingat pernah melihatnya sesaat setelah dirilis dan menjadi penggemar game retro pixel, saya sangat bersemangat untuk menginstalnya di komputer saya. Saya ingat kesan pertama saya: "Itu saja? Hanya berlari di sekitar ruang-ruang persegi? ”Pikirku setelah beberapa menit bermain. Saya tidak tahu apa yang menunggu saya saat itu. Segera setelah saya keluar dari lokasi awal, saya menemukan diri saya di dunia dua dimensi yang kecil tapi membingungkan dan penuh dengan pemandangan yang tidak biasa dan artefak pixel yang tidak saya ketahui.
Saya terbawa oleh permainan. Akhirnya, saya benar-benar mengalahkan permainan meskipun ada beberapa tantangan, seperti kompleksitas tinggi dengan kontrol permainan yang diterapkan dengan terampil, misalnya - karakter utama tidak dapat melompat, tetapi mampu membalikkan arah vektor gravitasi pada dirinya sendiri. Saya tidak tahu berapa kali karakter saya mati, tetapi saya yakin jumlah kematian diukur dalam puluhan ratusan. Lagipula, setiap game memiliki semangat uniknya sendiri :)
Pokoknya, mari kita kembali ke kode sumber, diposting
untuk menghormati ulang tahun permainan .
Saat ini, saya adalah pengembang PVS-Studio, yang merupakan penganalisa kode statis untuk C, C ++, C #, dan Java. Selain mengembangkan langsung, kami juga terlibat dalam promosi produk kami. Bagi kami, salah satu cara terbaik untuk melakukan ini adalah menulis artikel tentang memeriksa proyek sumber terbuka. Pembaca kami mendapatkan artikel menarik tentang topik pemrograman, dan kami mendapat kesempatan untuk menunjukkan kemampuan PVS-Studio. Jadi ketika saya mendengar tentang pembukaan kode sumber VVVVVV, saya tidak bisa melewatinya.
Pada artikel ini, kita akan melihat beberapa kesalahan menarik yang ditemukan oleh alat analisa PVS-Studio dalam kode VVVVVV, dan melihat detail kesalahan ini. Arahkan vektor gravitasi ke bawah dan buat diri Anda nyaman - kami baru akan memulai!
Ikhtisar Peringatan Analyzer
Peringatan 1
V512 Panggilan fungsi 'sprintf' akan menyebabkan meluapnya buffer 'fileSearch'. FileSystemUtils.cpp 307
#define MAX_PATH 260 .... void PLATFORM_migrateSaveData(char *output) { char oldLocation[MAX_PATH]; char newLocation[MAX_PATH]; char oldDirectory[MAX_PATH]; char fileSearch[MAX_PATH]; .... strcpy(oldDirectory, output); sprintf(fileSearch, "%s\\*.vvvvvv", oldDirectory); .... }
Seperti yang Anda lihat, string
fileSearch dan
oldDirectory berukuran sama: 260 karakter. Setelah menulis konten string
oldDirectory dalam string format (argumen
sprintf ketiga), akan terlihat seperti:
<i>contents_oldDirectory\*.vvvvvv</i>
Baris ini lebih panjang 9 karakter dari nilai asli
oldDirectory . Urutan karakter inilah yang akan ditulis dalam
fileSearch . Apa yang terjadi jika panjang string
oldDirectory lebih dari 251? String yang dihasilkan akan lebih panjang dari yang bisa ditemukan oleh
fileSearch , yang akan menyebabkan pelanggaran batas array. Data apa dalam RAM yang dapat rusak dan apa akibatnya adalah masalah retoris :)
Peringatan 2
V519 Variabel 'latar belakang' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 1367, 1373. Map.cpp 1373
void mapclass::loadlevel(....) { .... case 4:
Variabel yang sama diberikan nilai dua kali berturut-turut. Namun, variabel ini tidak digunakan di antara penugasan. Yang aneh ... Urutan ini mungkin tidak melanggar logika program, tetapi penugasan itu sendiri menunjukkan beberapa kebingungan saat menulis kode. Apakah ini kesalahan sebenarnya - hanya penulis yang bisa mengatakannya dengan pasti. Meskipun ada contoh yang lebih jelas dari kesalahan ini dalam kode:
void Game::loadquick(....) { .... else if (pKey == "frames") { frames = atoi(pText); frames = 0; } .... }
Dalam kasus ini, jelas bahwa kesalahan bersembunyi di suatu tempat baik dalam logika atau dalam tugas yang berlebihan. Mungkin, baris kedua ditulis sementara untuk debugging, dan kemudian dibiarkan terlupakan. Secara total, PVS-Studio mengeluarkan 8 peringatan tentang kasus tersebut.
Peringatan 3
Objek 'pKey' V808 dari tipe 'basic_string' telah dibuat tetapi tidak digunakan. editor.cpp 1866
void editorclass::load(std::string &_path) { .... std::string pKey(pElem->Value()); .... if (pKey == "edEntities") { int i = 0; for (TiXmlElement *edEntityEl = pElem->FirstChildElement(); edEntityEl; edEntityEl = edEntityEl->NextSiblingElement()) { std::string pKey(edEntityEl->Value());
Kode ini sangat aneh. Penganalisa memperingatkan tentang variabel
pKey yang dibuat tetapi tidak digunakan, tetapi pada kenyataannya, masalahnya lebih menarik. Saya sengaja menyoroti garis yang memicu peringatan dengan panah, karena fungsi ini berisi lebih dari satu definisi string dengan nama
pKey . Itu benar, variabel lain seperti itu dinyatakan di dalam
for loop. Ini tumpang tindih dengan yang dinyatakan di luar loop.
Jadi, jika Anda merujuk pada nilai string
pKey di luar
for loop, Anda akan mendapatkan nilai yang sama dengan
pElem-> Value () , tetapi ketika melakukan hal yang sama di dalam loop, Anda akan mendapatkan nilai yang sama dengan
edEntityEl -> Nilai () . Nama yang tumpang tindih adalah kesalahan yang agak kasar, yang mungkin sangat sulit ditemukan sendiri saat Anda meninjau kode.
Peringatan 4
V805 Menurunkan Kinerja. Tidak efisien untuk mengidentifikasi string kosong dengan menggunakan konstruksi 'strlen (str)> 0'. Cara yang lebih efisien adalah dengan memeriksa: str [0]! = '\ 0'. physfs.c 1604
static char *prefDir = NULL; .... const char *PHYSFS_getPrefDir(const char *org, const char *app) { .... assert(strlen(prefDir) > 0); ... return prefDir; }
Penganalisa menemukan sebuah fragmen untuk optimasi mikro potensial. Ini menggunakan fungsi
strlen untuk memeriksa apakah string kosong. Fungsi ini melintasi semua elemen string dan memeriksa masing-masingnya untuk null-terminator ('\ 0'). Jika kita mendapatkan string panjang, masing-masing karakter akan dibandingkan dengan null-terminator.
Tapi kita hanya perlu memeriksa bahwa string kosong! Yang perlu Anda lakukan adalah mencari tahu apakah karakter string pertama adalah null terminal. Oleh karena itu, untuk mengoptimalkan pemeriksaan ini di dalam pernyataan, ada baiknya menulis:
str[0] != '\0'
Itulah rekomendasi yang diberikan oleh analis. Tentu, panggilan fungsi strlen dalam kondisi makro yang
tegas , oleh karena itu hanya akan dijalankan dalam versi debugging, di mana kecepatannya tidak begitu penting. Dalam versi rilis, panggilan fungsi dan kode akan dieksekusi dengan cepat. Meskipun demikian, saya ingin menunjukkan apa yang dapat disarankan oleh analis kami dalam hal optimasi mikro.
Peringatan 5
Untuk menunjukkan inti dari kesalahan lain, saya harus mengutip dua fragmen kode di sini: deklarasi kelas
entclass dan konstruktornya. Mari kita mulai dengan deklarasi:
class entclass { public: entclass(); void clear(); bool outside(); public:
Konstruktor kelas ini terlihat sebagai berikut:
entclass::entclass() { clear(); } void entclass::clear() {
Cukup banyak bidang, bukan begitu? Tidak heran, PVS-Studio mengeluarkan peringatan untuk bug, bersembunyi di sini:
V730 Ada kemungkinan bahwa tidak semua anggota kelas diinisialisasi di dalam konstruktor. Pertimbangkan untuk memeriksa: oldxp, oldyp. Ent.cpp 3
Seperti yang Anda lihat, inisialisasi dua bidang kelas hilang dalam daftar yang begitu panjang. Akibatnya, nilai-nilai mereka tetap tidak terdefinisi, sehingga nilai-nilai tersebut dapat dibaca dan digunakan secara salah di tempat lain dalam program. Sangat sulit untuk mendeteksi kesalahan seperti itu hanya dengan meninjau.
Peringatan 6
Lihatlah kode ini:
void mapclass::loadlevel(....) { .... std::vector<std::string> tmap; .... tmap = otherlevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); ....
Peringatan PVS-Studio:
V688 Variabel lokal 'tmap' memiliki nama yang sama dengan salah satu anggota kelas, yang dapat mengakibatkan kebingungan. Map.cpp 1192
Memang, melihat ke dalam kelas
mapclass , Anda dapat menemukan vektor yang sama dengan nama yang sama di sana:
class mapclass { public: .... std::vector <int> roomdeaths; std::vector <int> roomdeathsfinal; std::vector <int> areamap; std::vector <int> contents; std::vector <int> explored; std::vector <int> vmult; std::vector <std::string> tmap;
Sayangnya, deklarasi vektor nama yang sama di dalam fungsi membuat vektor yang dinyatakan dalam kelas tidak terlihat. Ternyata vektor
tmap hanya diubah di dalam fungsi
loadlevel . Vektor yang dideklarasikan di kelas tetap sama!
Menariknya, PVS-Studio telah menemukan 20 fragmen kode seperti itu! Sebagian besar, mereka berhubungan dengan variabel sementara yang telah dinyatakan "untuk kenyamanan" sebagai anggota kelas. Penulis game (dan satu-satunya pengembangnya) menulis tentang dirinya sendiri yang dulunya memiliki kebiasaan buruk ini. Anda dapat membacanya di pos - tautan diberikan di awal artikel.
Dia juga mencatat bahwa nama-nama tersebut menyebabkan bug berbahaya yang sulit dideteksi. Yah, kesalahan seperti itu mungkin sangat merusak, tetapi menangkapnya menjadi lebih sulit jika Anda menggunakan analisis statis :)
Peringatan 7
V601 Tipe integer secara implisit dilemparkan ke tipe char. Game.cpp 4997
void Game::loadquick(....) { .... else if (pKey == "totalflips") { totalflips = atoi(pText); } else if (pKey == "hardestroom") { hardestroom = atoi(pText);
Untuk memahami apa yang terjadi, mari kita lihat definisi variabel dari bagian kode yang diberikan:
variabel
totalflips dan
hardestroomdeath adalah integer, jadi sangat normal untuk menetapkannya sebagai hasil dari fungsi
atoi . Tetapi apa yang terjadi jika Anda menetapkan nilai integer ke
std :: string ? Penugasan semacam itu ternyata valid dari perspektif bahasa. Akibatnya, nilai yang tidak jelas akan ditulis dalam variabel
hardestroom !
Peringatan 8
V1004 Pointer 'pElem' digunakan dengan tidak aman setelah diverifikasi terhadap nullptr. Periksa baris: 1739, 1744. editor.cpp 1744
void editorclass::load(std::string &_path) { .... TiXmlHandle hDoc(&doc); TiXmlElement *pElem; TiXmlHandle hRoot(0); version = 0; { pElem = hDoc.FirstChildElement().Element();
Penganalisa memperingatkan bahwa pointer
pElem tidak aman digunakan tepat setelah memeriksa
nullptr . Untuk memastikan penganalisisnya benar, mari kita periksa definisi fungsi
Element () yang mengembalikan nilai yang, pada gilirannya, menginisialisasi
poer pElem:
TiXmlElement *Element() const { return ToElement(); }
Seperti yang dapat kita lihat dari komentar, fungsi ini mungkin mengembalikan
nol .
Sekarang bayangkan itu benar-benar terjadi. Apa yang akan terjadi dalam kasus ini? Faktanya adalah bahwa situasi ini tidak akan ditangani dengan cara apa pun. Ya, akan ada pesan bahwa ada yang tidak beres, tetapi penunjuk yang salah akan disangkal hanya satu baris di bawah ini. Dereferencing seperti itu akan mengakibatkan crash program atau perilaku yang tidak terdefinisi. Ini adalah kesalahan yang cukup serius.
Peringatan 9
Fragmen kode ini memicu empat peringatan penganalisa PVS-Studio:
- V560 Bagian dari ekspresi kondisional selalu benar: x> = 0. editor.cpp 1137
- V560 Bagian dari ekspresi kondisional selalu benar: y> = 0. editor.cpp 1137
- V560 Bagian dari ekspresi kondisional selalu benar: x <40. editor.cpp 1137
- V560 Bagian dari ekspresi kondisional selalu benar: y <30. editor.cpp 1137
int editorclass::at( int x, int y ) { if(x<0) return at(0,y); if(y<0) return at(x,0); if(x>=40) return at(39,y); if(y>=30) return at(x,29); if(x>=0 && y>=0 && x<40 && y<30) { return contents[x+(levx*40)+vmult[y+(levy*30)]]; } return 0; }
Semua peringatan terkait dengan pernyataan
if terakhir. Masalahnya adalah bahwa keempat pemeriksaan, dilakukan di dalamnya, akan selalu kembali
benar . Saya tidak akan mengatakan itu adalah kesalahan serius, tapi itu cukup lucu. Penulis memutuskan untuk mengambil fungsi ini dengan serius dan untuk berjaga-jaga memeriksa setiap variabel lagi :)
Dia bisa saja menghapus cek ini, karena alur eksekusi tidak akan sampai ke ekspresi "
return 0; " Itu tidak akan mengubah logika program, tetapi akan membantu untuk menyingkirkan cek yang berlebihan dan kode mati.
Peringatan 10
Dalam artikelnya tentang ulang tahun permainan, Terry dengan ironisnya mencatat bahwa salah satu elemen yang mengendalikan logika permainan adalah pergantian besar dari fungsi
Game :: updatestate () , yang bertanggung jawab atas sejumlah besar kondisi permainan yang berbeda pada saat yang sama. . Dan sangat diharapkan bahwa saya akan menemukan peringatan berikut:
V2008 Kompleksitas siklus: 548. Pertimbangkan untuk melakukan refactoring pada fungsi 'Game :: updatestate'. Game.cpp 612
Ya, Anda benar: PVS-Studio memberi fungsi peringkat kompleksitas berikut - 548. Lima ratus empat puluh delapan !!! Ini adalah bagaimana "kode rapi" terlihat. Dan ini terlepas dari kenyataan bahwa, kecuali untuk pernyataan switch hampir tidak ada yang lain dalam fungsi. Di sakelar itu sendiri, saya menghitung lebih dari 300 ekspresi huruf.
Anda tahu, di perusahaan kami, kami memiliki persaingan kecil untuk artikel terpanjang. Saya ingin membawa seluruh kode fungsi (3.450 baris) ke sini, tetapi kemenangan seperti itu tidak adil, jadi saya hanya akan membatasi diri pada
tautan ke sakelar raksasa. Saya sarankan Anda mengikuti tautan dan lihat sendiri panjangnya! Untuk masalah itu, selain
Game :: updatestate () , PVS-Studio juga menemukan 44 fungsi dengan kompleksitas siklomatik yang meningkat, 10 di antaranya memiliki jumlah kompleksitas lebih dari 200.
Kesimpulan
Saya pikir kesalahan di atas sudah cukup untuk artikel ini. Ya, ada banyak kesalahan dalam proyek, tetapi ini semacam fitur. Dengan membuka kodenya, Terry Cavanagh menunjukkan bahwa Anda tidak harus menjadi programmer yang sempurna untuk menulis gim yang hebat. Sekarang, 10 tahun kemudian, Terry mengenang masa-masa itu dengan ironi. Penting untuk belajar dari kesalahan Anda, dan berlatih adalah cara terbaik untuk melakukannya. Dan jika latihan Anda dapat memunculkan game seperti VVVVVV, itu luar biasa! Yah ... Ini saatnya untuk memainkannya sekali lagi :)
Ini tidak semua kesalahan ditemukan dalam kode permainan. Jika Anda ingin melihat sendiri apa lagi yang bisa ditemukan - Saya sarankan Anda
mengunduh dan mencoba PVS-Studio ! Juga, jangan lupa bahwa kami
menyediakan proyek sumber terbuka dengan lisensi gratis.