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

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

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

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

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

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

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


Если вы так и не получили ответ, пожалуйста, проверьте папку
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 и попробовав на своём проекте. :-)

Популярные статьи по теме
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

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

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
PVS-Studio ROI

Дата: 30 Янв 2019

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

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Эффект последней строки

Дата: 31 Май 2014

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

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Зло живёт в функциях сравнения

Дата: 19 Май 2017

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

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…

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

Следующие комментарии

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