Geekly articles weekly

Analyse du code de ROOT, cadre d'analyse des données scientifiques

Image 3
Alors que Stockholm organisait la 118e semaine Nobel, j'étais assis dans notre bureau, où nous développons l'analyseur statique PVS-Studio, travaillant sur une revue d'analyse du projet ROOT, un cadre de traitement des mégadonnées utilisé dans la recherche scientifique. Ce code ne gagnerait pas de prix, bien sûr, mais les auteurs peuvent certainement compter sur un examen détaillé des défauts les plus intéressants et une licence gratuite pour vérifier eux-mêmes le projet.

Présentation


Image 1

ROOT est une boîte à outils logicielle scientifique modulaire. Il fournit toutes les fonctionnalités nécessaires au traitement du big data, à l'analyse statistique, à la visualisation et au stockage. Il est principalement écrit en C ++. ROOT est né au CERN , au cœur de la recherche en physique des hautes énergies. Chaque jour, des milliers de physiciens utilisent des applications ROOT pour analyser leurs données ou effectuer des simulations.

PVS-Studio est un outil pour détecter les bogues logiciels et les vulnérabilités potentielles dans le code source des programmes écrits en C, C ++, C # et Java. Il fonctionne sur Windows 64 bits, Linux et macOS et peut analyser le code source écrit pour les plates-formes ARM 32 bits, 64 bits et intégrées.

Les débuts d'un nouveau diagnostic


V1046 Utilisation non sûre des types bool 'et' int 'ensemble dans l'opération' & = '. GSLMultiRootFinder.h 175

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

Tout d'abord, voici un merveilleux bug trouvé par la version bêta de PVS-Studio, que j'utilisais pour cet examen.

Les attentes La fonction SetFunctionList traverse une liste d'itérateurs. Si au moins un itérateur n'est pas valide, la fonction renvoie false , ou true sinon.

La réalité La fonction SetFunctionList peut retourner false même pour les itérateurs valides. Voyons pourquoi. La fonction AddFunction renvoie le nombre d'itérateurs valides dans la liste fFunctions . Autrement dit, l'ajout d'itérateurs non nuls entraînera une augmentation incrémentielle de la liste: 1, 2, 3, 4, etc. C'est là que le bug entre en jeu:

 ret &= AddFunction(*f);

Étant donné que la fonction renvoie une valeur de type int plutôt que bool , l'opération '& =' renvoie false pour les valeurs paires car le bit le moins significatif d'un nombre pair est toujours défini sur zéro. C'est ainsi qu'un bug subtil peut casser la valeur de retour de SetFunctionsList même lorsque ses arguments sont valides.

Image 2

Erreurs dans les expressions conditionnelles


V501 Il existe des sous-expressions identiques à gauche et à droite de l'opérateur '&&': module && module rootcling_impl.cxx 3650

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

Commençons par le bug le moins nuisible. Le pointeur du module est vérifié deux fois. L'un des contrôles est probablement redondant, mais il serait toujours judicieux de le corriger pour éviter toute confusion à l'avenir.

V501 Il existe des sous-expressions identiques 'strchr (fHostAuth-> GetHost (),' * ')' à gauche et à droite de '||' opérateur. TAuthenticate.cxx 300

 TAuthenticate::TAuthenticate(TSocket *sock, const char *remote, const char *proto, const char *user) { .... // If generic THostAuth (ie 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); } .... }

La chaîne fHostAuth-> GetHost () est analysée deux fois pour le caractère '*'. L'une de ces vérifications était probablement destinée à rechercher le «?» car ces deux caractères sont généralement ceux utilisés pour spécifier divers masques génériques.

V517 L'utilisation du modèle 'if (A) {...} else if (A) {...}' a été détectée. Il y a une probabilité de présence d'erreur logique. Vérifiez les lignes: 163, 165. TProofMonSenderML.cxx 163

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

La variable fSummaryVrs est comparée à zéro deux fois, donc l'exécution n'atteint jamais le code dans la branche else-if . Et il y a pas mal de code là-dedans ...

V523 L' instruction 'then' est équivalente à l'instruction 'else'. TKDTree.cxx 805

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

Le même bloc de code, qui est un clone copier-coller, est exécuté quelle que soit la condition. Je suppose qu'il y a une confusion entre les mots gauche et droite .

Le projet est plein de points suspects comme ça:

  • V523 L'instruction 'then' est équivalente à l'instruction 'else'. TContainerConverters.cxx 51
  • V523 L'instruction 'then' est équivalente à l'instruction 'else'. TWebFile.cxx 1310
  • V523 L'instruction 'then' est équivalente à l'instruction 'else'. MethodMLP.cxx 423
  • V523 L'instruction 'then' est équivalente à l'instruction 'else'. RooAbsCategory.cxx 394

V547 L' expression '! File_name_value.empty ()' est toujours fausse. SelectionRules.cxx 1423

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

Ce n'est probablement pas un bug; l'analyseur vient de trouver du code qui peut être simplifié. Étant donné que la valeur de retour de file_name_value.empty () est déjà vérifiée au début de la boucle, la deuxième vérification en double peut être supprimée, jetant ainsi une bonne quantité de code inutile.

V590 Envisagez d'inspecter le '! File1 || c <= 0 || c == '*' || c! = '(' 'expression. L'expression est excessive ou contient une erreur d'impression. TTabCom.cxx 840

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

Voici la partie problématique de l'expression conditionnelle signalée par l'analyseur:

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

La vérification du caractère astérisque n'affectera pas le résultat de la condition. Cette partie sera toujours vraie pour tout personnage autre que '('. Vous pouvez facilement le vérifier par vous-même en dessinant une table de vérité.

Deux autres avertissements sur les conditions avec une logique étrange:

  • V590 Envisagez d'inspecter cette expression. L'expression est excessive ou contient une erreur d'impression. TFile.cxx 3963
  • V590 Envisagez d'inspecter cette expression. L'expression est excessive ou contient une erreur d'impression. TStreamerInfoActions.cxx 3084

V593 Envisagez de revoir l'expression du type «A = B <C». L'expression est calculée comme suit: «A = (B <C)». TProofServ.cxx 1903

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

Ce bogue ne se révèle que dans le cas d'un comportement défectueux du programme. La variable ret est censée stocker le code retour de la fonction AddWorkers et écrire cette valeur dans le journal en cas de condition d'erreur. Mais cela ne fonctionne pas comme prévu. La condition manque de parenthèses supplémentaires forçant l'ordre d'évaluation souhaité. Ce que la variable ret stocke réellement n'est pas le code retour mais le résultat de la comparaison logique, c'est-à-dire 0 ou 1.

Un autre problème similaire:

  • V593 Envisagez de revoir l'expression du type «A = B <C». L'expression est calculée comme suit: «A = (B <C)». TProofServ.cxx 3897

V768 La constante d'énumération 'kCostComplexityPruning' est utilisée comme variable de type booléen. MethodDT.cxx 283
 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; } .... }

Hm ... Pourquoi annuler la valeur constante kCostComplexityPruning ? Je soupçonne que le caractère de négation est une faute de frappe, ce qui fausse maintenant la logique d'exécution.

Erreurs de gestion du pointeur


V522 Le déréférencement du pointeur nul «pre» peut avoir lieu. TSynapse.cxx 61

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

J'ai fait de mon mieux pour comprendre cet étrange code, et il semble que l'idée était d'éviter d'attribuer une nouvelle valeur au champ fpre . Si c'est le cas, le programmeur vérifie accidentellement le mauvais pointeur. L'implémentation actuelle conduit à déréférencer un pointeur null si vous transmettez la valeur nullptr à la fonction SetPre .

Je pense que cet extrait devrait être corrigé comme suit:

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

Cependant, cela n'empêcherait pas le passage d'un pointeur nul à la fonction, mais au moins cette version est plus cohérente logiquement que la version d'origine.

Un clone légèrement modifié de ce code peut être trouvé à un autre endroit:

  • V522 Le déréférencement du pointeur nul «post» peut avoir lieu. TSynapse.cxx 74

V595 Le pointeur 'N' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 484, 488. Scanner.cxx 484

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

C'est un morceau de code extrêmement dangereux! Le pointeur N n'est pas vérifié pour null avant qu'il ne soit déréférencé la première fois. De plus, vous ne pouvez pas voir cela se produire ici car la déréférence a lieu à l'intérieur de la fonction shouldVisitDecl .

Ce diagnostic génère traditionnellement un ensemble d'avertissements pertinents. Voici quelques exemples:

  • V595 Le pointeur «fichier» a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 141, 153. TFileCacheRead.cxx 141
  • V595 Le pointeur 'fFree' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 2029, 2038. TFile.cxx 2029
  • V595 Le pointeur 'tbuf' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifier les lignes: 586, 591. TGText.cxx 586
  • V595 Le pointeur 'fPlayer' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 3425, 3430. TProof.cxx 3425
  • V595 Le pointeur 'gProofServ' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 1192, 1194. TProofPlayer.cxx 1192
  • V595 Le pointeur 'projDataTmp' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 791, 804. RooSimultaneous.cxx 791

Le suivant n'est pas un bogue, mais c'est encore un autre exemple de la façon dont les macros encouragent l' écriture de code défectueux ou redondant.

V571 Contrôle récurrent. La condition «if (fCanvasImp)» a déjà été vérifiée à la ligne 799. TCanvas.cxx 800

 #define SafeDelete(p) { if (p) { delete p; p = 0; } } void TCanvas::Close(Option_t *option) { .... if (fCanvasImp) SafeDelete(fCanvasImp); .... }

Le pointeur fCanvasImp est vérifié deux fois, l'une des vérifications étant déjà implémentée dans la macro SafeDelete . L'un des problèmes avec les macros est qu'elles sont difficiles à naviguer à partir du code, c'est pourquoi de nombreux programmeurs n'examinent pas leur contenu avant utilisation.

Erreurs de gestion des baies


V519 La variable 'Line [Cursor]' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 352, 353. Editor.cpp 353

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

L'élément Line [Cursor] se voit attribuer une nouvelle valeur, qui est alors immédiatement écrasée. Cela ne semble pas correct ...

V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 130

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

Faire cette erreur lors de la vérification des index de tableau est une tendance récente; nous le voyons dans presque tous les trois projets. Bien que l'indexation dans un tableau à l'intérieur d'une boucle soit facile - vous utilisez généralement l'opérateur '<' pour comparer l'index à la taille du tableau - les vérifications comme celle illustrée ci-dessus nécessitent l'opérateur '> =', pas '>'. Sinon, vous risquez d'indexer un élément au-delà de la limite du tableau.

Ce bug a été cloné plusieurs fois dans le code:

  • V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 186
  • V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 194
  • V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 209
  • V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 215
  • V557 Le dépassement de matrice est possible. L'index «ivar» pointe au-delà de la limite du tableau. BasicMinimizer.cxx 230

V621 Envisagez d'inspecter l'opérateur «pour». Il est possible que la boucle soit mal exécutée ou ne soit pas exécutée du tout. TDataMember.cxx 554

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

Dans la boucle for , les développeurs ont apparemment voulu comparer la variable dim avec dm-> fArrayDim plutôt que fArrayDim . La valeur de fArrayDim est négative, ce qui est garanti par la condition au début de la fonction. Par conséquent, cette boucle ne s'exécutera jamais.

V767 Accès suspect à l'élément du tableau 'courant' par un index constant à l'intérieur d'une boucle. TClingUtils.cxx 3082

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

Ce code analyse et vérifie une chaîne. Si le premier caractère de la chaîne actuelle (c'est-à-dire à l'index 0) a été reconnu comme un nombre, la boucle traversera tous les caractères restants pour s'assurer que tous sont des nombres. Eh bien, c'est du moins l'idée. Le problème est que le compteur i n'est pas utilisé dans la boucle. La condition doit être réécrite afin qu'elle vérifie le courant [i] plutôt que le courant [0] .

Image 4

Fuite de mémoire


V773 La fonction a été fermée sans relâcher le pointeur «liste d'options». Une fuite de mémoire est possible. TDataMember.cxx 355

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

Le pointeur optionList n'est pas libéré avant de revenir de la fonction. Je ne sais pas si une telle libération est nécessaire dans ce cas particulier, mais lorsque nous signalons de telles erreurs, les développeurs les corrigent généralement. Tout dépend si vous souhaitez ou non que votre programme continue de fonctionner en cas d'erreur. ROOT a un tas de défauts comme ça, donc je conseillerais aux auteurs de revérifier le projet eux-mêmes.

memset à nouveau


V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider le tampon 'x'. La fonction memset_s () doit être utilisée pour effacer les données privées. TMD5.cxx 366

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

Beaucoup pensent que le commentaire ne parviendra pas au fichier binaire après la compilation, et ils sont absolument corrects: D. Ce que certains ne savent peut-être pas, c'est que le compilateur supprimera également la fonction memset . Et cela arrivera à coup sûr. Si le tampon en question n'est plus utilisé dans le code, le compilateur optimisera l'appel de fonction. Techniquement, c'est une décision raisonnable, mais si le tampon stockait des données privées, ces données resteront là. Il s'agit d'une faiblesse de sécurité classique CWE-14 .

Divers


V591 La fonction non vide doit renvoyer une valeur. LogLikelihoodFCN.h 108

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

L'opérateur surchargé n'a pas de valeur de retour. Il s'agit d'une autre tendance récente.

V596 L'objet a été créé mais il n'est pas utilisé. Le mot clé «throw» peut être manquant: throw runtime_error (FOO); RTensor.hxx 363

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

Le problème est que le programmeur a accidentellement omis le mot clé throw , empêchant ainsi le lancement d'une exception en cas de condition d'erreur.

Il n'y avait que deux avertissements de ce type. Voici la seconde:

  • V596 L'objet a été créé mais il n'est pas utilisé. Le mot clé «throw» peut être manquant: throw runtime_error (FOO); Forest.hxx 137

V609 Divisez par zéro. Plage du dénominateur [0..100]. TGHtmlImage.cxx 340

 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 { .... } .... }

Celui-ci est similaire aux exemples de gestion de tableau discutés précédemment. La variable n est limitée à la plage de 0 à 100. Mais il y a ensuite une branche qui effectue la division par la variable n qui peut avoir la valeur 0. Je pense que les limites de la plage de n devraient être fixées comme suit:

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

V646 Envisagez d'inspecter la logique de l'application. Il est possible que le mot clé "else" soit manquant. TProofServ.cxx 729

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

L'analyseur signale une instruction if étrangement formatée avec le mot-clé else manquant. L'aspect de ce code suggère qu'il doit être corrigé.

Quelques avertissements supplémentaires de ce type:

  • V646 Envisagez d'inspecter la logique de l'application. Il est possible que le mot clé "else" soit manquant. TFormula_v5.cxx 3702
  • V646 Envisagez d'inspecter la logique de l'application. Il est possible que le mot clé "else" soit manquant. RooAbsCategory.cxx 604

La boucle infinie V663 est possible. La condition «cin.eof ()» est insuffisante pour rompre la boucle. Pensez à ajouter l'appel de fonction 'cin.fail ()' à l'expression conditionnelle. MethodKNN.cxx 602

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

Lorsque vous travaillez avec la classe std :: istream , appeler la fonction eof () ne suffit pas pour terminer la boucle. La fonction eof () retournera toujours false si les données ne peuvent pas être lues, et il n'y a pas d'autres points de terminaison dans ce code. Pour garantir la fin de la boucle, une vérification supplémentaire de la valeur retournée par la fonction fail () est requise:

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

Comme alternative, il peut être réécrit comme suit:

 while (is) { .... }

V678 Un objet est utilisé comme argument de sa propre méthode. Pensez à vérifier le premier argument réel de la fonction «Copier». TFormLeafInfo.cxx 2414

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

Terminons l'article avec cette jolie petite faute de frappe. La fonction de copie doit être appelée avec orig.fSizes , pas fSizes .

Conclusion


Il y a environ un an, nous avons vérifié le projet NCBI Genome Workbench , un autre programme utilisé dans la recherche scientifique qui traite de l'analyse du génome. Je le mentionne parce que la qualité des logiciels scientifiques est extrêmement cruciale, mais les développeurs ont tendance à les sous-estimer.

Soit dit en passant, macOS 10.15 Catalina a été publié l'autre jour, où ils ont cessé de prendre en charge les applications 32 bits. Heureusement, PVS-Studio propose un large éventail de diagnostics spécialement conçus pour détecter les bogues qui accompagnent le portage de programmes vers des systèmes 64 bits. En savoir plus dans ce post par l'équipe PVS-Studio.

Source: https://habr.com/ru/post/fr472492/

More articles:


All Articles