Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

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

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

14 Мар 2019

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

0616_Qalculate_ru/image1.png

Введение

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

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

0616_Qalculate_ru/image2.png

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

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

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

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

V523 The 'then' statement is equivalent to the 'else' statement. 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);
  }
  ....
}

Код в операторе if и else абсолютно одинаковый. Соседние фрагменты кода очень похожи на этот, но в них используются разные функции: internalLowerFloat() и internalUpperFloat(). Можно с уверенностью предположить, что здесь программист скопировал код и забыл поправить имя функции.

V501 There are identical sub-expressions '!mtr2.number().isReal()' to the left and to the right of the '||' operator. BuiltinFunctions.cc 6274

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

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

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

Найти аномалии в этом коде вручную нереально! Но они есть. Причём в оригинальном файле эти фрагменты записаны в одну строку. Анализатор обнаружил дублирующееся выражение 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 It is likely that a wrong variable is being compared inside the 'for' operator. Consider reviewing '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;
    }
  }
  ....
}

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

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

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. 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;
  ....
}

Насмотревшись на подобный код, 3 года назад я написал заметку для помощи себе и другим программистам: "Логические выражения в C/C++. Как ошибаются профессионалы". Встречая такой код, я убеждаюсь, что заметка ничуть не стала менее актуальной. Вы можете заглянуть в статью, найти паттерн ошибки, соответствующий коду, и узнать все нюансы.

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

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

V595 The 'o_data' pointer was utilized before it was verified against nullptr. Check lines: 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);
  }
  ....
}

Указатель 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 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

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 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

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 The 'cu' pointer was used after the memory was released. 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;
  }
  ....
}

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

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

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

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. Условие будет истинно, если точка будет найдена в любом месте строки, кроме случая, если точка стоит в начале. Это странная проверка. Я не уверен, но возможно, код следует переписать следующим образом:

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

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

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

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 и попробовав на своём проекте. :-)

Популярные статьи по теме
Как различить C и C++ разработчиков по их коду

Дата: 12 Май 2022

Автор: Гость

Так уж случилось, что я пишу код для разных IoT-железок, связанных с электричеством, типа зарядных станций автомобилей. Поскольку аппаратных ресурсов, как правило, вполне достаточно, то основным фоку…
Отладочный вывод на микроконтроллерах: как Concepts и Ranges отправили мой printf на покой

Дата: 06 Май 2022

Автор: Гость

Здравствуйте! Меня зовут Александр, и я работаю программистом микроконтроллеров.
Нереальный baselining или доработки PVS-Studio для Unreal Engine проектов

Дата: 26 Апр 2022

Автор: Валерий Комаров

Статический анализатор PVS-Studio постоянно развивается: улучшаются различные механизмы, происходит интеграция с игровыми движками, IDE, CI/CD и другими системами и сервисами. Благодаря этому несколь…
Разбор некоторых вредных советов для С++ программиста

Дата: 21 Апр 2022

Автор: Андрей Карпов

Юмор юмором, но осторожность не повредит. Вдруг кому-то не до конца понятно, почему какой-то из советов является вредным. Здесь можно найти соответствующие пояснения.
Четыре причины проверять, что вернула функция malloc

Дата: 20 Апр 2022

Автор: Андрей Карпов

Некоторые разработчики пренебрежительно относятся к проверкам: удалось ли выделить память с помощью функции malloc или нет. Их логика проста – памяти всегда должно хватить. А если не хватит, всё равн…

Комментарии (0)

Следующие комментарии
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять