Suivre les traces des calculatrices: SpeedCrunch

Nous y voilà, continuons d'explorer le code des calculatrices! Aujourd'hui, nous allons jeter un œil au projet appelé SpeedCrunch, la deuxième calculatrice gratuite la plus populaire.

Présentation

SpeedCrunch est une calculatrice scientifique de haute précision dotée d'une interface utilisateur rapide pilotée par clavier. Il s'agit d'un logiciel gratuit et open source, sous licence GPL et fonctionnant sous Windows, Linux et macOS.

Le code source est disponible sur BitBucket . J'ai été quelque peu déçu par la documentation de construction, qui pourrait être plus détaillée. Il dit que vous avez besoin de "Qt 5.2 ou version ultérieure" pour construire le projet, mais cela nécessitait en fait quelques packages spécifiques, ce qui n'était pas facile à comprendre à partir du journal CMake. Soit dit en passant, il est considéré de nos jours comme une bonne pratique d'inclure un Dockerfile dans le projet pour faciliter la configuration de l'environnement de développement par l'utilisateur.

Voici la sortie de l'utilitaire Cloc montrant comment SpeedCrunch se compare aux autres calculatrices:

Revues de bugs pour les autres projets:

• Compter les bogues dans la calculatrice Windows
• Sur les traces des calculatrices: calculez!

L'analyse a été effectuée avec l'analyseur statique PVS-Studio . Il s'agit d'un ensemble de solutions pour le contrôle qualité des logiciels et la détection des bogues et des vulnérabilités potentielles. PVS-Studio prend en charge C, C ++, C # et Java et s'exécute sur Windows, Linux et macOS.

Logique étrange dans une boucle

V560 Une partie de l'expression conditionnelle est toujours vraie:! RuleFound. evaluator.cpp 1410

void Evaluator::compile(const Tokens& tokens) { .... while (!syntaxStack.hasError()) { bool ruleFound = false; // <= // Rule for function last argument: id (arg) -> arg. if (!ruleFound && syntaxStack.itemCount() >= 4) { // <= Token par2 = syntaxStack.top(); Token arg = syntaxStack.top(1); Token par1 = syntaxStack.top(2); Token id = syntaxStack.top(3); if (par2.asOperator() == Token::AssociationEnd && arg.isOperand() && par1.asOperator() == Token::AssociationStart && id.isIdentifier()) { ruleFound = true; // <= syntaxStack.reduce(4, MAX_PRECEDENCE); m_codes.append(Opcode(Opcode::Function, argCount)); #ifdef EVALUATOR_DEBUG dbg << "\tRule for function last argument " << argCount << " \n"; #endif argCount = argStack.empty() ? 0 : argStack.pop(); } } .... } .... } 

Notez la variable ruleFound : elle est définie sur false à chaque itération. Dans le corps de la boucle, cependant, cette variable est définie sur true dans certaines conditions, mais elle sera redéfinie sur false à la prochaine itération. La variable ruleFound aurait probablement dû être déclarée avant la boucle.

Comparaisons suspectes

V560 Une partie de l'expression conditionnelle est toujours vraie: m_scrollDirection! = 0. resultdisplay.cpp 242

 void ResultDisplay::fullContentScrollEvent() { QScrollBar* bar = verticalScrollBar(); int value = bar->value(); bool shouldStop = (m_scrollDirection == -1 && value <= 0) || (m_scrollDirection == 1 && value >= bar->maximum()); if (shouldStop && m_scrollDirection != 0) { // <= stopActiveScrollingAnimation(); return; } scrollLines(m_scrollDirection * 10); } 

Si la valeur de la variable shouldStop est vraie , la variable m_scrollDirection prendra l'une des deux valeurs: -1 ou 1. Par conséquent, sa valeur sera certainement différente de zéro dans la prochaine instruction conditionnelle, ce à quoi l'analyseur met en garde.

V668 Il est inutile de tester le pointeur 'item' sur null, car la mémoire a été allouée à l'aide de l'opérateur 'new'. L'exception sera générée en cas d'erreur d'allocation de mémoire. editor.cpp 998

 void EditorCompletion::showCompletion(const QStringList& choices) { .... for (int i = 0; i < choices.count(); ++i) { QStringList pair = choices.at(i).split(':'); QTreeWidgetItem* item = new QTreeWidgetItem(m_popup, pair); if (item && m_editor->layoutDirection() == Qt::RightToLeft) item->setTextAlignment(0, Qt::AlignRight); .... } .... } 

La mémoire d'un objet de type QTreeWidgetItem est allouée à l'aide du nouvel opérateur. Cela signifie qu'un échec d'allocation de mémoire entraînera le lancement d'une exception std :: bad_alloc () . La vérification du pointeur d' élément est donc redondante et peut être supprimée.

Déréférence NULL potentielle

V595 Le pointeur «ioparams» a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 969, 983. floatio.c 969

 int cattokens(....) { .... if (printexp) { if (expbase < 2) expbase = ioparams->expbase; // <= .... } dot = '.'; expbegin = "("; expend = ")"; if (ioparams != NULL) // <= { dot = ioparams->dot; expbegin = ioparams->expbegin; expend = ioparams->expend; } .... } 

Le pointeur ioparams est déréférencé avant la vérification. Il semble qu'il y ait une erreur ici. Étant donné que la déréférence est précédée d'un certain nombre de conditions, le bogue n'apparaîtra pas souvent, mais il aura un effet drastique lorsqu'il le fera.

Division par zéro

V609 Divisez par zéro. Plage du dénominateur [0..4]. floatconvert.c 266

 static int lgbase( signed char base) { switch(base) { case 2: return 1; case 8: return 3; case 16: return 4; } return 0; // <= } static void _setlongintdesc( p_ext_seq_desc n, t_longint* l, signed char base) { int lg; n->seq.base = base; lg = lgbase(base); // <= n->seq.digits = (_bitlength(l) + lg - 1) / lg; // <= n->seq.leadingSignDigits = 0; n->seq.trailing0 = _lastnonzerobit(l) / lg; // <= n->seq.param = l; n->getdigit = _getlongintdigit; } 

La fonction lgbase peut retourner zéro, qui pourrait ensuite être utilisé comme diviseur. La fonction peut être potentiellement appelée avec n'importe quelle valeur, pas seulement 2, 8 ou 16.

Comportement indéfini

V610 Comportement indéfini . Vérifiez l'opérateur de décalage «<<». L'opérande gauche '(~ 0)' est négatif. floatlogic.c 64

 static char _signextend( t_longint* longint) { unsigned mask; signed char sign; sign = _signof(longint); mask = (~0) << SIGNBIT; // <= if (sign < 0) longint->value[MAXIDX] |= mask; else longint->value[MAXIDX] &= ~mask; return sign; } 

Étant donné que le résultat de l'inversion de zéro est stocké dans un entier signé, la valeur résultante sera un nombre négatif, qui est ensuite décalé. Décaler à gauche une valeur négative est un comportement non défini.

Voici une liste complète de tous ces cas:

• V610 Comportement indéfini. Vérifiez l'opérateur de décalage «<<». L'opérande gauche '(- 1)' est négatif. floatnum.c 289
• V610 Comportement indéfini. Vérifiez l'opérateur de décalage «<<». L'opérande gauche '(- 1)' est négatif. floatnum.c 325
• V610 Comportement indéfini. Vérifiez l'opérateur de décalage «<<». L'opérande gauche '(- 1)' est négatif. floatnum.c 344
• V610 Comportement indéfini. Vérifiez l'opérateur de décalage «<<». L'opérande gauche '(- 1)' est négatif. floatnum.c 351

Balises HTML non fermées

V735 Peut-être un HTML incorrect. La balise de fermeture "</body>" a été rencontrée, tandis que la balise "</div>" était attendue. book.cpp 127

 static QString makeAlgebraLogBaseConversionPage() { return BEGIN INDEX_LINK TITLE(Book::tr("Logarithmic Base Conversion")) FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a)) END; } 

Comme c'est souvent le cas avec le code C / C ++, l'étude de la source n'aide pas beaucoup à comprendre les choses, nous allons donc jeter un œil au code prétraité à la place:

L'analyseur a détecté une étiquette div non fermée. Ce fichier contient de nombreux extraits en HTML, et les développeurs devront également vérifier ce code.

Voici quelques autres cas suspects trouvés par PVS-Studio:

• V735 Peut-être un HTML incorrect. La balise de fermeture "</td>" a été rencontrée, tandis que la balise "</sub>" était attendue. book.cpp 344
• V735 Peut-être un HTML incorrect. La balise de fermeture "</td>" a été rencontrée, tandis que la balise "</sub>" était attendue. book.cpp 347

Opérateur d'affectation

V794 L'opérateur d'affectation doit être protégé contre le cas de 'this == & other'. quantité.cpp 373

 Quantity& Quantity::operator=(const Quantity& other) { m_numericValue = other.m_numericValue; m_dimension = other.m_dimension; m_format = other.m_format; stripUnits(); if(other.hasUnit()) { m_unit = new CNumber(*other.m_unit); m_unitName = other.m_unitName; } cleanDimension(); return *this; } 

Il est recommandé de vérifier les situations dans lesquelles un objet est attribué à lui-même en comparant les pointeurs. En d'autres termes, ajoutez les deux lignes suivantes au début du corps de la fonction:

 if (this == &other) return *this; 

Pour rappel

V601 La valeur 'false' est implicitement convertie en type entier. cmath.cpp 318

 /** * Returns -1, 0, 1 if n1 is less than, equal to, or more than n2. * Only valid for real numbers, since complex ones are not an ordered field. */ int CNumber::compare(const CNumber& other) const { if (isReal() && other.isReal()) return real.compare(other.real); else return false; // FIXME: Return something better. } 

Parfois, vous dites dans les commentaires que certains avertissements sont peut-être déclenchés par un code incomplet. Oui, cela se produit de temps en temps, mais nous signalons spécifiquement de tels cas.

Conclusion

Nous avons déjà vérifié le code de trois programmes de calculatrice - Windows Calculator, Qalculate! Et SpeedCrunch - et nous n'allons pas nous arrêter. N'hésitez pas à suggérer des projets que vous souhaitez que nous vérifions car les classements logiciels ne reflètent pas toujours l'état réel des choses.

Bienvenue pour télécharger PVS-Studio et l'essayer sur votre propre «calculatrice». :-)