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

15 Окт 2018

Теги:

#Cpp

Третья проверка Qt 5 с помощью PVS-Studio

15 Окт 2018

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

Qt
Неудачный Copy-Paste и опечатки
Дефекты безопасности
Нулевые указатели
Логические ошибки в условиях
Потенциальные утечки памяти
Разное
Заключение

Время от времени наша команда повторно проверяет проекты, про которые мы уже писали статьи. Очередным таким перепроверенным проектом стал Qt. Последний раз мы проверяли его с помощью PVS-Studio в 2014 году. Начиная с 2014 года проект начал регулярно проверяться с помощью Coverity. Это интересно. Давайте посмотрим, удастся ли нам теперь найти какие-то интересные ошибки с помощью PVS-Studio.

Qt

Предыдущие статьи:

"Как уменьшить вероятность ошибки на этапе написания кода", июль 2011.
"Проверка фреймворка Qt 5", апрель 2014.

В этот раз был проверен Qt Base (Core, Gui, Widgets, Network, ...) и Qt5 super module. Про Qt Creator мы позже планируем написать отдельную статью. Для проверки использовался статический анализатор PVS-Studio, пробную версию которого вы можете скачать с сайта.

На мой субъективный взгляд, код Qt стал более качественным. За годы с момента последней проверки, в анализаторе PVS-Studio появилось много новых диагностик. Несмотря на это, в ходе обзорного изучения предупреждений я выявил не так уж много ошибок для проекта такого размера. Ещё раз повторю, что это моё индивидуальное впечатление. Каких-то специальных исследований на тему плотности ошибок ни тогда, ни сейчас я не делал.

Положительно на качестве кода, скорее всего, сказались регулярные проверки с помощью статического анализатора Coverity. С 2014 года с помощью Coverity начал проверяться проект Qt, а с 2016 - Qt Creator. Моё мнение: если вы разрабатываете открытый проект, то Coverity Scan может стать хорошим бесплатным решением, которое существенно улучшит качество и надёжность ваших проектов.

Впрочем, как читатель может догадаться, если бы я ничего интересного не заметил в отчёте PVS-Studio, то и статьи бы не было :). А раз есть статья, то есть и дефекты. Давайте посмотрим на них. Всего я выписал 96 ошибок.

Неудачный Copy-Paste и опечатки

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

Эти ошибки межязыковые. Например, во второй статье приводится масса примеров ошибок в функциях сравнения, написанных на языках C, C++ и C#. Сейчас, реализуя в PVS-Studio поддержку языка Java, мы встречаем всё те же паттерны ошибок. Вот, например, ошибка, найденная нами недавно в библиотеке Hibernate:

public boolean equals(Object other) {
  if (other instanceof Id) {
    Id that = (Id) other;
    return purchaseSequence.equals(this.purchaseSequence) &&
           that.purchaseNumber == this.purchaseNumber;
  }
  else {
    return false;
  }
}

Если присмотреться, то оказывается, что поле purchaseSequence сравнивается само с собой. Правильный вариант:

return that.purchaseSequence.equals(this.purchaseSequence) &&
       that.purchaseNumber == this.purchaseNumber;

В общем, всё как всегда, и анализатору PVS-Studio предстоит "разгребать авгиевы конюшни" в Java-проектах. Кстати, приглашаем всех желающих принять участие в тестировании Beta-версии PVS-Studio for Java, которая должна появиться в ближайшее время. Для этого напишите нам (выберите пункт "Хочу анализатор для Java").

Теперь вернёмся к ошибкам в проекте Qt.

Дефект N1

static inline int windowDpiAwareness(HWND hwnd)
{
  return QWindowsContext::user32dll.getWindowDpiAwarenessContext &&
         QWindowsContext::user32dll.getWindowDpiAwarenessContext
    ? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext(
        QWindowsContext::user32dll.getWindowDpiAwarenessContext(hwnd))
    : -1;
}

Предупреждение PVS-Studio: V501 CWE-571 There are identical sub-expressions 'QWindowsContext::user32dll.getWindowDpiAwarenessContext' to the left and to the right of the '&&' operator. qwindowscontext.cpp 150

Какого-то специального пояснения помимо сообщения анализатора здесь не требуется. Мне кажется, что выражение должно было быть таким:

return QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext &&
       QWindowsContext::user32dll.getWindowDpiAwarenessContext
  ? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext(
      QWindowsContext::user32dll.getWindowDpiAwarenessContext(hwnd))
  : -1;

Дефект N2, N3

void QReadWriteLockPrivate::release()
{
  Q_ASSERT(!recursive);
  Q_ASSERT(!waitingReaders && !waitingReaders &&
           !readerCount && !writerCount);
  freelist->release(id);
}

Предупреждение PVS-Studio: V501 CWE-571 There are identical sub-expressions to the left and to the right of the '&&' operator: !waitingReaders &&!waitingReaders qreadwritelock.cpp 632

Ошибка находится внутри условия макроса Q_ASSERT, поэтому она незначительна. Но всё равно это ошибка. Дважды проверяется переменная waitingReaders. А какую-то другую переменную, видимо, забыли проверить.

Идентичная ошибка обнаруживает себя в 625 строке файла qreadwritelock.cpp. Да здравствует Copy-Paste! :)

Дефект N4

QString QGraphicsSceneBspTree::debug(int index) const
{
  ....
  if (node->type == Node::Horizontal) {
    tmp += debug(firstChildIndex(index));
    tmp += debug(firstChildIndex(index) + 1);
  } else {
    tmp += debug(firstChildIndex(index));
    tmp += debug(firstChildIndex(index) + 1);
  }
  ....
}

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

Скорее всего, блок текста был скопирован, но поправить его забыли.

Дефект N5

enum FillRule {
  OddEvenFill,
  WindingFill
};

QDataStream &operator>>(QDataStream &s, QPainterPath &p)
{
  ....
  int fillRule;
  s >> fillRule;
  Q_ASSERT(fillRule == Qt::OddEvenFill || Qt::WindingFill);
  ....
}

Предупреждение PVS-Studio: V768 CWE-571 The enumeration constant 'WindingFill' is used as a variable of a Boolean-type. qpainterpath.cpp 2479

Согласитесь, это красивый ляп! Q_ASSERT ничего не проверяет, так как условие всегда истинно. Истинно условие из-за того, что именованная константа Qt::WindingFill равна 1.

Дефект N6

bool QVariant::canConvert(int targetTypeId) const
{
  ....
  if (currentType == QMetaType::SChar || currentType == QMetaType::Char)
    currentType = QMetaType::UInt;
  if (targetTypeId == QMetaType::SChar || currentType == QMetaType::Char)
    targetTypeId = QMetaType::UInt;
  ....
}

Прежде чем читать предупреждение, попробуйте заметить опечатку самостоятельно. Добавив картинку, я помогу вам не прочитать сообщение анализатора сразу :).

Предупреждение PVS-Studio: V560 CWE-570 A part of conditional expression is always false: currentType == QMetaType::Char. qvariant.cpp 3529

Условие "currentType == QMetaType::Char" проверяется в первом if. Если условие выполняется, то переменной currentType присваивается значение QMetaType::UInt. Следовательно, далее переменная currentType уже никак не может быть равна QMetaType::Char. Поэтому анализатор и сообщает, что во втором if подвыражение "currentType == QMetaType::Char" всегда ложно.

На самом деле, второй if должен быть таким:

if (targetTypeId == QMetaType::SChar || targetTypeId == QMetaType::Char)
  targetTypeId = QMetaType::UInt;

Примечание по диагностики V560

В отчёте оказалось много предупреждений V560. Однако я не стал их более просматривать, как только нашёл интересный случай для статьи, который был рассмотрен выше как дефект N6.

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

QString QTextHtmlExporter::findUrlForImage(const QTextDocument *doc, ....)
{
  QString url;
  if (!doc)
    return url;

  if (QTextDocument *parent = qobject_cast<QTextDocument *>(doc->parent()))
      return findUrlForImage(parent, cacheKey, isPixmap);
 
  if (doc && doc->docHandle()) {       // <=
  ....
}

Предупреждение PVS-Stuidio: V560 CWE-571 A part of conditional expression is always true: doc. qtextdocument.cpp 2992

Анализатор абсолютно прав, что указатель doc при повторной проверке всегда не равен nullptr. Но это не ошибка, просто программист перестраховался. Можно упростить код, написав:

if (doc->docHandle()) {

Дефект N7

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

class QWindowsCursor : public QPlatformCursor
{
public:
  enum CursorState {
    CursorShowing,
    CursorHidden,
    CursorSuppressed
  };
  ....
}

QWindowsCursor::CursorState QWindowsCursor::cursorState()
{
  enum { cursorShowing = 0x1, cursorSuppressed = 0x2 };
  CURSORINFO cursorInfo;
  cursorInfo.cbSize = sizeof(CURSORINFO);
  if (GetCursorInfo(&cursorInfo)) {
    if (cursorInfo.flags & CursorShowing)
  ....
}

Предупреждение PVS-Studio: V616 CWE-480 The 'CursorShowing' named constant with the value of 0 is used in the bitwise operation. qwindowscursor.cpp 669

Более подробно я уже разбирал эту ошибку в отдельной маленькой заметке: "В очередной раз анализатор PVS-Studio оказался внимательнее человека".

Дефекты безопасности

На самом деле, все ошибки, которые рассматриваются в этой статье, можно назвать дефектами безопасности. Все они классифицируются согласно Common Weakness Enumeration (см. CWE ID в сообщениях анализатора). Если ошибки классифицируются как CWE, то они потенциально представляют угрозу с точки зрения безопасности. Более подробно это объясняется на странице PVS-Studio SAST.

Однако ряд ошибок так и хочется выделить в отдельную группу. Давайте взглянем на них.

Дефект N8, N9

bool QLocalServerPrivate::addListener()
{
  ....
  SetSecurityDescriptorOwner(pSD.data(), pTokenUser->User.Sid, FALSE);
  SetSecurityDescriptorGroup(pSD.data(), pTokenGroup->PrimaryGroup, FALSE);
  ....
}

Предупреждения PVS-Studio:

V530 CWE-252 The return value of function 'SetSecurityDescriptorOwner' is required to be utilized. qlocalserver_win.cpp 167
V530 CWE-252 The return value of function 'SetSecurityDescriptorGroup' is required to be utilized. qlocalserver_win.cpp 168

Существуют различные функции, связанные с разграничением доступа. Функции SetSecurityDescriptorOwner и SetSecurityDescriptorGroup относятся к их числу.

С такими функциями надо работать очень аккуратно. Например, обязательно надо проверять статус, который они возвращают. Что будет, если вызов этих функций закончится неудачей? Гадать не надо, надо написать код для обработки такого случая.

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

Дефект N10

bool QLocalServerPrivate::addListener()
{
  ....
  InitializeAcl(acl, aclSize, ACL_REVISION_DS);
  ....
}

Предупреждение PVS-Studio: V530 CWE-252 The return value of function 'InitializeAcl' is required to be utilized. qlocalserver_win.cpp 144

Ситуация аналогична рассмотренной выше.

Дефект N11, N12

static inline void sha1ProcessChunk(....)
{
  ....
  quint8 chunkBuffer[64];
  ....
#ifdef SHA1_WIPE_VARIABLES
  ....
  memset(chunkBuffer, 0, 64);
#endif
}

Предупреждение PVS-Studio: V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'chunkBuffer' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sha1.cpp 189

Компилятор удалит вызов функции memset. Уже много раз в статьях я разбирал эту ситуацию. Повторяться не хочется. Отсылаю к статье "Безопасная очистка приватных данных".

И ещё одна ошибка находится в том же файле sha1.cpp, в строке 247.

Нулевые указатели

Пришло время поговорить об указателях. На эту тему нашлось весьма много ошибок.

Дефект N13

QByteArray &QByteArray::append(const char *str, int len)
{
  if (len < 0)
    len = qstrlen(str);
  if (str && len) {
    ....
}

Предупреждение PVS-Studio: V595 CWE-476 The 'str' pointer was utilized before it was verified against nullptr. Check lines: 2118, 2119. qbytearray.cpp 2118

Классическая ситуация, когда указатель в начале используется, а затем проверяется на равенство nullptr. Это очень распространённый паттерн ошибки, и мы встречаем его регулярно практически во всех проектах.

Дефект N14, N15

static inline const QMetaObjectPrivate *priv(const uint* data)
{ return reinterpret_cast<const QMetaObjectPrivate*>(data); }

bool QMetaEnum::isFlag() const
{
  const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1;
  return mobj && mobj->d.data[handle + offset] & EnumIsFlag;
}

Предупреждение PVS-Studio: V595 CWE-476 The 'mobj' pointer was utilized before it was verified against nullptr. Check lines: 2671, 2672. qmetaobject.cpp 2671

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

#define priv(A) foo(sizeof(A))

Тогда всё будет работать.

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

Итак, указатель modj разыменовывается, а затем проверяется.

Дальше на сцену выходит "могучий и ужасный" Copy-Paste. Из-за чего в точности такая же ошибка обнаруживается в функции isScoped:

bool QMetaEnum::isScoped() const
{
  const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1;
  return mobj && mobj->d.data[handle + offset] & EnumIsScoped;
}

Предупреждение PVS-Studio: V595 CWE-476 The 'mobj' pointer was utilized before it was verified against nullptr. Check lines: 2683, 2684. qmetaobject.cpp 2683

Дефект N16-N21

Рассмотрим ещё один пример и, думаю, достаточно.

void QTextCursor::insertFragment(const QTextDocumentFragment &fragment)
{
  if (!d || !d->priv || fragment.isEmpty())
    return;

  d->priv->beginEditBlock();
  d->remove();
  fragment.d->insert(*this);
  d->priv->endEditBlock();

  if (fragment.d && fragment.d->doc)
    d->priv->mergeCachedResources(fragment.d->doc->docHandle());
}

Предупреждение PVS-Studio: V595 CWE-476 The 'fragment.d' pointer was utilized before it was verified against nullptr. Check lines: 2238, 2241. qtextcursor.cpp 2238

Всё то же самое. Обратите внимание на последовательность работы с указателем, хранящимся в переменной fragment.d.

Другие ошибки этого типа:

V595 CWE-476 The 'window' pointer was utilized before it was verified against nullptr. Check lines: 1846, 1848. qapplication.cpp 1846
V595 CWE-476 The 'window' pointer was utilized before it was verified against nullptr. Check lines: 1858, 1860. qapplication.cpp 1858
V595 CWE-476 The 'reply' pointer was utilized before it was verified against nullptr. Check lines: 492, 502. qhttpnetworkconnectionchannel.cpp 492
V595 CWE-476 The 'newHandle' pointer was utilized before it was verified against nullptr. Check lines: 877, 883. qsplitter.cpp 877
V595 CWE-476 The 'widget' pointer was utilized before it was verified against nullptr. Check lines: 2320, 2322. qwindowsvistastyle.cpp 2320
На самом деле ошибок больше. Мне быстро надоело изучать предупреждения V595, а для статьи я уже выписал достаточное количество фрагментов кода.

Дефект N22-N33

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

bool QTranslatorPrivate::do_load(const QString &realname,
                                 const QString &directory)
{
  ....
  d->unmapPointer = new char[d->unmapLength];
  if (d->unmapPointer) {
    file.seek(0);
    qint64 readResult = file.read(d->unmapPointer, d->unmapLength);
    if (readResult == qint64(unmapLength))
      ok = true;
  }
  ....
}

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

Проверка указателя не имеет смысла, так как в случае ошибки выделения памяти будет сгенерировано исключение std::bad_alloc. Если хочется, чтобы при нехватке памяти оператор new возвращал nullptr, то следовало написать:

d->unmapPointer = new (std::nothrow) char[d->unmapLength];

Анализатор знает про такой вариант использования оператора new и не выдал бы в этом случае предупреждение.

Другие ошибки: приведу их файлом qt-V668.txt.

Дефект N34-N70

Как и обещал, теперь очередь ошибок, когда не проверяют результат вызова функций malloc, calloc, strdup и т.д. Эти ошибки серьезнее, чем кажется на первый взгляд. Подробнее: "Почему важно проверять, что вернула функция malloc".

SourceFiles::SourceFiles()
{
  nodes = (SourceFileNode**)malloc(sizeof(SourceFileNode*)*(num_nodes=3037));
  for(int n = 0; n < num_nodes; n++)
    nodes[n] = nullptr;
}

Предупреждение PVS-Studio: V522 CWE-690 There might be dereferencing of a potential null pointer 'nodes'. Check lines: 138, 136. makefiledeps.cpp 138

Указатель используют без предварительной проверки.

Все эти ошибки однотипны, поэтому я не буду на них останавливаться подробнее. Приведу остальные предупреждения списком: qt-V522-V575.txt.

Логические ошибки в условиях

Дефект N71

QString QEdidParser::parseEdidString(const quint8 *data)
{
  QByteArray buffer(reinterpret_cast<const char *>(data), 13);

  // Erase carriage return and line feed
  buffer = buffer.replace('\r', '\0').replace('\n', '\0');

  // Replace non-printable characters with dash
  for (int i = 0; i < buffer.count(); ++i) {
    if (buffer[i] < '\040' && buffer[i] > '\176')
      buffer[i] = '-';
  }

  return QString::fromLatin1(buffer.trimmed());
}

Предупреждение PVS-Studio: V547 CWE-570 Expression 'buffer[i] < '\040' && buffer[i] > '\176'' is always false. qedidparser.cpp 169

Функция обязана выполнять следующее действие "Replace non-printable characters with dash". Однако она этого не делает. Взглянем внимательно вот на это условие:

if (buffer[i] < '\040' && buffer[i] > '\176')

Оно не имеет смысла. Символ не может одновременно быть меньше '\040' и больше '\176'. В условии необходимо использовать оператор '||'. Правильный вариант кода:

if (buffer[i] < '\040' || buffer[i] > '\176')

Дефект N72

Аналогичная ошибка, из-за которой не повезёт Windows-пользователям.

#if defined(Q_OS_WIN)
static QString driveSpec(const QString &path)
{
  if (path.size() < 2)
    return QString();
  char c = path.at(0).toLatin1();
  if (c < 'a' && c > 'z' && c < 'A' && c > 'Z')
    return QString();
  if (path.at(1).toLatin1() != ':')
    return QString();
  return path.mid(0, 2);
}
#endif

Анализатор выдаёт сразу два предупреждения:

V590 CWE-571 Consider inspecting the 'c < 'a' && c > 'z' && c < 'A' && c > 'Z'' expression. The expression is excessive or contains a misprint. qdir.cpp 77
V560 CWE-570 A part of conditional expression is always false: c > 'z'. qdir.cpp 77

Логическая ошибка находится в условии:

if (c < 'a' && c > 'z' && c < 'A' && c > 'Z')

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

if ((c < 'a' || c > 'z') && (c < 'A' || c > 'Z'))

Дефект N73

enum SelectionMode {
  NoSelection,
  SingleSelection,
  MultiSelection,
  ExtendedSelection,
  ContiguousSelection
};

void QAccessibleTableCell::unselectCell()
{
  QAbstractItemView::SelectionMode selectionMode = view->selectionMode();
  if (!m_index.isValid() || (selectionMode & QAbstractItemView::NoSelection))
    return;
  ....
}

Предупреждение PVS-Studio: V616 CWE-480 The 'QAbstractItemView::NoSelection' named constant with the value of 0 is used in the bitwise operation. itemviews.cpp 976

Именованная константа QAbstractItemView::NoSelection равна нулю. Поэтому подвыражение (selectionMode & QAbstractItemView::NoSelection) не имеет смысла. Всегда будет получаться 0.

Мне кажется, здесь следовало написать так:

if (!m_index.isValid() || (selectionMode == QAbstractItemView::NoSelection))

Дефект N74

Следующий код мне понять трудно. Он неправильный, но каким он должен быть я не знаю. Комментарий к функции тоже мне не помогает.

// Re-engineered from the inline function _com_error::ErrorMessage().
// We cannot use it directly since it uses swprintf_s(), which is not
// present in the MSVCRT.DLL found on Windows XP (QTBUG-35617).
static inline QString errorMessageFromComError(const _com_error &comError)
{
  TCHAR *message = nullptr;
  FormatMessage(
    FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
    NULL, DWORD(comError.Error()), MAKELANGID(LANG_NEUTRAL,SUBLANG_DEFAULT),
    message, 0, NULL);
  if (message) {
    const QString result = QString::fromWCharArray(message).trimmed();
    LocalFree(static_cast<HLOCAL>(message));
    return result;
  }
  if (const WORD wCode = comError.WCode())
    return QString::asprintf("IDispatch error #%u", uint(wCode));
  return QString::asprintf("Unknown error 0x0%x", uint(comError.Error()));
}

Предупреждение PVS-Studio: V547 CWE-570 Expression 'message' is always false. qwindowscontext.cpp 802

Программист, наверное, предполагает, что функция FormatMessage изменит значение указателя message. Но это не так. Функция FormatMessage не может изменить значение указателя, так как он передаётся в функцию по значению. Вот прототип этой функции:

DWORD
__stdcall
FormatMessageW(
  DWORD dwFlags,
  LPCVOID lpSource,
  DWORD dwMessageId,
  DWORD dwLanguageId,
  LPWSTR lpBuffer,
  DWORD nSize,
  va_list *Arguments
);

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

Дефект N75-N92

struct SourceDependChildren {
  SourceFile **children;
  int num_nodes, used_nodes;
  SourceDependChildren() : children(nullptr), num_nodes(0), used_nodes(0) { }
  ~SourceDependChildren() { if (children) free(children); children = nullptr; }
  void addChild(SourceFile *s) {
    if(num_nodes <= used_nodes) {
      num_nodes += 200;
      children = (SourceFile**)realloc(children,
                                       sizeof(SourceFile*)*(num_nodes));
    }
    children[used_nodes++] = s;
  }
};

Предупреждение PVS-Studio: V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'children' is lost. Consider assigning realloc() to a temporary pointer. makefiledeps.cpp 103

Увеличение буфера реализовано опасным образом. Если функция realloc не сможет выделить память, она вернёт значение NULL. Этот NULL будет сразу помещён в переменную children и не останется возможности как-то освободить буфер, выделенный ранее. Произойдёт утечка памяти.

Аналогичные ошибки: qt-701.txt.

Разное

Дефект N93

template<class GradientBase, typename BlendType>
static inline const BlendType * QT_FASTCALL
qt_fetch_linear_gradient_template(....)
{
  ....
  if (t+inc*length < qreal(INT_MAX >> (FIXPT_BITS + 1)) &&
      t+inc*length > qreal(INT_MIN >> (FIXPT_BITS + 1))) {
  ....
}

Предупреждение PVS-Studio: V610 CWE-758 Unspecified behavior. Check the shift operator '>>'. The left operand '(- 2147483647 - 1)' is negative. qdrawhelper.cpp 4015

Нельзя сдвигать отрицательное значение INT_MIN. Это неуточнённое поведение, и полагаться на результат такой операции нельзя. Старшие биты могут оказаться как равными 0, так и 1.

Дефект N94

void QObjectPrivate::addConnection(int signal, Connection *c)
{
  ....
  if (signal >= connectionLists->count())
    connectionLists->resize(signal + 1);

  ConnectionList &connectionList = (*connectionLists)[signal];
  ....
  if (signal < 0) {
  ....
}

Предупреждение PVS-Studio: V781 CWE-129 The value of the 'signal' variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 397, 413. qobject.cpp 397

Проверка (signal < 0) указывает на то, что значение аргумента signal может быть отрицательным. Однако этот аргумент ранее используется для индексации массива. Получается, что проверка выполняется слишком поздно. Работа программы уже будет нарушена.

Дефект N95

bool QXmlStreamWriterPrivate::finishStartElement(bool contents)
{
  ....
  if (inEmptyElement) {
    write("/>");
    QXmlStreamWriterPrivate::Tag &tag = tagStack_pop();
    lastNamespaceDeclaration = tag.namespaceDeclarationsSize;
    lastWasStartElement = false;
  } else {
    write(">");
  }
  inStartElement = inEmptyElement = false;
  lastNamespaceDeclaration = namespaceDeclarations.size();
  return hadSomethingWritten;
}

Предупреждение PVS-Studio: V519 CWE-563 The 'lastNamespaceDeclaration' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3188, 3194. qxmlstream.cpp 3194

Выделю суть ошибки:

if (inEmptyElement) {
  lastNamespaceDeclaration = tag.namespaceDeclarationsSize;
}
lastNamespaceDeclaration = namespaceDeclarations.size();

Дефект N96

void QRollEffect::scroll()
{
  ....
  if (currentHeight != totalHeight) {
    currentHeight = totalHeight * (elapsed/duration)
        + (2 * totalHeight * (elapsed%duration) + duration)
        / (2 * duration);
    // equiv. to int((totalHeight*elapsed) / duration + 0.5)
    done = (currentHeight >= totalHeight);
  }
  done = (currentHeight >= totalHeight) &&
         (currentWidth >= totalWidth);
  ....
}

V519 CWE-563 The 'done' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 509, 511. qeffects.cpp 511

Всё то же самое, что и в предыдущем случае. Обратите внимание на переменную done.

Заключение

Даже поверхностно просматривая отчёт, я выписал почти 100 ошибок. Я доволен результатами работы PVS-Studio.

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

Спасибо за внимание. Чтобы быть в курсе наших новых публикаций, приглашаю подписаться на один из наших каналов: