Geekly articles weekly

Auf den Spuren der Taschenrechner: Qalculate


Früher haben wir Codeüberprüfungen für große mathematische Pakete durchgeführt, z. B. Scilab und Octave, und Taschenrechner blieben als kleine Dienstprogramme beiseite, bei denen es aufgrund ihrer geringen Codegröße schwierig ist, Fehler zu machen. Wir haben uns geirrt, sie nicht zu beachten. Der Fall mit der Veröffentlichung des Quellcodes des Windows-Rechners hat gezeigt, dass jeder daran interessiert ist, zu diskutieren, welche Fehler dort versteckt sind, und dass es dort mehr als genug Fehler gibt, um einen Artikel darüber zu schreiben. Meine Kollegen und ich beschlossen, den Code einiger beliebter Taschenrechner zu untersuchen, und es stellte sich heraus, dass der Code des Windows-Taschenrechners nicht so schlecht war (Spoiler).

Einführung


Qalculate! - Universeller plattformübergreifender Rechner. Es ist einfach zu bedienen, bietet jedoch die Leistung und Vielseitigkeit, die normalerweise in komplexen Mathematikpaketen zu finden sind, sowie nützliche Tools für den täglichen Bedarf (wie Währungsumrechnung und Zinsberechnung). Das Projekt besteht aus zwei Komponenten: libqalculate (Bibliothek und CLI) und qalculate-gtk (GTK + UI). An der Studie ist nur libqalculate-Code beteiligt.

Um das Projekt bequemer mit demselben Windows-Rechner zu vergleichen, den wir kürzlich untersucht haben, gebe ich die Ausgabe des Cloc-Dienstprogramms für libqalculate an:

Bild 4

Subjektiv gibt es mehr Fehler und sie sind kritischer als im Code des Windows-Rechners. Ich empfehle jedoch, dass Sie selbst Schlussfolgerungen ziehen, indem Sie diese Codeüberprüfung lesen.

Hier ist übrigens ein Link zu einem Artikel über das Überprüfen eines Taschenrechners von Microsoft: " Zählen von Fehlern in einem Windows-Taschenrechner. "

PVS-Studio wurde als statisches Analysewerkzeug verwendet. Dies ist eine Reihe von Lösungen für die Codequalitätskontrolle, die Suche nach Fehlern und potenziellen Schwachstellen. Unterstützte Sprachen sind: C, C ++, C # und Java. Der Analysator kann unter Windows, Linux und MacOS gestartet werden.

Wieder kopieren, einfügen und Tippfehler!


V523 Die Anweisung 'then' entspricht der Anweisung 'else'. 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); } .... }

Der Code in der if- und else-Anweisung ist genau der gleiche. Die benachbarten Codefragmente sind dem sehr ähnlich, verwenden jedoch unterschiedliche Funktionen: internalLowerFloat () und internalUpperFloat () . Es ist davon auszugehen, dass der Programmierer hier den Code kopiert und vergessen hat, den Funktionsnamen zu korrigieren.

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke '! Mtr2.number (). IsReal ()'. Betreiber. BuiltinFunctions.cc 6274

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

Hier entstanden doppelte Ausdrücke, weil an einer Stelle anstelle des Namens mtr mtr2 geschrieben wurde . In der Bedingung gibt es daher keinen Aufruf der Funktion mtr.number (). IsReal () .

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke 'vargs [1] .representNonPositive ()'. Betreiber. BuiltinFunctions.cc 5785

Bild 6



Manuelles Auffinden von Anomalien in diesem Code ist unwirklich! Aber sie sind es. Darüber hinaus werden diese Fragmente in der Originaldatei in einer Zeile geschrieben. Der Analysator hat einen doppelten Ausdruck vargs [1] .representNonPositive () festgestellt , der auf einen Tippfehler und daher auf einen möglichen Fehler hinweisen kann.

Hier ist die ganze Liste verdächtiger Orte, die Sie kaum herausfinden können:

  • V501 Links und rechts vom '||' befinden sich identische Unterausdrücke 'vargs [1] .representNonPositive ()'. Betreiber. BuiltinFunctions.cc 5788
  • V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'append'. MathStructure.cc 1780
  • V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'append'. MathStructure.cc 2043
  • V501 Es gibt identische Unterausdrücke '(* v_subs [v_order [1]]). Repräsentiert Negativ (wahr)' links und rechts vom Operator '&&'. MathStructure.cc 5569

Ungültige Schleife


V534 Es ist wahrscheinlich, dass eine falsche Variable innerhalb des ' for' -Operators verglichen wird. Betrachten Sie die Überprüfung von '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; } } .... }

In der inneren Schleife ist der Zähler die Variable i2 , aber aufgrund eines Tippfehlers wurde ein Fehler gemacht - im Zustand des Stoppens der Schleife wird die Variable i aus der äußeren Schleife verwendet.

Redundanz oder Fehler?


V590 Überprüfen Sie diesen Ausdruck. Der Ausdruck ist übertrieben oder enthält einen Druckfehler. 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; .... }

Nachdem ich mir solchen Code angesehen hatte, schrieb ich vor 3 Jahren eine Notiz, um mir und anderen Programmierern zu helfen: " Logische Ausdrücke in C / C ++. Wie Profis falsch liegen ." Bei der Begegnung mit einem solchen Code bin ich überzeugt, dass die Notiz überhaupt nicht weniger relevant geworden ist. Sie können im Artikel nachsehen, das dem Code entsprechende Fehlermuster finden und alle Nuancen herausfinden.

In diesem Beispiel gehen Sie zum Abschnitt "Ausdruck == || ! = ”Und wir erfahren, dass der Ausdruck i2 == COMPARISON_RESULT_UNKNOWN nichts beeinflusst.

Dereferenzieren nicht verifizierter Zeiger


V595 Der Zeiger 'o_data' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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); } .... }

Der o_data- Zeiger in einer Funktion wird ohne Prüfung und mit Prüfung dereferenziert. Dies kann redundanter Code oder ein potenzieller Fehler sein. Ich bin zur letzten Option geneigt.

Es gibt zwei weitere ähnliche Orte:

  • V595 Der Zeiger 'o_assumption' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 229, 230. Variable.cc 229
  • V595 Der Zeiger 'i_value' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 3412, 3427. Number.cc 3412

free () oder delete []?


V611 Der Speicher wurde mit dem Operator 'new' zugewiesen, aber mit der Funktion 'free' freigegeben. Überprüfen Sie die Betriebslogik hinter der Variablen '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); // <= } } .... } .... }

Der Speicher für das Remcopy- Array wird auf verschiedene Weise zugewiesen und freigegeben, was ein schwerwiegender Fehler ist.

Verlorene Änderungen


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

Die Variable m wird in der Funktion als Referenz akzeptiert, was ihre Modifikation impliziert. Der Analysator stellte jedoch fest, dass der Code die gleichnamige lokale Variable enthält, die den Umfang des Funktionsparameters überlappt und den Verlust von Änderungen ermöglicht.

Seltsame Zeiger


V774 Der ' cu' -Zeiger wurde verwendet, nachdem der Speicher freigegeben wurde. 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; } .... }

Der Analysator warnt, dass der Code nach dem Freigeben des Speichers einen Aufruf der Methode des cu- Objekts enthält. Aber wenn Sie versuchen, den Code zu verstehen, wird es noch seltsamer. Erstens erfolgt der Aufruf zum Löschen von cu immer - in der Bedingung und danach. Zweitens geht der Code nach der Bedingung davon aus, dass die Zeiger u und cu nicht gleich sind. Nach dem Löschen des cu- Objekts ist es logisch, das u- Objekt zu verwenden. Höchstwahrscheinlich wurde ein Tippfehler im Code gemacht und es war geplant, nur die Variable u zu verwenden .

Verwenden der Suchfunktion


V797 Die Funktion 'find' wird so verwendet, als ob sie einen Bool-Typ zurückgeben würde. Der Rückgabewert der Funktion sollte wahrscheinlich mit std :: string :: npos verglichen werden. 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; } .... }

Obwohl der Code erfolgreich kompiliert wurde, sieht er verdächtig aus, da die Suchfunktion eine Anzahl vom Typ std :: string :: size_type zurückgibt . Die Bedingung ist erfüllt, wenn der Punkt irgendwo auf der Linie gefunden wird, außer wenn der Punkt am Anfang liegt. Dies ist ein seltsamer Test. Ich bin nicht sicher, aber vielleicht sollte der Code wie folgt umgeschrieben werden:

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

Möglicher Speicherverlust


V701 realloc () mögliches Leck: Wenn realloc () beim Zuweisen von Speicher fehlschlägt, geht der ursprüngliche Zeiger 'Puffer' verloren. Ziehen Sie in Betracht, einem temporären Zeiger realloc () zuzuweisen. 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 }

Bei der Arbeit mit der Funktion realloc () wird empfohlen, einen Zwischenpuffer zu verwenden, da der Zeiger auf den alten Speicher unwiederbringlich verloren geht, wenn kein Speicher zugewiesen werden kann.

Fazit


Projekt berechnen! führt die Liste der besten kostenlosen Taschenrechner an, enthält jedoch viele schwerwiegende Fehler. Aber wir haben seine Konkurrenten nicht gesehen. Wir werden versuchen, alle gängigen Taschenrechner durchzugehen.

Was den Vergleich mit der Qualität des Rechners aus der Windows-Welt betrifft, sieht das Dienstprogramm von Microsoft zuverlässiger und qualitativ hochwertiger aus.

Überprüfen Sie Ihren „Rechner“, indem Sie PVS-Studio herunterladen und in Ihrem Projekt ausprobieren. :-)



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Svyatoslav Razmyslov. Auf den Spuren von Taschenrechnern: Qalculate!

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

More articles:


All Articles