Seguindo os passos das calculadoras: Qalculate


Anteriormente, fizemos revisões de código de grandes pacotes matemáticos, por exemplo, Scilab e Octave, em que as calculadoras permaneciam distantes como pequenas utilidades, nas quais é difícil cometer erros devido à sua pequena base de código. Estávamos errados por não termos prestado atenção neles. O caso da publicação do código-fonte da calculadora do Windows mostrou que, na verdade, todos estavam interessados ​​em discutir os tipos de erros ocultos. Além disso, o número de erros foi mais do que suficiente para escrever um artigo sobre isso. Meus colegas e eu decidimos explorar o código de várias calculadoras populares, e o código da calculadora do Windows não era tão ruim (spoiler).

1. Introdução


Qalculate! é uma calculadora de desktop multiplataforma multiuso. É simples de usar, mas fornece energia e versatilidade normalmente reservadas para pacotes matemáticos complicados, além de ferramentas úteis para as necessidades diárias (como conversão de moeda e cálculo de porcentagem). O projeto consiste em dois componentes: libqalculate (biblioteca e CLI) e qalculate-gtk (GTK + UI). O estudo envolveu apenas o código libqalculate.

Para comparar facilmente o projeto com a Calculadora do Windows, que verificamos recentemente, cito a saída do utilitário Cloc para libqalculate:


Considerando-o subjetivamente, há mais erros e são mais críticos do que no código da calculadora do Windows. No entanto, eu recomendaria tirar conclusões por conta própria, depois de ler esta visão geral do código.

A propósito, aqui está um link para um artigo sobre a verificação da calculadora da Microsoft: " Contando bugs na calculadora do Windows ".

A ferramenta de análise é o analisador de código estático do PVS-Studio . É um conjunto de soluções para controle de qualidade de código, pesquisa de bugs e possíveis vulnerabilidades. Os idiomas suportados incluem: C, C ++, C # e Java. Você pode executar o analisador no Windows, Linux e macOS.

Copie e cole e erros de digitação novamente!


V523 A instrução 'then' é equivalente à instrução '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); } .... } 

O código é absolutamente o mesmo nos blocos if e else . Fragmentos de código adjacentes são muito semelhantes a este, mas funções diferentes são usadas neles: internalLowerFloat () e internalUpperFloat () . É seguro assumir que um desenvolvedor copiou o código e esqueceu de corrigir o nome da função aqui.

V501 Existem sub-expressões idênticas '! Mtr2.number (). IsReal ()' à esquerda e à direita do '||' operador. BuiltinFunctions.cc 6274

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

Nesse caso, expressões duplicadas apareceram devido ao fato de que em um local o mtr2 foi gravado em vez do mtr. Assim, uma chamada da função mtr.number (). IsReal () está ausente na condição.

V501 Existem subexpressões idênticas 'vargs [1] .represententsNonPositive ()' à esquerda e à direita da '||' operador. BuiltinFunctions.cc 5785



Nunca teríamos encontrado defeitos nesse código manualmente! Mas aqui está. Além disso, no arquivo original esses fragmentos são escritos em uma única linha. O analisador detectou uma expressão duplicada vargs [1] .represententsNonPositive () , que pode indicar um erro de digitação ou, consequentemente, um erro em potencial.

Aqui está toda a lista de lugares suspeitos, que mal se pode descobrir.

  • V501 Existem subexpressões idênticas 'vargs [1] .represententsNonPositive ()' à esquerda e à direita da '||' operador. BuiltinFunctions.cc 5788
  • V501 Existem subexpressões idênticas 'anexadas' à esquerda e à direita do operador '&&'. MathStructure.cc 1780
  • V501 Existem subexpressões idênticas 'anexadas' à esquerda e à direita do operador '&&'. MathStructure.cc 2043
  • V501 Existem subexpressões idênticas '(* v_subs [v_order [1]]). RepresentsNegative (true)' à esquerda e à direita do operador '&&'. MathStructure.cc 5569

Loop com condição incorreta


V534 É provável que uma variável incorreta esteja sendo comparada dentro do 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; } } .... } 

No loop interno, a variável i2 representa um contador, mas devido a um erro de digitação, um erro foi cometido - a variável i do loop externo é usada na condição de saída do loop.

Redundância ou erro?


V590 Considere inspecionar esta expressão. A expressão é excessiva ou contém uma impressão incorreta. 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; .... } 

Três anos atrás, depois de conhecer esse código, escrevi uma folha de dicas para mim e para outros desenvolvedores: " Expressões lógicas em C / C ++. Erros cometidos por profissionais ". Quando me deparei com esse código, verifique se a nota não se tornou menos relevante. Você pode examinar o artigo, encontrar um padrão de erro correspondente ao código e descobrir todas as nuances.

No caso deste exemplo, iremos para a seção "Expressão == || ! = "E descubra que a expressão i2 == COMPARISON_RESULT_UNKNOWN não afeta nada.

Desreferenciando ponteiros não verificados


V595 O ponteiro 'o_data' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 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); } .... } 

Em uma função, o ponteiro o_data é desreferenciado sem e com uma verificação. Pode ser um código redundante ou um erro em potencial. Estou inclinado para o último.

Existem dois lugares semelhantes:

  • V595 O ponteiro 'o_assumption' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 229, 230. Variable.cc 229
  • V595 O ponteiro 'i_value' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 3412, 3427. Number.cc 3412

grátis () ou excluir []?


V611 A memória foi alocada usando o operador 'novo', mas foi liberada usando a função 'livre'. Considere inspecionar as lógicas de operação por trás da variável '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); // <= } } .... } .... } 

A memória para a matriz remcopy é alocada e liberada de diferentes maneiras, o que é um erro grave.

Mudanças perdidas


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

A variável m na função é passada por referência, o que significa sua modificação. No entanto, o analisador detectou que o código contém a variável com o mesmo nome, que se sobrepõe ao escopo do parâmetro da função, permitindo a perda de alterações.

Ponteiros estranhos


V774 O ponteiro 'cu' foi usado após o lançamento da memória. 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; } .... } 

O analisador avisa que o código chama um método do objeto cu logo após desalocar a memória. Mas, ao tentar lidar com isso, o código acaba sendo ainda mais estranho. Em primeiro lugar, a chamada delete cu acontece sempre - na condição e depois disso. Em segundo lugar, o código após a condição implica que os ponteiros u e cu não sejam iguais, o que significa que, após a exclusão do objeto cu , é bastante lógico usar o objeto u . Provavelmente, um erro de digitação foi feito no código e o autor do código queria usar apenas a variável u .

Uso da função find


V797 A função 'find' é usada como se retornasse um tipo de bool. O valor de retorno da função provavelmente deve ser comparado com 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; } .... } 

Mesmo que o código possa ser compilado com êxito, ele parece suspeito, pois a função find retorna o número do tipo std :: string :: size_type . A condição será verdadeira se o ponto for encontrado em qualquer parte da string, exceto se o ponto estiver no início. É um cheque estranho. Não tenho certeza, mas talvez esse código seja reescrito da seguinte maneira:

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

Potencial vazamento de memória


V701 realloc () possível vazamento: quando realloc () falha na alocação de memória, o ponteiro original 'buffer' é perdido. Considere atribuir realloc () a um ponteiro temporário. 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 } 

Ao trabalhar com a função realloc () , é recomendável usar um buffer intermediário, pois, se for impossível alocar memória, o ponteiro para a área de memória antiga será irremediavelmente perdido.

Conclusão


O Qalculate! O projeto encabeça a lista das melhores calculadoras gratuitas, enquanto contém muitos erros sérios. Por outro lado, ainda não verificamos seus concorrentes. Vamos tentar analisar todas as calculadoras populares.

Quanto à comparação com a qualidade da calculadora do mundo Windows, o utilitário da Microsoft parece mais confiável e bem trabalhado até agora.

Verifique sua própria “Calculadora” - faça o download do PVS-Studio e experimente para o seu projeto. :-)

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


All Articles