# Анализ кода ROOT \- фреймворка для анализа данных научных исследований

Пока в Стокгольме проходила 118\-я Нобелевская неделя, в офисе разработки статического анализатора кода PVS\-Studio готовился обзор кода проекта ROOT, используемого в научных исследованиях для обработки больших данных\. Премию за такой код, конечно, не дашь, а вот подробный обзор интересных дефектов кода и лицензию для полной проверки проекта разработчики получат\.

![0682_ROOT_ru/image1.png](https://import.viva64.com/docx/blog/0682_ROOT_ru/image1.png)

## Введение

![0682_ROOT_ru/image2.png](https://import.viva64.com/docx/blog/0682_ROOT_ru/image2.png)

[ROOT](https://root.cern/) \- набор утилит для работы с данными научных исследований\. Он обеспечивает все функциональные возможности, необходимые для обработки больших данных, статистического анализа, визуализации и хранения\. В основном написан на языке C\+\+\. Разработка началась в [CERN](https://home.cern/) \(Европейская организация по ядерным исследованиям\) для исследований по физике высоких энергий\. Каждый день тысячи физиков используют ROOT\-приложения для анализа своих данных или для моделирования\.

[PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) \- это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C\+\+, C\# и Java\. Работает в 64\-битных системах на Windows, Linux и macOS и может анализировать код, предназначенный для 32\-битных, 64\-битных и встраиваемых ARM платформ\.

## Дебют новой диагностики

[V1046](https://pvs-studio.ru/ru/docs/warnings/v1046/) Unsafe usage of the bool' and 'int' types together in the operation '&\='\. GSLMultiRootFinder\.h 175

```cpp
int AddFunction(const ROOT::Math::IMultiGenFunction & func) {
  ROOT::Math::IMultiGenFunction * f = func.Clone();
  if (!f) return 0;
  fFunctions.push_back(f);
  return fFunctions.size();
}

template<class FuncIterator>
bool SetFunctionList( FuncIterator begin, FuncIterator end) {
  bool ret = true;
  for (FuncIterator itr = begin; itr != end; ++itr) {
    const ROOT::Math::IMultiGenFunction * f = *itr;
    ret &= AddFunction(*f);
  }
  return ret;
}
```

Бета\-версия анализатора, которая использовалась при проверке, нашла вот такую потрясающую ошибку\.

**Ожидание\.** Функция _SetFunctionList_ обходит список итераторов\. Если хоть один из них будет невалидным, то возвращаемое значение будет _false_, иначе _true_\.

**Реальность\.** Функция _SetFunctionList_ может возвращать значение _false_ даже для валидных итераторов\. Разберёмся в ситуации\.** **Функция _AddFunction_ возвращает количество валидных итераторов в списке _fFunctions_\. Т\.е\. при добавлении ненулевых итераторов, размер этого списка будет последовательно увеличиваться: 1, 2, 3, 4 и т\.д\. Вот тут и начнёт проявлять себя ошибка в коде:

```cpp
ret &= AddFunction(*f);
```

Т\.к\. функция возвращает результат типа _int_, а не _bool_, то операция '&\=' с чётными числами будет давать значение _false_\. Ведь младший бит чётных чисел всегда будет равен нулю\. Следовательно, такая неочевидная ошибка будет портить результат функции _SetFunctionsList_ даже для валидных аргументов\.

![0682_ROOT_ru/image3.png](https://import.viva64.com/docx/blog/0682_ROOT_ru/image3.png)

## Ошибки в условных выражениях

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '&&' operator: module && module rootcling\_impl\.cxx 3650

```cpp
virtual void HandleDiagnostic(....) override
{
  ....
  bool isROOTSystemModuleDiag = module && ....;
  bool isSystemModuleDiag = module && module && module->IsSystem;
  if (!isROOTSystemModuleDiag && !isSystemModuleDiag)
    fChild->HandleDiagnostic(DiagLevel, Info);
  ....
}
```

Начнём с самого безобидного примера\. Указатель module проверяется два раза\. Скорее всего, одна проверка лишняя\. Но код лучше исправить, чтобы не возникало лишних вопросов\.

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'strchr\(fHostAuth\-\>GetHost\(\), '\*'\)' to the left and to the right of the '\|\|' operator\. TAuthenticate\.cxx 300

```cpp
TAuthenticate::TAuthenticate(TSocket *sock, const char *remote,
                             const char *proto, const char *user)
{
  ....
  // If generic THostAuth (i.e. with wild card or user == any)
  // make a personalized memory copy of this THostAuth
  if (strchr(fHostAuth->GetHost(),'*') || strchr(fHostAuth->GetHost(),'*') ||
     fHostAuth->GetServer() == -1 ) {
    fHostAuth = new THostAuth(*fHostAuth);
    fHostAuth->SetHost(fqdn);
    fHostAuth->SetUser(checkUser);
    fHostAuth->SetServer(servtype);
  }
  ....
}
```

Тут в строке _fHostAuth\-\>GetHost\(\)_ ищется один и тот же символ \- '\*'\. Возможно, одним из них должен быть символ '?'\. Обычно их используют для задания разных масок\.

[V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 163, 165\. TProofMonSenderML\.cxx 163

```cpp
Int_t TProofMonSenderML::SendSummary(TList *recs, const char *id)
{
  ....
  if (fSummaryVrs == 0) {
    if ((dsn = recs->FindObject("dataset"))) recs->Remove(dsn);
  } else if (fSummaryVrs == 0) {
    // Only the first records
    xrecs = new TList;
    xrecs->SetOwner(kFALSE);
    TIter nxr(recs);
    TObject *o = 0;
    while ((o = nxr())) {
       if (!strcmp(o->GetName(), "vmemmxw")) break;
       xrecs->Add(o);
    }
  }
  ....
}
```

Переменная _fSummaryVrs_ дважды сравнивается с нулём\. Это приводит к тому, что код в ветви else\-if никогда не выполняется\. А кода там написано немало\.\.\.

[V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. TKDTree\.cxx 805

```cpp
template <typename  Index, typename Value>
void TKDTree<Index, Value>::UpdateRange(....)
{
  ....
  if (point[fAxis[inode]]<=fValue[inode]){
    //first examine the node that contains the point
    UpdateRange(GetLeft(inode),point, range, res);
    UpdateRange(GetRight(inode),point, range, res);
  } else {
    UpdateRange(GetLeft(inode),point, range, res);
    UpdateRange(GetRight(inode),point, range, res);
  }
  ....
}
```

Один и тот же copy\-paste\-код выполняется при любом условии\. Возможно, есть опечатка в словах _left_ и _right_\.

Подобного подозрительного кода в проекте немало:

* V523 The 'then' statement is equivalent to the 'else' statement\. TContainerConverters\.cxx 51
* V523 The 'then' statement is equivalent to the 'else' statement\. TWebFile\.cxx 1310
* V523 The 'then' statement is equivalent to the 'else' statement\. MethodMLP\.cxx 423
* V523 The 'then' statement is equivalent to the 'else' statement\. RooAbsCategory\.cxx 394

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression '\!file\_name\_value\.empty\(\)' is always false\. SelectionRules\.cxx 1423

```cpp
bool SelectionRules::AreAllSelectionRulesUsed() const {
  for(auto&& rule : fClassSelectionRules){
    ....
    std::string file_name_value;
    if (!rule.GetAttributeValue("file_name", file_name_value))
     file_name_value.clear();

    if (!file_name_value.empty()) {                  // <=
      // don't complain about defined_in rules
      continue;
    }

    const char* attrName = nullptr;
    const char* attrVal = nullptr;
    if (!file_name_value.empty()) {                  // <=
      attrName = "file name";
      attrVal = file_name_value.c_str();
    } else {
      attrName = "class";
      if (!name.empty()) attrVal = name.c_str();
    }
    ROOT::TMetaUtils::Warning(0,"Unused %s rule: %s\n", attrName, attrVal);
  }
  ....
}
```

Скорее всего, тут нет ошибки\. Анализатор обнаружил код, который можно сократить\. Т\.к\. значение _file\_name\_value\.empty\(\)_ проверяется в начале цикла, то ниже по коду эту проверку можно убрать, заметно сократив количество ненужного кода\.

[V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting the '\!file1 \|\| c <\= 0 \|\| c \=\= '\*' \|\| c \!\= '\('' expression\. The expression is excessive or contains a misprint\. TTabCom\.cxx 840

```cpp
TString TTabCom::DetermineClass(const char varName[])
{
  ....
  c = file1.get();
  if (!file1 || c <= 0 || c == '*' || c != '(') {
    Error("TTabCom::DetermineClass", "variable \"%s\" not defined?",
        varName);
    goto cleanup;
  }
  ....
}
```

Рассмотрим сокращённую часть условного выражения, на которое указал анализатор:

```cpp
if (.... || c == '*' || c != '(') {
  ....
}
```

Условие не зависит от того, равен символ "звёздочке" или нет\. Эта часть условного выражения всегда будет истинна для любого символа, отличного от '\('\. В этом легко убедиться, если построить таблицу истинности\.

Ещё два места со странной логикой в условных выражениях:

* V590 Consider inspecting this expression\. The expression is excessive or contains a misprint\. TFile\.cxx 3963
* V590 Consider inspecting this expression\. The expression is excessive or contains a misprint\. TStreamerInfoActions\.cxx 3084

[V593](https://pvs-studio.ru/ru/docs/warnings/v593/) Consider reviewing the expression of the 'A \= B < C' kind\. The expression is calculated as following: 'A \= \(B < C\)'\. TProofServ\.cxx 1903

```cpp
Int_t TProofServ::HandleSocketInput(TMessage *mess, Bool_t all)
{
  ....
  if (Int_t ret = fProof->AddWorkers(workerList) < 0) {
    Error("HandleSocketInput:kPROOF_GETSLAVEINFO",
          "adding a list of worker nodes returned: %d", ret);
  }
  ....
}
```

Ошибка, которую обнаружил анализатор, проявляет себя только при некорректной работе программы\. Переменная _ret_ должна хранить код возврата функции _AddWorkers_ и в случае нештатной ситуации выводить это значение в лог\. Но код работает не так\. В условии не хватает дополнительных скобок, задающих приоритет операций\. В переменную _ret_ сохраняется не код возврата, а результат логического сравнения, т\.е\. только 0 или 1\.

Есть ещё одно место с похожим дефектом:

* V593 Consider reviewing the expression of the 'A \= B < C' kind\. The expression is calculated as following: 'A \= \(B < C\)'\. TProofServ\.cxx 3897

[V768](https://pvs-studio.ru/ru/docs/warnings/v768/) The enumeration constant 'kCostComplexityPruning' is used as a variable of a Boolean\-type\. MethodDT\.cxx 283

```cpp
enum EPruneMethod {kExpectedErrorPruning=0, kCostComplexityPruning, kNoPruning};

void TMVA::MethodDT::ProcessOptions()
{
  ....
  if (fPruneStrength < 0) fAutomatic = kTRUE;
  else fAutomatic = kFALSE;
  if (fAutomatic && fPruneMethod==!DecisionTree::kCostComplexityPruning){
    Log() << kFATAL
          <<  "Sorry automatic pruning strength determination is ...." << Endl;
  }
  ....
}
```

Хм\.\.\. Зачем делать отрицание константного значения _kCostComplexityPruning_? Скорее всего, символ отрицания случайно добавился и теперь приводит к неправильной логике выполнения кода\.

## Некорректный код с указателями

[V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'pre' might take place\. TSynapse\.cxx 61

```cpp
void TSynapse::SetPre(TNeuron * pre)
{
  if (pre) {
    Error("SetPre","this synapse is already assigned to a pre-neuron.");
    return;
  }
  fpre = pre;
  pre->AddPost(this);
}
```

Я попытался разобраться в этом странном коде\. Вроде задумка в том, чтобы не выставлять новое значение полю _fpre_\. Тогда тут допустили ошибку, перепутав указатель, который следует проверить в условии\. В текущей реализации, если передать значение _nullptr_ в функцию _SetPre_, то произойдёт разыменование нулевого указателя\.

Скорее всего, правильно так:

```cpp
void TSynapse::SetPre(TNeuron * pre)
{
  if (fpre) {
    Error("SetPre","this synapse is already assigned to a pre-neuron.");
    return;
  }
  fpre = pre;
  pre->AddPost(this);
}
```

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

Вот ещё одно место, которое скопировано отсюда с небольшими изменениями:

* V522 Dereferencing of the null pointer 'post' might take place\. TSynapse\.cxx 74

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'N' pointer was utilized before it was verified against nullptr\. Check lines: 484, 488\. Scanner\.cxx 484

```cpp
bool RScanner::shouldVisitDecl(clang::NamedDecl *D)
{
   if (auto M = D->getOwningModule()) {                      // <= 2
      return fInterpreter.getSema().isModuleVisible(M);
   }
   return true;
}

bool RScanner::VisitNamespaceDecl(clang::NamespaceDecl* N)
{
 if (fScanType == EScanType::kOnePCM)
  return true;

 if (!shouldVisitDecl(N))                                    // <= 1
  return true;

 if((N && N->isImplicit()) || !N){                           // <= 3
    return true;
 }
 ....
}
```

Анализатор обнаружил очень опасный код\! Указатель _N_ в первом случае разыменовывается без проверки на нулевое значение\. Причём обращение к непроверенному указателю и не разглядишь\. Это происходит внутри функции _shouldVisitDecl_\.

Традиционно, это диагностическое правило выдаёт много полезных предупреждений\. Вот некоторые из них:

* V595 The 'file' pointer was utilized before it was verified against nullptr\. Check lines: 141, 153\. TFileCacheRead\.cxx 141
* V595 The 'fFree' pointer was utilized before it was verified against nullptr\. Check lines: 2029, 2038\. TFile\.cxx 2029
* V595 The 'tbuf' pointer was utilized before it was verified against nullptr\. Check lines: 586, 591\. TGText\.cxx 586
* V595 The 'fPlayer' pointer was utilized before it was verified against nullptr\. Check lines: 3425, 3430\. TProof\.cxx 3425
* V595 The 'gProofServ' pointer was utilized before it was verified against nullptr\. Check lines: 1192, 1194\. TProofPlayer\.cxx 1192
* V595 The 'projDataTmp' pointer was utilized before it was verified against nullptr\. Check lines: 791, 804\. RooSimultaneous\.cxx 791

Следующий пример не является ошибкой, но в очередной раз демонстрирует, что макросы [поощряют](https://habr.com/ru/company/pvs-studio/blog/444612/) написание неправильного или избыточного кода\.

[V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(fCanvasImp\)' condition was already verified in line 799\. TCanvas\.cxx 800

```cpp
#define SafeDelete(p) { if (p) { delete p; p = 0; } }

void TCanvas::Close(Option_t *option)
{
  ....
  if (fCanvasImp)
    SafeDelete(fCanvasImp);
  ....
}
```

Указатель _fCanvasImp_ проверяется дважды\. Одна из проверок уже реализована в макросе _SafeDelete_\. Одна из проблем с макросами в том, что к ним часто затруднена навигация из кода, поэтому многие не изучают их содержимое перед использованием\.

## Ошибки при работе с массивами

[V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'Line\[Cursor\]' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 352, 353\. Editor\.cpp 353

```cpp
size_t find_last_non_alnum(const std::string &str,
                             std::string::size_type index = std::string::npos) {
  ....
  char tmp = Line.GetText()[Cursor];
  Line[Cursor] = Line[Cursor - 1];
  Line[Cursor] = tmp;
  ....
}
```

Новое значение элемента _Line\[Cursor\]_ сразу же перезаписывается\. Что\-то здесь не так\.\.\.

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 130

```cpp
bool BasicMinimizer::SetVariableValue(unsigned int ivar, double val) {
  if (ivar > fValues.size() ) return false;
  fValues[ivar] = val;
  return true;
}
```

Так ошибаться в проверке индекса массива \- просто массовая проблема в последнее время\. Чуть ли не в каждом третьем проекте встречается\. Если при индексации массива в цикле всё просто \- почти всегда используется оператор '<' для сравнения индекса с размером массива, то при такой проверке, как здесь, надо использовать оператор '\>\=', а не '\>'\. Иначе возможен выход за границу массива на один элемент\.

Эту ошибку расплодили по файлу несколько раз:

* V557 Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 186
* V557 Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 194
* V557 Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 209
* V557 Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 215
* V557 Array overrun is possible\. The 'ivar' index is pointing beyond array bound\. BasicMinimizer\.cxx 230

[V621](https://pvs-studio.ru/ru/docs/warnings/v621/) Consider inspecting the 'for' operator\. It's possible that the loop will be executed incorrectly or won't be executed at all\. TDataMember\.cxx 554

```cpp
Int_t TDataMember::GetArrayDim() const
{
 if (fArrayDim<0 && fInfo) {
    R__LOCKGUARD(gInterpreterMutex);
    TDataMember *dm = const_cast<TDataMember*>(this);
    dm->fArrayDim = gCling->DataMemberInfo_ArrayDim(fInfo);
    // fArrayMaxIndex should be zero
    if (dm->fArrayDim) {
       dm->fArrayMaxIndex = new Int_t[fArrayDim];
       for(Int_t dim = 0; dim < fArrayDim; ++dim) {
          dm->fArrayMaxIndex[dim] = gCling->DataMemberInfo_MaxIndex(fInfo,dim);
       }
    }
 }
 return fArrayDim;
}
```

Скорее всего, в цикле for хотели сравнивать переменную _dim_ с _dm\-\>fArrayDim_, а не _fArrayDim_\. Значение используемой переменной \- отрицательное, благодаря условию в начале функции\. Такой цикл никогда не выполняется\.

[V767](https://pvs-studio.ru/ru/docs/warnings/v767/) Suspicious access to element of 'current' array by a constant index inside a loop\. TClingUtils\.cxx 3082

```cpp
llvm::StringRef ROOT::TMetaUtils::DataMemberInfo__ValidArrayIndex(....)
{
  ....
  while (current!=0) {
    // Check the token
    if (isdigit(current[0])) {
       for(i=0;i<strlen(current);i++) {
          if (!isdigit(current[0])) {
             if (errstr) *errstr = current;
             if (errnum) *errnum = NOT_INT;
             return llvm::StringRef();
          }
       }
    } else { // current token is not a digit
      ....
    }
    ....
  }
  ....
}
```

Этот фрагмент кода парсит некую строку и проверяет её корректность\. После того, как нулевой символ строки _current_ определяется как число, выполняется обход всех остальных символов, чтобы убедиться, что до конца строки все символы \- числа\. Ну как выполняется\.\.\. счётчик цикла _i_ не используется в цикле\. Надо было написать _current\[i\]_, а не _current\[0\]_ в условии\.

![0682_ROOT_ru/image4.png](https://import.viva64.com/docx/blog/0682_ROOT_ru/image4.png)

## Утечка памяти

[V773](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'optionlist' pointer\. A memory leak is possible\. TDataMember\.cxx 355

```cpp
void TDataMember::Init(bool afterReading)
{
  ....
  TList *optionlist = new TList();       //storage for options strings

  for (i=0;i<token_cnt;i++) {
     if (strstr(tokens[i],"Items")) {
        ptr1 = R__STRTOK_R(tokens[i], "()", &rest);
        if (ptr1 == 0) {
           Fatal("TDataMember","Internal error, found \"Items....",GetTitle());
           return;
        }
        ptr1 = R__STRTOK_R(nullptr, "()", &rest);
        if (ptr1 == 0) {
           Fatal("TDataMember","Internal error, found \"Items....",GetTitle());
           return;
        }
        ....
     }
     ....
  }
  ....
  // dispose of temporary option list...
  delete optionlist;
  ....
}
```

Во время выхода из функции не предусмотрено освобождение памяти по указателю _optionList_\. Нужно ли это в данном конкретном случае \- мне сложно сказать\. Но обычно такие ошибки исправляют в проектах по отчётам PVS\-Studio\. Всё зависит от того, должна ли программа пытаться продолжить работать в нештатной ситуации или нет\. Таких предупреждений в проекте много, разработчикам лучше перепроверить проект самостоятельно и посмотреть полный отчёт анализатора\.

## Снова про функцию memset

[V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'x' buffer\. The memset\_s\(\) function should be used to erase the private data\. TMD5\.cxx 366

```cpp
void TMD5::Transform(UInt_t buf[4], const UChar_t in[64])
{
  UInt_t a, b, c, d, x[16];
  ....
  // Zero out sensitive information
  memset(x, 0, sizeof(x));
}
```

Многие подумают, что когда код скомпилируется, в бинарный файл не попадёт этот комментарий, и будут правы :D\. А вот что функция _memset_ тоже будет удалена компилятором, догадываются не все\. А это произойдёт\. Если очищаемый буфер больше не будет использоваться в коде, то компилятор оптимизирует код и удалит вызов функции\. Технически он прав, но если в буфере были секретные данные, то они там и останутся\. Это классический дефект безопасности [CWE\-14](https://cwe.mitre.org/data/definitions/14.html)\.

## Разные предупреждения

[V591](https://pvs-studio.ru/ru/docs/warnings/v591/) Non\-void function should return a value\. LogLikelihoodFCN\.h 108

```cpp
LogLikelihoodFCN & operator = (const LogLikelihoodFCN & rhs) {
  SetData(rhs.DataPtr() );
  SetModelFunction(rhs.ModelFunctionPtr() );
  fNEffPoints = rhs.fNEffPoints;
  fGrad = rhs.fGrad;
  fIsExtended = rhs.fIsExtended;
  fWeight = rhs.fWeight;
  fExecutionPolicy = rhs.fExecutionPolicy;
}
```

В перегруженном операторе отсутствует возвращаемое значение\. Тоже очень распространённая проблема в последнее время\.

[V596](https://pvs-studio.ru/ru/docs/warnings/v596/) The object was created but it is not being used\. The 'throw' keyword could be missing: throw runtime\_error\(FOO\); RTensor\.hxx 363

```cpp
template <typename Value_t, typename Container_t>
inline RTensor<Value_t, Container_t> RTensor<Value_t, Container_t>::Transpose()
{
  if (fLayout == MemoryLayout::RowMajor) {
    fLayout = MemoryLayout::ColumnMajor;
  } else if (fLayout == MemoryLayout::ColumnMajor) {
    fLayout = MemoryLayout::RowMajor;
  } else {
    std::runtime_error("Memory layout is not known.");
  }
  ....
}
```

Ошибка заключается в том, что случайно забыто ключевое слово _throw_\. В результате данный код не генерирует исключение в случае ошибочной ситуации\.

Всего нашлось два таких места\. Второе:

* V596 The object was created but it is not being used\. The 'throw' keyword could be missing: throw runtime\_error\(FOO\); Forest\.hxx 137

[V609](https://pvs-studio.ru/ru/docs/warnings/v609/) Divide by zero\. Denominator range \[0\.\.100\]\. TGHtmlImage\.cxx 340

```cpp
const char *TGHtml::GetPctWidth(TGHtmlElement *p, char *opt, char *ret)
{
  int n, m, val;
  ....
  if (n < 0 || n > 100) return z;
  if (opt[0] == 'h') {
    val = fCanvas->GetHeight() * 100;
  } else {
    val = fCanvas->GetWidth() * 100;
  }
  if (!fInTd) {
    snprintf(ret, 15, "%d", val / n);  // <=
  } else {
    ....
  }
  ....
}
```

Случай, похожий на те, что были описаны ранее про массивы\. Здесь переменная _n_ ограничивается диапазоном значений от 0 до 100\. В таком случае существует ветвь кода, в которой произойдет деление на переменную n со значением 0\. Скорее всего, ограничение значения n следует переписать таким образом:

```cpp
if (n <= 0 || n > 100) return z;
```

[V646](https://pvs-studio.ru/ru/docs/warnings/v646/) Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. TProofServ\.cxx 729

```cpp
TProofServ::TProofServ(Int_t *argc, char **argv, FILE *flog)
       : TApplication("proofserv", argc, argv, 0, -1)
{
  ....
  if (!logmx.IsDigit()) {
    if (logmx.EndsWith("K")) {
      xf = 1024;
      logmx.Remove(TString::kTrailing, 'K');
    } else if (logmx.EndsWith("M")) {
      xf = 1024*1024;
      logmx.Remove(TString::kTrailing, 'M');
    } if (logmx.EndsWith("G")) {
      xf = 1024*1024*1024;
      logmx.Remove(TString::kTrailing, 'G');
    }
  }
  ....
}
```

Анализатор обнаружил странное форматирование\. В одном месте отсутствует ключевое слово _else_\. По коду можно предположить, что код действительно стоит исправить\.

И ещё пару мест исправить заодно:

* V646 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. TFormula\_v5\.cxx 3702
* V646 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. RooAbsCategory\.cxx 604

[V663](https://pvs-studio.ru/ru/docs/warnings/v663/) Infinite loop is possible\. The 'cin\.eof\(\)' condition is insufficient to break from the loop\. Consider adding the 'cin\.fail\(\)' function call to the conditional expression\. MethodKNN\.cxx 602

```cpp
void TMVA::MethodKNN::ReadWeightsFromStream(std::istream& is)
{
  ....
  while (!is.eof()) {
    std::string line;
    std::getline(is, line);

    if (line.empty() || line.find("#") != std::string::npos) {
       continue;
    }
    ....
  }
  ....
}
```

При работе с классом _std::istream_ недостаточно вызова функции _eof\(\)_ для завершения цикла\. В случае возникновения сбоя при чтении данных, вызов функции _eof\(\)_ будет всегда возвращать значение _false_, а других точек выхода из цикла в этом коде нет\. Для завершения цикла в этом случае необходима дополнительная проверка значения, возвращаемого функцией _fail\(\)_:

```cpp
while (!is.eof() && !is.fail())
{ 
....
}
```

Или просто написать:

```cpp
while (is)
{ 
....
}
```

[V678](https://pvs-studio.ru/ru/docs/warnings/v678/) An object is used as an argument to its own method\. Consider checking the first actual argument of the 'Copy' function\. TFormLeafInfo\.cxx 2414

```cpp
TFormLeafInfoMultiVarDim::TFormLeafInfoMultiVarDim(
  const TFormLeafInfoMultiVarDim& orig) : TFormLeafInfo(orig)
{
   fNsize = orig.fNsize;
   fSizes.Copy(fSizes);   // <=
   fCounter2 = orig.fCounter2?orig.fCounter2->DeepCopy():0;
   fSumOfSizes = orig.fSumOfSizes;
   fDim = orig.fDim;
   fVirtDim = orig.fVirtDim;
   fPrimaryIndex = orig.fPrimaryIndex;
   fSecondaryIndex = orig.fSecondaryIndex;
}
```

Напоследок вот такая опечаточка\. Вместо _fSizes_ надо было передать _orig\.fSizes_ в функцию _Copy_\.

## Заключение

Примерно год назад делался обзор кода проекта [NCBI Genome Workbench](https://www.ncbi.nlm.nih.gov/tools/gbench/)\. Этот проект тоже используется в научных исследованиях, но генома\. К чему я веду, программное обеспечение в этой сфере очень важно, но его качеству не уделяют должного внимания\.

Кстати, на днях вышла macOS 10\.15 Catalina, в которой отказались от поддержки 32\-х битных приложений\. И в PVS\-Studio есть большой набор правил для выявления проблем при портировании программ на 64\-х битные системы\. Подробнее об этом в [посте](https://pvs-studio.ru/ru/blog/posts/0680/) блога анализатора\.