🌃 🙌🏽 👩🏼‍🤝‍👨🏻 Suivre la trace des calculatrices: Qalculate ➿ 👩🏾‍🤝‍👨🏼 🗿

Plus tôt, nous avons effectué des révisions de code de grands packages mathématiques, par exemple, Scilab et Octave, et les calculatrices sont restées de côté en tant que petits utilitaires dans lesquels il est difficile de faire des erreurs en raison de leur petite taille de code. Nous nous sommes trompés en ne leur prêtant pas attention. Le cas de la publication du code source de la calculatrice Windows a montré que tout le monde est intéressé à discuter des erreurs qui y sont cachées, et il y a plus qu'assez d'erreurs pour écrire un article à ce sujet. Mes collègues et moi avons décidé d'examiner le code d'un certain nombre de calculatrices populaires, et il s'est avéré que le code de la calculatrice Windows n'était pas si mauvais (spoiler).

Présentation

Calculez! - calculatrice multi-plateforme universelle. Il est facile à utiliser, mais offre la puissance et la polyvalence généralement présentes dans les packages mathématiques complexes, ainsi que des outils utiles pour les besoins quotidiens (tels que la conversion de devises et le calcul des intérêts). Le projet se compose de deux composants: libqalculate (bibliothèque et CLI) et qalculate-gtk (GTK + UI). Seul le code libqalculate est impliqué dans l'étude.

Pour comparer plus facilement le projet avec la même calculatrice Windows que nous avons récemment examinée, je donne la sortie de l'utilitaire Cloc pour libqalculate:

Subjectivement, il y a plus d'erreurs, et elles sont plus critiques que dans le code de la calculatrice Windows. Mais je vous recommande de tirer des conclusions vous-même en lisant cette revue de code.

À propos, voici un lien vers un article sur la vérification d'une calculatrice de Microsoft: « Compter les bogues dans une calculatrice Windows » .

PVS-Studio a été utilisé comme outil d'analyse statique. Il s'agit d'un ensemble de solutions pour le contrôle de la qualité du code, la recherche d'erreurs et les vulnérabilités potentielles. Les langages pris en charge incluent: C, C ++, C # et Java. L'analyseur peut être lancé sur Windows, Linux et macOS.

Copiez et collez à nouveau!

V523 L' instruction 'then' est équivalente à l'instruction '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); } .... }

Le code de l' instruction if et else est exactement le même. Les fragments de code voisins sont très similaires à cela, mais ils utilisent différentes fonctions: internalLowerFloat () et internalUpperFloat () . Il est sûr de supposer qu'ici le programmeur a copié le code et oublié de corriger le nom de la fonction.

V501 Il existe des sous-expressions identiques '! Mtr2.number (). IsReal ()' à gauche et à droite de '||' opérateur. BuiltinFunctions.cc 6274

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

Ici, des expressions en double sont apparues car à un endroit au lieu du nom mtr, elles ont écrit mtr2 . Ainsi, dans la condition, il n'y a aucun appel à la fonction mtr.number (). IsReal () .

V501 Il existe des sous-expressions identiques «vargs [1] .representNonPositive ()» à gauche et à droite de «||» opérateur. BuiltinFunctions.cc 5785

Trouver des anomalies dans ce code manuellement est irréel! Mais ils le sont. De plus, dans le fichier d'origine, ces fragments sont écrits sur une seule ligne. L'analyseur a détecté une expression en double vargs [1] .representNonPositive () , qui peut indiquer une faute de frappe et, par conséquent, une erreur potentielle.

Voici toute la liste des endroits suspects que vous pouvez à peine comprendre:

V501 Il existe des sous-expressions identiques «vargs [1] .representNonPositive ()» à gauche et à droite de «||» opérateur. BuiltinFunctions.cc 5788
V501 Il existe des sous-expressions identiques «ajouter» à gauche et à droite de l'opérateur «&&». MathStructure.cc 1780
V501 Il existe des sous-expressions identiques «ajouter» à gauche et à droite de l'opérateur «&&». MathStructure.cc 2043
V501 Il existe des sous-expressions identiques '(* v_subs [v_order [1]]). ReprésenteNégatif (vrai)' à gauche et à droite de l'opérateur '&&'. MathStructure.cc 5569

Boucle non valide

V534 Il est probable qu'une mauvaise variable soit comparée à l'intérieur de l'opérateur 'for'. Pensez à revoir «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; } } .... }

Dans la boucle interne, le compteur est la variable i2 , mais en raison d'une faute de frappe, une erreur a été commise - à condition d'arrêter la boucle, la variable i de la boucle externe est utilisée.

Redondance ou erreur?

V590 Envisagez d'inspecter cette expression. L'expression est excessive ou contient une erreur d'impression. 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; .... }

Après avoir examiné un tel code, il y a 3 ans, j'ai écrit une note pour m'aider moi-même et d'autres programmeurs: " Expressions logiques en C / C ++. Comment les professionnels se trompent ." En rencontrant un tel code, je suis convaincu que la note n'est pas du tout devenue moins pertinente. Vous pouvez regarder dans l'article, trouver le modèle d'erreur correspondant au code et découvrir toutes les nuances.

Dans le cas de cet exemple, allez dans la section "Expression == || ! = ”Et nous apprenons que l'expression i2 == COMPARISON_RESULT_UNKNOWN n'affecte rien.

Déréférencer des pointeurs non vérifiés

V595 Le pointeur 'o_data' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 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); } .... }

Le pointeur o_data dans une fonction est déréférencé sans vérification et avec vérification. Il peut s'agir d'un code redondant ou d'une erreur potentielle. Je suis enclin à la dernière option.

Il y a deux autres endroits similaires:

V595 Le pointeur 'o_assumption' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 229, 230. Variable.cc 229
V595 Le pointeur 'i_value' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 3412, 3427. Number.cc 3412

gratuit () ou supprimer []?

V611 La mémoire a été allouée à l'aide de l'opérateur "nouveau" mais a été libérée à l'aide de la fonction "libre". Pensez à inspecter les logiques d'opération derrière la variable «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); // <= } } .... } .... }

La mémoire du tableau remcopy est allouée et libérée de diverses manières, ce qui est une grave erreur.

Changements perdus

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

La variable m est acceptée dans la fonction par référence, ce qui implique sa modification. Mais l'analyseur a constaté que le code contient la variable locale du même nom, qui chevauche la portée du paramètre de fonction, permettant la perte de modifications.

Pointeurs étranges

V774 Le pointeur «cu» a été utilisé après la libération de la mémoire. 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; } .... }

L'analyseur avertit que le code contient un appel à la méthode de l'objet cu après avoir libéré de la mémoire. Mais si vous essayez de comprendre le code, ce sera encore plus étrange. Premièrement, l'appel à supprimer cu se produit toujours - dans l'état et après. Deuxièmement, le code après la condition suppose que les pointeurs u et cu ne sont pas égaux, donc après avoir effacé l'objet cu , il est logique d'utiliser l'objet u . Très probablement, une faute de frappe a été faite dans le code et il était prévu d'utiliser uniquement la variable u .

Utilisation de la fonction de recherche

V797 La fonction 'find' est utilisée comme si elle retournait un type booléen. La valeur de retour de la fonction doit probablement être comparée à 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; } .... }

Bien que le code se compile correctement, il semble suspect car la fonction find renvoie un certain nombre de type std :: string :: size_type . La condition sera vraie si le point se trouve n'importe où sur la ligne, sauf si le point est au début. Ceci est un test étrange. Je ne suis pas sûr, mais peut-être que le code devrait être réécrit comme suit:

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

Fuite potentielle de mémoire

V701 realloc () fuite possible: lorsque realloc () échoue dans l'allocation de mémoire, le «tampon» du pointeur d'origine est perdu. Pensez à affecter realloc () à un pointeur temporaire. 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 }

Lorsque vous travaillez avec la fonction realloc () , il est recommandé d'utiliser un tampon intermédiaire, car s'il est impossible d'allouer de la mémoire, le pointeur vers l'ancienne mémoire sera irrémédiablement perdu.

Conclusion

Projet Qalculate! en tête de liste des meilleures calculatrices gratuites, alors qu'il contient de nombreuses erreurs graves. Mais nous n'avons pas vu ses concurrents. Nous allons essayer de parcourir toutes les calculatrices populaires.

Quant à la comparaison avec la qualité de la calculatrice du monde Windows, alors que l'utilitaire de Microsoft semble plus fiable et de haute qualité.

Vérifiez votre «calculatrice» en téléchargeant PVS-Studio et en l'essayant sur votre projet. :-)

Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Svyatoslav Razmyslov. Sur les traces des calculatrices: calculez!

Suivre la trace des calculatrices: Qalculate