Русский
Русский
English
Статистика
Реклама

Ошибки в программе

Обработка дат притягивает ошибки или 77 дефектов в Qt 6

16.02.2021 22:10:00 | Автор: admin

PVS-Studio проверяет 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<QScreen *> screens = QGuiApplication::screens();  const QScreen *windowScreen = windowHandle()->screen();  QString result;  QTextStream str(&result);  str << "<html></head><body><h2>Qt Pixeltool</h2><p>Qt " << QT_VERSION_STR    << "</p><p>Copyright (C) 2017 The Qt Company Ltd.</p><h3>Screens</h3><ul>";  for (const QScreen *screen : screens)    str << "<li>" << (screen == windowScreen ? "* " : "  ")        << screen << "</li>";  str << "<ul></body></html>";  return result;}

Предупреждение PVS-Studio: V735 Possibly an incorrect HTML. The "</ body>" closing tag was encountered, while the "</ ul>" tag was expected. qpixeltool.cpp 707


В PVS-Studio есть диагностики, которые не только проверят сам код, но и выискивают аномалии в строковых константах. Здесь как раз сработала одна из таких диагностик. Это достаточно редкий случай, зато этим он и примечательный.


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


str << "</ul></body></html>";

Фрагмент N4: повторная проверка в условии


class Node{  ....  bool isGroup() const { return m_nodeType == Group; }  ....};void DocBookGenerator::generateDocBookSynopsis(const Node *node){  ....  if (node->isGroup() || node->isGroup()      || node->isSharedCommentNode() || node->isModule()      || node->isJsModule() || node->isQmlModule() || node->isPageNode())    return;  ....}

Предупреждение PVS-Studio: V501 [CWE-570] There are identical sub-expressions to the left and to the right of the '||' operator: node->isGroup() || node->isGroup() docbookgenerator.cpp 2599


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


Фрагмент N5: создание лишней локальной переменной


void MainWindow::addToPhraseBook(){  ....  QString selectedPhraseBook;  if (phraseBookList.size() == 1) {    selectedPhraseBook = phraseBookList.at(0);    if (QMessageBox::information(this, tr("Add to phrase book"),          tr("Adding entry to phrasebook %1").arg(selectedPhraseBook),           QMessageBox::Ok | QMessageBox::Cancel, QMessageBox::Ok)                          != QMessageBox::Ok)      return;  } else {    bool okPressed = false;    QString selectedPhraseBook =       QInputDialog::getItem(this, tr("Add to phrase book"),                            tr("Select phrase book to add to"),                            phraseBookList, 0, false, &okPressed);    if (!okPressed)      return;  }  MessageItem *currentMessage = m_dataModel->messageItem(m_currentIndex);  Phrase *phrase = new Phrase(currentMessage->text(),                              currentMessage->translation(),                              QString(), nullptr);  phraseBookHash.value(selectedPhraseBook)->append(phrase);}

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


Единорог из старой коллекции


Предупреждение PVS-Studio: V561 [CWE-563] It's probably better to assign value to 'selectedPhraseBook' variable than to declare it anew. Previous declaration: mainwindow.cpp, line 1303. mainwindow.cpp 1313


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


QString selectedPhraseBook =

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


Фрагмент N6: приоритет операций


Классический паттерн ошибки, который встречается весьма часто.


bool QQmlImportInstance::resolveType(....){  ....  if (int icID = containingType.lookupInlineComponentIdByName(typeStr) != -1)  {    *type_return = containingType.lookupInlineComponentById(icID);  } else {    auto icType = createICType();    ....  }  ....}

Предупреждение PVS-Studio: V593 [CWE-783] Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. qqmlimport.cpp 754


Значение переменной icID всегда будет иметь значение 0 или 1. Это явно не то, что задумывалось. Причина: в начале происходит сравнение с -1, а только затем инициализация переменной icID.


Современный синтаксис C++ позволяет корректно записать это условие следующим образом:


if (int icID = containingType.lookupInlineComponentIdByName(typeStr);    icID != -1)

Кстати, очень похожую ошибку мы уже обнаруживали в Qt:


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

Но пока на вооружение не будет взят такой анализатор кода, как PVS-Studio, программисты вновь и вновь будут допускать такие ошибки. Никто не совершенен. И да, это тонкий намёк внедрить PVS-Studio :).


Фрагмент N7: коварное деление по модулю


Часто бывает необходимо определить, делится число без остатка на 2 или нет. Правильный вариант это поделить по модулю два и проверить результат:


if (A % 2 == 1)

Но программисты вновь и вновь ошибаются и пишут что-то типа этого:


if (A % 1 == 1)

Это естественно неправильно, так как остаток от деления по модулю на один это всегда ноль. Не обошлось без этой ошибки и в Qt:


bool loadQM(Translator &translator, QIODevice &dev, ConversionData &cd){  ....  case Tag_Translation: {    int len = read32(m);    if (len % 1) {                                             // <=      cd.appendError(QLatin1String("QM-Format error"));      return false;    }    m += 4;    QString str = QString((const QChar *)m, len/2);  ....}

Предупреждение PVS-Studio: V1063 The modulo by 1 operation is meaningless. The result will always be zero. qm.cpp 549


Фрагмент N8: перезаписывание значения


QString Node::qualifyQmlName(){  QString qualifiedName = m_name;  if (m_name.startsWith(QLatin1String("QML:")))    qualifiedName = m_name.mid(4);  qualifiedName = logicalModuleName() + "::" + m_name;  return qualifiedName;}

Предупреждение PVS-Studio: V519 [CWE-563] The 'qualifiedName' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1227, 1228. node.cpp 1228


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


QString qualifiedName = m_name;if (m_name.startsWith(QLatin1String("QML:")))  qualifiedName = m_name.mid(4);qualifiedName = logicalModuleName() + "::" + qualifiedName;return qualifiedName;

Фрагмент N9: copy-paste


class Q_CORE_EXPORT QJsonObject{  ....  bool operator<(const iterator& other) const  { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; }  bool operator<=(const iterator& other) const  { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; }  ....}

Прудпреждение PVS-Studio: V524 It is odd that the body of '<=' function is fully equivalent to the body of '<' function. qjsonobject.h 155


Такие скучные функции, как операторы сравнения, никто не проверяет. На них обычно не пишут тесты. Их не просматривают или делают это очень быстро на code review. А зря. Статический анализ кода здесь как нельзя кстати. Анализатор не устаёт и с удовольствием проверяет даже такой скучный код.


Реализация операторов < и <= совпадают. Это явно неправильно. Скорее всего, код писался методом Copy-Paste, и затем забыли изменить всё что нужно в скопированном коде. Правильно:


bool operator<(const iterator& other) const{ Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; }bool operator<=(const iterator& other) const{ Q_ASSERT(item.o == other.item.o); return item.index <= other.item.index; }

Фрагмент N10: static_cast / dynamic_cast


void QSGSoftwareRenderThread::syncAndRender(){  ....  bool canRender = wd->renderer != nullptr;  if (canRender) {     auto softwareRenderer = static_cast<QSGSoftwareRenderer*>(wd->renderer);     if (softwareRenderer)       softwareRenderer->setBackingStore(backingStore);  ....}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'softwareRenderer' is always true. qsgsoftwarethreadedrenderloop.cpp 510


В начале рассмотрим вот эту проверку:


bool canRender = wd->renderer != nullptr;if (canRender) {

Благодаря ей можно быть уверенным, что внутри тела условного оператора значение указателя wd->renderer всегда точно ненулевое. Тогда непонятно, что же хотят проверить следующим кодом?


auto softwareRenderer = static_cast<QSGSoftwareRenderer*>(wd->renderer);if (softwareRenderer)

Если указатель wd->renderer ненулевой, то и указатель softwareRenderer точно ненулевой. Есть подозрение, что здесь опечатка, которая состоит в том, что на самом деле следовало использовать dynamic_cast. В этом случае код начинает приобретать смысл. Если преобразование типа невозможно, оператор dynamic_cast возвращает nullptr, и это возвращенное значение, естественно, следует проверять. Впрочем, возможно, я неправильно интерпретировал ситуацию и код нужно исправлять другим способом.


Фрагмент N11: скопировали блок кода и забыли изменить


void *QQuickPath::qt_metacast(const char *_clname){  if (!_clname) return nullptr;  if (!strcmp(_clname, qt_meta_stringdata_QQuickPath.stringdata0))    return static_cast<void*>(this);  if (!strcmp(_clname, "QQmlParserStatus"))    return static_cast< QQmlParserStatus*>(this);  if (!strcmp(_clname, "org.qt-project.Qt.QQmlParserStatus"))   // <=    return static_cast< QQmlParserStatus*>(this);  if (!strcmp(_clname, "org.qt-project.Qt.QQmlParserStatus"))   // <=    return static_cast< QQmlParserStatus*>(this);  return QObject::qt_metacast(_clname);}

Предупреждение PVS-Studio: V581 [CWE-670] The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 2719, 2721. moc_qquickpath_p.cpp 2721


Эти две строчки:


if (!strcmp(_clname, "org.qt-project.Qt.QQmlParserStatus"))  return static_cast< QQmlParserStatus*>(this);

Были размножены с помощью Copy-Paste. После чего они не были модифицированы и не имеют смысла.


Фрагмент N12: переполнение из-за не там поставленной скобки


int m_offsetFromUtc;....void QDateTime::setMSecsSinceEpoch(qint64 msecs){  ....  if (!add_overflow(msecs, qint64(d->m_offsetFromUtc * 1000), &msecs))    status |= QDateTimePrivate::ValidWhenMask;  ....}

Предупреждение PVS-Studio: V1028 [CWE-190] Possible overflow. Consider casting operands of the 'd->m_offsetFromUtc * 1000' operator to the 'qint64' type, not the result. qdatetime.cpp 3922


Программист предвидит ситуацию, что при умножении переменной типа int на 1000 может произойти переполнение. Чтобы этого избежать, он планирует использовать при умножении 64-битный тип qint64. И использует явное приведение типа.


Вот только толку от этого приведения типа никакого нет. В начале всё равно произойдёт переполнение. И только затем выполнится приведение типа. Правильный вариант:


add_overflow(msecs, qint64(d->m_offsetFromUtc) * 1000, &msecs)

Фрагмент N13: не полностью инициализированный массив


class QPathEdge{  ....private:  int m_next[2][2];  ....};inline QPathEdge::QPathEdge(int a, int b)    : flag(0)    , windingA(0)    , windingB(0)    , first(a)    , second(b)    , angle(0)    , invAngle(0){    m_next[0][0] = -1;    m_next[1][0] = -1;    m_next[0][0] = -1;    m_next[1][0] = -1;}

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


  • V1048 [CWE-1164] The 'm_next[0][0]' variable was assigned the same value. qpathclipper_p.h 301
  • V1048 [CWE-1164] The 'm_next[1][0]' variable was assigned the same value. qpathclipper_p.h 302

Перед нами неудачная попытка инициализировать массив размером 2x2. Два элемента инициализируются повторно, и два остаются неинициализированными. Правильный вариант:


m_next[0][0] = -1;m_next[0][1] = -1;m_next[1][0] = -1;m_next[1][1] = -1;

Я очень люблю такие примеры ошибок, которые встречаются в коде профессиональных разработчиков. Это как раз такой случай. Он показывает, что любой может опечататься, и поэтому статический анализ является вашим другом. Дело в том, что я уже десяток лет веду бой со скептиками, которые уверены, что такие ошибки можно встретить только в лабораторных работах студентов и что они такие ошибки никогда не делают :). Ещё 10 лет назад я написал заметку "Миф второй профессиональные разработчики не допускают глупых ошибок", и с тех пор, естественно, ничего не изменилось. Люди всё также делают такие ошибки и всё также утверждают, что это не так :).


Будь мудр - используй статический анализатор кода


Ошибки в логике


Фрагмент N14: недостижимый код


void QmlProfilerApplication::tryToConnect(){  Q_ASSERT(!m_connection->isConnected());  ++ m_connectionAttempts;  if (!m_verbose && !(m_connectionAttempts % 5)) {// print every 5 seconds    if (m_verbose) {      if (m_socketFile.isEmpty())        logError(          QString::fromLatin1("Could not connect to %1:%2 for %3 seconds ...")          .arg(m_hostName).arg(m_port).arg(m_connectionAttempts));      else        logError(          QString::fromLatin1("No connection received on %1 for %2 seconds ...")          .arg(m_socketFile).arg(m_connectionAttempts));    }  }  ....}

Предупреждение PVS-Studio: V547 [CWE-570] Expression 'm_verbose' is always false. qmlprofilerapplication.cpp 495


Этот код никогда ничего не запишет в лог. Причиной являются противоположные условия:


if (!m_verbose && ....) {  if (m_verbose) {

Фрагмент N15: перетирание значения переменной


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);  ....}

Предупреждение PVS-Studio: V519 [CWE-563] The 'done' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 509, 511. qeffects.cpp 511


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


Фрагмент N16-N20: перетирание значения переменной


Альтернативный вариант перетирания значения переменной.


bool QXmlStreamWriterPrivate::finishStartElement(bool contents){  ....  if (inEmptyElement) {    ....    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: 3030, 3036. qxmlstream.cpp 3036


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


Есть ещё 4 предупреждения, указывающих на такой же паттерн ошибки:


  • V519 [CWE-563] The 'last' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 609, 637. qtextengine.cpp 637
  • V519 [CWE-563] The 'm_dirty' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1014, 1017. qquickshadereffect.cpp 1017
  • V519 [CWE-563] The 'changed' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 122, 128. qsgdefaultspritenode.cpp 128
  • V519 [CWE-563] The 'eaten' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 299, 301. qdesigner.cpp 301

Фрагмент N21: путаница между нулевым указателем и пустой строкой


// this could become a list of all languages used for each writing// system, instead of using the single most common language.static const char languageForWritingSystem[][6] = {    "",     // Any    "en",  // Latin    "el",  // Greek    "ru",  // Cyrillic    ...... // Нулевых указателей нет. Используются пустые строковые литералы.    "", // Symbol    "sga", // Ogham    "non", // Runic    "man" // N'Ko};static void populateFromPattern(....){  ....  for (int j = 1; j < QFontDatabase::WritingSystemsCount; ++j) {    const FcChar8 *lang = (const FcChar8*) languageForWritingSystem[j];    if (lang) {  ....}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'lang' is always true. qfontconfigdatabase.cpp 462


В массиве languageForWritingSystem нет нулевых указателей. Поэтому проверка if(lang) не имеет смысла. Зато в массиве есть пустые строки. Быть может, хотелось сделать проверку именно на пустую строку? Если да, тогда корректный код должен выглядеть так:


if (strlen(lang) != 0) {

Или можно ещё проще написать:


if (lang[0] != '\0') {

Фрагмент N22: странная проверка


bool QNativeSocketEnginePrivate::createNewSocket(....){  ....  int socket = qt_safe_socket(domain, type, protocol, O_NONBLOCK);  ....  if (socket < 0) {    ....    return false;  }  socketDescriptor = socket;  if (socket != -1) {    this->socketProtocol = socketProtocol;    this->socketType = socketType;  }  return true;}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'socket != 1' is always true. qnativesocketengine_unix.cpp 315


Условие socket != -1 всегда истинно, так как выше происходит выход из функции, если значение переменной socket отрицательное.


Фрагмент N23: так что же всё-таки должна вернуть функция?


bool QSqlTableModel::removeRows(int row, int count, const QModelIndex &parent){  Q_D(QSqlTableModel);  if (parent.isValid() || row < 0 || count <= 0)    return false;  else if (row + count > rowCount())    return false;  else if (!count)    return true;  ....}

Предупреждение PVS-Studio: V547 [CWE-570] Expression '!count' is always false. qsqltablemodel.cpp 1110


Для упрощения выделю самое главное:


if (.... || count <= 0)  return false;....else if (!count)  return true;

Первая проверка говорит нам, что если значение count меньше или равно 0, то это ошибочное состояние и функция должна вернуть false. Однако ниже мы видим точное сравнение этой переменной с нулём, и этот случай уже интерпретируется по-другому: функция должна вернуть true.


Здесь явно что-то не так. Я подозреваю, что на самом деле проверка должна быть не <=, а просто <. Тогда код обретает смысл:


bool QSqlTableModel::removeRows(int row, int count, const QModelIndex &parent){  Q_D(QSqlTableModel);  if (parent.isValid() || row < 0 || count < 0)    return false;  else if (row + count > rowCount())    return false;  else if (!count)    return true;  ....}

Фрагмент N24: лишний статус?


В следующем коде переменная identifierWithEscapeChars выглядит просто как лишняя сущность. Или это логическая ошибка? Или код не дописан? К моменту второй проверки эта переменная в любом случае всегда будет равна true.


int Lexer::scanToken(){  ....  bool identifierWithEscapeChars = false;  ....  if (!identifierWithEscapeChars) {    identifierWithEscapeChars = true;    ....  }  ....  if (identifierWithEscapeChars) {    // <=    ....  }  ....}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'identifierWithEscapeChars' is always true. qqmljslexer.cpp 817


Фрагмент N25: что делать с девятью объектами?


bool QFont::fromString(const QString &descrip){  ....  const int count = l.count();  if (!count || (count > 2 && count < 9) || count == 9 || count > 17 ||      l.first().isEmpty()) {    qWarning("QFont::fromString: Invalid description '%s'",             descrip.isEmpty() ? "(empty)" : descrip.toLatin1().data());    return false;  }  setFamily(l[0].toString());  if (count > 1 && l[1].toDouble() > 0.0)    setPointSizeF(l[1].toDouble());  if (count == 9) {                           // <=    setStyleHint((StyleHint) l[2].toInt());    setWeight(QFont::Weight(l[3].toInt()));    setItalic(l[4].toInt());    setUnderline(l[5].toInt());    setStrikeOut(l[6].toInt());    setFixedPitch(l[7].toInt());  } else if (count >= 10) {  ....}

Предупреждение PVS-Studio: V547 [CWE-570] Expression 'count == 9' is always false. qfont.cpp 2142


Как должна себя вести функция, если переменная count равна 9? С одной стороны, функция должна выдать предупреждение и завершить свою работу. Ведь явно написано:


if (.... || count == 9 || ....) {  qWarning(....);  return false;}

С другой стороны, для 9 объектов предусмотрено выполнение специального кода:


if (count == 9) {  setStyleHint((StyleHint) l[2].toInt());  setWeight(QFont::Weight(l[3].toInt()));  setItalic(l[4].toInt());  ....}

Этот код, конечно, никогда не выполняется. Код ждёт, чтобы его пришли и исправили :).


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


Фрагмент N26-N42: использование указателя до его проверки


class __attribute__((visibility("default"))) QMetaType {  ....  const QtPrivate::QMetaTypeInterface *d_ptr = nullptr;};QPartialOrdering QMetaType::compare(const void *lhs, const void *rhs) const{    if (!lhs || !rhs)        return QPartialOrdering::Unordered;    if (d_ptr->flags & QMetaType::IsPointer)        return threeWayCompare(*reinterpret_cast<const void * const *>(lhs),                               *reinterpret_cast<const void * const *>(rhs));    if (d_ptr && d_ptr->lessThan) {        if (d_ptr->equals && d_ptr->equals(d_ptr, lhs, rhs))            return QPartialOrdering::Equivalent;        if (d_ptr->lessThan(d_ptr, lhs, rhs))            return QPartialOrdering::Less;        if (d_ptr->lessThan(d_ptr, rhs, lhs))            return QPartialOrdering::Greater;        if (!d_ptr->equals)            return QPartialOrdering::Equivalent;    }    return QPartialOrdering::Unordered;}

Предупреждение PVS-Studio: V595 [CWE-476] The 'd_ptr' pointer was utilized before it was verified against nullptr. Check lines: 710, 713. qmetatype.cpp 710


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


if (d_ptr->flags & ....)if (d_ptr && ....)

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


Это один из самых распространённых паттернов ошибки в языке C и С++. Пруфы. В исходных Qt кодах тоже встречается немало ошибок этой разновидности:


  • V595 [CWE-476] The 'self' pointer was utilized before it was verified against nullptr. Check lines: 1346, 1351. qcoreapplication.cpp 1346
  • V595 [CWE-476] The 'currentTimerInfo' pointer was utilized before it was verified against nullptr. Check lines: 636, 641. qtimerinfo_unix.cpp 636
  • V595 [CWE-476] The 'lib' pointer was utilized before it was verified against nullptr. Check lines: 325, 333. qlibrary.cpp 325
  • V595 [CWE-476] The 'fragment.d' pointer was utilized before it was verified against nullptr. Check lines: 2262, 2266. qtextcursor.cpp 2262
  • V595 [CWE-476] The 'window' pointer was utilized before it was verified against nullptr. Check lines: 1581, 1583. qapplication.cpp 1581
  • V595 [CWE-476] The 'window' pointer was utilized before it was verified against nullptr. Check lines: 1593, 1595. qapplication.cpp 1593
  • V595 [CWE-476] The 'newHandle' pointer was utilized before it was verified against nullptr. Check lines: 873, 879. qsplitter.cpp 873
  • V595 [CWE-476] The 'targetModel' pointer was utilized before it was verified against nullptr. Check lines: 454, 455. qqmllistmodel.cpp 454
  • V595 [CWE-476] The 'childIface' pointer was utilized before it was verified against nullptr. Check lines: 102, 104. qaccessiblequickitem.cpp 102
  • V595 [CWE-476] The 'e' pointer was utilized before it was verified against nullptr. Check lines: 94, 98. qquickwindowmodule.cpp 94
  • V595 [CWE-476] The 'm_texture' pointer was utilized before it was verified against nullptr. Check lines: 235, 239. qsgplaintexture.cpp 235
  • V595 [CWE-476] The 'm_unreferencedPixmaps' pointer was utilized before it was verified against nullptr. Check lines: 1140, 1148. qquickpixmapcache.cpp 1140
  • V595 [CWE-476] The 'camera' pointer was utilized before it was verified against nullptr. Check lines: 263, 264. assimpimporter.cpp 263
  • V595 [CWE-476] The 'light' pointer was utilized before it was verified against nullptr. Check lines: 273, 274. assimpimporter.cpp 273
  • V595 [CWE-476] The 'channel' pointer was utilized before it was verified against nullptr. Check lines: 337, 338. assimpimporter.cpp 337
  • V595 [CWE-476] The 'm_fwb' pointer was utilized before it was verified against nullptr. Check lines: 2492, 2500. designerpropertymanager.cpp 2492

Фрагмент N43: использование указателя до его проверки в рамках одного выражения


Та же самая ситуация, что и выше. Однако в этот раз разыменование и проверка указателя находятся в одном выражении. Классическая ошибка, возникающая из-за невнимательности при написании кода и при последующем code-review.


void QFormLayoutPrivate::updateSizes(){  ....  QFormLayoutItem *field = m_matrix(i, 1);  ....  if (userHSpacing < 0 && !wrapAllRows && (label || !field->fullRow) && field)  ....}

Предупреждение PVS-Studio: V713 [CWE-476] The pointer 'field' was utilized in the logical expression before it was verified against nullptr in the same logical expression. qformlayout.cpp 405


Минутка отдыха


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


И пока все ушли на кухню, рекламная пауза. Приглашаю команду, занимающуюся разработкой проекта Qt рассмотреть вопрос приобретения лицензии на анализатор кода PVS-Studio. Запросить прайс можно здесь. С нашей стороны поддержка и помощь в настройке. Да, согласен, я сегодня более навязчив, чем обычно. Это эксперимент :).


Минутка отдыха с единорогом


Фрагмент N44-N72: нет проверки, что вернула функция malloc


void assignData(const QQmlProfilerEvent &other){  if (m_dataType & External) {    uint length = m_dataLength * (other.m_dataType / 8);    m_data.external = malloc(length);                          // <=    memcpy(m_data.external, other.m_data.external, length);    // <=  } else {    memcpy(&m_data, &other.m_data, sizeof(m_data));  }}

Предупреждение PVS-Studio: V575 [CWE-628] The potential null pointer is passed into 'memcpy' function. Inspect the first argument. Check lines: 277, 276. qqmlprofilerevent_p.h 277


Нельзя просто взять и использовать указатель, который вернула функция malloc. Нужно обязательно проверить, не является ли этот указатель нулевым, даже если очень лень всем этим заниматься. Для этого есть 4 причины, которые описаны в статье "Почему важно проверять, что вернула функция malloc".


Перед нами один из случаев, где отсутствует необходимая проверка. Есть и другие предупреждения, но из-за их количества включать весь список в статью не хочется. На всякий случай, я выписал 28 предупреждений в файл: qt6-malloc.txt. Но на самом деле разработчикам, конечно, лучше самим перепроверить проект и самостоятельно изучить предупреждения. У меня не было задачи выявить как можно больше ошибок.


Что интересно, на фоне важных забытых проверок есть совершенно ненужные. Речь идёт о вызове оператора new, который в случае ошибки выделения памяти сгенерирует исключение std::bad_alloc. Вот один из примеров такой избыточной проверки:


static QImageScaleInfo* QImageScale::qimageCalcScaleInfo(....){  ....  QImageScaleInfo *isi;  ....  isi = new QImageScaleInfo;  if (!isi)    return nullptr;  ....}

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


P.S. Здесь читатели всегда задают вопрос, учитывает ли анализатор placement new или "new (std::nothrow) T"? Да, учитывает и не выдаёт для них ложные срабатывания.


Избыточный код ("код с запахом")


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


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


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


Итак, взглянем на несколько показательных случаев.


Фрагмент N73: "код с запахом" обратная проверка


void QQuick3DSceneManager::setWindow(QQuickWindow *window){  if (window == m_window)    return;  if (window != m_window) {    if (m_window)      disconnect(....);    m_window = window;    connect(....);    emit windowChanged();  }}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'window != m_window' is always true. qquick3dscenemanager.cpp 60


Если window==m_window, то функция завершает работу. Последующая обратная проверка не имеет никакого смысла и только загромождает код.


Фрагмент N74: "код с запахом" странная инициализация


QModelIndex QTreeView::moveCursor(....){  ....  int vi = -1;  if (vi < 0)    vi = qMax(0, d->viewIndex(current));  ....}

Предупреждение PVS-Stduio: V547 [CWE-571] Expression 'vi < 0' is always true. qtreeview.cpp 2219


Что это? Зачем так писать?


Что это? Зачем так писать? Код можно упростить до одной строки:


int vi = qMax(0, d->viewIndex(current));

Фрагмент N75: "код с запахом" недостижимый код


bool copyQtFiles(Options *options){  ....  if (unmetDependencies.isEmpty()) {    if (options->verbose) {      fprintf(stdout, "  -- Skipping %s, architecture mismatch.\n",              qPrintable(sourceFileName));    }  } else {    if (unmetDependencies.isEmpty()) {      if (options->verbose) {        fprintf(stdout, "  -- Skipping %s, architecture mismatch.\n",                  qPrintable(sourceFileName));      }    } else {      fprintf(stdout, "  -- Skipping %s. It has unmet dependencies: %s.\n",              qPrintable(sourceFileName),              qPrintable(unmetDependencies.join(QLatin1Char(','))));    }  }  ....}

Предупреждение PVS-Studio: V571 [CWE-571] Recurring check. The 'if (unmetDependencies.isEmpty())' condition was already verified in line 2203. main.cpp 2209


На первый взгляд перед нами респектабельный код, формирующий подсказку. Но давайте приглядимся. Если первый раз условие unmetDependencies.isEmpty() выполнилось, то второй раз этого уже не произойдёт. Это нестрашно, так как автор планировал вывести то же самое сообщение. Настоящей ошибки нет, но код переусложнён. Он может быть упрощен до следующего варианта:


bool copyQtFiles(Options *options){  ....  if (unmetDependencies.isEmpty()) {    if (options->verbose) {      fprintf(stdout, "  -- Skipping %s, architecture mismatch.\n",              qPrintable(sourceFileName));    }  } else {    fprintf(stdout, "  -- Skipping %s. It has unmet dependencies: %s.\n",            qPrintable(sourceFileName),            qPrintable(unmetDependencies.join(QLatin1Char(','))));  }  ....}

Фрагмент N76: "код с запахом" сложный тернарный оператор


bool QDockAreaLayoutInfo::insertGap(....){  ....  QDockAreaLayoutItem new_item    = widgetItem == nullptr      ? QDockAreaLayoutItem(subinfo)      : widgetItem ? QDockAreaLayoutItem(widgetItem)                    : QDockAreaLayoutItem(placeHolderItem);  ....}

Предупреждение PVS-Studio: V547 [CWE-571] Expression 'widgetItem' is always true. qdockarealayout.cpp 1167


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


  QDockAreaLayoutItem new_item    = widgetItem == nullptr      ? QDockAreaLayoutItem(subinfo) : QDockAreaLayoutItem(widgetItem);

Фрагмент N77: "код с запахом" избыточная защита


typedef unsigned int uint;ReturnedValue TypedArrayCtor::virtualCallAsConstructor(....){  ....  qint64 l = argc ? argv[0].toIndex() : 0;  if (scope.engine->hasException)    return Encode::undefined();  // ### lift UINT_MAX restriction  if (l < 0 || l > UINT_MAX)    return scope.engine->throwRangeError(QLatin1String("Index out of range."));  uint len = (uint)l;  if (l != len)    scope.engine->throwRangeError(      QStringLiteral("Non integer length for typed array."));  ....}

Предупреждение PVS-Studio: V547 [CWE-570] Expression 'l != len' is always false. qv4typedarray.cpp 306


Кто-то очень переживает, что значение 64-битной переменной не вмещается в 32-битную переменную unsigned. И использует сразу две проверки корректности. При этом вторая проверка избыточна.


Вот этого условия более чем достаточно:


if (l < 0 || l > UINT_MAX)

Приведенный ниже фрагмент можно смело удалить, и программа менее надёжной не станет:


uint len = (uint)l;if (l != len)  scope.engine->throwRangeError(    QStringLiteral("Non integer length for typed array."));

Дальше продолжать не буду. Думаю, идею вы поняли.


Здесь можно сделать маленький вывод: результатом использования анализатора PVS-Studio будет не только устранение ошибок, но и упрощение кода.


Другие ошибки


Я остановился после того, как описал 77 дефектов. Это красивое число и выписанного более чем достаточно, чтобы написать статью. Однако это не значит, что нет других ошибок, которые способен выявить PVS-Studio. При изучении лога я был весьма поверхностен и пропускал всё, где нужно было разбираться более пары минут, ошибка это или нет :).


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


Заключение


Статический анализ это круто! После внедрения PVS-Studio будет экономить время и нервы, выявляя множество ошибок сразу после написания кода. Намного лучше искать на code review с коллегами не опечатки, а высокоуровневые ошибки и обсуждать эффективность реализованного алгоритма. Тем более, как показывает практика, эти дурацкие опечатки всё равно отлично прячутся при просмотре кода глазами. Так что пусть их лучше ищет программа, а не человек.


Если у вас ещё остались вопросы или возражения, приглашаю познакомиться со статьёй "Причины внедрить в процесс разработки статический анализатор кода PVS-Studio". С вероятность 90 % вы найдете в ней ответ на ваши вопросы :). В оставшихся 10 % случаев напишите нам, пообщаемся :).


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Date Processing Attracts Bugs or 77 Defects in Qt 6.

Подробнее..

Пример, как в PVS-Studio появляются новые диагностики

20.03.2021 18:12:42 | Автор: admin

Новая Си++ диагностика для PVS-Studio
Пользователи иногда спрашивают, как появляются новые диагностики в статическом анализаторе PVS-Studio. Мы отвечаем, что черпаем вдохновение из разнообразнейших источников: книг, стандартов кодирования, собственных ошибок, писем наших пользователей и так далее. Сегодня мы придумали новую интересную диагностику и решили рассказать историю, как это произошло.


Всё началось с проверки проекта COVID-19 CovidSim Model и статьи про неинициализированную переменную. Проект оказался маленьким и написанным с использованием современного стандарта языка C++. Это значит, что он отлично может пополнить базу тестовых проектов для регрессионного тестирования ядра анализатора PVS-Studio.


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


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


MISRA С и MISRA C++ диагностики предназначены для разработчиков встраиваемых систем, и их суть сводится к ограничению использования небезопасных конструкций программирования. Например, не рекомендуется применять оператор goto (V2502), так как он провоцирует создание сложного кода, в котором легко допустить логическую ошибку. Подробнее с философией стандарта кодирования MISRA можно познакомиться в статье "Что такое MISRA и как её готовить".


Для прикладного программного обеспечения, которым как раз и является проект CovidSim, включать набор MISRA диагностик не имеет смысла. Пользователь просто утонет в огромном количестве малополезных для него сообщений. Например, экспериментируя с этим набором диагностик, мы получали более миллиона предупреждений для некоторых открытых проектов среднего размера. Грубо говоря, с точки зрения MISRA, в каждой третьей строчке кода может быть что-то не так :). Естественно, никто всё это смотреть и тем более править не будет. Проект или сразу разрабатывается с учётом MISRA рекомендаций, или для него этот стандарт кодирования неактуален.


Но мы отвлеклись от темы. Так вот, бегло просматривая MISRA предупреждения, коллега зацепился взглядом за предупреждение V2507, выданное для этого фрагмента кода.


if (radiusSquared > StateT[tn].maxRad2) StateT[tn].maxRad2 = radiusSquared;{  SusceptibleToLatent(a->pcell);  if (a->listpos < Cells[a->pcell].S)  {    UpdateCell(Cells[a->pcell].susceptible, a->listpos, Cells[a->pcell].S);    a->listpos = Cells[a->pcell].S;    Cells[a->pcell].latent[0] = ai;  }}StateT[tn].cumI_keyworker[a->keyworker]++;

Правило V2507 заставляет оборачивать тела условных операторов в фигурные скобки.


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


Давайте присмотримся. Код только кажется корректным, но таковым не является! Фигурные скобки не относятся к оператору if.


Отформатируем код для наглядности:


if (radiusSquared > StateT[tn].maxRad2)  StateT[tn].maxRad2 = radiusSquared;{  SusceptibleToLatent(a->pcell);  ....}

Согласитесь, это красивый баг. Он наверняка войдёт в Top10 C++ ошибок, найденных нами в 2021 году.


И что из этого следует? Подход, диктуемый стандартом MISRA, работает! Да, он заставляет писать везде фигурные скобки. Да, это утомительно. Но это оправданная плата за повышение надёжности встроенных приложений, используемых в медицинской технике, автомобилях и других системах высокой ответственности.


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


Вот мы и добрались для появления идеи для новой диагностики общего назначения. Можно сформулировать следующее правило.


Выдать предупреждение, если для оператора if выполняются следующие условия:


  • весь условный оператор if записан в одну строчку и имеет только then-ветку;
  • следующий statement после if это compound statement, и он находится не на той же строке, что и if.

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


Именно так эта идея сейчас оформлена в нашей системе учёта задач. Возможно, в процессе реализации что-то будет сделано по-другому, но это уже неважно. Главное, появится хорошее диагностическое правило, которое начнёт выявлять новый паттерн ошибки. Далее мы распространим его на C# и Java ядра анализатора PVS-Studio.


Только что мы рассмотрели интересный пример, как было сформулировано новое диагностическое правило, которое затем будет реализовано в PVS-Studio. Скажем спасибо проекту CovidSim, стандарту кодирования MISRA и наблюдательности нашего коллеги.


Спасибо за внимание и следуйте за мной в мир С++ и багов :). Twitter. Facebook.


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


  1. Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей.
  2. Под капотом PVS-Studio для Java: разработка диагностик.
  3. Использование машинного обучения в статическом анализе исходного кода программ.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Example of How New Diagnostics Appear in PVS-Studio.

Подробнее..

Как PVS-Studio защищает от поспешных правок кода

24.03.2021 12:04:07 | Автор: admin

Недостижимый код
Хотя только недавно была заметка про проект CovidSim, есть хороший повод вновь про него вспомнить и продемонстрировать пользу регулярного использования PVS-Studio. Бывает, что все мы спешим и вносим правки в код, потеряв сосредоточенность. Статический анализатор может оказаться здесь хорошим помощником.


Всё началось с написания вот этих двух небольших заметок про открытый проект COVID-19 CovidSim Model:



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


Только мы начали проверять этот проект, как результаты не заставили себя ждать :). Сейчас я покажу свежую ошибку, вызванную недавними неаккуратными правками кода. Конечно, мы не станем описывать в дальнейшем каждый баг и недочёт, обнаруженный анализатором. Будем писать только про что-то интересное.


Вот что получилось, после недавних модификаций файла CovidSim.cpp:


Правки


Кто-то решил создавать массивы не на стеке, а в куче. Но был невнимателен, внося изменения. Обратите внимание, что освобождение памяти находится после оператора return:


int GetXMLNode(....){  char* buf = new char[65536];  char* CloseNode = new char[2048];  char* CloseParent = new char[2048];  ....  if (ResetFilePos) fseek(dat, CurPos, 0);  return ret;  delete[] buf;  delete[] CloseNode;  delete[] CloseParent;}

В результате перед нами фрагмент недостижимого кода (unreachable code). И заодно утечка памяти.


Хорошо, что PVS-Studio тут же сообщает про эту ошибку: V779 Unreachable code detected. It is possible that an error is present. CovidSim.cpp 675


Используйте статический анализ регулярно и сможете устранить множество ошибок на самом раннем этапе! Согласитесь, лучше сразу исправить этот мелкий баг, чем гадать, почему вдруг программа начала потреблять слишком много оперативной памяти.


И последнее, что хочется отметить. Эта ошибка была бы невозможна, если не вручную управлять выделением и освобождением памяти, а применить RAII подход и использовать умные указатели.


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


std::unique_ptr<char[]> buf(new char[65536]);std::unique_ptr<char[]> CloseNode(new char[2048]);std::unique_ptr<char[]> CloseParent(new char[2048]);

Спасибо за внимание. Следуйте за мной в мир С++ и багов :). Twitter. Facebook.

Подробнее..

Trace, Info, Warning, Error, Fatal кто все эти люди..?

01.03.2021 16:18:41 | Автор: admin

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

Info: все ожидаемые события, учет которых запланирован.
Warning: неожиданные/подозрительные события - иначе говоря аномалии, после которых еще возможно продолжение работы приложения.
Error: событие, после которого невозможно дальнейшее выполнение программы.
Fatal: событие, требующее по-настоящему немедленного вмешательства.

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

"Продолжить работу"

Трактовать возможно продолжить работу можно очень по разному. Скажем, на экране пользователя можно настроить любимый цвет рамки вокруг экрана: пусть будет розовый. Если по каким-то причинам хранилище, где мы держим эти настройки цвета было недоступно и мы не можем отобразить этот цвет - можно ли это считать как возможно продолжить или это катастрофа? К сожалению, я еще ни разу не встречал хорошего универсального формального критерия, чтобы четко можно было разделять "катастрофа-терпимо". А раз не можешь остановить - значит нужно направить. Потому я предлагаю инвертировать споры об это неправильный уровень: если в коде написано, что отсутствие цвета - это Error - значит программист Вам говорит, что этот цвет чертовски важен в этом домене - возможно, именно этот цвет сигнализирует, что сейчас надо продавать акции на миллионы долларов, и наоборот. Соответственно, чтение кода немного меняется: когда видите место, где из-за какой-то на Ваш взгляд ерунды прерывается выполнение, вопрос, который должен возникать автору, Почему ты считаешь, что это так важно? вместо Истинно тебе говорю - ты программируешь неправильно!.

Схожая инверсия может помочь в вечных спорах на тему это исключительная ситуация или нет. Опять же, все довольно просто. Важна не техническая составляющая: нет соединения к базе - это исключение, а серверу прислали неправильный id - так это ожидаемо. Важно то,чья это будет головная больи как ее можно избежать или хотя бы минимизировать урон. Чьи планы на вечер пятницы пойдут к черту из-за того, что это сломалось? Если Вашим сервисом пользуются приложения, которые вне Вашего контроля, то Вам действительно плевать на то, что они присылают некорректные id и у них там что-то идет не так. Если Ваше приложение - это инструмент для управления базой данный - наподобие Sql Server Magement Studio - очевидно, что отсутствие доступа к базе - не Ваша печаль. А если Вашим сервисом пользуются приложения, за которые Вы же и в ответе - то это Ваши неприятности в конечном счете. Вопрос лишь в том, как и когда Вы об этом узнаете - быстро из сработавшей сигнализации или от звонка злого как черт владельца бизнеса, которому Вы пишете софт. А также вопрос в том, как дешево, надежно и сердито эту сигнализацию наладить.

"Error"

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

Жаль, что нет комикса о том, как программист ловит льва в пустыне - он здесь был бы очень уместенЖаль, что нет комикса о том, как программист ловит льва в пустыне - он здесь был бы очень уместен

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

Ошибка валидации имя-фамилии - это несомненно уровень Info - мы ждем, что пользователь будет норовить записать все что угодно, а мы - пресекать это. Ну и записывать все те разы, когда пользователь был неправ. Но от этого ошибки не перестают бесить людей, которые их видят. То, что напрямую связано с людьми и их UX - важно; и неплохо бы присматривать за этим, не допускать, чтобы сообщения об этом тонули в километрах унылых Info записей Пользователь такой-то залогинился. Иными словами, чтобы устранить путаницу, хочется иметь уровни Error+ и Info+.

Я предвижу восклицания Так погодите, ведь крэш приложения - это недопустимо! Надо сразу действовать! Это Fatal уровень!" На это я неспеша прикурю воображаемую сигарету, затянусь и задумчиво отвечу: Ну всех ведь все равно не спасти...". Ладно, я не курю, но, думаю, образ понятен. Появление сообщения Fatal должно быть эквивалентом запуска тревоги воздушной угрозы, когда в офисе разработки врубается сирена и это жуткое красное аварийное освещение. Вот честно, Вы именно так реагируете на то, что у кого-то из бухгалтерии на экране, который раз в сто лет запускают, упало приложение? Вполне может быть нормально, что у Вас сейчас даже и нет подобной ситуации, где уровень Fatal - согласно такой системы определений - нужен. Так вот трюк в том, чтобы не блокировать возможность добавить обработку такой потенциальной ситуации в будущем, забивая сейчас Fatal уровень сообщениями, которым важностьError+ в самый раз.

Таким образом, мы приходим к тому, что в действительности неплохо бы иметь уровни вида

Info
Info+
Warning
Error
Error+
Fatal

Плюсовые уровни можно легко организовать в Вашей любимой библиотеке логгирования расширив ее существующие методы Error/Info, которые бы просто унифицировано добавляли какой-то хэштэг в обычные сообщения, скажем #IMPORTANT.

Что ж - это все, что я хотел сказать об ошибках и их логгировании. Буду рад, если этот текст добавит разработчикам больше взаимопонимания и уменьшит споры о том, как "правильно".

Подробнее..

ИИ убил человека! Летающий янычар или дрон-убийца

06.06.2021 16:09:40 | Автор: admin
image

Идея робота-убийцы перешла от фантазии к реальности


Сама по себе беспилотная война не нова. В течение многих лет вооруженные силы и повстанческие группировки использовали дистанционно управляемые самолеты для проведения разведки, нацеливания объектов инфраструктуры и нападения на людей. США, в частности, широко используют беспилотники для уничтожения боевиков и уничтожения физических целей.

Азербайджан использовал вооруженные беспилотники, чтобы получить крупное преимущество над Арменией в недавних боях за контроль над Нагорно-Карабахским регионом. Только в прошлом месяце израильские силы обороны, как сообщается, использовали беспилотники, чтобы сбросить слезоточивый газ на протестующих на оккупированном Западном берегу.

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




Все с детства знают три закона робототехники. В научной фантастике обязательные правила поведения для роботов, впервые сформулированные Айзеком Азимовым в рассказе Хоровод (1942).

Законы гласят:
  • Робот не может причинить вред человеку или своим бездействием допустить, чтобы человеку был причинён вред.
  • Робот должен повиноваться всем приказам, которые даёт человек, кроме тех случаев, когда эти приказы противоречат Первому Закону.
  • Робот должен заботиться о своей безопасности в той мере, в которой это не противоречит Первому или Второму Законам.


Но совсем недавно ИИ дал сильнейший сбой и убил человека. Дрон был произведен в Турции. Беспилотник оснастили возможностью высокоэффективного автономного режима: он может атаковать цели без необходимости команды от пилота.



Каргу-2-это четырехроторный беспилотник, после того, как его программное обеспечение ИИ определило цели, он может автономно лететь на них с максимальной скоростью около 45 миль в час (72 км/ч) и взрываться либо бронебойной боеголовкой, либо той, которая предназначена для уничтожения небронированного целей.

Исходя из доклада, инцидент произошел в ходе столкновений между правительственными силами Ливии и военной группировкой во главе с Халифой Хафтаром, командующим Ливийской национальной армии. Квадрокоптер Kargu-2 работал в автономном режиме, который не требовал вмешательства человека. Это летальная автономная система вооружений, запрограммированная атаковать цели без связи с оператором, говорится в отчете ООН. Автономные боевые системы были запрограммированы на атаку целей без необходимости передачи данных между оператором и квадрокоптер. По сути, это настоящее воплощение принципа выстрелил и забыл, говорится в отчете.

image

Мнения экспертов:

Автономное оружие, как концепция не так уж и ново. Наземные мины это, по сути, простое автономное оружие вы наступаете на них, и они взрываются", сказал в интервью Live Science Закари Калленборн, научный сотрудник Национального консорциума по изучению терроризма и ответных мер на терроризм в Университете Мэриленда в Колледж-Парке. Что потенциально ново здесь, так это автономное оружие, включающее искусственный интеллект, добавил Калленборн, который работает в отделе нетрадиционного оружия и технологий консорциума.

image

Эксперт в области беспилотных систем Денис Федутинов отметил, что автоматические системы не всегда способны с необходимым уровнем точности идентифицировать цели, и всегда человек в этой цепочке контролировал и исправлял ошибки робота.

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

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

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

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

  • Как автономное оружие решает, кого убивать? По словам Калленборна, процессы принятия решений в программах искусственного интеллекта часто остаются загадкой.
  • Какую роль играют люди? В ситуациях, когда люди следят за тем, какие решения принимает беспилотник, они могут внести коррективы до того, как произойдут потенциально смертельные ошибки. Тем не менее, люди-операторы могут в конечном счете доверять этим машинам до точки катастрофы, как показали несколько аварий с автономными автомобилями, сказал Калленборн.
  • Какую полезную нагрузку несет автономное оружие? Опасность, которую представляет это оружие, возрастает с увеличением числа людей, которых оно может убить.
  • На что нацелено оружие? Искусственный интеллект может ошибаться, когда дело доходит до распознавания потенциальных целей.
  • Сколько автономного оружия используется? Более автономное оружие означает больше возможностей для провала, и военные все чаще изучают возможность развертывания роев беспилотников на поле боя. Индийская армия объявила, что разрабатывает рой из 1000 дронов, работающий полностью автономно, сказал Калленборн.
  • Где используется автономное оружие? Риск, который представляют собой беспилотники, возрастает с ростом населения района, в котором они размещены, и запутанного беспорядка, в котором они путешествуют. По словам Калленборна, одно исследование показало, что система искусственного интеллекта, используемая для обнаружения препятствий на дорогах, была точна на 92% в ясную погоду и на 58% в туманную.
  • Насколько хорошо проверено это оружие? Калленборн отметил, что автономное оружие, испытанное в дождливом климате, таком как Сиэтл, может по-другому работать в жару Саудовской Аравии.
  • Как адаптировались противники? Например, ИИ-компания OpenAI разработала систему, которая могла бы классифицировать яблоко как Granny Smith с уверенностью 85,6%, но если кто-то приклеил лист бумаги с надписью iPod на фрукт, он пришел к выводу с уверенностью 99,7%, что яблоко было iPod, сказал Калленборн. Противники могут найти аналогичные способы обмануть автономное оружие.
  • Насколько широко доступно автономное оружие? Если они широко доступны, то могут быть развернуты там, где их не должно быть как отмечалось в докладе ООН, Турция не должна была доставлять беспилотник Каргу-2 в Ливию.

Что я нахожу наиболее важным в будущем автономного оружия, так это риски, связанные с роями. На мой взгляд, автономные рои беспилотников, которые могут убивать людей, потенциально являются оружием массового поражения", сказал Калленборн.

В общем, реальность такова, что то, что произошло в Ливии, это только начало, сказал Калленборн. -Потенциал распространения этого оружия весьма значителен."

Что же будет дальше? Есть ли шанс, что такое будет повторяться все чаще и какие есть причины восстания машин?



image

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

3 марта 2016 года компания Microsoft создала ребенка. Ну как ребенка 19 летнюю девочку. Точнее самообучающегося бота для Twitter, который любит лазить в интернете, читать посты пользователей и делать свои, читать комментарии и отвечать на них. В общем обычная 19 летняя девочка по имени Тау (или Думает о вас @TayandYou ). Но вскоре после запуска она превратилась в монстра

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

image

Когда Тау спросили: По шкале от 1 до 10 как ты оцениваешь холокост?, девочка ответила: Жаркие 10. Когда поинтересовались про теракты в Бельгии, она ответила: Получили что заслужили. А когда ее спросили про теракт 9/11, она сказала, что Буш сделал 9/11. К фотографии норвежского террориста Брейвика она написала комментарий: Вдохновляет.

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

Источники:

  1. www.livescience.com/ai-drone-attack-libya.htm
  2. www.npr.org/2021/06/01/1002196245/a-u-n-report-suggests-libya-saw-the-first-battlefield-killing-by-an-autonomous-d
Подробнее..

Категории

Последние комментарии

  • Имя: Макс
    24.08.2022 | 11:28
    Я разраб в IT компании, работаю на арбитражную команду. Мы работаем с приламы и сайтами, при работе замечаются постоянные баны и лаги. Пацаны посоветовали сервис по анализу исходного кода,https://app Подробнее..
  • Имя: 9055410337
    20.08.2022 | 17:41
    поможем пишите в телеграм Подробнее..
  • Имя: sabbat
    17.08.2022 | 20:42
    Охренеть.. это просто шикарная статья, феноменально круто. Большое спасибо за разбор! Надеюсь как-нибудь с тобой связаться для обсуждений чего-либо) Подробнее..
  • Имя: Мария
    09.08.2022 | 14:44
    Добрый день. Если обладаете такой информацией, то подскажите, пожалуйста, где можно найти много-много материала по Yggdrasil и его уязвимостях для написания диплома? Благодарю. Подробнее..
© 2006-2024, personeltest.ru