Pull to refresh

Comments 14

Проекту можно только посочувствовать. Да, можно разработчикам порекомендовать статический анализ, но этого явно мало, если в проекте на С++ (не С даже) появляются конструкции вида
if (sscanf(header, "%u.%u.%u.%u%n", &i1, &i2, &i3, &i4, &n) < 4
      ||  sscanf(header + n, "%hu%x%n", &uuu->port, &tkt, &m) < 2
      || (header[m += n]  &&  !(header[m] == '$')  &&
          !isspace((unsigned char)((header + m)
                                   [header[m] == '$'])))) {
      break/*failed - unreadable connection info*/;
  }

Это явно говорит о системных проблемах: отсутствие внятного стиля и практик кодирования, видимо отсутствие культуры ревью.
Другие примеры только подтверждают это наблюдение. (авторы Cpp Core Guidelines заплакали)
Да, культура написания кода не исключит все ошибки (но 9/10 явно можно будет избежать). Для остального уже стат анализаторы придут на помощь :)
Да, такие вещи часто пишут не профессиональные программисты, а просто учёные, поэтому качество хромает. Но стоит помнить, что те, кто мог бы остаться в науке и помочь развитию подобных проектов, погнались за длинным рублём и ушли в индустрию. Остаются в основном энтузиасты этого дела, а на энтузиазме далеко не уедешь.
упрекать надо тут государство, а не тех кто погнался за длинным рублем
А я людей и не упрекаю, они сделали свой выбор. Замечу, что я сам в науке давно варюсь, поэтому я прекрасно знаю, почему люди так поступают.
Заметьте, я в своем комментарии ни слова про профессионализм разработчиков не сказал.
Есть просто огромная разница даже между
«три джуна коммитят в одну репу» и «три джуна проводят обязательное код ревью друг друга». Даже при условии отсутствии опыта результат совершенно разный (ну только если три джуна это три брата близнеца разве что), просто за счет того что люди мыслят и видят все по-разному.
К сожалению, я не нашел git репозитория данного проекта, на сайте только tarболлы. Поэтому не могу сделать далекоидущих выводов о коллективной разработке; но поисследовав с десяток файлов, ощущение что у одного файла обычно не больше 2 авторов. Т.е. могу сделать предположение, что разработка ведется по следующей схеме:
— имеется директория со всей библиотекой;
— один из сотрудников университета решает расширить функциональность
— пишет новый класс, добавляет к нему автотесты, пишет документацию
— после сдачи статьи/диссертации код продолжает лежать мертвым грузом
В лучшем случае придет человек который решит исправить какие-то нюансы реализации.
Повторюсь, это лишь мои домыслы.

Что интересно, я не нашел в коде использований STL. вообще.
NCBI Workbench — это далеко не одноразовый исследовательский код, это массовый рабочий софт. GUI-обёртка вокруг кучи разного рабочего софта, которая позволяет неспециалисту пользоваться и не думать про консоль, компиляцию, настройку, администрирование баз и прочие такие неприятные вещи. Скорее тут дело в том, что разрабам интереснее пилить собственно софт, чем вылизывать обёртку.
Да, код ревью это хорошая вещь, только кто этим будет заниматься? Беда тут скорее в том, что подобной культуре не учат толком почти нигде, поэтому один и тот же код может выглядеть совершенно по-разному у разных людей. а уж после совместного ревью из-за нехватки времени и желания всё свалят (как обычно) на аспирантов, скажут, чтобы сделал как надо, на этом и заканчивается.
/// Recursive assignment
CBioNode& operator=(const CBioNode& tree)
{
  TParent::operator=(tree);
  TBioTree* pt = (TBioTree*)tree.GetParentTree();
  SetParentTree(pt);
}

виртуальный оператор присваивания — «казалось бы, что может пойти не так»?
Приглашаю разработчиков проекта NCBI Genome Workbench связаться с нами, и мы предоставим полный отчёт, выданный анализатором PVS-Studio.

В мэйл-лист можно послать, gbench-bugs@ncbi.nlm.nih.gov

V590 Consider inspecting the 'ch != '\0' && ch == ' '' expression. The expression is excessive or contains a misprint. cleanup_utils.cpp 580

Интересно, а это у вас pattern-based или dataflow?

Это очень старая pattern-based-диагностика, но пару лет назад мы максимально внедрили DataFlow-анализ и сейчас этот случай находится им. На практике ни тот, ни другой подход не являются универсальными, поэтому мы совместили эти две технологии.

Интересно, спасибо! Я на таких условиях пропускаю DFA справа налево, и если он находит истинные операнды, которые не являются таковыми сами по себе, то это подозрительно. Но это работает только для условий без сайд-эффектов, в том числе потенциальных исключений, поэтому действительно все случаи не поймаешь.

Продолжаем изучать библиотеки для анализа данных в научных исследований: ROOT.
Sign up to leave a comment.