Geekly articles weekly

Mengikuti Jejak Kalkulator: Hitung Ulang


Sebelumnya kami melakukan review kode dari paket matematika besar, misalnya, Scilab dan Oktaf, di mana kalkulator tetap menyendiri sebagai utilitas kecil, di mana sulit untuk membuat kesalahan karena basis kode kecil mereka. Kami salah bahwa kami tidak memperhatikan mereka. Kasus dengan memposting kode sumber kalkulator Windows menunjukkan bahwa sebenarnya semua orang tertarik untuk mendiskusikan jenis kesalahan yang bersembunyi di dalamnya. Apalagi jumlah kesalahan di sana lebih dari cukup untuk menulis artikel tentang itu. Kolega saya dan saya, kami memutuskan untuk menjelajahi kode sejumlah kalkulator populer, dan ternyata kode kalkulator Windows tidak terlalu buruk (spoiler).

Pendahuluan


Hitung Ulang! adalah kalkulator desktop lintas platform multi guna. Mudah digunakan tetapi memberikan daya dan keserbagunaan yang biasanya dicadangkan untuk paket matematika yang rumit, serta alat yang berguna untuk kebutuhan sehari-hari (seperti konversi mata uang dan perhitungan persen). Proyek ini terdiri dari dua komponen: libqalculate (library dan CLI) dan qalculate-gtk (GTK + UI). Studi ini hanya melibatkan kode libqalculate.

Untuk membandingkan proyek dengan Windows Calculator dengan mudah, yang baru-baru ini kami periksa, saya mengutip keluaran dari utilitas Cloc untuk libqalculate:


Mempertimbangkannya secara subjektif, ada lebih banyak kesalahan di dalamnya dan mereka lebih kritis daripada dalam kode kalkulator Windows. Namun demikian, saya akan merekomendasikan membuat kesimpulan sendiri, setelah membaca ikhtisar kode ini.

Ngomong-ngomong, inilah tautan ke artikel tentang pemeriksaan kalkulator dari Microsoft: " Menghitung Bug di Kalkulator Windows ".

Alat analisis adalah penganalisa kode statis PVS-Studio . Ini adalah serangkaian solusi untuk kontrol kualitas kode, mencari bug dan kerentanan potensial. Bahasa yang didukung meliputi: C, C ++, C # dan Java. Anda dapat menjalankan penganalisis pada Windows, Linux dan macOS.

Salin dan tempel lagi dan lagi!


V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. Number.cc 4018

bool Number::square() { .... if(mpfr_cmpabs(i_value->internalLowerFloat(), i_value->internalUpperFloat()) > 0) { mpfr_sqr(f_tmp, i_value->internalLowerFloat(), MPFR_RNDU); mpfr_sub(f_rl, f_rl, f_tmp, MPFR_RNDD); } else { mpfr_sqr(f_tmp, i_value->internalLowerFloat(), MPFR_RNDU); mpfr_sub(f_rl, f_rl, f_tmp, MPFR_RNDD); } .... }

Kode ini benar-benar sama di blok if and else . Fragmen kode yang berdekatan sangat mirip dengan yang ini, tetapi fungsi yang berbeda digunakan di dalamnya: internalLowerFloat () dan internalUpperFloat () . Aman untuk mengasumsikan bahwa pengembang menyalin kode dan lupa untuk memperbaiki nama fungsi di sini.

V501 Ada sub-ekspresi identik '! Mtr2.number (). IsReal ()' di kiri dan di kanan '||' operator. BuiltinFunctions.cc 6274

 int IntegrateFunction::calculate(....) { .... if(!mtr2.isNumber() || !mtr2.number().isReal() || !mtr.isNumber() || !mtr2.number().isReal()) b_unknown_precision = true; .... }

Dalam hal ini, ekspresi duplikat muncul karena fakta bahwa di satu tempat mtr2 ditulis, bukan mtr. Dengan demikian, panggilan fungsi mtr.number (). IsReal () tidak ada dalam kondisi tersebut.

V501 Ada sub-ekspresi identik 'muatan [1] .representsNonPositive ()' ke kiri dan ke kanan '||' operator. BuiltinFunctions.cc 5785



Kami tidak akan pernah menemukan cacat dalam kode ini secara manual! Tapi di sini ada. Selain itu, dalam file asli fragmen ini ditulis dalam satu baris. Penganalisis telah mendeteksi duplikasi ekspresi muatan [1] .representsNonPositive () , yang dapat menunjukkan kesalahan ketik atau, akibatnya, kesalahan potensial.

Inilah seluruh daftar tempat mencurigakan, yang sulit ditemukan.

  • V501 Ada sub-ekspresi identik 'muatan [1] .representsNonPositive ()' ke kiri dan ke kanan '||' operator. BuiltinFunctions.cc 5788
  • V501 Ada sub-ekspresi identik 'tambahkan' ke kiri dan ke kanan operator '&&'. MathStructure.cc 1780
  • V501 Ada sub-ekspresi identik 'tambahkan' ke kiri dan ke kanan operator '&&'. MathStructure.cc 2043
  • V501 Ada sub-ekspresi yang identik '(* v_subs [v_order [1]]). Merupakan Representatif (benar)' di sebelah kiri dan di sebelah kanan operator '&&'. MathStructure.cc 5569

Loop Dengan Kondisi Yang Tidak Benar


V534 Kemungkinan variabel yang salah sedang dibandingkan di dalam operator 'untuk'. Pertimbangkan untuk meninjau 'i'. MathStructure.cc 28741

 bool MathStructure::isolate_x_sub(....) { .... for(size_t i = 0; i < mvar->size(); i++) { if((*mvar)[i].contains(x_var)) { mvar2 = &(*mvar)[i]; if(mvar->isMultiplication()) { for(size_t i2 = 0; i < mvar2->size(); i2++) { if((*mvar2)[i2].contains(x_var)) {mvar2 = &(*mvar2)[i2]; break;} } } break; } } .... }

Di loop dalam, variabel i2 mewakili penghitung, tetapi karena kesalahan ketik kesalahan dibuat - variabel i dari loop luar digunakan dalam kondisi loop keluar.

Redundansi atau kesalahan?


V590 Pertimbangkan untuk memeriksa ungkapan ini. Ekspresi berlebihan atau mengandung kesalahan cetak. Number.cc 6564

 bool Number::add(const Number &o, MathOperation op) { .... if(i1 >= COMPARISON_RESULT_UNKNOWN && (i2 == COMPARISON_RESULT_UNKNOWN || i2 != COMPARISON_RESULT_LESS)) return false; .... }

3 tahun yang lalu setelah saya melihat kode semacam itu, saya menulis lembar contekan untuk saya dan pengembang lain: " Ekspresi logis dalam C / C ++. Kesalahan yang Dibuat oleh Profesional ". Ketika saya menemukan kode seperti itu, saya memastikan bahwa catatan itu belum menjadi kurang relevan. Anda dapat melihat ke dalam artikel, menemukan pola kesalahan yang sesuai dengan kode, dan mencari tahu semua nuansanya.

Dalam kasus contoh ini, kita akan pergi ke bagian “Ekspresi == || ! = "Dan cari tahu bahwa ekspresi i2 == COMPARISON_RESULT_UNKNOWN tidak mempengaruhi apa pun.

Dereferencing Pointer Tidak Dicentang


V595 Pointer 'o_data' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 1108, 1112. DataSet.cc 1108

 string DataObjectArgument::subprintlong() const { string str = _("an object from"); str += " \""; str += o_data->title(); // <= str += "\""; DataPropertyIter it; DataProperty *o = NULL; if(o_data) { // <= o = o_data->getFirstProperty(&it); } .... }

Dalam satu fungsi pointer o_data dereferenced baik tanpa maupun dengan cek. Ini bisa berupa kode yang berlebihan, atau kesalahan potensial. Saya condong ke arah yang terakhir.

Ada dua tempat serupa:

  • V595 Pointer 'o_assumption' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 229, 230. Variable.cc 229
  • V595 Pointer 'i_value' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 3412, 3427. Number.cc 3412

gratis () atau hapus []?


V611 Memori dialokasikan menggunakan operator 'baru' tetapi dirilis menggunakan fungsi 'bebas'. Pertimbangkan untuk memeriksa logika operasi di belakang variabel 'remcopy'. Number.cc 8123

 string Number::print(....) const { .... while(!exact && precision2 > 0) { if(try_infinite_series) { remcopy = new mpz_t[1]; // <= mpz_init_set(*remcopy, remainder); } mpz_mul_si(remainder, remainder, base); mpz_tdiv_qr(remainder, remainder2, remainder, d); exact = (mpz_sgn(remainder2) == 0); if(!started) { started = (mpz_sgn(remainder) != 0); } if(started) { mpz_mul_si(num, num, base); mpz_add(num, num, remainder); } if(try_infinite_series) { if(started && first_rem_check == 0) { remainders.push_back(remcopy); } else { if(started) first_rem_check--; mpz_clear(*remcopy); free(remcopy); // <= } } .... } .... }

Memori untuk array salinan dialokasikan dan dirilis dengan cara yang berbeda, yang merupakan kesalahan serius.

Perubahan yang hilang


 bool expand_partial_fractions(MathStructure &m, ....) { .... if(b_poly && !mquo.isZero()) { MathStructure m = mquo; if(!mrem.isZero()) { m += mrem; m.last() *= mtest[i]; m.childrenUpdated(); } expand_partial_fractions(m, eo, false); return true; } .... }

Variabel m dalam fungsi dilewatkan oleh referensi, yang berarti modifikasi. Namun, penganalisa telah mendeteksi bahwa kode tersebut berisi variabel dengan nama yang sama, yang tumpang tindih dengan cakupan parameter fungsi, yang memungkinkan hilangnya perubahan.

Pointer aneh


V774 Penunjuk 'cu' digunakan setelah memori dilepaskan. Calculator.cc 3595

 MathStructure Calculator::convertToBestUnit(....) { .... CompositeUnit *cu = new CompositeUnit("", "...."); cu->add(....); Unit *u = getBestUnit(cu, false, eo.local_currency_conversion); if(u == cu) { delete cu; // <= return mstruct_new; } delete cu; // <= if(eo.approximation == APPROXIMATION_EXACT && cu->hasApproximateRelationTo(u, true)) { // <= if(!u->isRegistered()) delete u; return mstruct_new; } .... }

Penganalisa memperingatkan bahwa kode memanggil metode objek cu setelah deallocating memori. Tetapi ketika mencoba untuk bergulat dengannya, kode tersebut ternyata menjadi lebih aneh. Pertama, panggilan delete cu selalu terjadi - baik dalam kondisi dan setelah itu. Kedua, kode setelah kondisi menyiratkan bahwa pointer u dan cu tidak sama, yang berarti bahwa setelah menghapus objek cu , cukup logis untuk menggunakan objek u . Kemungkinan besar, kesalahan ketik dibuat dalam kode dan pembuat kode hanya ingin menggunakan variabel u .

Penggunaan fungsi find


V797 Fungsi 'find' digunakan seolah-olah mengembalikan tipe bool. Nilai pengembalian fungsi mungkin harus dibandingkan dengan std :: string :: npos. Unit.cc 404

 MathStructure &AliasUnit::convertFromFirstBaseUnit(....) const { if(i_exp != 1) mexp /= i_exp; ParseOptions po; if(isApproximate() && suncertainty.empty() && precision() == -1) { if(sinverse.find(DOT) || svalue.find(DOT)) po.read_precision = READ_PRECISION_WHEN_DECIMALS; else po.read_precision = ALWAYS_READ_PRECISION; } .... }

Meskipun kode berhasil dikompilasi, kode tersebut terlihat mencurigakan, karena fungsi find mengembalikan jumlah tipe std :: string :: size_type . Kondisi ini akan benar jika titik ditemukan di bagian mana pun dari string kecuali jika titiknya ada di awal. Ini cek aneh. Saya tidak yakin tetapi, mungkin, kode ini harus ditulis ulang sebagai berikut:

 if( sinverse.find(DOT) != std::string::npos || svalue.find(DOT) != std::string::npos) { po.read_precision = READ_PRECISION_WHEN_DECIMALS; }

Potensi kebocoran memori


V701 realloc () kemungkinan kebocoran: ketika realloc () gagal mengalokasikan memori, 'buffer' pointer asli hilang. Pertimbangkan untuk menetapkan realloc () ke pointer sementara. util.cc 703

 char *utf8_strdown(const char *str, int l) { #ifdef HAVE_ICU .... outlength = length + 4; buffer = (char*) realloc(buffer, outlength * sizeof(char)); // <= .... #else return NULL; #endif }

Ketika bekerja dengan fungsi realloc () disarankan untuk menggunakan buffer perantara, seperti dalam kasus jika tidak mungkin mengalokasikan memori, penunjuk ke area memori lama akan hilang.

Kesimpulan


The Qalculate! proyek menduduki puncak daftar kalkulator gratis terbaik, sedangkan itu berisi banyak kesalahan serius. Di sisi lain, kami belum memeriksa pesaingnya. Kami akan mencoba memeriksa semua kalkulator populer.

Sedangkan untuk membandingkan dengan kualitas kalkulator dari dunia Windows, utilitas dari Microsoft terlihat lebih andal dan berfungsi dengan baik sejauh ini.

Periksa "Kalkulator" Anda sendiri - unduh PVS-Studio dan coba untuk proyek Anda. :-)

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

More articles:


All Articles