Auf den Spuren von Taschenrechnern: Qalculate

Zuvor haben wir Codeüberprüfungen für große mathematische Pakete durchgeführt, z. B. Scilab und Octave, wobei Taschenrechner als kleine Dienstprogramme fern blieben, bei denen es aufgrund ihrer kleinen Codebasis schwierig ist, Fehler zu machen. Wir haben uns geirrt, dass wir ihnen nicht Beachtung geschenkt haben. Der Fall mit der Veröffentlichung des Quellcodes des Windows-Rechners zeigte, dass tatsächlich jeder daran interessiert war, Arten von Fehlern zu diskutieren, die sich darin verstecken. Darüber hinaus war die Anzahl der Fehler mehr als ausreichend, um einen Artikel darüber zu schreiben. Meine Kollegen und ich haben beschlossen, den Code einer Reihe beliebter Taschenrechner zu untersuchen, und es stellte sich heraus, dass der Code des Windows-Taschenrechners nicht so schlecht war (Spoiler).

Einführung

Qalculate! ist ein plattformübergreifender Mehrzweck-Desktop-Rechner. Es ist einfach zu bedienen, bietet jedoch Leistung und Vielseitigkeit, die normalerweise komplizierten Mathematikpaketen vorbehalten sind, sowie nützliche Tools für den täglichen Bedarf (wie Währungsumrechnung und Prozentberechnung). Das Projekt besteht aus zwei Komponenten: libqalculate (Bibliothek und CLI) und qalculate-gtk (GTK + UI). Die Studie umfasste nur den libqalculate-Code.

Um das Projekt einfach mit Windows Calculator zu vergleichen, den wir kürzlich überprüft haben, zitiere ich die Ausgabe des Cloc-Dienstprogramms für libqalculate:

Subjektiv betrachtet gibt es mehr Fehler und diese sind kritischer als im Windows-Rechnercode. Trotzdem würde ich empfehlen, selbst Schlussfolgerungen zu ziehen, nachdem Sie diese Codeübersicht gelesen haben.

Hier ist übrigens ein Link zu einem Artikel über die Überprüfung des Rechners von Microsoft: " Counting Bugs in Windows Calculator ".

Das Analysetool ist der statische Code-Analysator PVS-Studio . Es handelt sich um 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. Sie können den Analyzer unter Windows, Linux und macOS ausführen.

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 ist in den if- und else- Blöcken absolut identisch . Benachbarte Codefragmente sind diesem sehr ähnlich, es werden jedoch verschiedene Funktionen verwendet: internalLowerFloat () und internalUpperFloat () . Es ist davon auszugehen, dass ein Entwickler den Code kopiert und vergessen hat, den Namen der Funktion hier 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; .... } 

In diesem Fall wurden doppelte Ausdrücke angezeigt , da an einer Stelle mtr2 anstelle von mtr geschrieben wurde. Daher fehlt in der Bedingung ein Aufruf der Funktion mtr.number (). IsReal () .

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



Wir hätten niemals manuell Fehler in diesem Code gefunden! Aber hier gibt es. Darüber hinaus werden diese Fragmente in der Originaldatei in einer einzigen Zeile geschrieben. Der Analysator hat einen doppelten Ausdruck vargs [1] .representNonPositive () erkannt , der auf einen Tippfehler oder folglich auf einen möglichen Fehler hinweisen kann.

Hier ist die gesamte Liste verdächtiger Orte, die man kaum herausfinden kann.

• 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

Schleife mit falschem Zustand

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 stellt die i2- Variable einen Zähler dar, aber aufgrund eines Tippfehlers wurde ein Fehler gemacht - die i- Variable aus der äußeren Schleife wird in der Schleifenausgangsbedingung verwendet.

Redundanz oder ein 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; .... } 

Vor 3 Jahren, nachdem ich einen Blick auf solchen Code geworfen hatte, schrieb ich einen Spickzettel für mich und andere Entwickler: " Logische Ausdrücke in C / C ++. Fehler von Fachleuten ". Wenn ich auf solchen Code stoße, stelle ich sicher, dass der Hinweis ist nicht weniger relevant geworden. Sie können in den Artikel schauen, ein Muster des Fehlers finden, das dem Code entspricht, und alle Nuancen herausfinden.

In diesem Beispiel gehen wir zum Abschnitt „Ausdruck == || ! = "Und finde heraus, dass der Ausdruck i2 == COMPARISON_RESULT_UNKNOWN nichts beeinflusst.

Dereferenzieren von nicht aktivierten Zeigern

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

In einer Funktion wird der Zeiger o_data sowohl ohne als auch mit einer Prüfung dereferenziert. Dies kann redundanter Code oder ein potenzieller Fehler sein. Ich neige mich zu Letzterem.

Es gibt zwei ä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 unterschiedliche 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 in der Funktion wird als Referenz übergeben, was ihre Änderung bedeutet. Der Analysator hat jedoch festgestellt, dass der Code die gleichnamige Variable enthält, die den Funktionsumfang 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 direkt nach der Freigabe des Speichers eine Methode des cu- Objekts aufruft. Aber wenn man versucht, sich damit auseinanderzusetzen, stellt sich heraus, dass der Code noch seltsamer ist. Erstens geschieht der Aufruf von delete cu immer - sowohl in der Bedingung als auch danach. Zweitens impliziert der Code nach der Bedingung, dass die Zeiger u und cu nicht gleich sind, was bedeutet, dass es nach dem Löschen des cu- Objekts ziemlich logisch ist, das u- Objekt zu verwenden. Höchstwahrscheinlich wurde ein Tippfehler im Code gemacht und der Autor des Codes wollte nur die Variable u verwenden .

Verwendung 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 werden kann, sieht er verdächtig aus, da die Suchfunktion die Nummer vom Typ std :: string :: size_type zurückgibt . Die Bedingung ist erfüllt, wenn der Punkt in einem Teil der Zeichenfolge gefunden wird, außer wenn sich der Punkt am Anfang befindet. Es ist eine seltsame Prüfung. Ich bin nicht sicher, aber vielleicht sollte dieser 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. Wenn kein Speicher zugewiesen werden kann, geht der Zeiger auf den alten Speicherbereich unwiederbringlich verloren.

Fazit

Das Qalculate! Das Projekt führt die Liste der besten kostenlosen Taschenrechner an, enthält jedoch viele schwerwiegende Fehler. Auf der anderen Seite haben wir die Konkurrenten noch nicht überprüft. Wir werden versuchen, alle gängigen Taschenrechner durchzugehen.

Im Vergleich zur Qualität des Taschenrechners aus der Windows-Welt sieht das Dienstprogramm von Microsoft bisher zuverlässiger und gut aus.

Überprüfen Sie Ihren eigenen „Rechner“ - laden Sie PVS-Studio herunter und probieren Sie es für Ihr Projekt aus. :-)