Menjelang perilisan Amnesia: Rebirth, Fractional Games telah merilis kode sumber untuk Amnesia: The Dark Descent yang legendaris dan sekuelnya, Amnesia: A Machine For Pigs. Mengapa tidak menggunakan alat analisis statis untuk melihat seberapa parah kesalahan tersembunyi di dalam game horor kultus ini?
Melihat di Reddit berita bahwa kode sumber dari game " Amnesia: The Dark Descend " dan " Amnesia: A Machine for Pigs " telah diterbitkan, saya tidak dapat berjalan melewati dan memeriksa kode ini menggunakan PVS-Studio, dan pada saat yang sama menulis tentang artikel ini. Terutama mengingat fakta bahwa pada tanggal 20 Oktober (dan pada saat artikel ini diterbitkan, sudah keluar) bagian baru dari seri ini: " Amnesia: Rebirth ".
Amnesia: The Dark Descent dirilis pada tahun 2010 dan telah menjadi game horor survival ikonik. Sejujurnya, saya tidak pernah bisa melewatinya, bahkan sedikit pun, karena saya memainkan game horor menurut algoritme yang sama: bertaruh, enter selama lima menit, keluar dengan "alt + f4" pada saat creep pertama dan hapus game tersebut.Tapi saya suka menonton bagian dari permainan ini di YouTube.
Dan jika seseorang belum terbiasa dengan PVS-Studio, ini adalah penganalisis statis yang mencari kesalahan dan tempat mencurigakan di kode sumber program.
Saya terutama suka melihat kode sumber permainan, jadi jika Anda bertanya-tanya kesalahan apa yang dibuat di dalamnya, Anda dapat membaca artikel saya sebelumnya. Nah, atau simak artikel rekan saya tentang mengecek source code game.
Setelah diperiksa, ternyata banyak kode di "The Dark Descend" dan "A Machine For Pigs" yang tumpang tindih, dan laporan untuk kedua project tersebut sangat mirip. Jadi hampir semua error yang akan saya kutip di bawah ini terdapat di kedua project tersebut.
Dan lapisan kesalahan terbesar dari semua penganalisis yang terdeteksi dalam proyek ini adalah lapisan kesalahan "salin-tempel". Karena itulah judul artikelnya. Alasan utama kesalahan ini adalah " efek baris terakhir ".
Baiklah, mari kita mulai bisnis.
Kesalahan salin-tempel
Ada banyak tempat mencurigakan yang mirip dengan penyalinan lalai. Beberapa kasus, mungkin, entah bagaimana disebabkan oleh logika internal dari game itu sendiri. Tetapi jika mereka mempermalukan saya dan penganalisis, setidaknya ada baiknya memberikan komentar tentang mereka. Lagipula, pengembang lain bisa juga sama lambannya dengan saya.
Cuplikan 1.
Mari kita mulai dengan contoh di mana seluruh fungsi terdiri dari membandingkan hasil metode dan bidang dari dua objek aObjectDataA dan aObjectDataB . Saya akan memberikan fungsi ini sepenuhnya untuk kejelasan. Coba perhatikan sendiri di mana kesalahan itu dibuat dalam fungsi:
static bool SortStaticSubMeshesForBodies(const ....& aObjectDataA,
const ....& aObjectDataB)
{
//Is shadow caster check
if( aObjectDataA.mpObject->GetRenderFlagBit(....)
!= aObjectDataB.mpObject->GetRenderFlagBit(....))
{
return aObjectDataA.mpObject->GetRenderFlagBit(....)
< aObjectDataB.mpObject->GetRenderFlagBit(....);
}
//Material check
if( aObjectDataA.mpPhysicsMaterial != aObjectDataB.mpPhysicsMaterial)
{
return aObjectDataA.mpPhysicsMaterial < aObjectDataB.mpPhysicsMaterial;
}
//Char collider or not
if( aObjectDataA.mbCharCollider != aObjectDataB.mbCharCollider)
{
return aObjectDataA.mbCharCollider < aObjectDataB.mbCharCollider;
}
return aObjectDataA.mpObject->GetVertexBuffer()
< aObjectDataA.mpObject->GetVertexBuffer();
}Sebuah gambar, agar tidak secara tidak sengaja memata-matai jawabannya:
Bisakah Anda menemukan kesalahannya? Ya, hasil terakhir adalah perbandingan menggunakan aObjectDataA di kedua sisi. Perhatikan bahwa semua ekspresi dalam kode asli ditulis dalam satu baris, di sini saya memiliki tanda hubung sehingga semuanya pas dengan lebar garis. Bayangkan apa yang akan dicari cacat seperti itu di penghujung hari kerja. Dan penganalisis akan menemukannya segera setelah merakit dan menjalankan analisis inkremental.
V501 Ada sub-ekspresi identik 'aObjectDataA.mpObject-> GetVertexBuffer ()' di kiri dan kanan operator '<'. WorldLoaderHplMap.cpp 1123
Akibatnya, kesalahan seperti itu akan ditemukan hampir pada saat menulis kode, alih-alih bersembunyi di kedalaman kode dari beberapa tahap QA, membuat pencarian Anda berkali-kali lebih sulit.
Catatan oleh kolega Andrey Karpov. Ya, ini adalah "kesalahan baris terakhir" klasik. Namun, ini juga merupakan pola kesalahan klasik saat membandingkan dua objek. Lihat artikel " Kehidupan Jahat dalam Fungsi Perbandingan ".Fragmen 2.
Mari kita lihat kode yang menyebabkan peringatan:
Saya menyajikan kode dengan tangkapan layar untuk kejelasan.
Peringatan itu sendiri terlihat seperti ini:
V501 Ada sub-ekspresi identik 'lType == eLuxJournalState_OpenNote' di kiri dan kanan '||' operator. LuxJournal.cpp 2262
Penganalisis telah mendeteksi bahwa ada kesalahan dalam memeriksa nilai variabel lType . Kesetaraan diperiksa dua kali dengan anggota enumerator eLuxJournalState_OpenNote yang sama .
Pertama, saya ingin kondisi seperti itu ditulis dalam bentuk "tabel" agar lebih mudah dibaca. Lihat bab N13 dalam buku mini The Biggest Question of Programming, Refactoring, and All That .
if(!( lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenDiary
|| lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenNarratedDiary))
return false;Dalam formulir ini, akan lebih mudah untuk menemukan kesalahan bahkan tanpa penganalisis.
Namun, muncul pertanyaan, apakah pemeriksaan yang salah seperti itu menyebabkan distorsi logika program? Lagi pula, mungkin Anda perlu memeriksa beberapa nilai lType lainnya , tetapi pemeriksaan itu terlewatkan karena kesalahan salin-tempel. Mari kita lihat pencacahan itu sendiri:
enum eLuxJournalState
{
eLuxJournalState_Main,
eLuxJournalState_Notes,
eLuxJournalState_Diaries,
eLuxJournalState_QuestLog,
eLuxJournalState_OpenNote,
eLuxJournalState_OpenDiary,
eLuxJournalState_OpenNarratedDiary,
eLuxJournalState_LastEnum,
};Hanya ada tiga arti yang memiliki kata "Open" di namanya. Dan ketiganya hadir di cek. Kemungkinan besar, tidak ada distorsi logika di sini, tetapi masih tidak mungkin untuk mengatakan dengan pasti. Jadi, penganalisis menemukan kesalahan logis yang dapat diperbaiki oleh pengembang game, atau menemukan tempat tertulis "jelek" yang seharusnya ditulis ulang untuk kejelasan yang lebih baik.
Fragmen 3.
Kasus berikut umumnya merupakan contoh kesalahan salin-tempel yang paling jelas.
V778 Dua fragmen kode serupa ditemukan. Mungkin, ini salah ketik dan variabel 'mvSearcherIDs' harus digunakan sebagai pengganti 'mvAttackerIDs'. LuxSavedGameTypes.cpp 615
void cLuxMusicHandler_SaveData::ToMusicHandler(....)
{
....
// Enemies
//Attackers
for(size_t i=0; i<mvAttackerIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap
->GetEntityByID(mvAttackerIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
//Searchers
for(size_t i=0; i<mvSearcherIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap->GetEntityByID(mvSearcherIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
}Dalam siklus pertama, pekerjaan berjalan dengan penunjuk pEntity , yang diterima melalui mvAttackerID, dan jika kondisi tidak terpenuhi, pesan untuk debugging akan dikeluarkan untuk mvAttackerID yang sama . Namun, di loop berikutnya, yang gayanya persis sama seperti bagian kode sebelumnya, pEntity diperoleh menggunakan mvSearcherIDs . Namun peringatan tersebut masih dikeluarkan dengan menyebutkan mvAttackerIDs .
Kemungkinan besar, blok kode bertanda "Pencari" disalin dari blok "Penyerang", mvAttackerID diganti dengan mvSearcherID , tetapi blok lain tidak diubah. Akibatnya, pesan kesalahan menggunakan elemen dari larik yang salah.
Kesalahan ini tidak mempengaruhi logika permainan, tetapi dengan cara ini Anda dapat menempatkan babi serius pada orang yang harus men-debug tempat ini dan membuang waktu bekerja dengan elemen mvSearcherIDs yang salah .
Fragmen 4.
Penganalisis menunjukkan tempat yang mencurigakan berikut dengan tiga peringatan:
- V547 Ekspresi 'pEntity == 0' selalu salah. LuxScriptHandler.cpp 2444
- V649 Ada dua pernyataan 'jika' dengan ekspresi kondisional yang identik. Pernyataan 'if' pertama berisi pengembalian fungsi. Ini berarti pernyataan 'jika' kedua tidak masuk akal. Periksa baris: 2433, 2444. LuxScriptHandler.cpp 2444
- V1051 Pertimbangkan untuk memeriksa kesalahan cetak. Ada kemungkinan bahwa 'pTargetEntity' harus diperiksa di sini. LuxScriptHandler.cpp 2444
Mari kita lihat kodenya:
void __stdcall cLuxScriptHandler::PlaceEntityAtEntity(....)
{
cLuxMap *pMap = gpBase->mpMapHandler->GetCurrentMap();
iLuxEntity *pEntity = GetEntity(....);
if(pEntity == NULL) return;
if(pEntity->GetBodyNum() == 0)
{
....
}
iPhysicsBody *pBody = GetBodyInEntity(....);
if(pBody == NULL) return;
iLuxEntity *pTargetEntity = GetEntity(....);
if(pEntity == NULL) return; // <=
iPhysicsBody *pTargetBody = GetBodyInEntity(....);
if(pTargetBody == NULL) return;
....
}Peringatan V547 dikeluarkan untuk pemeriksaan kedua pEntity == NULL . Untuk penganalisis, pemeriksaan ini akan selalu salah , karena jika kondisi ini benar , maka keluar dari fungsi akan terjadi lebih tinggi karena pemeriksaan serupa sebelumnya.
Peringatan berikutnya, V649 dikeluarkan justru karena kami memiliki dua pemeriksaan yang identik. Biasanya kasus ini mungkin bukan error. Tiba-tiba, satu bagian kode mengimplementasikan satu logika, dan di bagian lain kode, menurut pemeriksaan yang sama, sesuatu yang lain harus diterapkan. Tetapi dalam hal ini, badan pemeriksaan pertama terdiri dari pengembalian, jadi cek kedua, kalau kondisinya ternyata benar, malah tidak akan mencapai titik. Dengan melacak logika tersebut, penganalisis mengurangi jumlah pesan palsu untuk kode yang mencurigakan dan mengeluarkannya hanya untuk logika yang sangat aneh.
Kesalahan yang ditunjukkan oleh peringatan terakhir sangat mirip dengan contoh sebelumnya. Kemungkinan besar, semua pemeriksaan diduplikasi dari pemeriksaan pertama if (pEntity == NULL) , dan kemudian objek yang dicentang diganti dengan yang diperlukan. Dalam kasus objek pBody dan pTargetBody, penggantian telah dilakukan, tetapi objek pTargetEntity dilupakan. Akibatnya, objek ini tidak dicentang.
Pada contoh yang kami pertimbangkan, jika Anda menggali sedikit lebih dalam ke kode, ternyata kesalahan seperti itu secara umum tidak akan mempengaruhi jalannya program. Pointer pTargetBody mendapatkan nilainya dari fungsi GetBodyInEntity :
iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity,
asTargetBodyName);Argumen pertama di sini hanyalah penunjuk yang sebelumnya tidak dicentang, yang tidak digunakan di tempat lain. Dan untungnya, di dalam fungsi ini ada pemeriksaan argumen pertama untuk NULL :
iPhysicsBody* ....::GetBodyInEntity(iLuxEntity* apEntity, ....)
{
if(apEntity == NULL){
return NULL;
}
....
}Alhasil, kode ini pada akhirnya berfungsi dengan baik, meski mengandung error.
Fragmen 5.
Dan satu lagi tempat mencurigakan dengan copy-paste!
Metode ini membersihkan bidang objek kelas cLuxPlayer .
void cLuxPlayer::Reset()
{
....
mfRoll=0;
mfRollGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
mfLeanRoll=0;
mfLeanRollGoal=0;
mfLeanRollSpeedMul=0;
mfLeanRollMaxSpeed=0;
mvCamAnimPos =0;
mvCamAnimPosGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
....
}Tetapi untuk beberapa alasan, dua variabel mfRollSpeedMul dan mfRollMaxSpeed dibatalkan dua kali:
- V519 Variabel 'mfRollSpeedMul' diberi nilai dua kali berturut-turut. Mungkin ini salah. Periksa baris: 298, 308. LuxPlayer.cpp 308
- V519 Variabel 'mfRollMaxSpeed' diberi nilai dua kali berturut-turut. Mungkin ini salah. Periksa baris: 299, 309. LuxPlayer.cpp 309
Mari kita lihat kelas itu sendiri dan lihat bidang apa yang dikandungnya:
class cLuxPlayer : ....
{
....
private:
....
float mfRoll;
float mfRollGoal;
float mfRollSpeedMul;
float mfRollMaxSpeed;
float mfLeanRoll;
float mfLeanRollGoal;
float mfLeanRollSpeedMul;
float mfLeanRollMaxSpeed;
cVector3f mvCamAnimPos;
cVector3f mvCamAnimPosGoal;
float mfCamAnimPosSpeedMul;
float mfCamAnimPosMaxSpeed;
....
}Menariknya, ada tiga blok variabel yang mirip dengan nama terkait: mfRoll , mfLeanRoll , dan mvCamAnimPos . Dalam Reset, ketiga blok ini dihapus, kecuali dua variabel terakhir dari blok ketiga, mfCamAnimPosSpeedMul dan mfCamAnimPosMaxSpeed . Dan di tempat kedua variabel inilah tugas duplikat ditemukan. Kemungkinan besar, semua tugas ini disalin dari blok tugas pertama, dan kemudian nama variabel diganti dengan yang diperlukan.
Mungkin dua variabel yang hilang seharusnya tidak disetel menjadi nol, tetapi probabilitas kebalikannya juga sangat tinggi. Dan penugasan ulang jelas tidak akan membantu dalam memelihara kode ini. Seperti yang Anda lihat, dalam footcloth panjang dari tindakan yang sama, Anda mungkin tidak melihat kesalahan seperti itu, dan penganalisis membantu di sini.
Fragmen 5.5.
Contoh ini sangat mirip dengan yang sebelumnya. Saya akan segera mengutip potongan kode dan peringatan penganalisis untuknya.
V519 Variabel 'mfTimePos' diberi nilai dua kali berturut-turut. Mungkin ini salah. Periksa baris: 49, 53. AnimationState.cpp 53
cAnimationState::cAnimationState(....)
{
....
mfTimePos = 0;
mfWeight = 1;
mfSpeed = 1.0f;
mfBaseSpeed = 1.0f;
mfTimePos = 0;
mfPrevTimePos=0;
....
}Variabel mfTimePos telah diberi nilai 0 dua kali.Seperti pada contoh sebelumnya, mari kita lihat deklarasi bidang ini:
class cAnimationState
{
....
private:
....
//Properties of the animation
float mfLength;
float mfWeight;
float mfSpeed;
float mfTimePos;
float mfPrevTimePos;
....
}Anda dapat melihat bahwa blok deklarasi ini juga cocok dengan urutan penetapan di cuplikan kode kesalahan, seperti pada contoh sebelumnya. Di sini, dalam tugas, alih- alih variabel mfLength, nilainya mendapat mfTimePos . Tetapi di sini kesalahan tidak dapat dijelaskan dengan menyalin blok dan "efek baris terakhir". Mungkin mfLength tidak perlu diberi nilai baru, tetapi lokasi ini masih mencurigakan.
Fragmen 6.
Penganalisis mengeluarkan sejumlah besar peringatan untuk fragmen kode berikutnya dari "Amnesia: A Machine For Pigs". Saya hanya akan memberikan sebagian dari kode yang kesalahan jenisnya sama dikeluarkan:
void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep)
{
....
if(prevMoveState != mMoveState)
{
....
//Backward
if(mMoveState == eLuxEnemyMoveState_Backward)
{
....
}
....
//Walking
else if(mMoveState == eLuxEnemyMoveState_Walking)
{
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;
....
}
....
}
}Dimana kesalahannya disini?
Penganalisis mengeluarkan peringatan berikut:
- V768 Konstanta enumerasi 'eLuxEnemyMoveState_Jogging' digunakan sebagai variabel tipe Boolean. LuxEnemyMover.cpp 672
- V768 Konstanta enumerasi 'eLuxEnemyMoveState_Walking' digunakan sebagai variabel tipe Boolean. LuxEnemyMover.cpp 680
- V768 Konstanta enumerasi 'eLuxEnemyMoveState_Jogging' digunakan sebagai variabel tipe Boolean. LuxEnemyMover.cpp 688
Rantai if-else-if diulangi dalam kode asli, dan kemudian peringatan ini hanya dikeluarkan untuk setiap badan dari masing-masing if .
Perhatikan garis yang ditunjukkan oleh parser:
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;Tidaklah mengherankan bahwa kesalahan telah merayap ke ekspresi seperti itu, yang juga ditulis dalam satu baris dalam bahasa aslinya. Dan Anda mungkin sudah menyadarinya. Anggota enumerasi eLuxEnemyMoveState_Jogging tidak dibandingkan dengan apa pun; nilainya dicentang. Kemungkinan besar, ekspresi 'prevMoveState == eLuxEnemyMoveState_Jogging' tersirat.
Kesalahan seperti itu mungkin tampak sama sekali tidak berbahaya. Tetapi di artikel saya yang lain, tentang memeriksa Bullet Engine , di antara komitmen untuk proyek tersebut, saya menemukan perbaikan bugdari jenis yang sama, mengakibatkan gaya diterapkan ke objek dari sisi yang salah. Dan dalam kasus ini, kesalahan ini dilakukan beberapa kali. Nah, saya juga mencatat bahwa kondisi terner sama sekali tidak berarti, karena akan dipenuhi, terakhir, untuk hasil Boolean operator logika.
Fragmen 7.
Terakhir, beberapa contoh kesalahan salin-tempel. Kali ini lagi dalam pernyataan bersyarat. Penganalisis mengeluarkan peringatan untuk potongan kode berikut:
void iParticleEmitter::SetSubDivUV(const cVector2l &avSubDiv)
{
//Check so that there is any subdivision
// and that no sub divison axis is
//equal or below zero
if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x >0 && avSubDiv.y >0))
{
....
}
....
}Saya rasa cukup mudah untuk melihat tempat yang mencurigakan dalam sebuah fragmen, terpisah dari semua kode. Namun, kesalahan ini berhasil disembunyikan dari pengembang game ini.
Penganalisis mengeluarkan peringatan berikut:
V501 Ada sub-ekspresi identik di kiri dan kanan '||' operator: avSubDiv.x> 1 || avSubDiv.x> 1 ParticleEmitter.cpp 199
Tanda kurung kedua dalam kondisi menunjukkan bahwa bidang x dan y dicentang . Tetapi dalam tanda kurung pertama, karena suatu alasan momen ini terlewatkan dan hanya bidang x yang dicentang... Selain itu, dilihat dari komentar validasi, kedua kolom seharusnya sudah divalidasi. Di sini, bagaimanapun, bukan "efek baris terakhir" yang berfungsi, melainkan yang pertama, karena dalam tanda kurung pertama mereka lupa mengganti panggilan ke bidang x dengan panggilan ke bidang y .
Jadi kesalahan semacam itu cukup berbahaya, karena dalam hal ini pengembang bahkan tidak terbantu dengan menulis komentar penjelasan atas kondisi tersebut.
Saya akan merekomendasikan dalam kasus seperti itu untuk menganggapnya sebagai kebiasaan untuk mencatat pemeriksaan terkait dalam bentuk tabel. Lebih mudah untuk mengedit dengan cara ini, dan kekurangannya akan lebih terlihat:
if( (avSubDiv.x > 1 || avSubDiv.x > 1)
&& (avSubDiv.x > 0 && avSubDiv.y > 0))Fragmen 7.5.
Benar-benar sama, pada kenyataannya, kesalahan ditemukan di tempat yang sama sekali berbeda:
static bool EdgeTriEqual(const cTriEdge &edge1, const cTriEdge &edge2)
{
if(edge1.tri1 == edge2.tri1 && edge1.tri2 == edge2.tri2)
return true;
if(edge1.tri1 == edge1.tri1 && edge1.tri2 == edge2.tri1)
return true;
return false;
}Nah, bagaimana Anda bisa segera melihat di mana dia bersembunyi? Bukan alasan mengapa begitu banyak contoh telah disortir :)
Penganalisis mengeluarkan peringatan berikut:
V501 Ada sub-ekspresi identik di kiri dan kanan operator '==': edge1.tri1 == edge1.tri1 Math.cpp 2914
Mari kita analisis ini fragmen secara berurutan. Jelas, pemeriksaan pertama memeriksa kesetaraan bidang edge1.tri1 dan edge2.tri2 , dan pada saat yang sama persamaan edge1.tri2 dan edge2.tri2 :
edge1.tri1 -> edge2.tri1
edge1.tri2 -> edge2.tri2Dan pada pemeriksaan kedua, dilihat dari bagian yang benar dari pemeriksaan 'edge1.tri2 == edge2.tri1', perlu untuk memeriksa kesetaraan bidang ini "melintang":
Tetapi alih-alih memeriksa edge1.tri1 == edge2.tri2 , pemeriksaan yang tidak berarti dilakukan edge1.tri1 == edge1.tri1 . Tapi ini semua konten fungsinya, saya tidak menghapus apa pun darinya. Dan semua sama, kesalahan seperti itu masuk ke dalam kode.
Kesalahan lainnya
Fragmen 1.
Saya akan memberikan fragmen kode berikut dengan indentasi asli.
void iCharacterBody::CheckMoveCollision(....)
{
....
/////////////////////////////////////
//Forward velocity reflection
//Make sure that new velocity points in the right direction
//and that it is not too large!
if(mfMoveSpeed[eCharDir_Forward] != 0)
{
vForwardVel = ....;
float fForwardSpeed = vForwardVel.Length();
if(mfMoveSpeed[eCharDir_Forward] > 0)
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
else
if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}
....
}Peringatan PVS-Studio: V563 Ada kemungkinan bahwa cabang 'else' ini harus berlaku untuk pernyataan 'jika' sebelumnya. CharacterBody.cpp 1591
Contoh ini dapat membingungkan. Kenapa lagi menjorok sama dengan jika luar ? Apakah yang lain untuk kondisi luar? Kalau begitu, Anda perlu menempatkan tanda kurung, jika tidak mengacu pada if .
if(mfMoveSpeed[eCharDir_Forward] > 0)
{
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
}
else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
{
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}Atau tidak? Dalam proses penulisan artikel ini, saya mengubah pendapat saya beberapa kali tentang varian mana dari urutan tindakan yang dimaksudkan untuk kode ini yang paling mungkin.
Jika kita mempelajari lebih dalam kode ini, ternyata variabel fForwardSpeed , yang perbandingannya dilakukan di lower if , tidak dapat memiliki nilai kurang dari nol, karena ia menerima nilai dari metode Length :
inline T Length() const
{
return sqrt( x * x + y * y + z * z);
}Kemudian, kemungkinan besar, inti dari pemeriksaan ini adalah pertama-tama kami memeriksa apakah elemen mfMoveSpeed lebih besar dari nol, lalu kami memeriksa nilainya relatif terhadap fForwardSpeed . Selain itu, dua if yang terakhir berhubungan satu sama lain dalam kata-katanya.
Dalam hal ini, kode asli akan berfungsi sebagaimana mestinya! Tapi itu pasti akan membuat orang yang datang untuk mengedit / mengubah itu menjadi teka-teki.
Saya pikir saya tidak akan pernah menemukan kode yang terlihat seperti ini. Karena tertarik, saya melihat database kesalahan yang ditemukan dalam proyek sumber terbuka dan dijelaskan dalam artikel. Dan contoh kesalahan ini ditemukan di proyek lain - Anda dapat melihatnya sendiri .
Dan tolong jangan menulis seperti itu, bahkan jika Anda sendiri jelas dalam kasus ini. Atau kurung kurawal atau lekukan yang benar, atau lebih baik, keduanya. Jangan terjun ke penderitaan orang-orang yang memahami kode Anda, dan diri mereka sendiri di masa depan;)
Fragmen 2.
Kesalahan berikutnya sedikit membingungkan saya, dan saya mencoba untuk menemukan logika di sini untuk waktu yang lama. Tetapi pada akhirnya, bagi saya masih terlihat bahwa ini kemungkinan besar adalah kesalahan, dan agak menjijikkan.
Mari kita lihat kodenya:
bool cBinaryBuffer::DecompressAndAdd(char *apSrcData, size_t alSize)
{
....
///////////////////////////
// Init decompression
int ret = inflateInit(&zipStream);
if (ret != Z_OK) return false;
///////////////////////////
// Decompress, chunk by chunk
do
{
//Set current output chunk
zipStream.avail_out = lMaxChunkSize;
....
//Decompress as much as possible to current chunk
int ret = inflate(&zipStream, Z_NO_FLUSH);
if(ret != Z_OK && ret != Z_STREAM_END)
{
inflateEnd(&zipStream);
return false;
}
....
}
while (zipStream.avail_out == 0 && ret != Z_STREAM_END);
....
return true;
}V711 Berbahaya untuk membuat variabel lokal di dalam loop dengan nama yang sama sebagai variabel yang mengontrol loop ini. BinaryBuffer.cpp 371
Jadi, kami memiliki variabel ret , yang mengontrol keluar dari loop do-while . Tetapi di dalam loop ini, alih-alih memberikan nilai baru ke variabel eksternal ini, variabel baru bernama ret dideklarasikan . Akibatnya, itu menimpa variabel eksternal ret, dan variabel yang diperiksa dalam kondisi loop tidak akan pernah berubah.
Dalam kombinasi keadaan yang tidak menguntungkan, siklus seperti itu bisa menjadi tak terbatas. Kemungkinan besar, dalam kasus ini, kode ini menyimpan kondisi internal yang memeriksa nilai ret variabel internal dan mengarah keluar dari fungsi.
Kesimpulan
Sangat sering, pengembang menggunakan analisis statis tidak secara teratur, tetapi dengan jeda yang lama. Atau mereka bahkan menjalankan proyek melalui penganalisis satu kali. Sebagai hasil dari pendekatan ini, penganalisis sering tidak mendeteksi sesuatu yang serius atau menemukan sesuatu seperti contoh yang kami pertimbangkan, yang mungkin tidak terlalu memengaruhi fungsionalitas game. Orang mendapat kesan bahwa penganalisis tidak terlalu berguna. Yah, dia menemukan tempat-tempat seperti itu, tetapi masih berfungsi.
Dan faktanya adalah bahwa tempat-tempat serupa, tetapi di mana kesalahan itu tidak tertutup, tetapi jelas menyebabkan bug dalam program tersebut, telah diperbaiki dengan berjam-jam debugging, berjalan melalui tes, dan departemen pengujian. Akibatnya, saat memeriksa proyek, penganalisis hanya menampilkan masalah yang belum terwujud dengan cara apa pun. Kadang-kadang di antara masalah seperti itu ada juga momen serius yang sangat mempengaruhi pengoperasian program, tetapi kemungkinan program akan mengikuti jalurnya rendah, dan oleh karena itu kesalahan ini tidak diketahui oleh pengembang.
Oleh karena itu, sangat penting untuk mengevaluasi kegunaan analisis statis hanya setelah penggunaan rutinnya. Jika satu kali dijalankan melalui PVS-Studio mengungkapkan tempat-tempat yang mencurigakan dan ceroboh dalam kode permainan ini, maka berapa banyak kesalahan yang jelas seperti ini harus dilokalkan dan diperbaiki dalam proses pengembangan.
Gunakan penganalisis statis secara teratur!
Jika Anda ingin berbagi artikel ini dengan pembaca berbahasa Inggris, silakan gunakan tautan terjemahan: Victoria Khanieva. Amnesia: The Dark Descent atau Cara Lupa Memperbaiki Salin Tempel .