Проверка фреймворка Qt 5

• Qt
• Особенности проверки проекта Qt
• Результаты проверки
  • Окунёмся в удивительный мир опечаток
  • Неправильное использование языка Си++
  • Тёмная сторона
  • Разыменование нулевых указателей
  • Вопросы без ответов
  • Разное
• Унаследованные ошибки
• Выводы
• Дополнительные ссылки

Инструменты статического анализа кода позволяют устранить множество ошибок ещё на этапе написания текста программы. Например, можно быстро выявить и устранить опечатки. Некоторые программисты искренне уверены, что они не делают опечатки и глупые ошибки. Это не так. Такие ошибки делают все. Эта статья послужит хорошим демонстрационным примером. Опечатки можно найти даже в таком качественном и оттестированном проекте, как Qt.

Qt

Qt - кроссплатформенный инструментарий разработки ПО на языке программирования C++. Позволяет запускать написанное с его помощью ПО в большинстве современных операционных систем путём простой компиляции программы для каждой ОС без изменения исходного кода. Включает в себя все основные классы, которые могут потребоваться при разработке прикладного программного обеспечения, начиная от элементов графического интерфейса и заканчивая классами для работы с сетью, базами данных и XML. Qt является полностью объектно-ориентированным, легко расширяемым и поддерживающим технику компонентного программирования. [источник: Wikipedia]

Ссылки:

• Wikipedia - Qt;
• Сайт Qt Project;
• Статья о проверке нами Qt 4.7.3 в 2011 году.

Мы будем работать с Qt версии 5.2.1. Для анализа мы используем PVS-Studio версии 5.15.

Хочу обратить внимание, что PVS-Studio сумел найти ошибки, несмотря на то, что проект Qt проверялся с помощью анализаторов Klocwork и Coverity. Насколько регулярно используются эти анализаторы я не знаю. Однако, Klocwork и Coverity упоминается в bugtracker и файлах ChangeLog-xxx. Ещё я нашел упоминание в интернете, что Qt регулярно проверяется с использованием PC-lint.

Особенности проверки проекта Qt

Для разнообразия мы проверили Qt с помощью нового механизма, появившегося в PVS-Studio Standalone. Про механизм пока никто не знает, и мы будем время от времени напоминать в статьях о его существовании. Что-же это за новый таинственный и прекрасный механизм?

Иногда затруднительно проверить проект с помощью PVS-Studio, если он собирается с помощью nmake и тому подобного. Требуется интегрировать PVS-Studio в сборку, что не так просто. По крайней мере это явно мешает по-быстрому попробовать и оценить инструмент.

Теперь в PVS-Studio появился новый режим работы, который существенно облегчит работу с такими проектами. Анализатор умеет следить, с какими параметрами запускается компилятор и собирает всю необходимую для проверки информацию. Достаточно сказать анализатору, когда начать слежку и когда её прекратить.

Слежение за компилятором можно осуществлять как из GUI приложения, так и из командной строки. Подробнее как всё это работает и как использовать новый режим, описано в статье:

Евгений Рыжков. PVS-Studio теперь поддерживает любую сборочную систему на Windows и любой компилятор. Легко и "из коробки".

В это статье рассказывается, как был проверен проект Qt в режиме запуска мониторинга из командной строки.

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

Результаты проверки

В целом, у меня сложилось следующее впечатление о коде:

Код Qt качественный и практически не содержит ошибок, связанных с опасными особенностями языка Си++. Зато много обыкновенных опечаток.

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

Окунёмся в удивительный мир опечаток

Опечатка N1

bool QWindowsUser32DLL::initTouch()
{
  QSystemLibrary library(QStringLiteral("user32"));

  registerTouchWindow   = ....;
  unregisterTouchWindow = ....;
  getTouchInputInfo     = ....;
  closeTouchInputHandle = ....;

  return registerTouchWindow &&
         unregisterTouchWindow &&
         getTouchInputInfo &&
         getTouchInputInfo;
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'getTouchInputInfo' to the left and to the right of the '&&' operator. qwindowscontext.cpp 216

Четырём переменным присваиваются значения. Эти четыре переменные должны быть проверены. Но из-за опечатки проверяется только 3. В последней строчке вместо 'getTouchInputInfo' должно быть написано 'closeTouchInputHandle'.

Опечатка N2

QWindowsNativeImage *QWindowsFontEngine::drawGDIGlyph(....)
{
  ....
  int iw = gm.width.toInt();
  int ih = gm.height.toInt();
  if (iw <= 0 || iw <= 0)
    return 0;
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator: iw <= 0 || iw <= 0 qwindowsfontengine.cpp 1095

Нет проверки высоты, хранящейся в переменной 'ih'.

Опечатка N3, N4

Эта ошибка относится к тестам. Хороший пример, как статический анализ дополняет юнит-тесты. Подробнее про эту тему: "Как статический анализ дополняет TDD".

inline bool qCompare(QImage const &t1, QImage const &t2, ....)
{
  ....
  if (t1.width() != t2.width() || t2.height() != t2.height()) {
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '!=' operator: t2.height() != t2.height() qtest_gui.h 101

Функция сравнения двух картинок неправильно сравнивает их высоту. Вернее, совсем не сравнивает.

Ошибка размножена с помощью Copy-Paste. Точно такое-же сравнение можно увидеть в этом же файле чуть ниже (строка 135).

Опечатка N5

Прошу прощения, что я некрасиво отформатировал код. Строки были слишком длинные.

void QXmlSimpleReader::setFeature(
  const QString& name, bool enable)
{
  ....
  } else if (   name == QLatin1String(
    "http://trolltech.com/xml/features/report-start-end-entity")
             || name == QLatin1String(
    "http://trolltech.com/xml/features/report-start-end-entity"))
  {
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator. qxml.cpp 3249

Переменная 'name' два раза сравнивается с одной и той-же строкой. Выше в коде, есть схожее сравнение. Там переменная сравнивается с этими двумя с строками:

• http://trolltech.com/xml/features/report-whitespace-only-CharData
• http://qt-project.org/xml/features/report-whitespace-only-CharData

По аналогии, можно предположить, что в приведённом коде переменная 'name' должна была сравниваться с:

• http://trolltech.com/xml/features/report-start-end-entity
• http://qt-project.org/xml/features/report-start-end-entity

Опечатка N6, N7, N8, N9

QString DayTimeDuration::stringValue() const
{
  ....
  if(!m_hours && !m_minutes && !m_seconds && !m_seconds)
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions '!m_seconds' to the left and to the right of the '&&' operator. qdaytimeduration.cpp 148

Забыли про миллисекунды. Миллисекунды хранятся в переменной 'm_mseconds'. Проверка должна быть такой:

if(!m_hours && !m_minutes && !m_seconds && !m_mseconds)

Идентичная ошибка с миллисекундами есть ещё в трёх местах:

• qdaytimeduration.cpp 170
• qduration.cpp 167
• qduration.cpp 189

Опечатка N10

QV4::ReturnedValue
QQuickJSContext2DPrototype::method_getImageData(
  QV4::CallContext *ctx)
{
  ....
  qreal x = ctx->callData->args[0].toNumber();
  qreal y = ctx->callData->args[1].toNumber();
  qreal w = ctx->callData->args[2].toNumber();
  qreal h = ctx->callData->args[3].toNumber();
  if (!qIsFinite(x) || !qIsFinite(y) ||
      !qIsFinite(w) || !qIsFinite(w))
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions '!qIsFinite(w)' to the left and to the right of the '||' operator. qquickcontext2d.cpp 3305

Нет проверки переменной 'h'. Вместо этого два раза проверяется переменная 'w'.

Опечатка N11

AtomicComparator::ComparisonResult
IntegerComparator::compare(const Item &o1,
                           const AtomicComparator::Operator,
                           const Item &o2) const
{
  const Numeric *const num1 = o1.as<Numeric>();
  const Numeric *const num2 = o1.as<Numeric>();
 
  if(num1->isSigned() || num2->isSigned())
  ....
}

V656 Variables 'num1', 'num2' are initialized through the call to the same function. It's probably an error or un-optimized code. Consider inspecting the 'o1.as < Numeric > ()' expression. Check lines: 220, 221. qatomiccomparators.cpp 221

Переменные 'num1' и 'num2' инициализируются одним и тем же значением. Ниже обе эти переменные проверяются. Это подозрительно. Ведь достаточно проверить значение только одной переменной.

Скорее всего, переменная 'num2' должна инициализироваться выражением, где используется аргумент 'o2':

const Numeric *const num1 = o1.as<Numeric>();
const Numeric *const num2 = o2.as<Numeric>();

Опечатка N12

void Atlas::uploadBgra(Texture *texture)
{
  const QRect &r = texture->atlasSubRect();
  QImage image = texture->image();

  if (image.format() != QImage::Format_ARGB32_Premultiplied ||
      image.format() != QImage::Format_RGB32) {
  ....
}

V547 Expression is always true. Probably the '&&' operator should be used here. qsgatlastexture.cpp 271

Условие в этом коде не имеет смысла. Она всегда истинно. Чтобы легче понять, что здесь не так, я приведу упрощенный пример:

int a = ...;
if (a != 1 || a != 2)

Переменная всегда будет чему-то неравна.

Я не знаю, как должен выглядеть правильный код. Возможно должно быть так:

if (image.format() == QImage::Format_ARGB32_Premultiplied ||
    image.format() == QImage::Format_RGB32) {

Или так:

if (image.format() != QImage::Format_ARGB32_Premultiplied &&
    image.format() != QImage::Format_RGB32) {

Опечатка N13

void QDeclarativeStateGroupPrivate::setCurrentStateInternal(
  const QString &state, 
  bool ignoreTrans)
{
  ....
  QDeclarativeTransition *transition =
    (ignoreTrans || ignoreTrans) ?
      0 : findTransition(currentState, state);
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator: ignoreTrans || ignoreTrans qdeclarativestategroup.cpp 442

Что-то здесь не так. Как именно хотели написать проверку, мне не ясно.

Опечатка N14

QV4::ReturnedValue
QQuickJSContext2DPrototype::method_createPattern(....)
{
  ....
  if (repetition == QStringLiteral("repeat") ||
      repetition.isEmpty()) {
    pattern->patternRepeatX = true;
    pattern->patternRepeatY = true;
  } else if (repetition == QStringLiteral("repeat-x")) {
    pattern->patternRepeatX = true;
  } else if (repetition == QStringLiteral("repeat-y")) {
    pattern->patternRepeatY = true;
  } else if (repetition == QStringLiteral("no-repeat")) {
    pattern->patternRepeatY = false;
    pattern->patternRepeatY = false;
  } else {
    //TODO: exception: SYNTAX_ERR
  }
  ....
}

Предупреждение PVS-Studio: V519 The 'pattern->patternRepeatY' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1775, 1776. qquickcontext2d.cpp 1776

Два раза подряд выполняется присваивание переменной 'patternRepeatY':

pattern->patternRepeatY = false;
pattern->patternRepeatY = false;

Я думаю, здесь должно быть написано так:

} else if (repetition == QStringLiteral("no-repeat")) {
  pattern->patternRepeatX = false;
  pattern->patternRepeatY = false;
} else {

Неправильное использование языка Си++

Как я уже говорил, большинство ошибок - это обыкновенные опечатки. Почти нет ошибок, связанных с неправильным использованием языка Си++. Но парочка таких ошибок всё-таки нашлась.

Красивая ошибка, связанная с приоритетом операций

bool QConfFileSettingsPrivate::readIniLine(....)
{
  ....
  char ch;
  while (i < dataLen &&
         ((ch = data.at(i) != '\n') && ch != '\r'))
    ++i;
  ....
}

V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. qsettings.cpp 1702

Цикл предназначен для того, чтобы найти конец строки. Признаком конца строки является символ '\n' или '\r'.

Внутри условия должен браться символ и сравниваться с '\n' и '\r'. Ошибка возникает из-за того, что приоритет оператора '!=' выше, чем оператора '='. Из-за этого в переменную 'ch' записывается не код символа, а 'true' или 'false'. В результате, сравнение '\r' уже не играет никакой роли.

Расставим скобки, чтобы ошибка была лучше заметна:

while (i < dataLen &&
       ((ch = (data.at(i) != '\n')) && ch != '\r'))

Из-за ошибки концом строки считается только символ '\n'. Если строки будут заканчиваться символом '\r', функция будет работать неправильно.

Правильный код:

while (i < dataLen &&
       (ch = data.at(i)) != '\n' && ch != '\r')

Потеря точности

bool QWindowsTabletSupport::translateTabletPacketEvent()
{
  ....
  const double radAzim =
    (packet.pkOrientation.orAzimuth / 10) * (M_PI / 180);
  ....
}

V636 The 'packet.pkOrientation.orAzimuth / 10' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. qwindowstabletsupport.cpp 467

Переменная 'packet.pkOrientation.orAzimuth' имеет тип 'int'. Эта целочисленная переменная делится на число 10. Подозрительно то, что потом результат деления используется совместно со значениями типа 'double'. Конечный результат тоже помещается в переменную типа 'double'.

Такое целочисленное деление вовсе не обязательно ошибка. Возможно, код написан именно так, как и задумывалась. Но как показывает практика, часто это недочёт, приводящий к потере точности вычислений.

Например, пусть переменная 'packet.pkOrientation.orAzimuth' равна 55. Тогда результат вычислений будет равен:

(55 / 10) * (3.14159... / 180) = 5 * 0,01745... = 0,087266...

Точность можно существенно увеличить. Достаточно объявить константу 10, как тип double: "(packet.pkOrientation.orAzimuth / 10.0) * (M_PI / 180)". Теперь результат вычислений равен:

(55 / 10.0) * (3.14159... / 180) = 5.5 * 0,01745... = 0,095993...

Такие неточности возникают часто, так как программисты невнимательно относятся к выражениям, где смешиваются переменные разных типов. Такая невнимательность часто приводит ещё и к 64-битным ошибкам (см. смешанная арифметика).

Анализатор обнаруживает ещё 51 подозрительное целочисленное деление. Возможно, некоторые выражения окажутся менее точными, чем хотелось разработчикам. Привожу их списком: qt-v636.txt.

Бессмысленные проверки указателя

Если память выделяется с помощью оператора 'new', уже давно нет смысла проверять указатель на равенство нулю. Если память не удастся выделить, будет сгенерировано исключение. Конечно, можно сделать, чтобы оператор 'new' вернул 0, но сейчас речь не про эти случаи.

Однако, время от времени программисты забывают, и в коде вновь появляется бессмысленная проверка.

HRESULT STDMETHODCALLTYPE QWindowsEnumerate::Clone(
  IEnumVARIANT **ppEnum)
{
  QWindowsEnumerate *penum = 0;
  *ppEnum = 0;
  
  penum = new QWindowsEnumerate(array);
  if (!penum)
    return E_OUTOFMEMORY;
  ....
}

Предупреждение PVS-Studio: V668 There is no sense in testing the 'penum' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. qwindowsmsaaaccessible.cpp 141

Есть ещё несколько таких проверок: main.cpp 127, qaudiodevicefactory.cpp 236, qaudiodevicefactory.cpp 263, qaudiobuffer.cpp 488, mfvideorenderercontrol.cpp 143, mfvideorenderercontrol.cpp 158, mfvideorenderercontrol.cpp 1193, mfvideorenderercontrol.cpp 1199, qaxserverbase.cpp 1006, positionpollfactory.cpp 60.

Тёмная сторона

Есть два фрагмента кода, про которые я не могу сказать, что это точно ошибка. Я не знаю архитектуру проекта и особенности реализации. Однако, даже если это и не ошибки, то это тёмная сторона программирования на Си++.

class Q_CORE_EXPORT QObject
{
  ....
  virtual ~QObject();
  virtual bool event(QEvent *);
  virtual bool eventFilter(QObject *, QEvent *);
  ....
};

QObject *QQmlVME::run(....)
{
  ....
  QObject *o = (QObject *)operator
    new(instr.typeSize + sizeof(QQmlData));   
  ::memset(static_cast<void *>(o), 0,
           instr.typeSize + sizeof(QQmlData));
  ....
}

Предупреждение PVS-Studio: V598 The 'memset' function is used to nullify the fields of 'QObject' class. Virtual method table will be damaged by this. qqmlvme.cpp 658

Класс QObject имеет виртуальные функции. Это значит, что объект хранит указатель на таблицу виртуальных методов. Мне кажется не очень хорошей идея инициализировать такие объекты с помощью функции memset().

Ещё одно предупреждение: V598 The 'memset' function is used to nullify the fields of 'QObject' class. Virtual method table will be damaged by this. qdeclarativevme.cpp 286

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

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

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

Вернёмся к нулевым указателям.

Разыменование нулевого указателя из-за опечатки

QV4::ReturnedValue QQuickJSContext2DPixelData::getIndexed(
  QV4::Managed *m, uint index, bool *hasProperty)
{
  ....
  if (!m)
    return m->engine()->currentContext()->throwTypeError();
  ....
}

Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'm' might take place. qquickcontext2d.cpp 3169

По всей видимости, оператор '!' здесь лишний. Простая опечатка, приводящая к серьезной ошибке.

Разыменование нулевого указателя в обработчике ошибки

void QDocIndexFiles::readIndexSection(....)
{
  ....
  DocNode* dn = qdb_->findGroup(groupNames[i]);
  if (dn) {
    dn->addMember(node);
  }
  else {
    ....
    qDebug() << "DID NOT FIND GROUP:" << dn->name()
             << "for:" << node->name();
  }
  ....
}

Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'dn' might take place. qdocindexfiles.cpp 539

Если произойдёт ошибка, то должно быть распечатано диагностическое сообщение. В нём попытаются взять имя у несуществующего объекта: dn->name().

82 потенциальных разыменовывания нулевого указателя

Почти во всех проектах, в том и числе в Qt, есть недочёты в работе с нулевыми указателями. Часто проверка осуществляется уже после того, как указатель был использован. Не всегда это ошибка. Бывает, что указатель в принципе не может быть равен нулю.

В любом случае, такие места заслуживают изучения и рефакторинга. Даже если ошибки нет, лишняя проверка указателя сбивает программиста с толка при чтении кода.

Рассмотрим один из фрагментов опасного кода:

static int gray_raster_render(....)
{
  const QT_FT_Outline* outline =
    (const QT_FT_Outline*)params->source;

  ....

  /* return immediately if the outline is empty */
  if ( outline->n_points == 0 || outline->n_contours <= 0 )
    return 0;

  if ( !outline || !outline->contours || !outline->points )
    return ErrRaster_Invalid_Outline;  

  ....
}

Предупреждение PVS-Studio: V595 The 'outline' pointer was utilized before it was verified against nullptr. Check lines: 1746, 1749. qgrayraster.c 1746

Я думаю, ошибка появилась в тот момент, когда захотели оптимизировать работу функции gray_raster_render(). Скорее всего, в уже законченный код функции, были вставлены эти строки:

/* return immediately if the outline is empty */
if ( outline->n_points == 0 || outline->n_contours <= 0 )
  return 0;

Беда в том, что указатель 'outline' может оказаться нулевым. Проверка, на равенство указателя нулю находится чуть ниже.

Анализатор нашёл ещё 81 потенциальную ошибку. Приведу диагностические сообщения списком: qt-v595.txt.

Вопросы без ответов

Есть странные фрагменты кода, про которые сложно сказать, как они появились и что на самом деле планировалось написать. Возможно, это опечатки, возможно недописанный код, возможно неудачный рефакторинг.

Двойная проверка

QWindowsFontEngine::~QWindowsFontEngine()
{
  ....
  if (QWindowsContext::verboseFonts)
    if (QWindowsContext::verboseFonts)
      qDebug("%s: font='%s", __FUNCTION__, qPrintable(_name));
  ....
}

Предупреждение PVS-Studio: V571 Recurring check. The 'if (QWindowsContext::verboseFonts)' condition was already verified in line 369. qwindowsfontengine.cpp 370

Зачем два раза проверять одно и то же? Возможно, одна проверка лишняя. Возможно, забыли проверить что-то ещё.

Двойное присваивание

void Moc::parse()
{
  ....
  index = def.begin + 1;
  namespaceList += def;
  index = rewind;
  ....
}

Предупреждение PVS-Studio: V519 The 'index' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 568, 570. moc.cpp 570

Почему переменной 'index' присваиваются разные значения?

Есть ещё несколько аналогичных странных фрагментов кода:

• V519 The 'exitCode' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 807, 815. qprocess.cpp 815
• V519 The 'detecting' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 163, 164. qhoversensorgesturerecognizer.cpp 164
• V519 The 'increaseCount' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 185, 186. qtwistsensorgesturerecognizer.cpp 186

Подозрение на пропущенный оператор 'break'

bool GSuggestCompletion::eventFilter(QObject *obj, QEvent *ev)
{
  ....
  switch (key) {
  case Qt::Key_Enter:
  case Qt::Key_Return:
    doneCompletion();
    consumed = true;

  case Qt::Key_Escape:
    editor->setFocus();
    popup->hide();
    consumed = true;

  case Qt::Key_Up:
  case Qt::Key_Down:
  case Qt::Key_Home:
  case Qt::Key_End:
  case Qt::Key_PageUp:
  case Qt::Key_PageDown:
    break;
  ....
}

Предупреждение PVS-Studio: V519 The 'consumed' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 110, 115. googlesuggest.cpp 115

Забыт здесь оператор break или нет?

Анализатору показалось странно, что переменной 'consumed' может два раза подряд присваиваться значение 'true'. Есть вероятность, что здесь забыт оператор break. Но я в этом не уверен. Быть может, просто нужно удалить первое присваивание: "consumed = true;".

Подозрение на лишний оператор 'break'

bool QHelpGenerator::registerVirtualFolder(....)
{
  ....
  while (d->query->next()) {
    d->namespaceId = d->query->value(0).toInt();
    break;
  }
  ....
}

Предупреждение PVS-Studio: V612 An unconditional 'break' within a loop. qhelpgenerator.cpp 429

Это правильно, что цикл сразу остановится из-за оператора 'break'?

Ещё один такой фрагмент кода живёт здесь: qhelpgenerator.cpp 642

Разное

Потерпите. Статья уже скоро закончится. Осталось ещё несколько разношёрстных ошибок.

Неправильное использование функции toLower()

int main(int argc, char **argv)
{
  ....
  QByteArray arg(argv[a]);
  ....
  arg = arg.mid(1);
  arg.toLower();
  if (arg == "o")
  ....
}

Предупреждение PVS-Studio: V530 The return value of function 'toLower' is required to be utilized. main.cpp 72

Функция 'toLower()' не изменяет объект. Она возвращает копию объекта, который будет хранить в себе символы в нижнем регистре.

Ещё одна ошибка: V530 The return value of function 'toLower' is required to be utilized. main.cpp 1522

Выход за границы массива

Ситуация сложная. Прошу сосредоточиться.

Существует вот такой нумерованный тип:

typedef enum {
    JNone,
    JCausing,
    JDual,
    JRight,
    JTransparent
} Joining;

Запомним, что JTransparent == 4.

Теперь рассмотрим функцию getNkoJoining():

static Joining getNkoJoining(unsigned short uc)
{
  if (uc < 0x7ca)
    return JNone;
  if (uc <= 0x7ea)
    return JDual;
  if (uc <= 0x7f3)
    return JTransparent;
  if (uc <= 0x7f9)
    return JNone;
  if (uc == 0x7fa)
    return JCausing;
  return JNone;
}

Нам важно то, что эта функция может возвращать 'JTransparent'. То есть функция может вернуть 4.

В программе есть двумерный массив 'joining_table':

static const JoiningPair joining_table[5][4] = { .... };

А теперь собственно место, где может возникнуть ошибка:

static void getNkoProperties(....)
{
  ....
  Joining j = getNkoJoining(chars[0]);
  ArabicShape shape = joining_table[XIsolated][j].form2;
  ....
}

Предупреждение PVS-Studio: V557 Array overrun is possible. The value of 'j' index could reach 4. harfbuzz-arabic.c 516

Как мы помним, функция getNkoJoining() может вернуть значение 4. Таким образом, мы обращаемся к ячейке массива joining_table[...][4]. Это не допустимо. Произойдёт выход за границу массива.

Одинаковые условия

void Node::setPageType(const QString& t)
{
    if ((t == "API") || (t == "api"))
        pageType_ = ApiPage;
    else if (t == "howto")
        pageType_ = HowToPage;
    else if (t == "overview")
        pageType_ = OverviewPage;
    else if (t == "tutorial")
        pageType_ = TutorialPage;
    else if (t == "howto")
        pageType_ = HowToPage;
    else if (t == "article")
        pageType_ = ArticlePage;
    else if (t == "example")
        pageType_ = ExamplePage;
    else if (t == "ditamap")
        pageType_ = DitaMapPage;
}

Предупреждение PVS-Studio: V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 386, 392. node.cpp 386

Два раза выполняется проверка (t == "howto"). Думаю, одна из них лишняя.

Вот ещё пара аналогичных предупреждений:

• V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 188, 195. qmaintainingreader_tpl_p.h 188
• V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 299, 303. mfmetadatacontrol.cpp 299

Выполнение одинаковых действий

void
QBluetoothServiceDiscoveryAgentPrivate::_q_deviceDiscovered(
  const QBluetoothDeviceInfo &info)
{
  if(mode == QBluetoothServiceDiscoveryAgent::FullDiscovery) {
    for(int i = 0; i < discoveredDevices.count(); i++){
      if(discoveredDevices.at(i).address() == info.address()){
        discoveredDevices.removeAt(i);
      }
    }
    discoveredDevices.prepend(info);
  }
  else {
    for(int i = 0; i < discoveredDevices.count(); i++){
      if(discoveredDevices.at(i).address() == info.address()){
        discoveredDevices.removeAt(i);
      }
    }
    discoveredDevices.prepend(info);
  }
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. qbluetoothservicediscoveryagent.cpp 402

Независимо от условия, выполняются одни и те-же действия.

Аналогично: pcre_exec.c 5577, ditaxmlgenerator.cpp 1722, htmlgenerator.cpp 388.

Унаследованные ошибки

В Qt используется несколько сторонних библиотек. В этих библиотеках тоже есть ошибки. Получается, что эти ошибки есть и в Qt. В статье их описывать я не стал, но решил, что про них можно упомянуть.

Внимательно сторонние библиотеки я не смотрел, но кое-что выписал: qt-3rdparty.txt.

Примечание. Впрочем, не надо думать, что зато я очень внимательно смотрел Qt. Проект большой и даже поверхностного осмотра достаточно, чтобы написать вот такую статью.

Выводы

PVS-Studio отличный, мощный анализатор, который способен находить ошибки даже в таких качественных и вылизанных проектах, как фреймворк Qt.

Он поможет экономить массу времени коллективу разработчиков, обнаруживая многие ошибки на самом раннем этапе. Если использовать инкрементальный анализ, то ошибки будут обнаруживаться сразу после компиляции файлов.

Дополнительные ссылки

• Мы регулярно проверяем open-source проекты. Например, Tor, Chromium, Clang, Firebird, OpenCV. Всем, кому интересно, приходите сюда: "Обновляемый список open-source проектов, которые мы проверили с помощью PVS-Studio".
• Здесь можно скачать пробную версию PVS-Studio. В начале даётся 20 кликов для навигации по предупреждениям. После сообщения информации о себе, ещё 200 кликов.