Обработка дат притягивает ошибки или 77 дефектов в Qt 6
Относительно недавно состоялся релиз фреймворка Qt 6, и это стало поводом вновь проверить его с помощью PVS-Studio. В статье будут рассмотрены различные интересные ошибки, например, связанные с обработкой дат. Обнаружение всех этих ошибок хорошо демонстрирует пользу, которую может получить проект от использования таких инструментов, как PVS-Studio, особенно если они применяются регулярно.
Это классическая статья о проверке открытого проекта, которая пополнит нашу «доказательную базу» полезности и эффективности использования PVS-Studio для контроля качества кода. Хотя мы уже писали про поверку проекта Qt (в 2011, в 2014 и в 2018), очень полезно сделать это вновь. Так, мы на практике демонстрируем простую, но очень важную мысль: статический анализ должен применяться регулярно!
Наши статьи показывают, что анализатор PVS-Studio умеет находить множество разнообразнейших ошибок. И, как правило, авторы проектов быстро исправляют описанные нами ошибки. Однако всё это не имеет ничего общего с правильной и полезной методикой регулярного применения статических анализаторов. Когда анализатор встроен в процесс разработки, это позволяет быстро находить ошибки в новом или изменённом коде и тем самым их исправление обходится максимально дёшево.
Всё, теория заканчивается. Давайте посмотрим, что интересного нас ждёт в коде. А пока вы будете читать статью, предлагаю скачать PVS-Studio и запросить демонстрационный ключ, чтобы посмотреть, что интересного найдётся в ваших собственных проектах :).
Даты
Кажется, намечается ещё один паттерн кода, где любят собираться ошибки. Он, конечно, не такой масштабный, как функции сравнения или последние строки в однотипных блоках. Речь идёт про фрагменты кода, работающие с датами. Наверное, такой код сложно тестировать, и в итоге эти недотестированные функции будут давать некорректный результат на определённых наборах входных данных. Про пару таких случаев уже было рассказано в статье «Почему PVS-Studio не предлагает автоматические правки кода».
Встретились ошибки обработки дат и в Qt. Давайте с них и начнём.
Фрагмент N1: неправильная интерпретация статуса ошибки
Для начала нам следует посмотреть, как устроена функция, возвращающая номер месяца по его сокращённому названию.
static const char qt_shortMonthNames[][4] = {
"Jan", "Feb", "Mar", "Apr", "May", "Jun",
"Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
};
static int fromShortMonthName(QStringView monthName)
{
for (unsigned int i = 0;
i < sizeof(qt_shortMonthNames) / sizeof(qt_shortMonthNames[0]); ++i)
{
if (monthName == QLatin1String(qt_shortMonthNames[i], 3))
return i + 1;
}
return -1;
}
В случае успеха функция возвращает номер месяца (значение от 1 до 12). Если имя месяца некорректно, то функция возвращает отрицательное значение (-1). Обратите внимание, что функция не может вернуть значение 0.
Но там, где только что рассмотренная функция используется, программист как раз рассчитывает, что в качестве статуса ошибки ему будет возвращено нулевое значение. Фрагмент кода, некорректно использующий функцию fromShortMonthName:
QDateTime QDateTime::fromString(QStringView string, Qt::DateFormat format)
{
....
month = fromShortMonthName(parts.at(1));
if (month)
day = parts.at(2).toInt(&ok);
// If failed, try day then month
if (!ok || !month || !day) {
month = fromShortMonthName(parts.at(2));
if (month) {
QStringView dayPart = parts.at(1);
if (dayPart.endsWith(u'.'))
day = dayPart.chopped(1).toInt(&ok);
}
}
....
}
Проверки номера месяца на равенство нулю никогда не сработают, и программа продолжит выполняться с некорректным отрицательным номером месяца. Анализатор PVS-Studio видит здесь целый букет несостыковок, о чем сообщает сразу четырьмя сообщениями:
- V547 [CWE-571] Expression 'month' is always true. qdatetime.cpp 4907
- V560 [CWE-570] A part of conditional expression is always false: ! month. qdatetime.cpp 4911
- V547 [CWE-571] Expression 'month' is always true. qdatetime.cpp 4913
- V560 [CWE-570] A part of conditional expression is always false: ! month. qdatetime.cpp 4921
Фрагмент N2: ошибка логики обработки даты
Для начала посмотрим на реализацию функции, возвращающей количество секунд.
enum {
....
MSECS_PER_DAY = 86400000,
....
SECS_PER_MIN = 60,
};
int QTime::second() const
{
if (!isValid())
return -1;
return (ds() / 1000)%SECS_PER_MIN;
}
Рассмотренная функция может вернуть значение в диапазоне [0…59] или статус ошибки -1.
В одном месте эта функция используется очень странным образом:
static qint64 qt_mktime(QDate *date, QTime *time, ....)
{
....
} else if (yy == 1969 && mm == 12 && dd == 31
&& time->second() == MSECS_PER_DAY - 1) {
// There was, of course, a last second in 1969, at time_t(-1); we won't
// rescue it if it's not in normalised form, and we don't know its DST
// status (unless we did already), but let's not wantonly declare it
// invalid.
} else {
....
}
Предупреждение PVS-Studio: V560 [CWE-570] A part of conditional expression is always false: time→second () == MSECS_PER_DAY — 1. qdatetime.cpp 2488
Согласно комментарию, если что-то пошло не так, то лучше ничего не делать. Однако, условие всегда будет ложным, поэтому всегда выполняется else-ветка.
Ошибочно вот это сравнение:
time->second() == MSECS_PER_DAY - 1
MSECS_PER_DAY — 1 это 86399999. Функция second, как мы уже знаем, никак не может вернуть такое значение. Таким образом, здесь какая-то логическая ошибка и код заслуживает пристального внимания разработчиков.
Сильная сторона статических анализаторов кода в том, что они проверяют все ситуации, независимо от частоты их выполнения. Таким образом, статический анализ хорошо дополняет юнит-тесты и другие методы контроля качества кода.
Опечатки
Фрагмент N3: неожиданно, мы поговорим о… HTML!
QString QPixelTool::aboutText() const
{
const QList screens = QGuiApplication::screens();
const QScreen *windowScreen = windowHandle()->screen();
QString result;
QTextStream str(&result);
str << "Qt Pixeltool
Qt " << QT_VERSION_STR
<< "
Copyright (C) 2017 The Qt Company Ltd.
Screens
";
for (const QScreen *screen : screens)
str << "- " << (screen == windowScreen ? "* " : " ")
<< screen << "
";
str << "