Geekly articles weekly

Mengikuti jejak kalkulator: Hitung Ulang


Sebelumnya kami melakukan review kode dari paket matematika besar, misalnya, Scilab dan Oktaf, dan kalkulator tetap sebagai utilitas kecil di mana sulit untuk membuat kesalahan karena ukuran kode yang kecil. Kami keliru karena tidak memperhatikan mereka. Kasus dengan penerbitan kode sumber kalkulator Windows menunjukkan bahwa semua orang tertarik untuk membahas kesalahan apa yang disembunyikan di sana, dan ada lebih dari cukup kesalahan di sana untuk menulis artikel tentang itu. Saya dan kolega saya memutuskan untuk memeriksa kode sejumlah kalkulator populer, dan ternyata kode kalkulator Windows tidak terlalu buruk (spoiler).

Pendahuluan


Hitung Ulang! - Kalkulator lintas platform universal. Mudah digunakan, tetapi memberikan kekuatan dan keserbagunaan yang biasanya ditemukan dalam paket matematika yang kompleks, serta alat yang berguna untuk kebutuhan sehari-hari (seperti konversi mata uang dan perhitungan bunga). Proyek ini terdiri dari dua komponen: libqalculate (library dan CLI) dan qalculate-gtk (GTK + UI). Hanya kode libqalculate yang terlibat dalam penelitian ini.

Untuk lebih mudah membandingkan proyek dengan kalkulator Windows yang sama dengan yang baru-baru ini kami teliti, saya memberikan output dari utilitas Cloc untuk libqalculate:

Gambar 4

Secara subyektif, ada lebih banyak kesalahan, dan lebih kritis daripada dalam kode kalkulator Windows. Tapi saya sarankan Anda menarik kesimpulan sendiri dengan membaca ulasan kode ini.

Omong-omong, berikut ini tautan ke artikel tentang memeriksa kalkulator dari Microsoft: " Menghitung bug di kalkulator Windows. "

PVS-Studio digunakan sebagai alat analisis statis. Ini adalah serangkaian solusi untuk kontrol kualitas kode, mencari kesalahan dan kerentanan potensial. Bahasa yang didukung meliputi: C, C ++, C #, dan Java. Alat analisis dapat diluncurkan pada Windows, Linux dan macOS.

Salin-tempel dan salah ketik 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 dalam pernyataan if dan else persis sama. Fragmen kode tetangga sangat mirip dengan ini, tetapi mereka menggunakan fungsi yang berbeda: internalLowerFloat () dan internalUpperFloat () . Aman untuk berasumsi bahwa di sini programmer menyalin kode dan lupa untuk memperbaiki nama fungsinya.

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; .... }

Di sini ekspresi rangkap muncul karena di satu tempat bukan nama mtr mereka menulis mtr2 . Jadi, dalam kondisi tidak ada panggilan ke fungsi mtr.number (). IsReal () .

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

Gambar 6



Menemukan anomali dalam kode ini secara manual tidak nyata! Tetapi mereka. Selain itu, dalam file asli, fragmen-fragmen ini ditulis dalam satu baris. Penganalisa mendeteksi duplikat ekspresi muatan [1]. Merepresentasikan NonPositive () , yang dapat mengindikasikan kesalahan ketik dan, oleh karena itu, kesalahan potensial.

Berikut adalah seluruh daftar tempat mencurigakan yang sulit Anda ketahui:

  • 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 Tidak Valid


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; } } .... }

Dalam loop internal, penghitung adalah variabel i2 , tetapi karena kesalahan ketik, kesalahan dibuat - dalam kondisi loop berhenti, variabel i dari loop eksternal digunakan.

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; .... }

Setelah melihat kode seperti itu, 3 tahun yang lalu saya menulis catatan untuk membantu diri saya sendiri dan programmer lain: " Ekspresi logis dalam C / C ++. Betapa profesionalnya salah ." Setelah menemukan kode seperti itu, saya yakin bahwa catatan itu menjadi kurang relevan sama sekali. Anda dapat melihat di artikel, menemukan pola kesalahan yang sesuai dengan kode, dan mencari tahu semua nuansanya.

Dalam kasus contoh ini, buka bagian "Ekspresi == || ! = ”Dan kita belajar bahwa ungkapan i2 == COMPARISON_RESULT_UNKNOWN tidak memengaruhi apa pun.

Merujuk referensi yang tidak diverifikasi


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); } .... }

Pointer o_data dalam satu fungsi dereferenced tanpa memeriksa dan dengan memeriksa. Ini mungkin kode yang berlebihan, atau kesalahan potensial. Saya cenderung ke opsi 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 dibebaskan dalam berbagai cara, 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 diterima dalam fungsi dengan referensi, yang menyiratkan modifikasinya. Tetapi penganalisa menemukan bahwa kode tersebut berisi variabel lokal dengan nama yang sama, yang tumpang tindih lingkup 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 berisi panggilan ke metode objek cu setelah membebaskan memori. Tetapi jika Anda mencoba memahami kode tersebut, maka itu akan menjadi lebih aneh. Pertama, panggilan untuk menghapus cu selalu terjadi - dalam kondisi dan setelah. Kedua, kode setelah kondisi mengasumsikan bahwa pointer u dan cu tidak sama, jadi setelah membersihkan objek cu , adalah logis untuk menggunakan objek u . Kemungkinan besar, kesalahan ketik dibuat dalam kode dan direncanakan hanya menggunakan variabel u .

Menggunakan 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, itu terlihat mencurigakan karena fungsi find mengembalikan sejumlah tipe std :: string :: size_type . Kondisi akan benar jika titik ditemukan di mana saja di telepon, kecuali jika titik di awal. Ini adalah tes yang aneh. Saya tidak yakin, tapi mungkin kodenya 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, karena jika tidak mungkin mengalokasikan memori, pointer ke memori lama akan hilang.

Kesimpulan


Proyek perhitungan ulang! berada di puncak daftar kalkulator gratis terbaik, sementara itu berisi banyak kesalahan serius. Tapi kita belum melihat kompetitornya. Kami akan mencoba menelusuri semua kalkulator populer.

Sedangkan untuk perbandingannya dengan kualitas kalkulator dari dunia Windows, sedangkan utilitas dari Microsoft terlihat lebih andal dan berkualitas tinggi.

Periksa "Kalkulator" Anda dengan mengunduh PVS-Studio dan mencobanya di proyek Anda. :-)



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov. Mengikuti Jejak Kalkulator: Hitung Ulang!

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

More articles:


All Articles