⚱️ 👩🏻‍🏫 🖐🏽 Siguiendo el rastro de las calculadoras: calcular 🔢 🧓🏻 🗓️

Anteriormente realizamos revisiones de código de paquetes matemáticos grandes, por ejemplo, Scilab y Octave, y las calculadoras se quedaron a un lado como pequeñas utilidades en las que es difícil cometer errores debido a su pequeño tamaño de código. Nos equivocamos al no prestarles atención. El caso con la publicación del código fuente de la calculadora de Windows mostró que todos están interesados en discutir qué errores están ocultos allí, y que hay errores más que suficientes para escribir un artículo al respecto. Mis colegas y yo decidimos examinar el código de varias calculadoras populares, y resultó que el código de la calculadora de Windows no era tan malo (spoiler).

Introduccion

Qalculate! - calculadora universal multiplataforma. Es fácil de usar, pero proporciona el poder y la versatilidad que generalmente se encuentran en paquetes matemáticos complejos, así como herramientas útiles para las necesidades diarias (como la conversión de moneda y el cálculo de intereses). El proyecto consta de dos componentes: libqalculate (biblioteca y CLI) y qalculate-gtk (GTK + UI). Solo el código libqalculate está involucrado en el estudio.

Para comparar más convenientemente el proyecto con la misma calculadora de Windows que examinamos recientemente, proporciono el resultado de la utilidad Cloc para libqalculate:

Subjetivamente, hay más errores, y son más críticos que en el código de la calculadora de Windows. Pero le recomiendo que saque conclusiones usted mismo leyendo esta revisión de código.

Por cierto, aquí hay un enlace a un artículo sobre cómo verificar una calculadora de Microsoft: " Contando errores en una calculadora de Windows " .

PVS-Studio se utilizó como herramienta de análisis estático. Este es un conjunto de soluciones para el control de calidad del código, la búsqueda de errores y posibles vulnerabilidades. Los lenguajes compatibles incluyen: C, C ++, C # y Java. El analizador se puede iniciar en Windows, Linux y macOS.

Copiar y pegar errores tipográficos de nuevo!

V523 La declaración 'then' es equivalente a la declaración '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); } .... }

El código en la instrucción if y else es exactamente el mismo. Los fragmentos de código vecinos son muy similares a esto, pero utilizan diferentes funciones: internalLowerFloat () e internalUpperFloat () . Es seguro asumir que aquí el programador copió el código y olvidó corregir el nombre de la función.

V501 Hay subexpresiones idénticas '! Mtr2.number (). IsReal ()' a la izquierda y a la derecha de '||' operador BuiltinFunctions.cc 6274

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

Aquí surgieron expresiones duplicadas porque en un lugar en lugar del nombre mtr escribieron mtr2 . Por lo tanto, en la condición no hay llamada a la función mtr.number (). IsReal () .

V501 Hay subexpresiones idénticas 'vargs [1] .representsNonPositive ()' a la izquierda y a la derecha de '||' operador BuiltinFunctions.cc 5785

Cuadro 6

¡Encontrar anomalías en este código manualmente es irreal! Pero ellos son. Además, en el archivo original, estos fragmentos están escritos en una línea. El analizador detectó una expresión duplicada vargs [1] .representsNonPositive () , que puede indicar un error tipográfico y, por lo tanto, un error potencial.

Aquí está la lista completa de lugares sospechosos que difícilmente puede entender:

V501 Hay subexpresiones idénticas 'vargs [1] .representsNonPositive ()' a la izquierda y a la derecha de '||' operador BuiltinFunctions.cc 5788
V501 Hay subexpresiones idénticas 'anexar' a la izquierda y a la derecha del operador '&&'. MathStructure.cc 1780
V501 Hay subexpresiones idénticas 'anexar' a la izquierda y a la derecha del operador '&&'. MathStructure.cc 2043
V501 Hay subexpresiones idénticas '(* v_subs [v_order [1]]). Representa Negative (true)' a la izquierda y a la derecha del operador '&&'. MathStructure.cc 5569

Bucle inválido

V534 Es probable que se compare una variable incorrecta dentro del operador 'for'. Considere revisar '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; } } .... }

En el bucle interno, el contador es la variable i2 , pero debido a un error tipográfico, se cometió un error: en la condición de que el bucle se detenga, se usa la variable i del bucle externo.

¿Redundancia o error?

V590 Considere inspeccionar esta expresión. La expresión es excesiva o contiene un error de imprenta. 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; .... }

Después de mirar ese código, hace 3 años escribí una nota para ayudarme a mí mismo y a otros programadores: " Expresiones lógicas en C / C ++. Cómo se equivocan los profesionales ". Al encontrar dicho código, estoy convencido de que la nota no se ha vuelto menos relevante en absoluto. Puede consultar el artículo, encontrar el patrón de error correspondiente al código y conocer todos los matices.

En el caso de este ejemplo, vaya a la sección "Expresión == || ! = ”Y aprendemos que la expresión i2 == COMPARISON_RESULT_UNKNOWN no afecta nada.

Desreferenciar punteros no verificados

V595 El puntero 'o_data' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 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); } .... }

El puntero o_data en una función se desreferencia sin verificar y con verificar. Esto puede ser código redundante o un posible error. Estoy inclinado a la última opción.

Hay dos lugares más similares:

V595 El puntero 'o_assumption' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 229, 230. Variable.cc 229
V595 El puntero 'i_value' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 3412, 3427. Number.cc 3412

free () o eliminar []?

V611 La memoria fue asignada usando el operador 'nuevo' pero fue liberada usando la función 'libre'. Considere inspeccionar las lógicas de operación detrás de 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 memoria para la matriz remcopy se asigna y libera de varias maneras, lo cual es un grave error.

Cambios perdidos

 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 se acepta en la función por referencia, lo que implica su modificación. Pero el analizador descubrió que el código contiene la variable local del mismo nombre, que se superpone al alcance del parámetro de la función, lo que permite la pérdida de cambios.

Punteros extraños

V774 El puntero 'cu' se usó después de liberar la memoria. 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; } .... }

El analizador advierte que el código contiene una llamada al método del objeto cu después de liberar memoria. Pero si intentas entender el código, será aún más extraño. En primer lugar, la llamada para eliminar cu siempre ocurre, en la condición y después. En segundo lugar, el código después de la condición supone que los punteros u y cu no son iguales, por lo que después de borrar el objeto cu , es lógico usar el objeto u . Lo más probable es que se haya cometido un error tipográfico en el código y se planeó usar solo la variable u .

Usando la función de búsqueda

V797 La función 'buscar' se usa como si devolviera un tipo bool. El valor de retorno de la función probablemente debería compararse con 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; } .... }

Aunque el código se compila correctamente, parece sospechoso ya que la función de búsqueda devuelve un número de tipo std :: string :: size_type . La condición será verdadera si el punto se encuentra en cualquier lugar de la línea, excepto si el punto está al principio. Esta es una prueba extraña. No estoy seguro, pero quizás el código debería reescribirse de la siguiente manera:

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

Posible fuga de memoria

V701 realloc () posible fuga: cuando realloc () falla en la asignación de memoria, el puntero original 'buffer' se pierde. Considere asignar realloc () a un puntero temporal. 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 }

Cuando se trabaja con la función realloc () , se recomienda utilizar un búfer intermedio, ya que si es imposible asignar memoria, el puntero a la memoria anterior se perderá irremediablemente.

Conclusión

¡Qalculate proyecto! encabeza la lista de las mejores calculadoras gratuitas, mientras que contiene muchos errores graves. Pero no hemos visto a sus competidores. Intentaremos revisar todas las calculadoras populares.

En cuanto a la comparación con la calidad de la calculadora del mundo de Windows, mientras que la utilidad de Microsoft se ve más confiable y de alta calidad.

Verifique su "Calculadora" descargando PVS-Studio y probándolo en su proyecto. :-)

Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov. Siguiendo los pasos de las calculadoras: ¡Calcule!

Siguiendo el rastro de las calculadoras: calcular