Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Единорог заинтересовался KDE

Единорог заинтересовался KDE

29 Сен 2014

KDE (сокращение от K Desktop Environment) - среда рабочего стола, преимущественно для Linux и других UNIX-подобных систем. Если простым языком, то это та штука, которая отвечает за всё графическое оформление. Среда построена на основе кроссплатформенного инструментария разработки пользовательского интерфейса Qt. Разработкой занимаются несколько сотен программистов со всего мира, преданных идее свободного программного обеспечения. KDE предлагает полный набор приложений пользовательского окружения, который позволяет взаимодействовать с операционной системой в современном графическом интерфейсе. Давайте же посмотрим, что у KDE под капотом.

0283_KDE_ru/image1.png

Следующие пакеты проекта KDE версии 4.14 проверялись с помощью PVS-Studio 5.19 в OpenSUSE Factory:

  • KDE PIM Libraries
  • KDE Base Libraries
  • KDE Base Apps
  • KDE Development

KDE PIM Libraries

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

enum PartStat {
  ....
  Accepted,
  Tentative,
  ....
};

static QString formatICalInvitationHelper(....)
{
  ....
  a = findDelegatedFromMyAttendee( inc );
  if ( a ) {
    if ( a->status() != Attendee::Accepted ||      // <=
         a->status() != Attendee::Tentative ) {    // <=
      html += responseButtons( inc, rsvpReq, rsvpRec, helper );
      break;
    }
  }
  ....
}

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

Аналогичное подозрительное место:

  • V547 Expression is always true. Probably the '&&' operator should be used here. incidenceformatter.cpp 3293

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

void LDAPProtocol::del( const KUrl &_url, bool )
{
  ....
  if ( (id = mOp.del( usrc.dn() ) == -1) ) {
    LDAPErr();
    return;
  }
  ret = mOp.waitForResult( id, -1 );
  ....
}

Приоритет оператора сравнения (==) выше приоритета оператора присваивания (=). Благодаря везению, условие выполняется как задумно, но дальше используется некорректное значение идентификатора 'id', равное 0.

V595 The 'incBase' pointer was utilized before it was verified against nullptr. Check lines: 2487, 2491. incidenceformatter.cpp 2487

static QString formatICalInvitationHelper(....)
{
  ....
  incBase->shiftTimes( mCalendar->timeSpec(), ....);

  Incidence *existingIncidence = 0;
  if ( incBase && helper->calendar() ) {
    ....
  }
  ....
}

Разыменование указателя 'incBase' выполняется перед его проверкой.

V622 Consider inspecting the 'switch' statement. It's possible that the first 'case' operator is missing. listjob.cpp 131

void ListJob::doStart()
{
  Q_D( ListJob );

  switch ( d->option ) {
    break;                          // <=
  case IncludeUnsubscribed:
    d->command = "LIST";
    break;
  case IncludeFolderRoleFlags:
    d->command = "XLIST";
    break;
  case NoOption:
  default:
    d->command = "LSUB";
  }
  ....
}

В блоке оператора 'switch' первым оператором не является 'case'. Это приводит к тому, что фрагмент кода никогда не получит управление. В лучшем случае оператор 'break' остался после неполного удаления старого условия, но в худшем случае тут пропущен ещё один 'case'.

V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'lexBuf.strs' is lost. Consider assigning realloc() to a temporary pointer. vcc.y 638

static void lexAppendc(int c)
{
  lexBuf.strs = (char *) realloc(lexBuf.strs, (size_t) .... + 1);  
  lexBuf.strs[lexBuf.strsLen] = c;
  ....
}

Данное выражение является потенциально опасным: рекомендуется результат функции realloc сохранять в другой переменной. Функция realloc() производит изменение размера некоторого блока памяти. В случае ошибки указатель на старую область памяти будет утерян.

Да и вообще, такой код плох. Нет провреки того, что вернула функция realloc(). Указатель сразу разыменовывается в следующей строке.

Аналогичные места:

  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'mods' is lost. Consider assigning realloc() to a temporary pointer. ldapoperation.cpp 534
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'mods[i]->mod_vals.modv_bvals' is lost. Consider assigning realloc() to a temporary pointer. ldapoperation.cpp 579
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'ctrls' is lost. Consider assigning realloc() to a temporary pointer. ldapoperation.cpp 624
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'fp->s' is lost. Consider assigning realloc() to a temporary pointer. vobject.c 1055
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'lexBuf.strs' is lost. Consider assigning realloc() to a temporary pointer. vcc.y 635
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'lexBuf.strs' is lost. Consider assigning realloc() to a temporary pointer. vcc.y 643
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'bytes' is lost. Consider assigning realloc() to a temporary pointer. vcc.y 928
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'fp->s' is lost. Consider assigning realloc() to a temporary pointer. vobject.c 1050

KDE Base Libraries

V523 The 'then' statement is equivalent to the 'else' statement. kconfig_compiler.cpp 1051

QString newItem( const QString &type, ....)
{
  QString t = "new "+cfg.inherits+"::Item" + ....;
  if ( type == "Enum" ) t += ", values" + name;
  if ( !defaultValue.isEmpty() ) {
    t += ", ";
    if ( type == "String" ) t += defaultValue;        // <=
    else t+= defaultValue;                            // <=
  }
  t += " );";

  return t;
}

Одинаковые истинная и ложная ветвь оператора 'if' выглядят крайне подозрительно. Если код всё-таки не содержит опечатку, то его можно упростить, например, так:

if ( !defaultValue.isEmpty() )
    t += ", " + defaultValue;

Похожее место:

  • V523 The 'then' statement is equivalent to the 'else' statement. installation.cpp 589

V595 The 'priv->slider' pointer was utilized before it was verified against nullptr. Check lines: 786, 792. knuminput.cpp 786

void KDoubleNumInput::spinBoxChanged(double val)
{
  ....
  const double slidemin = priv->slider->minimum();      // <=
  const double slidemax = priv->slider->maximum();      // <=
  ....
  if (priv->slider) {                                   // <=
    priv->slider->blockSignals(true);
    priv->slider->setValue(qRound(slidemin + rel * (....)));
    priv->slider->blockSignals(false);
  }
}

Разыменование указателя 'priv' выполняется перед его проверкой.

Похожие опасные места:

  • V595 The 'm_instance' pointer was utilized before it was verified against nullptr. Check lines: 364, 376. ksystemtimezone.cpp 364
  • V595 The 'job' pointer was utilized before it was verified against nullptr. Check lines: 778, 783. knewfilemenu.cpp 778

V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. karchive.cpp 187

*bool KArchive::close()
{
  ....
  // if d->saveFile is not null then it is equal to d->dev.
  if ( d->saveFile ) {
    closeSucceeded = d->saveFile->finalize();
    delete d->saveFile;
    d->saveFile = 0;
  } if ( d->deviceOwned ) {                                 // <=
    delete d->dev; // we created it ourselves in open()
  }
  ....
}

Данный фрагмент кода может свидетельствовать о пропуске ключевого слова 'else', либо это просто крайне ненаглядное и сбивающие с толку оформление кода.

V655 The strings was concatenated but are not utilized. Consider inspecting the expression. entrydetailsdialog.cpp 225

void EntryDetails::updateButtons()
{
  ....
  foreach (....) {
    QString text = info.name;
    if (!info.distributionType.trimmed().isEmpty()) {
        text + " (" + info.distributionType.trimmed() + ")";// <=
    }
    QAction* installAction =
      installMenu->addAction(KIcon("dialog-ok"), text);
    installAction->setData(info.id);
  }
  ....
}

Анализатор обнаружил неиспользуемое объединение строковых переменных. Возможно, код должен быть таким:

text += " (" + info.distributionType.trimmed() + ")";

Аналогичные места:

  • V655 The strings was concatenated but are not utilized. Consider inspecting the expression. itemsgridviewdelegate.cpp 365
  • V655 The strings was concatenated but are not utilized. Consider inspecting the expression. itemsviewdelegate.cpp 159

V705 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics. entrydetailsdialog.cpp 149

void EntryDetails::entryChanged(const KNS3::EntryInternal& entry)
{
  ....
  if(m_entry.previewUrl(EntryInternal::PreviewSmall1).isEmpty()){
    ui->previewBig->setVisible(false);
  } else                                // <=

  if (!m_entry.previewUrl((....)type).isEmpty()) {
    ....
  }
  ....
}

Оформление этого фрагмента кода тоже выглядит неоднозначно. Планировалась ли тут конструкция "else if" или же просто забыли удалить 'else'?

V612 An unconditional 'return' within a loop. bufferfragment_p.h 94

BufferFragment split(char c, unsigned int* start) 
{
  while (*start < len) {
    int end = indexOf(c, *start);
    if (end == -1) end = len;
    BufferFragment line(d + (*start), end - (*start));
    *start = end + 1;
    return line;
  }
  return BufferFragment();
}

Стоило ли писать цикл из-за одной итерации? Или же не хватает условного оператора?

V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'd' is lost. Consider assigning realloc() to a temporary pointer. netwm.cpp 596

template <class Z>
void NETRArray<Z>::reset() {
    sz = 0;
    capacity = 2;
    d = (Z*) realloc(d, sizeof(Z)*capacity);
    memset( (void*) d, 0, sizeof(Z)*capacity );
}

Как и в "KDE PIM Libraries", здесь не рекомендутся использовать один указатель с функцией realloc(), так как указатель на старую область памяти может быть утерян, если память не удастся увеличить.

Аналогичные места:

  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'handlers' is lost. Consider assigning realloc() to a temporary pointer. kxerrorhandler.cpp 94
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'buffer' is lost. Consider assigning realloc() to a temporary pointer. netwm.cpp 528
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'd' is lost. Consider assigning realloc() to a temporary pointer. netwm.cpp 608
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'ptr' is lost. Consider assigning realloc() to a temporary pointer. kdesu_stub.c 119
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'addr.generic' is lost. Consider assigning realloc() to a temporary pointer. k3socketaddress.cpp 372

KDE Base Apps

V501 There are identical sub-expressions 'mimeData->hasFormat(QLatin1String("application/x-kde-ark-dndextract-service"))' to the left and to the right of the '&&' operator. iconview.cpp 2357

void IconView::dropEvent(QGraphicsSceneDragDropEvent *event)
{
  ....
  if (mimeData->hasFormat(QLatin1String(
       "application/x-kde-ark-dndextract-service")) &&      // <=
      mimeData->hasFormat(QLatin1String(
       "application/x-kde-ark-dndextract-service")))        // <=
  {
    const QString remoteDBusClient = mimeData->data(
      QLatin1String("application/x-kde-ark-dndextract-service"));
    const QString remoteDBusPath = mimeData->data(
      QLatin1String("application/x-kde-ark-dndextract-path"));
    ....
  }
  ....
}

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

if (mimeData->hasFormat(QLatin1String(
       "application/x-kde-ark-dndextract-service")) &&     // <=
      mimeData->hasFormat(QLatin1String(
       "application/x-kde-ark-dndextract-path ")))         // <=
{
  ....
}

V595 The 'm_view' pointer was utilized before it was verified against nullptr. Check lines: 797, 801. kitemlistcontroller.cpp 797

bool KItemListController::mouseDoubleClickEvent(....)
{
  const QPointF pos = transform.map(event->pos());
  const int index = m_view->itemAt(pos);

  // Expand item if desired - See Bug 295573
  if (m_mouseDoubleClickAction != ActivateItemOnly) {
    if (m_view && m_model && ....) {
      const bool expanded = m_model->isExpanded(index);
      m_model->setExpanded(index, !expanded);
    }
  }
  ....
}

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

V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 410, 412. kebsearchline.cpp 410

void
KViewSearchLine::slotColumnsRemoved(const QModelIndex &,
                                    int first, int last)
{
  if(d->treeView)
    updateSearch();
  else
  {
    if(d->listView->modelColumn() >= first &&
       d->listView->modelColumn() <= last)
    {
      if(d->listView->modelColumn()>last)   // <=
        kFatal()<<"...."<<endl;
      updateSearch();
    }
  }
}

Вложенное условие будет всегда ложным. Можно предположить, что такое условие было актуальным до некоторых правок.

V654 The condition 'state != 1' of loop is always true. passwd.cpp 255

int PasswdProcess::ConversePasswd(....)
{
  ....
  state = 0;
  while (state != 1)
  {
    line = readLine();
    if (line.isNull())
    {
      // No more input... OK
      return 0;
    }
    if (isPrompt(line, "password"))
    {
      // Uh oh, another prompt. Not good!
      kill(m_Pid, SIGKILL);
      waitForChild();
      return PasswordNotGood;
    }
    m_Error += line + '\n'; // Collect error message
  }
  ....
}

Значение переменной 'state' не изменяется в цикле, следовательно, условием остановки является только вызов 'return'.

KDE Development

V501 There are identical sub-expressions 'file == rhs.file' to the left and to the right of the '&&' operator. pp-macro.cpp 44

bool pp_macro::operator==(const pp_macro& rhs) const {
  if(completeHash() != rhs.completeHash())
    return false;
  
  return name == rhs.name && file == rhs.file &&      // <=
         file == rhs.file &&                          // <=
         sourceLine == rhs.sourceLine &&
         defined == rhs.defined &&
         hidden == rhs.hidden &&
         function_like == rhs.function_like &&
         variadics == rhs.variadics &&
         fixed == rhs.fixed &&
         defineOnOverride == rhs.defineOnOverride &&
         listsEqual(rhs);
}

Анализатор обнаружил несколько мест с повторяющимися условными выраженями. Что-то из этого вполне может быть серьёзной опечаткой.

Ещё такие места:

  • V501 There are identical sub-expressions 'tokenKind == Token_not_eq' to the left and to the right of the '||' operator. builtinoperators.cpp 174
  • V501 There are identical sub-expressions '!context->owner()' to the left and to the right of the '||' operator. typeutils.cpp 194

V595 The 'parentJob()->cpp()' pointer was utilized before it was verified against nullptr. Check lines: 437, 438. cppparsejob.cpp 437

void CPPInternalParseJob::run()
{
    ....
    QReadLocker lock(parentJob()->parentPreprocessor() ?
      0: parentJob()->cpp()->language()->parseLock());      // <=
    if(.... || !parentJob()->cpp())                         // <=
      return;
    ....
}

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

Ещё место:

  • V595 The 'parentContext()' pointer was utilized before it was verified against nullptr. Check lines: 692, 695. context.cpp 692

V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. usedecoratorvisitor.cpp 40

DataAccess::DataAccessFlags typeToDataAccessFlags(....)
{
  DataAccess::DataAccessFlags ret = DataAccess::Read;
  TypePtr< ReferenceType > reftype=type.cast<ReferenceType>();
  if(reftype && reftype->baseType() &&
     !reftype->baseType()->modifiers() &    // <=
     AbstractType::ConstModifier)
    ret |= DataAccess::Write;
  
  return ret;
}

Авторам лучше знать, ошибка здесь или нет, но оператор '&' выглядит подозрительно. Обратите внимание, что выражение "!reftype->baseType()->modifiers()" имеет тип 'bool'.

V555 The expression 'm_pos - backOffset > 0' will work as 'm_pos != backOffset'. pp-stream.cpp 225

unsigned int rpp::Stream::peekLastOutput(uint backOffset) const {
  if(m_pos - backOffset > 0)
    return m_string->at(m_pos - backOffset - 1);
  return 0;
}

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

if(m_pos > backOffset)
    return m_string->at(m_pos - backOffset - 1);

Похожее место:

  • V555 The expression 'nextOffset - currentOffset > 0' will work as 'nextOffset != currentOffset'. pp-location.cpp 211

Заключение

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

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

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

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

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

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Эффект последней строки

Дата: 31 Май 2014

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

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

Дата: 21 Ноя 2018

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

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…

Комментарии (0)

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