Siguiendo los pasos de las calculadoras: calcular


Anteriormente realizamos revisiones de código de grandes paquetes matemáticos, por ejemplo, Scilab y Octave, por lo que las calculadoras se mantuvieron distantes como pequeñas utilidades, en las que es difícil cometer errores debido a su pequeña base de código. Nos equivocamos al no haberles prestado atención. El caso de publicar el código fuente de la calculadora de Windows mostró que en realidad todos estaban interesados ​​en discutir los tipos de errores que se ocultan en él. Además, la cantidad de errores allí fue más que suficiente para escribir un artículo sobre eso. Mis colegas y yo decidimos explorar 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! es una calculadora de escritorio multiplataforma multipropósito. Es fácil de usar, pero proporciona potencia y versatilidad normalmente reservadas para paquetes matemáticos complicados, así como herramientas útiles para las necesidades diarias (como conversión de moneda y cálculo de porcentajes). El proyecto consta de dos componentes: libqalculate (biblioteca y CLI) y qalculate-gtk (GTK + UI). El estudio incluyó solo el código libqalculate.

Para comparar fácilmente el proyecto con la Calculadora de Windows, que hemos verificado recientemente, estoy citando el resultado de la utilidad Cloc para libqalculate:


Considerando esto subjetivamente, hay más errores y son más críticos que en el código de la calculadora de Windows. Sin embargo, recomendaría sacar conclusiones por su cuenta, después de leer esta descripción general del código.

Por cierto, aquí hay un enlace a un artículo sobre la verificación de la calculadora de Microsoft: " Contando errores en la calculadora de Windows ".

La herramienta de análisis es el analizador de código estático PVS-Studio . Es un conjunto de soluciones para el control de calidad del código, búsqueda de errores y vulnerabilidades potenciales. Los lenguajes compatibles incluyen: C, C ++, C # y Java. Puede ejecutar el analizador 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 es absolutamente el mismo en los bloques if y else . Los fragmentos de código adyacentes son muy similares a este, pero se utilizan diferentes funciones en ellos: internalLowerFloat () y internalUpperFloat () . Es seguro asumir que un desarrollador copió el código y se olvidó de corregir el nombre de la función aquí.

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

En este caso, aparecieron expresiones duplicadas debido al hecho de que en un lugar se escribió mtr2 en lugar de mtr. Por lo tanto, una llamada de la función mtr.number (). IsReal () está ausente en la condición.

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



¡Nunca habríamos encontrado defectos en este código manualmente! Pero aquí los hay. Además, en el archivo original, estos fragmentos están escritos en una sola línea. El analizador ha detectado una expresión duplicada vargs [1] .representsNonPositive () , que puede indicar un error tipográfico o, en consecuencia, un posible error.

Aquí está la lista completa de lugares sospechosos, que apenas se pueden descifrar.

  • 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 con condición incorrecta


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, la variable i2 representa un contador, pero debido a un error tipográfico se cometió un error: la variable i del bucle externo se usa en la condición de salida del bucle.

¿Redundancia o un 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; .... } 

Hace 3 años, después de haber visto ese código, escribí una hoja de trucos para mí y otros desarrolladores: " Expresiones lógicas en C / C ++. Errores cometidos por profesionales ". Cuando me encuentro con ese código, me aseguro de que la nota no se ha vuelto menos relevante. Puede consultar el artículo, encontrar un patrón del error correspondiente al código y descubrir todos los matices.

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

Desreferencia de punteros no controlados


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

En una función, el puntero o_data se desreferencia tanto con un check como sin él. Esto puede ser código redundante o un posible error. Me estoy inclinando hacia lo último.

Hay dos lugares 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 de remcopy se asigna y libera de diferentes maneras, lo cual es un error grave.

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 en la función se pasa por referencia, lo que significa su modificación. Sin embargo, el analizador ha detectado que el código contiene la variable con el 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 llama a un método del objeto cu justo después de desasignar la memoria. Pero cuando se trata de lidiar con él, el código resulta aún más extraño. En primer lugar, llamar a delete cu siempre ocurre, tanto en la condición como después de eso. En segundo lugar, el código después de la condición implica que los punteros u y cu no son iguales, lo que significa que después de eliminar el objeto cu es bastante lógico usar el objeto u . Lo más probable es que se haya cometido un error tipográfico en el código y el autor del código quería usar solo la variable u .

Uso de 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 puede compilar con éxito, parece sospechoso, ya que la función de búsqueda devuelve el número del tipo std :: string :: size_type . La condición será verdadera si el punto se encuentra en cualquier parte de la cadena, excepto si el punto está al principio. Es un cheque extraño. No estoy seguro, pero, tal vez, este 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 en caso de que sea imposible asignar memoria, el puntero al área de memoria anterior se perderá irremediablemente.

Conclusión


El Qalculate! El proyecto encabeza la lista de las mejores calculadoras gratuitas, mientras que contiene muchos errores graves. Por otro lado, aún no hemos revisado sus competidores. Intentaremos repasar todas las calculadoras populares.

En cuanto a la comparación con la calidad de la calculadora del mundo de Windows, la utilidad de Microsoft parece más confiable y bien trabajada hasta ahora.

Compruebe su propia "Calculadora": descargue PVS-Studio y pruébelo para su proyecto. :-)

Source: https://habr.com/ru/post/443656/


All Articles