Ответ о проекте factplusplus

Меня попросили проверить логику вывода движка в логике описания приложения factplusplus. Итак, я посмотрел проект. Он был небольшим, его плотность ошибок — низкая. Так что в целом все выглядело хорошо. По крайней мере, я не нашел ничего, что стоило бы писать статью. Я даже не планировал писать публичный ответ. Но, разговаривая с человеком через сообщения на Хабре, я понял, что это не удобно — я пытался вставить код в сообщение, и оно оказалось не таким организованным, как хотелось бы. Поэтому я решил написать здесь мини-пост и отправить ссылку человеку.

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

Однако есть несколько вещей, которые я заметил:

Опечатка. PVS-Studio предупреждает: V524 Странно, что тело функции p_begin полностью эквивалентно телу функции s_end. ТаксономияCreator.h 82

ss_iterator s_begin ( void ) override { return beg; }
ss_iterator s_end ( void ) override { return end; }
ss_iterator p_begin ( void ) override { return end; }      // <=
ss_iterator p_end ( void ) override { return end; }

Оператор new явно лишний. PVS-Studio предупреждает: V1022 [CWE-755] Исключение было вызвано указателем. Вместо этого попробуйте бросить его по значению. Расширенныйдатаранж.cpp 44

static bool
checkDataRelation ( const DataTypeReasoner& Op1, const DataTypeReasoner& Op2, int op )
{
  switch (op)
  {
  ....
  default:
    throw new EFaCTPlusPlus("Illegal operation in checkDataRelation()");
  }
}

Конечно, нет никакого способа избежать ложных срабатываний или предупреждений о низком значении. Но и их можно победить. Обычно анализатор выдает такие предупреждения, когда код недостаточно точен и нуждается в доработке или рефакторинге. От таких исправлений кода выигрывают все. Взгляните, например, на этот код:

if ( value == nullptr )
  parseError("Data value expected");
TDataTypeExpr* type = const_cast<TDataTypeExpr*>(value->getExpr());

Анализатор предупреждает, что здесь происходит разыменование нулевого указателя: V522 [CWE-690] Возможно, происходит разыменование потенциального значения нулевого указателя. parser.cpp 611

Причина в том, что функция

void parseError ( const char* p )

не помечен как возвращающий управление. Если вы измените его на

[[noreturn]] void parseError ( const char* p )

предупреждение исчезает. И код становится понятнее и легче читается.

Это все. Приглашаю всех в наш блог, где мы проверяем различные проекты и пишем о любопытных вещах, которые там находим.