Auparavant, nous avons effectué des révisions de code de grands packages mathématiques, par exemple, Scilab et Octave, où les calculatrices sont restées à l'écart en tant que petits utilitaires, dans lesquels il est difficile de faire des erreurs en raison de leur petite base de code. Nous avions tort de ne pas leur avoir prêté attention. Le cas de la publication du code source de la calculatrice Windows a montré qu'en fait, tout le monde était intéressé à discuter des types d'erreurs qui s'y cachent. De plus, le nombre d'erreurs était largement suffisant pour écrire un article à ce sujet. Mes collègues et moi, nous avons décidé d'explorer 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! est une calculatrice de bureau multi-plateforme multi-usages. Il est simple à utiliser, mais offre une puissance et une polyvalence normalement réservées aux packages mathématiques complexes, ainsi que des outils utiles pour les besoins quotidiens (tels que la conversion de devises et le calcul du pourcentage). Le projet se compose de deux composants:
libqalculate (bibliothèque et CLI) et
qalculate-gtk (GTK + UI). L'étude ne portait que sur le code libqalculate.
Pour comparer facilement le projet avec Windows Calculator, que nous avons récemment vérifié, je cite la sortie de l'utilitaire Cloc pour libqalculate:
En le considérant subjectivement, il contient plus d'erreurs et elles sont plus critiques que dans le code de la calculatrice Windows. Néanmoins, je recommanderais de tirer des conclusions par vous-même, après avoir lu cet aperçu du code.
À propos, voici un lien vers un article sur la vérification de la calculatrice de Microsoft: "
Counting Bugs in Windows Calculator ".
L'outil d'analyse est l'analyseur de code statique
PVS-Studio . Il s'agit d'un ensemble de solutions pour le contrôle de la qualité du code, la recherche de bogues et les vulnérabilités potentielles. Les langages pris en charge incluent: C, C ++, C # et Java. Vous pouvez exécuter l'analyseur sur Windows, Linux et macOS.
Copiez-collez et Typos à 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 est absolument le même dans les blocs
if et
else . Les fragments de code adjacents sont très similaires à celui-ci, mais différentes fonctions y sont utilisées:
internalLowerFloat () et
internalUpperFloat () . Il est sûr de supposer qu'un développeur a copié le code et oublié de corriger le nom de la fonction ici.
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; .... }
Dans ce cas, des expressions dupliquées sont apparues en raison du fait qu'à un endroit
mtr2 a été écrit au lieu de
mtr. Ainsi, un appel de la fonction
mtr.number (). IsReal () est absent dans la condition.
V501 Il existe des sous-expressions identiques «vargs [1] .representNonPositive ()» à gauche et à droite de «||» opérateur. BuiltinFunctions.cc 5785
Nous n'aurions jamais trouvé de défauts dans ce code manuellement! Mais voilà. De plus, dans le fichier d'origine, ces fragments sont écrits sur une seule ligne. L'analyseur a détecté une expression dupliquée
vargs [1] .representNonPositive () , qui peut indiquer une faute de frappe ou, par conséquent, une erreur potentielle.
Voici la liste complète des endroits suspects, que l'on peut à peine décoder.
- 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 avec condition incorrecte
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, la variable
i2 représente un compteur, mais en raison d'une faute de frappe, une erreur a été commise - la variable
i de la boucle externe est utilisée dans la condition de sortie de la boucle.
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; .... }
Il y a 3 ans après avoir eu un aperçu de ce code, j'ai écrit une feuille de triche pour moi et d'autres développeurs: "
Expressions logiques en C / C ++. Erreurs commises par des professionnels ". Quand je tombe sur un tel code, je m'assure que la note n'est pas devenu moins pertinent. Vous pouvez consulter l'article, trouver un modèle d'erreur correspondant au code et découvrir toutes les nuances.
Dans le cas de cet exemple, nous irons à la section «Expression == || ! = "Et découvrez que l'expression
i2 == COMPARISON_RESULT_UNKNOWN n'affecte rien.
Déréférencer les pointeurs non contrôlé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();
Dans une fonction, le pointeur
o_data est déréférencé à la fois sans et avec une vérification. Cela peut être du code redondant ou une erreur potentielle. Je me penche vers ce dernier.
Il y a deux 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];
La mémoire du tableau
remcopy est allouée et libérée de différentes 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 dans la fonction est passée par référence, ce qui signifie sa modification. Cependant, l'analyseur a détecté que le code contient la variable avec le même nom, qui chevauche la portée du paramètre de la 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;
L'analyseur avertit que le code appelle une méthode de l'objet
cu juste après la désallocation de la mémoire. Mais quand on essaie de s'y attaquer, le code s'avère encore plus étrange. Premièrement, appeler
delete cu se produit toujours - à la fois dans la condition et après cela. Deuxièmement, le code après la condition implique que les pointeurs
u et
cu ne sont pas égaux, ce qui signifie qu'après la suppression de l'objet
cu , il est tout à fait logique d'utiliser l'objet
u . Très probablement, une faute de frappe a été faite dans le code et l'auteur du code voulait 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; } .... }
Même si le code peut être compilé avec succès, il semble suspect, car la fonction
find renvoie le numéro du type
std :: string :: size_type . La condition sera vraie si le point est trouvé dans n'importe quelle partie de la chaîne sauf si le point est au début. C'est un étrange chèque. Je ne suis pas sûr mais, peut-être, ce 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));
Lorsque vous travaillez avec la fonction
realloc () , il est recommandé d'utiliser un tampon intermédiaire, car dans le cas où il est impossible d'allouer de la mémoire, le pointeur vers l'ancienne zone de mémoire sera irrémédiablement perdu.
Conclusion
Le Qalculate! Le projet arrive en tête de liste des meilleures calculatrices gratuites, alors qu'il contient de nombreuses erreurs graves. D'un autre côté, nous n'avons pas encore vérifié ses concurrents. Nous allons essayer de parcourir toutes les calculatrices populaires.
En ce qui concerne la comparaison avec la qualité de la calculatrice du monde Windows, l'utilitaire de Microsoft semble jusqu'à présent plus fiable et bien travaillé.
Vérifiez votre propre «calculatrice» - téléchargez
PVS-Studio et essayez-le pour votre projet. :-)