Geekly articles weekly

NCBI Genome Workbench: Endangered Research

Teknologi komputer modern, solusi teknis dan perangkat lunak - semua ini sangat memudahkan dan mempercepat implementasi berbagai penelitian ilmiah. Seringkali, simulasi komputer adalah satu-satunya cara untuk menguji banyak teori. Perangkat lunak ilmiah memiliki karakteristiknya sendiri. Sebagai contoh, perangkat lunak seperti itu sering mengalami pengujian yang sangat teliti, tetapi tidak terdokumentasi dengan baik. Namun, perangkat lunak ditulis oleh orang-orang, dan orang membuat kesalahan. Kesalahan dalam program ilmiah dapat menimbulkan keraguan pada keseluruhan penelitian. Artikel ini akan mencantumkan lusinan masalah yang ditemukan dalam kode paket perangkat lunak NCBI Genome Workbench.

Pendahuluan


NCBI Genome Workbench menawarkan kepada para peneliti berbagai alat untuk mempelajari dan menganalisis data genetik. Pengguna dapat meneliti dan membandingkan data dari beberapa sumber, termasuk database NCBI (Pusat Nasional untuk Informasi Bioteknologi) atau data pribadi mereka sendiri.

Seperti disebutkan sebelumnya, perangkat lunak ilmiah biasanya tercakup dengan baik oleh unit test. Saat memeriksa proyek ini, 85 direktori dengan file uji dikeluarkan dari analisis. Ini sekitar seribu file. Mungkin ini disebabkan oleh persyaratan untuk menguji berbagai algoritma kompleks yang ditemukan untuk berbagai penelitian. Tetapi kualitas sisa kode (bukan yang diuji) tidak pada tingkat setinggi yang kita inginkan. Namun, seperti dalam setiap proyek di mana mereka belum mengurus memperkenalkan alat analisis kode statis :).

Data untuk tinjauan kode (atau bahkan penelitian) disediakan oleh penganalisa kode statis untuk C / C ++ / C # / Java - PVS-Studio .

Hanya dua angka yang merusak proyek Anda




Berdasarkan database kesalahan kami, yang saat ini berjumlah lebih dari 12 ribu contoh yang dipilih, kami memperhatikan dan menjelaskan pola spesifik untuk penulisan kode yang mengarah pada banyak kesalahan. Sebagai contoh, kami melakukan studi berikut:

  1. Efek dari baris terakhir ;
  2. Fungsi paling berbahaya di dunia C / C ++ ;
  3. Ekspresi logis dalam C / C ++. Betapa salahnya para profesional ;
  4. Jahat hidup dalam fungsi perbandingan .

Proyek ini menandai awal dari deskripsi pola baru. Kita berbicara tentang angka 1 dan 2 dalam nama variabel, misalnya, file1 dan file2 , dll. Sangat mudah untuk membingungkan dua variabel tersebut. Ini adalah kasus kesalahan ketik khusus dalam kode, tetapi satu kesalahan seperti itu menghasilkan keinginan untuk bekerja dengan variabel-variabel dengan nama yang sama, yang hanya berbeda dengan angka 1 dan 2 pada akhir nama.

Ke depan sedikit, saya akan mengatakan bahwa semua studi yang tercantum di atas dikonfirmasi dalam kode proyek ini: D.

Pertimbangkan contoh pertama dari proyek Genome Workbench:

V501 Ada sub-ekspresi identik '(! Loc1.IsInt () &&! Loc1.IsWhole ())' di sebelah kiri dan di sebelah kanan '||' operator. nw_aligner.cpp 480

CRef<CSeq_align> CNWAligner::Run(CScope &scope, const CSeq_loc &loc1, const CSeq_loc &loc2, bool trim_end_gaps) { if ((!loc1.IsInt() && !loc1.IsWhole()) || (!loc1.IsInt() && !loc1.IsWhole())) { NCBI_THROW(CException, eUnknown, "Only whole and interval locations supported"); } .... }

Kami melihat dua variabel bernama loc1 dan loc2 . Dan juga kesalahan dalam kode: variabel loc2 tidak digunakan, karena alih-alih itu loc1 digunakan sekali lagi.

Contoh lain:

V560 Bagian dari ekspresi kondisional selalu salah: s1.IsSet (). valid_biosource.cpp 3073

 static bool s_PCRPrimerSetLess(const CPCRPrimerSet& s1, const CPCRPrimerSet& s2) { if (!s1.IsSet() && s1.IsSet()) { return true; } else if (s1.IsSet() && !s2.IsSet()) { return false; } else if (!s1.IsSet() && !s2.IsSet()) { return false; } else if (s1.Get().size() < s2.Get().size()) { return true; } else if (s1.Get().size() > s2.Get().size()) { return false; } else { ..... }

Baris pertama dari kode mencampur variabel s1 dan s2 . Berdasarkan namanya, ini adalah fungsi perbandingan. Tetapi kesalahan seperti itu bisa terjadi di mana saja, karena dengan memberi nama variabel Nomor 1 dan Nomor 2 , programmer hampir pasti akan membuat kesalahan di masa depan. Dan semakin banyak menggunakan nama-nama tersebut dalam suatu fungsi, semakin tinggi kemungkinan kesalahan.

Kesalahan ketik dan salin-tempel lainnya




V501 Ada sub-ekspresi identik ke kiri dan ke kanan Operator '! =': Bd.bit_.bits [i]! = Bd.bit_.bits [i] bm.h 296

 bool compare_state(const iterator_base& ib) const { .... if (this->block_type_ == 0 { if (bd.bit_.ptr != ib_db.bit_.ptr) return false; if (bd.bit_.idx != ib_db.bit_.idx) return false; if (bd.bit_.cnt != ib_db.bit_.cnt) return false; if (bd.bit_.pos != ib_db.bit_.pos) return false; for (unsigned i = 0; i < bd.bit_.cnt; ++i) { if (bd.bit_.bits[i] != bd.bit_.bits[i]) return false; } } .... }

Saya percaya bahwa setelah semua pemeriksaan, ukuran array bit dari objek bd.bit_ dan ib_db.bit_ sama. Oleh karena itu, penulis kode menulis satu siklus untuk perbandingan elemen array bit , tetapi membuat kesalahan ketik atas nama salah satu objek yang dibandingkan. Akibatnya, objek yang dibandingkan dapat secara keliru dianggap sama dalam beberapa situasi.

Contoh ini layak untuk artikel " Jahat hidup dalam fungsi perbandingan "

V501 Ada sub-ekspresi identik 'CFieldHandler :: QualifierNamesAreEquivalent (bidang, kFieldTypeSeqId)' ke kiri dan ke kanan '||' operator. field_handler.cpp 152

 bool CFieldHandlerFactory::s_IsSequenceIDField(const string& field) { if ( CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId) || CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId)) { return true; } else { return false; } }

Kemungkinan besar, salah satu dari cek itu berlebihan. Saya tidak menemukan variabel kode yang mirip dengan kFieldTypeSeqId . Namun demikian, panggilan fungsi tambahan dimungkinkan karena operator "||", yang menurunkan kinerja.

Beberapa lagi dari jenis tempat yang sama dengan peringatan penganalisa, membutuhkan verifikasi:

  • V501 Ada sub-ekspresi identik 'uf-> GetData (). IsBool ()' di sebelah kiri dan di sebelah kanan operator '&&'. variasi_utils.cpp 1711
  • V501 Ada sub-ekspresi identik 'uf-> GetData (). IsBool ()' di sebelah kiri dan di sebelah kanan operator '&&'. variasi_utils.cpp 1735

V766 Item dengan kunci yang sama 'kArgRemote' telah ditambahkan. blast_args.cpp 3262

 void CBlastAppArgs::x_IssueWarningsForIgnoredOptions(const CArgs& args) { set<string> can_override; .... can_override.insert(kArgOutputFormat); can_override.insert(kArgNumDescriptions); can_override.insert(kArgNumAlignments); can_override.insert(kArgMaxTargetSequences); can_override.insert(kArgRemote); // <= can_override.insert(kArgNumThreads); can_override.insert(kArgInputSearchStrategy); can_override.insert(kArgRemote); // <= can_override.insert("remote_verbose"); can_override.insert("verbose"); .... }

Alat analisis mendeteksi penambahan 2 nilai identik ke wadah yang ditetapkan . Ingat bahwa wadah ini hanya menyimpan nilai unik, jadi duplikat tidak ditambahkan ke dalamnya.

Kode seperti di atas sering ditulis menggunakan metode salin-tempel. Mungkin saja ada nilai tambahan, atau mungkin penulis lupa untuk mengganti nama salah satu variabel ketika ia menyalin. Saat Anda menghapus panggilan tambahan untuk disisipkan, kode sedikit dioptimalkan, yang, bagaimanapun, tidak signifikan. Lebih penting lagi, kesalahan serius dapat disembunyikan di sini karena elemen yang hilang di set.

V523 Pernyataan 'maka' setara dengan fragmen kode berikutnya. vcf_reader.cpp 1105

 bool CVcfReader::xAssignFeatureLocationSet(....) { .... if (data.m_SetType == CVcfData::ST_ALL_DEL) { if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); //-1 for 0-based, //another -1 for inclusive end-point ( ie [], not [) ) pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; } //default: For MNV's we will use the single starting point //NB: For references of size >=2, this location will not //match the reference allele. Future Variation-ref //normalization code will address these issues, //and obviate the need for this code altogether. if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; }

Fungsi ini berisi fragmen kode besar dan benar-benar identik. Namun, mereka berisi berbagai komentar yang menyertainya. Kode tidak ditulis secara optimal, membingungkan, dan mungkin mengandung kesalahan.

Seluruh daftar tempat mencurigakan dengan pernyataan if-else terlihat seperti ini:

  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. blk.c 2142
  • V523 Pernyataan 'maka' setara dengan fragmen kode berikutnya. odbc.c 379
  • V523 Pernyataan 'maka' setara dengan fragmen kode berikutnya. odbc.c 1414
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. seqdbvol.cpp 1922
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. seqdb_demo.cpp 466
  • V523 Pernyataan 'maka' setara dengan fragmen kode berikutnya. blast_engine.c 1917
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. blast_filter.c 420
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. blast_parameters.c 636
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. unordered_spliter.cpp 684
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. bme.cpp 333
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. gme.cpp 484

/ * dengan keamanan paling baik menjadi bertele-tele * /



V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'passwd_buf'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 366

 /** * Crypt a given password using schema required for NTLMv1 authentication * @param passwd clear text domain password * @param challenge challenge data given by server * @param flags NTLM flags from server side * @param answer buffer where to store crypted password */ void tds_answer_challenge(....) { #define MAX_PW_SZ 14 .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); memset(ntlm2_challenge, 0, sizeof(ntlm2_challenge)); } else { .... } }

Seperti yang mungkin sudah Anda tebak, komentar lucu tentang keamanan dari kode digunakan pada judul bagian.

Singkatnya, fungsi memset akan dihapus oleh kompiler, karena buffer flush tidak lagi digunakan. Dan data seperti hash atau passwd_buf tidak akan benar-benar nol. Untuk informasi lebih lanjut tentang mekanisme kompiler yang tidak jelas ini, lihat artikel " Aman Membersihkan Data Pribadi ".

V597 Kompilator dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'jawab'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 561

 static TDSRET tds7_send_auth(....) { .... /* for security reason clear structure */ memset(&answer, 0, sizeof(TDSANSWER)); return tds_flush_packet(tds); }

Itu bukan satu-satunya contoh dengan komentar tentang "keamanan". Dilihat dari komentar, dapat diasumsikan bahwa keamanan sangat penting untuk proyek. Oleh karena itu, saya melampirkan seluruh daftar masalah yang tidak kecil:

  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk menyiram objek 'heap'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. ncbi_heapmgr.c 1300
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'konteks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 167
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 339
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'md5_ctx'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 353
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'hash'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 365
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 406
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ntlm_v2_response'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. login.c 795
  • V597 Kompilator dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'jawab'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. login.c 801
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'paket'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. numeric.c 256
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'paket'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. numeric.c 110
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'pwd'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. getpassarg.c 50
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'konteks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 188
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'buf'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 243
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'ntlm_v2_hash'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 309
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'md5_ctx'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 354
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'passwd_buf'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 380
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 393
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'hash'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 394
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'ntlm2_challenge'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. tantangan.c 395
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ks'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 419
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ntlm_v2_response'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. challenge.c 556

Siklus yang mencurigakan




V534 Kemungkinan variabel yang salah sedang dibandingkan di dalam operator 'untuk'. Pertimbangkan untuk meninjau 'i'. taxFormat.cpp 569

 void CTaxFormat::x_LoadTaxTree(void) { .... for(size_t i = 0; i < alignTaxids.size(); i++) { int tax_id = alignTaxids[i]; .... for(size_t j = 0; i < taxInfo.seqInfoList.size(); j++) { SSeqInfo* seqInfo = taxInfo.seqInfoList[j]; seqInfo->taxid = newTaxid; } .... } .... }

Saya pikir, dalam kondisi loop dalam, variabel saya mendapatkannya secara acak. Sebagai gantinya, variabel j harus digunakan.

V535 Variabel 'i' digunakan untuk loop ini dan untuk loop luar. Periksa baris: 302, 309. sls_alp.cpp 309

 alp::~alp() { .... if(d_alp_states) { for(i=0;i<=d_nalp;i++) // <= { if(i<=d_alp_states->d_dim) { if(d_alp_states->d_elem[i]) { for(i=0;i<=d_nalp;i++) // <= { .... .... }

Dua siklus identik bersarang, di mana penghitung global juga diatur ulang, terlihat sangat mencurigakan. Pengembang harus memeriksa apa yang terjadi di sini.

Array pengindeksan tidak normal




V520 Operator koma ',' dalam ekspresi indeks larik '[- ​​i2, - k]'. nw_spliced_aligner16.cpp 564

 void CSplicedAligner16::x_DoBackTrace ( const Uint2* backtrace_matrix, CNWAligner::SAlignInOut* data, int i_global_max, int j_global_max) { .... while(intron_length < m_IntronMinSize || (Key & donor) == 0) { Key = backtrace_matrix[--i2, --k]; ++intron_length; data->m_transcript.push_back(eTS_Intron); } .... }

Saya harus mengatakan segera bahwa sepertinya tidak ada kesalahan (untuk saat ini, lol). Pertimbangkan baris berikut:

 Key = backtrace_matrix[--i2, --k];

Kata 'matrix' dan pengindeksan ganda dapat menunjukkan bahwa array adalah dua dimensi, tetapi tidak. Ini adalah pointer reguler ke array bilangan bulat. Tetapi diagnostik V520 tidak muncul begitu saja. Pemrogram benar-benar bingung tentang cara mengindeks array dua dimensi.

Dalam hal ini, penulis hanya memutuskan untuk menghemat satu baris kode, meskipun ia dapat menulis seperti ini:

 --i2; Key = backtrace_matrix[--k];

V661 Ekspresi mencurigakan 'A [B == C]'. Mungkin berarti 'A [B] == C'. ncbi_service_connector.c 180

 static EHTTP_HeaderParse s_ParseHeader(const char* header, ....) { .... if (sscanf(header, "%u.%u.%u.%u%n", &i1, &i2, &i3, &i4, &n) < 4 || sscanf(header + n, "%hu%x%n", &uuu->port, &tkt, &m) < 2 || (header[m += n] && !(header[m] == '$') && !isspace((unsigned char)((header + m) [header[m] == '$'])))) { break/*failed - unreadable connection info*/; } .... }

Contoh lain dari kode di mana saya menghabiskan waktu lama mencoba memahami apa yang sedang terjadi: D. Fungsi isspace () memeriksa karakter dengan indeks m , tetapi jika karakter ini adalah '$', maka karakter dengan indeks m + 1 diteruskan ke fungsi. Apalagi perbandingan dengan '$' sudah lebih dulu. Mungkin tidak ada kesalahan di sini, tetapi kode pasti dapat ditulis ulang lebih jelas.

V557 Array overrun dimungkinkan. Indeks 'baris' menunjuk di luar batas array. aln_reader.cpp 412

 bool CAlnReader::x_IsGap(TNumrow row, TSeqPos pos, const string& residue) { if (m_MiddleSections.size() == 0) { x_CalculateMiddleSections(); } if (row > m_MiddleSections.size()) { return false; } if (pos < m_MiddleSections[row].first) { .... } .... }

Di sini ada kesalahan serius. Pemeriksaan indeks baris yang benar harus seperti ini:

 if (row >= m_MiddleSections.size()) { return false; }

Jika tidak, dimungkinkan untuk mengakses data di luar vektor MiddleSections .

Banyak tempat seperti itu:

  • V557 Array overrun dimungkinkan. Indeks 'i' menunjuk di luar batas array. resource_pool.hpp 388
  • V557 Array overrun dimungkinkan. Indeks 'baris' menunjuk di luar batas array. aln_reader.cpp 418
  • V557 Array overrun dimungkinkan. Indeks 'fmt_idx' menunjuk di luar batas array. seq_writer.cpp 384
  • V557 Array overrun dimungkinkan. Indeks 'fmt_idx' menunjuk di luar batas array. blastdb_formatter.cpp 183
  • V557 Array overrun dimungkinkan. Indeks 'num' menunjuk di luar batas array. newcleanupp.cpp 13035

Cara mendapatkan ketidakpercayaan fungsi




V570 Variabel 'm_onClickFunction' ditugaskan untuk dirinya sendiri. alngraphic.hpp 103

 void SetOnClickFunctionName(string onClickFunction) { m_onClickFunction = m_onClickFunction; }

Bahkan tidak ada yang bisa dikomentari. Anda hanya dapat bersimpati dengan orang yang mengklik sesuatu, mengklik, tetapi tidak ada yang berubah.

Dua kasus penugasan variabel untuk saya sendiri akan menghasilkan daftar:

  • V570 Variabel 'iter-> level' ditugaskan untuk dirinya sendiri. align_format_util.cpp 189
  • V570 Variabel 'd_elements_values ​​[ind]' ditugaskan untuk dirinya sendiri. sls_alp_data.cpp 1416

V763 Parameter 'w1' selalu ditulis ulang di badan fungsi sebelum digunakan. bmfunc.h 5363

 /// Bit COUNT functor template<typename W> struct bit_COUNT { W operator()(W w1, W w2) { w1 = 0; BM_INCWORD_BITCOUNT(w1, w2); return w1; } };

Sebuah fungsi di mana argumen menjadi usang segera setelah memasuki fungsi dapat menyesatkan para pengembang yang menggunakannya. Kode harus diperiksa ulang.

Kesalahan Desain Kelas




V688 Argumen fungsi 'm_qsrc' memiliki nama yang sama dengan salah satu anggota kelas, yang dapat mengakibatkan kebingungan. compart_matching.cpp 873

 class CElementaryMatching: public CObject { .... ISequenceSource * m_qsrc; .... void x_CreateIndex (ISequenceSource *m_qsrc, EIndexMode index_more, ....); void x_CreateRemapData(ISequenceSource *m_qsrc, EIndexMode mode); void x_LoadRemapData (ISequenceSource *m_qsrc, const string& sdb); .... };

Segera 3 fungsi kelas berisi argumen yang namanya bertepatan dengan bidang kelas. Ini dapat menyebabkan kesalahan dalam badan fungsi: programmer mungkin berpikir bahwa ia bekerja dengan anggota kelas, sebenarnya mengubah nilai variabel lokal.

V614 Variabel tidak diinisialisasi 'm_BitSet' digunakan. SnpBitAttributes.hpp 187

 /// SNP bit attribute container. class CSnpBitAttributes { public: .... private: /// Internal storage for bits. Uint8 m_BitSet; }; inline CSnpBitAttributes::CSnpBitAttributes(Uint8 bits) : m_BitSet(bits) { } inline CSnpBitAttributes::CSnpBitAttributes(const vector<char>& octet_string) { auto count = sizeof(m_BitSet); auto byte = octet_string.end(); do m_BitSet = (m_BitSet << 8) | *--byte; while (--count > 0); }

Salah satu konstruktor dengan sembrono bekerja dengan variabel m_BitSet . Faktanya adalah bahwa variabel tersebut tidak diinisialisasi. Nilai "sampah" digunakan pada iterasi pertama loop, setelah inisialisasi terjadi. Ini adalah kesalahan yang sangat serius, yang mengarah ke perilaku program yang tidak ditentukan.

V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> SIntervalComparisonResult :: SIntervalComparisonResult (....)' harus digunakan. compare_feats.hpp 100

 //This struct keeps the result of comparison of two exons struct SIntervalComparisonResult : CObject { public: SIntervalComparisonResult(unsigned pos1, unsigned pos2, FCompareLocs result, int pos_comparison = 0) : m_exon_ordinal1(pos1), m_exon_ordinal2(pos2), m_result(result), m_position_comparison(pos_comparison) {} SIntervalComparisonResult() { SIntervalComparisonResult(0, 0, fCmp_Unknown, 0); } .... };

Dulu sekali saya tidak menemui kesalahan seperti itu saat memeriksa proyek. Namun masalahnya masih relevan. Kesalahannya adalah bahwa memanggil konstruktor parameter dengan cara ini membuat dan menghapus objek sementara. Dan bidang kelas tetap tidak diinisialisasi. Konstruktor lain harus dipanggil melalui daftar inisialisasi (lihat Mendelegasikan konstruktor ).

Fungsi V591 Non-void harus mengembalikan nilai. bio_tree.hpp 266

 /// Recursive assignment CBioNode& operator=(const CBioNode& tree) { TParent::operator=(tree); TBioTree* pt = (TBioTree*)tree.GetParentTree(); SetParentTree(pt); }

Penganalisa menganggap bahwa garis hilang dalam pernyataan kelebihan beban:

 return *this;

V670 Anggota kelas yang tidak diinisialisasi 'm_OutBlobIdOrData' digunakan untuk menginisialisasi anggota 'm_StdOut'. Ingat bahwa anggota diinisialisasi dalam urutan deklarasi mereka di dalam kelas. remote_app.hpp 215

 class NCBI_XCONNECT_EXPORT CRemoteAppResult { public: CRemoteAppResult(CNetCacheAPI::TInstance netcache_api, size_t max_inline_size = kMaxBlobInlineSize) : m_NetCacheAPI(netcache_api), m_RetCode(-1), m_StdOut(netcache_api, m_OutBlobIdOrData, m_OutBlobSize), m_OutBlobSize(0), m_StdErr(netcache_api, m_ErrBlobIdOrData, m_ErrBlobSize), m_ErrBlobSize(0), m_StorageType(eBlobStorage), m_MaxInlineSize(max_inline_size) { } .... };

3 peringatan penganalisa segera dikeluarkan untuk fragmen kode ini. Bidang kelas diinisialisasi bukan dalam urutan yang tercantum dalam daftar inisialisasi, tetapi dengan cara mereka dinyatakan dalam kelas. Penyebab klasik kesalahan ini adalah bahwa tidak semua programmer ingat atau tahu tentang aturan ini. Di sini dan di daftar inisialisasi hanya urutan yang salah. Orang mendapat perasaan bahwa daftar bidang dimasukkan dalam urutan acak.

V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. cobalt.cpp 247

 void CMultiAligner::SetQueries(const vector< CRef<objects::CBioseq> >& queries) { .... try { seq_loc->SetId(*it->GetSeqId()); } catch (objects::CObjMgrException e) { NCBI_THROW(CMultiAlignerException, eInvalidInput, (string)"Missing seq-id in bioseq. " + e.GetMsg()); } m_tQueries.push_back(seq_loc); .... }

Menangkap pengecualian berdasarkan nilai dapat menyebabkan hilangnya beberapa informasi tentang pengecualian karena pembuatan objek baru. Jauh lebih baik dan lebih aman untuk menangkap pengecualian dengan referensi.

Tempat serupa:

  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. agp_validate_reader.cpp 562
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. aln_build_app.cpp 320
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. aln_test_app.cpp 458
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. cobalt.cpp 691
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. cobalt.cpp 719
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. cobalt.cpp 728
  • V746 Mengiris objek. Pengecualian harus ditangkap dengan referensi bukan oleh nilai. cobalt.cpp 732

Tentang kode yang tidak terjangkau dan masalah eksekusi kode lainnya



V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. merge_tree_core.cpp 627

 bool CMergeTree::x_FindBefores_Up_Iter(....) { .... FirstFrame->Curr = StartCurr; FirstFrame->Returned = false; FirstFrame->VisitCount = 0; FrameStack.push_back(FirstFrame); while(!FrameStack.empty()) { .... if(Rel == CEquivRange::eAfter) { Frame->Returned = false; FrameStack.pop_back(); continue; } else if(Rel == CEquivRange::eBefore) { .... continue; } else { if(Frame->VisitCount == 0) { .... continue; } else { .... continue; } } Frame->Returned = false; // <= FrameStack.pop_back(); continue; } // end stack loop FirstFrame->ChildFrames.clear(); return FirstFrame->Returned; }

Kode pernyataan kondisional ditulis sehingga benar-benar semua cabang kode diakhiri dengan pernyataan continue . Hal ini menyebabkan beberapa baris pembentukan kode yang tidak terjangkau dalam loop while . Garis-garis ini terlihat sangat mencurigakan. Kemungkinan besar, masalah ini muncul setelah refactoring kode, dan sekarang membutuhkan tinjauan kode yang cermat.

V519 Variabel 'interval_width' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 454, 456. aln_writer.cpp 456

 void CAlnWriter::AddGaps(....) { .... switch(exon_chunk->Which()) { case CSpliced_exon_chunk::e_Match: interval_width = exon_chunk->GetMatch(); case CSpliced_exon_chunk::e_Mismatch: interval_width = exon_chunk->GetMismatch(); case CSpliced_exon_chunk::e_Diag: interval_width = exon_chunk->GetDiag(); genomic_string.append(....); product_string.append(....); genomic_pos += interval_width; product_pos += interval_width/res_width; break; .... } .... }

Variabel interval_width ditimpa beberapa kali, karena tidak ada pernyataan istirahat di cabang kasus . Meski klasik, tapi kesalahannya sangat buruk.

Beberapa tempat mencurigakan:

  • V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. dbapi_driver_utils.cpp 351
  • V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. net.c 780
  • V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. bcp.c 1495
  • V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. remote_blast.cpp 1470
  • V779 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. remote_blast.cpp 1522

V571 Cek berulang. Kondisi 'if (m_QueryOpts-> filtering_options)' sudah diverifikasi di baris 703. blast_options_local_priv.hpp 713

 inline void CBlastOptionsLocal::SetFilterString(const char* f) { .... if (m_QueryOpts->filtering_options) // <= { SBlastFilterOptions* old_opts = m_QueryOpts->filtering_options; m_QueryOpts->filtering_options = NULL; SBlastFilterOptionsMerge(&(m_QueryOpts->filtering_options), old_opts, new_opts); old_opts = SBlastFilterOptionsFree(old_opts); new_opts = SBlastFilterOptionsFree(new_opts); } else { if (m_QueryOpts->filtering_options) // <= m_QueryOpts->filtering_options = SBlastFilterOptionsFree(m_QueryOpts->filtering_options); m_QueryOpts->filtering_options = new_opts; new_opts = NULL; } .... }

Jelas, cabang lain membutuhkan penulisan ulang. Saya punya beberapa ide yang ingin saya lakukan dengan pointer m_QueryOpts-> filtering_options , tetapi kodenya entah bagaimana masih membingungkan. Saya memohon kepada penulis kode.

Nah, masalahnya tidak datang sendiri:

  • V571 Cek berulang. Kondisi 'if (sleeptime)' sudah diverifikasi di baris 205. request_control.cpp 208
  • V571 Cek berulang. Kondisi 'jika (assignValue.empty ())' sudah diverifikasi di baris 712. classstr.cpp 718

Kesalahan Baca Data



V739 EOF tidak boleh dibandingkan dengan nilai tipe 'char'. 'Linestring [0]' harus dari tipe 'int'. alnread.c 3509

 static EBool s_AfrpInitLineData( .... char* linestring = readfunc (pfile); .... while (linestring != NULL && linestring [0] != EOF) { s_TrimSpace (&linestring); .... } .... }

Karakter yang Anda rencanakan untuk dibandingkan dengan EOF tidak boleh disimpan dalam variabel char . Jika tidak, ada risiko bahwa karakter dengan nilai 0xFF (255) akan berubah menjadi -1 dan akan ditafsirkan dengan cara yang sama dengan akhir file (EOF). Juga (untuk berjaga-jaga) ada baiknya memeriksa implementasi fungsi readfunc .

V663 Infinite loop dimungkinkan. Kondisi 'cin.eof ()' tidak cukup untuk memutuskan dari loop.Pertimbangkan untuk menambahkan pemanggilan fungsi 'cin.fail ()' ke ekspresi kondisional. ncbicgi.cpp 1564

 typedef std::istream CNcbiIstream; void CCgiRequest::Serialize(CNcbiOstream& os) const { .... CNcbiIstream* istrm = GetInputStream(); if (istrm) { char buf[1024]; while(!istrm->eof()) { istrm->read(buf, sizeof(buf)); os.write(buf, istrm->gcount()); } } }

Penganalisa mendeteksi kesalahan potensial karena loop infinite dapat terjadi. Jika kegagalan terjadi saat membaca data, memanggil fungsi eof () akan selalu kembali salah . Untuk menyelesaikan loop dalam kasus ini, diperlukan pemeriksaan tambahan dari nilai yang dikembalikan oleh fungsi gagal () .

Kesalahan lain-lain



V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '&&'. ncbi_connutil.c 1135

 static const char* x_ClientAddress(const char* client_host, int/*bool*/ local_host) { .... if ((client_host == c && x_IsSufficientAddress(client_host)) || !(ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)) || SOCK_ntoa(ip, addr, sizeof(addr)) != 0 || !(s = (char*) malloc(strlen(client_host) + strlen(addr) + 3))) { return client_host/*least we can do :-/*/; } .... }

Perhatikan ungkapan:

 !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)


Itu tidak dihitung seperti yang diharapkan oleh programmer, karena seluruh ekspresi seperti ini:

 ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(...)


Prioritas && operator lebih tinggi daripada ?: . Karena alasan ini, kode tidak dieksekusi sebagaimana dimaksud.

V561 Mungkin lebih baik untuk memberikan nilai pada variabel 'seq' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: validator.cpp, baris 490. validator.cpp 492

 bool CValidator::IsSeqLocCorrectlyOrdered(const CSeq_loc& loc, CScope& scope) { CBioseq_Handle seq; try { CBioseq_Handle seq = scope.GetBioseqHandle(loc); } catch (CObjMgrException& ) { // no way to tell return true; } catch (const exception& ) { // no way to tell return true; } if (seq && seq.GetInst_Topology() == CSeq_inst::eTopology_circular) { // no way to check if topology is circular return true; } return CheckConsecutiveIntervals(loc, scope, x_IsCorrectlyOrdered); }

Karena programmer mendeklarasikan variabel seq baru di dalam bagian try / catch, variabel seq lainnya tetap tidak diinisialisasi dan digunakan di bawah ini dalam kode.

V562 Aneh membandingkan nilai tipe bool dengan nilai 0: (((status) & 0x7f) == 0)! = 0. ncbi_process.cpp 111

 bool CProcess::CExitInfo::IsExited(void) const { EXIT_INFO_CHECK; if (state != eExitInfo_Terminated) { return false; } #if defined(NCBI_OS_UNIX) return WIFEXITED(status) != 0; #elif defined(NCBI_OS_MSWIN) // The process always terminates with exit code return true; #endif }

Tidak ada yang sakit, tapi WIFEXITED ternyata menjadi pembukaan makro dengan cara ini:

 return (((status) & 0x7f) == 0) != 0;

Ternyata fungsi mengembalikan nilai yang berlawanan.

Dalam kode tersebut, ada fungsi lain seperti itu, yang mengeluarkan peringatan:

  • V562 Aneh membandingkan nilai tipe bool dengan nilai 0. ncbi_process.cpp 126

V595 Pointer 'dst_len' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 309, 315. zlib.cpp 309

 bool CZipCompression::CompressBuffer( const void* src_buf, size_t src_len, void* dst_buf, size_t dst_size, /* out */ size_t* dst_len) { *dst_len = 0; // Check parameters if (!src_len && !F_ISSET(fAllowEmptyData)) { src_buf = NULL; } if (!src_buf || !dst_buf || !dst_len) { SetError(Z_STREAM_ERROR, "bad argument"); ERR_COMPRESS(48, FormatErrorMessage("CZipCompression::CompressBuffer")); return false; } .... }

Pointer dst_len ditereferensi pada awal fungsi, sementara lebih lanjut pada kode diperiksa untuk kesetaraan ke nol. Kesalahan telah dibuat dalam kode yang mengarah ke perilaku tidak terdefinisi jika pointer dst_len adalah nullptr .

V590 Pertimbangkan untuk memeriksa ekspresi 'ch! =' \ 0 '&& ch ==' ''. Ekspresi berlebihan atau mengandung kesalahan cetak. cleanup_utils.cpp 580

 bool Asn2gnbkCompressSpaces(string& val) { .... while (ch != '\0' && ch == ' ') { ptr++; ch = *ptr; } .... }

Kondisi untuk menghentikan loop hanya bergantung pada apakah karakter ch adalah spasi atau tidak. Ekspresi dapat disederhanakan sebagai berikut:

 while (ch == ' ') { .... }

Kesimpulan


Penggunaan program komputer dalam penelitian ilmiah membantu dan akan membantu membuat penemuan. Mari kita berharap bahwa yang sangat penting tidak akan terlewatkan karena kesalahan ketik.

Saya mengundang pengembang proyek NCBI Genome Workbench untuk menghubungi kami, dan kami akan memberikan laporan lengkap yang dikeluarkan oleh penganalisa PVS-Studio.

Semoga penelitian kode kecil ini membantu memperbaiki banyak bug dan umumnya meningkatkan keandalan proyek. Coba jalankan PVS-Studio pada kode proyek Anda, jika Anda belum melakukannya. Anda mungkin menyukainya :).



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov. NCBI Genome Workbench: Riset Ilmiah di Bawah Ancaman

Source: https://habr.com/ru/post/id430476/

More articles:


All Articles