# По следам калькуляторов: Qalculate\!

Ранее мы делали обзоры кода крупных математических пакетов, например, Scilab и Octave, а калькуляторы оставались в стороне как небольшие утилиты, в которых сложно допустить ошибки из\-за их малого объёма кода\. Мы ошиблись, не уделив им внимания\. Случай с публикацией исходного кода калькулятора Windows показал, что всем интересно пообсуждать, какие ошибки там прячутся, а ошибок там более чем достаточно, чтобы написать про это статью\. Мы с коллегами решили исследовать код ряда популярных калькуляторов и оказалось, что код калькулятора Windows был не так уж и плох \(спойлер\)\.

![0616_Qalculate_ru/image1.png](https://import.viva64.com/docx/blog/0616_Qalculate_ru/image1.png)

## Введение

[Qalculate\!](https://qalculate.github.io/) \- универсальный кроссплатформенный калькулятор\. Он прост в использовании, но обеспечивает мощь и универсальность, обычно характерную для сложных математических пакетов, а также полезные инструменты для повседневных нужд \(таких как конвертация валюты и расчет процентов\)\. Проект состоит из двух компонентов: [libqalculate](https://github.com/Qalculate/libqalculate) \(library and CLI\) и [qalculate\-gtk](https://github.com/Qalculate/qalculate-gtk) \(GTK\+ UI\)\. В исследовании участвует только код libqalculate\.

Чтобы удобнее сравнить проект с тем же калькулятором Windows, который мы недавно исследовали, привожу вывод утилиты Cloc для libqalculate:

![0616_Qalculate_ru/image2.png](https://import.viva64.com/docx/blog/0616_Qalculate_ru/image2.png)

Субъективно, ошибок больше, и они более критичные, чем в коде калькулятора Windows\. Но рекомендую сделать выводы самостоятельно, ознакомившись с данным обзором кода\.

Обзоры ошибок в других проектах:

* [Подсчитаем баги в калькуляторе Windows](https://pvs-studio.ru/ru/blog/posts/cpp/0615/)
* [По следам калькуляторов: SpeedCrunch](https://pvs-studio.ru/ru/blog/posts/cpp/0618/)

В качестве инструмента статического анализа использовался [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. Это комплекс решений для контроля качества кода, поиска ошибок и потенциальных уязвимостей\. В поддерживаемые языки входят: C, C\+\+, C\# и Java\. Запуск анализатора возможен на Windows, Linux и macOS\.

## Снова copy\-paste и опечатки\!

[V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. Number\.cc 4018

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

Код в операторе _if_ и _else_ абсолютно одинаковый\. Соседние фрагменты кода очень похожи на этот, но в них используются разные функции: _internalLowerFloat\(\)_ и _internalUpperFloat\(\)_\. Можно с уверенностью предположить, что здесь программист скопировал код и забыл поправить имя функции\.

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\!mtr2\.number\(\)\.isReal\(\)' to the left and to the right of the '\|\|' operator\. BuiltinFunctions\.cc 6274

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

Здесь дублирующиеся выражения возникли из\-за того, что в одном месте вместо имени _mtr_ написали _mtr2_\. Таким образом, в условии отсутствует вызов функции _mtr\.number\(\)\.isReal\(\)_\.

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'vargs\[1\]\.representsNonPositive\(\)' to the left and to the right of the '\|\|' operator\. BuiltinFunctions\.cc 5785

![0616_Qalculate_ru/image3.png](https://import.viva64.com/docx/blog/0616_Qalculate_ru/image3.png)

Найти аномалии в этом коде вручную нереально\! Но они есть\. Причём в оригинальном файле эти фрагменты записаны в одну строку\. Анализатор обнаружил дублирующееся выражение _vargs\[1\]\.representsNonPositive\(\)_, что может свидетельствовать об опечатке и, следовательно, о потенциальной ошибке\.

Вот весь список подозрительных мест, в которых едва ли можно разобраться:

* V501 There are identical sub\-expressions 'vargs\[1\]\.representsNonPositive\(\)' to the left and to the right of the '\|\|' operator\. BuiltinFunctions\.cc 5788
* V501 There are identical sub\-expressions 'append' to the left and to the right of the '&&' operator\. MathStructure\.cc 1780
* V501 There are identical sub\-expressions 'append' to the left and to the right of the '&&' operator\. MathStructure\.cc 2043
* V501 There are identical sub\-expressions '\(\* v\_subs\[v\_order\[1\]\]\)\.representsNegative\(true\)' to the left and to the right of the '&&' operator\. MathStructure\.cc 5569

## Цикл с неверным условием

[V534](https://pvs-studio.ru/ru/docs/warnings/v534/) It is likely that a wrong variable is being compared inside the 'for' operator\. Consider reviewing 'i'\. MathStructure\.cc 28741

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

Во внутреннем цикле счётчиком является переменная _i2_, но из\-за опечатки допущена ошибка \- в условии остановки цикла используется переменная _i_ от внешнего цикла\.

## Избыточность или ошибка?

[V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting this expression\. The expression is excessive or contains a misprint\. Number\.cc 6564

```cpp
bool Number::add(const Number &o, MathOperation op)
{
  ....
  if(i1 >= COMPARISON_RESULT_UNKNOWN &&
    (i2 == COMPARISON_RESULT_UNKNOWN || i2 != COMPARISON_RESULT_LESS))
    return false;
  ....
}
```

Насмотревшись на подобный код, 3 года назад я написал заметку для помощи себе и другим программистам: "[Логические выражения в C/C\+\+\. Как ошибаются профессионалы](https://pvs-studio.ru/ru/blog/posts/cpp/0390/)"\. Встречая такой код, я убеждаюсь, что заметка ничуть не стала менее актуальной\. Вы можете заглянуть в статью, найти паттерн ошибки, соответствующий коду, и узнать все нюансы\.

В случае этого примера переходим в раздел "Выражение \=\= \|\| \!\=" и узнаём, что выражение _i2 \=\= COMPARISON\_RESULT\_UNKNOWN_ ни на что не влияет\.

## Разыменование непроверенных указателей

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'o\_data' pointer was utilized before it was verified against nullptr\. Check lines: 1108, 1112\. DataSet\.cc 1108

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

Указатель _o\_data_ в одной функции разыменовывается без проверки и с проверкой\. Это может быть избыточный код, либо потенциальная ошибка\. Я склоняюсь к последнему варианту\.

Есть ещё два похожих места:

* V595 The 'o\_assumption' pointer was utilized before it was verified against nullptr\. Check lines: 229, 230\. Variable\.cc 229
* V595 The 'i\_value' pointer was utilized before it was verified against nullptr\. Check lines: 3412, 3427\. Number\.cc 3412

## free\(\) или delete \[\]?

[V611](https://pvs-studio.ru/ru/docs/warnings/v611/) The memory was allocated using 'new' operator but was released using the 'free' function\. Consider inspecting operation logics behind the 'remcopy' variable\. Number\.cc 8123

```cpp
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);                                 // <=
      }
    }
    ....
  }
  ....
}
```

Память под массив _remcopy_ выделяется и освобождается разными способами, что является серьёзной ошибкой\.

## Потерянные изменения

[V672](https://pvs-studio.ru/ru/docs/warnings/v672/) There is probably no need in creating the new 'm' variable here\. One of the function's arguments possesses the same name and this argument is a reference\. Check lines: 25600, 25626\. MathStructure\.cc 25626

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

Переменная _m_ принимается в функции по ссылке, что подразумевает её модификацию\. Но анализатор обнаружил, что в коде присутствует одноимённая локальная переменная, которая перекрывает область видимости параметра функции, допуская потерю изменений\.

## Странные указатели

[V774](https://pvs-studio.ru/ru/docs/warnings/v774/) The 'cu' pointer was used after the memory was released\. Calculator\.cc 3595

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

Анализатор предупреждает, что в коде присутствует обращение к методу объекта _cu_ уже после освобождения памяти\. Но если попытаться разобраться в коде, то он окажется ещё более странным\. Во\-первых, вызов _delete cu_ происходит всегда \- в условии и после\. Во\-вторых, код после условия предполагает, что указатели _u_ и _cu_ не равны, значит после очистки объекта _cu_ логично использовать объект _u_\. Скорее всего, в коде была допущена опечатка и планировалось использовать только переменную _u_\.

## Использование функции find

[V797](https://pvs-studio.ru/ru/docs/warnings/v797/) The 'find' function is used as if it returned a bool type\. The return value of the function should probably be compared with std::string::npos\. Unit\.cc 404

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

Хотя код успешно компилируется, он выглядит подозрительным, так как функция _find_ возвращает число типа _std::string::size\_type_\. Условие будет истинно, если точка будет найдена в любом месте строки, кроме случая, если точка стоит в начале\. Это странная проверка\. Я не уверен, но возможно, код следует переписать следующим образом:

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

## Потенциальная утечка памяти

[V701](https://pvs-studio.ru/ru/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'buffer' is lost\. Consider assigning realloc\(\) to a temporary pointer\. util\.cc 703

```cpp
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
}
```

При работе с функцией _realloc\(\)_ рекомендуется использовать промежуточный буфер, так как в случае невозможности выделения памяти, указатель на старый участок памяти будет безвозвратно утерян\.

## Заключение

Проект Qalculate\! возглавляет список лучших бесплатных калькуляторов, при этом содержит много серьёзных ошибок\. Но мы ещё не видели его конкурентов\. Постараемся пройтись по всем популярным калькуляторам\.

Что касается сравнения с качеством калькулятора из мира Windows, пока утилита от Microsoft выглядит более надёжной и качественной\.

Проверь свой "Калькулятор", скачав [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/) и попробовав на своём проекте\. :\-\)