Geekly articles weekly

Analyse des ROOT-Codes, Scientific Data Analysis Framework

Bild 3
Während Stockholm die 118. Nobelwoche abhielt, saß ich in unserem Büro, wo wir den statischen Analysator PVS-Studio entwickelten und an einer Analyseüberprüfung des ROOT-Projekts arbeiteten, einem Rahmen für die Verarbeitung großer Datenmengen, der in der wissenschaftlichen Forschung verwendet wird. Dieser Code würde natürlich keinen Preis gewinnen, aber die Autoren können sich definitiv auf eine detaillierte Überprüfung der interessantesten Mängel sowie eine kostenlose Lizenz verlassen, um das Projekt selbst gründlich zu überprüfen.

Einführung


Bild 1

ROOT ist ein modulares wissenschaftliches Software-Toolkit. Es bietet alle Funktionen, die für die Verarbeitung, statistische Analyse, Visualisierung und Speicherung von Big Data erforderlich sind. Es ist hauptsächlich in C ++ geschrieben. ROOT wurde am CERN geboren , im Zentrum der Forschung zur Hochenergiephysik. Täglich verwenden Tausende von Physikern ROOT-Anwendungen, um ihre Daten zu analysieren oder Simulationen durchzuführen.

PVS-Studio ist ein Tool zum Erkennen von Softwarefehlern und potenziellen Schwachstellen im Quellcode von Programmen, die in C, C ++, C # und Java geschrieben wurden. Es läuft unter 64-Bit-Windows, Linux und MacOS und kann Quellcode analysieren, der für 32-Bit-, 64-Bit- und eingebettete ARM-Plattformen geschrieben wurde.

Das Debüt einer neuen Diagnose


V1046 Unsichere Verwendung der Typen bool 'und' int 'zusammen in der Operation' & = '. 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; }

Zunächst einmal ist hier ein wunderbarer Fehler, der von der Beta-Version von PVS-Studio gefunden wurde, die ich für diesen Test verwendet habe.

Erwartungen Die SetFunctionList- Funktion durchläuft eine Iteratorliste. Wenn mindestens ein Iterator ungültig ist, gibt die Funktion false oder andernfalls true zurück .

Realität Die SetFunctionList- Funktion kann auch für gültige Iteratoren false zurückgeben . Lassen Sie uns herausfinden, warum. Die AddFunction- Funktion gibt die Anzahl der gültigen Iteratoren in der fFunctions- Liste zurück. Das heißt, das Hinzufügen von Nicht-Null-Iteratoren führt dazu, dass die Liste schrittweise größer wird: 1, 2, 3, 4 usw. Hier kommt der Fehler ins Spiel:

 ret &= AddFunction(*f);

Da die Funktion einen Wert vom Typ int anstelle von bool zurückgibt, gibt die Operation '& =' für gerade Werte false zurück , da das niedrigstwertige Bit einer geraden Zahl immer auf Null gesetzt wird. Auf diese Weise kann ein subtiler Fehler den Rückgabewert von SetFunctionsList auch dann aufheben , wenn seine Argumente gültig sind.

Bild 2

Fehler in bedingten Ausdrücken


V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: Modul && Modul rootcling_impl.cxx 3650

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

Beginnen wir mit dem am wenigsten schädlichen Fehler. Der Modulzeiger wird zweimal überprüft. Eine der Überprüfungen ist wahrscheinlich überflüssig, es wäre jedoch ratsam, sie zu beheben, um künftige Verwirrung zu vermeiden.

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke 'strchr (fHostAuth-> GetHost (),' * ')'. Betreiber. 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); } .... }

Die Zeichenfolge fHostAuth-> GetHost () wird zweimal nach dem Zeichen '*' durchsucht. Eine dieser Überprüfungen sollte wahrscheinlich nach dem '?' Zeichen, da diese beiden Zeichen normalerweise zum Angeben verschiedener Platzhaltermasken verwendet werden.

V517 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Überprüfen Sie die Zeilen: 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); } } .... }

Die Variable fSummaryVrs wird zweimal mit Null verglichen, sodass die Ausführung niemals den Code im Zweig else-if erreicht . Und da ist ziemlich viel Code ...

V523 Die Anweisung 'then' entspricht der Anweisung '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); } .... }

Der gleiche Codeblock, bei dem es sich um einen Copy-Paste-Klon handelt, wird unabhängig von der Bedingung ausgeführt. Ich denke, es gibt eine Verwechslung zwischen den Wörtern links und rechts .

Das Projekt ist voll von solchen verdächtigen Stellen:

  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. TContainerConverters.cxx 51
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. TWebFile.cxx 1310
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. MethodMLP.cxx 423
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. RooAbsCategory.cxx 394

V547 Ausdruck '! File_name_value.empty ()' ist immer falsch. 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); } .... }

Dies ist wahrscheinlich kein Fehler; Der Analysator hat gerade einen Code gefunden, der vereinfacht werden kann. Da der Rückgabewert von file_name_value.empty () bereits zu Beginn der Schleife überprüft wird, kann die zweite doppelte Prüfung entfernt werden, wodurch eine große Menge unnötigen Codes weggeworfen wird.

V590 Überprüfen Sie die '! File1 || c <= 0 || c == '*' || c! = '(' 'Ausdruck. Der Ausdruck ist übermäßig oder enthält einen Druckfehler. 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; } .... }

Hier ist der Problemteil des vom Analysator gemeldeten bedingten Ausdrucks:

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

Die Prüfung auf das Sternchen wirkt sich nicht auf das Ergebnis der Bedingung aus. Dieser Teil gilt immer für andere Zeichen als '('. Sie können ihn leicht selbst überprüfen, indem Sie eine Wahrheitstabelle zeichnen.

Zwei weitere Warnungen vor Zuständen mit seltsamer Logik:

  • V590 Überprüfen Sie diesen Ausdruck. Der Ausdruck ist übertrieben oder enthält einen Druckfehler. TFile.cxx 3963
  • V590 Überprüfen Sie diesen Ausdruck. Der Ausdruck ist übertrieben oder enthält einen Druckfehler. TStreamerInfoActions.cxx 3084

V593 Überprüfen Sie den Ausdruck der Art 'A = B <C'. Der Ausdruck wird wie folgt berechnet: '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); } .... }

Dieser Fehler zeigt sich nur bei fehlerhaftem Verhalten des Programms. Die Variable ret soll den Rückkehrcode der AddWorkers- Funktion speichern und diesen Wert im Fehlerfall in das Protokoll schreiben. Aber es funktioniert nicht wie beabsichtigt. Der Bedingung fehlen zusätzliche Klammern, die die gewünschte Reihenfolge der Bewertung erzwingen. Was die Variable ret tatsächlich speichert, ist nicht der Rückkehrcode, sondern das Ergebnis des logischen Vergleichs, dh entweder 0 oder 1.

Ein weiteres ähnliches Problem:

  • V593 Überprüfen Sie den Ausdruck der Art 'A = B <C'. Der Ausdruck wird wie folgt berechnet: 'A = (B <C)'. TProofServ.cxx 3897

V768 Die Aufzählungskonstante 'kCostComplexityPruning' wird als Variable eines Booleschen Typs verwendet. 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 ... Warum den konstanten Wert kCostComplexityPruning negieren? Ich vermute, dass das Negationszeichen ein Tippfehler ist, der jetzt die Ausführungslogik verzerrt.

Zeigerhandhabungsfehler


V522 Eine Dereferenzierung des Nullzeigers 'pre' kann stattfinden. 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); }

Ich habe mein Bestes getan, um diesen seltsamen Code zu verstehen, und es scheint, dass die Idee darin bestand, dem Feld fpre keinen neuen Wert zuzuweisen . In diesem Fall überprüft der Programmierer versehentlich den falschen Zeiger. Die aktuelle Implementierung führt zur Dereferenzierung eines Nullzeigers, wenn Sie den Nullptr- Wert an die SetPre- Funktion übergeben.

Ich denke, dieses Snippet sollte wie folgt repariert werden:

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

Dies würde jedoch die Übergabe eines Nullzeigers an die Funktion nicht verhindern, aber zumindest ist diese Version logisch konsistenter als die ursprüngliche.

Ein leicht modifizierter Klon dieses Codes befindet sich an einer anderen Stelle:

  • V522 Es kann zu einer Dereferenzierung des Nullzeigers 'post' kommen. TSynapse.cxx 74

V595 Der ' N' -Zeiger wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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; } .... }

Dies ist ein äußerst gefährlicher Code! Der N- Zeiger wird nicht auf Null geprüft, bevor er beim ersten Mal dereferenziert wird. Darüber hinaus können Sie dies hier nicht sehen, da die Dereferenzierung innerhalb der Funktion shouldVisitDecl stattfindet .

Diese Diagnose generiert traditionell eine Reihe relevanter Warnungen. Hier nur einige Beispiele:

  • V595 Der 'Datei'-Zeiger wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 141, 153. TFileCacheRead.cxx 141
  • V595 Der Zeiger 'fFree' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 2029, 2038. TFile.cxx 2029
  • V595 Der Zeiger 'tbuf' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 586, 591. TGText.cxx 586
  • V595 Der Zeiger 'fPlayer' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 3425, 3430. TProof.cxx 3425
  • V595 Der Zeiger 'gProofServ' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 1192, 1194. TProofPlayer.cxx 1192
  • V595 Der Zeiger 'projDataTmp' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 791, 804. RooSimultaneous.cxx 791

Der nächste ist kein Fehler, aber ein weiteres Beispiel dafür, wie Makros das Schreiben von fehlerhaftem oder redundantem Code fördern .

V571 Wiederkehrende Prüfung. Die Bedingung 'if (fCanvasImp)' wurde bereits in Zeile 799 überprüft. TCanvas.cxx 800

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

Der fCanvasImp- Zeiger wird zweimal überprüft, wobei eine der Überprüfungen bereits im SafeDelete- Makro implementiert ist. Eines der Probleme mit Makros besteht darin, dass es schwierig ist, innerhalb des Codes zu navigieren. Dies ist der Grund, warum viele Programmierer ihren Inhalt vor der Verwendung nicht untersuchen.

Fehler bei der Array-Behandlung


V519 Der Variablen 'Line [Cursor]' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 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; .... }

Dem Element Zeile [Cursor] wird ein neuer Wert zugewiesen, der dann sofort überschrieben wird. Das sieht nicht richtig aus ...

V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 130

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

Dieser Fehler beim Überprüfen von Array-Indizes ist ein aktueller Trend. Wir sehen es in fast jedem dritten Projekt. Während die Indizierung in ein Array innerhalb einer Schleife einfach ist - normalerweise verwenden Sie den Operator '<', um den Index mit der Größe des Arrays zu vergleichen - erfordern Überprüfungen wie die oben gezeigte den Operator '> =', nicht '>'. Andernfalls besteht die Gefahr, dass Sie ein Element über die Grenzen des Arrays hinaus indizieren.

Dieser Fehler wurde einige Male im gesamten Code geklont:

  • V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 186
  • V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 194
  • V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 209
  • V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 215
  • V557 Array-Überlauf ist möglich. Der 'ivar'-Index zeigt über die Array-Grenze hinaus. BasicMinimizer.cxx 230

V621 Überprüfen Sie den ' for' -Operator. Es ist möglich, dass die Schleife falsch oder gar nicht ausgeführt wird. 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; }

In der for- Schleife wollten die Entwickler anscheinend die dim- Variable mit dm-> fArrayDim und nicht mit fArrayDim vergleichen . Der Wert von fArrayDim ist negativ, was durch die Bedingung am Anfang der Funktion garantiert wird. Folglich wird diese Schleife niemals ausgeführt.

V767 Verdächtiger Zugriff auf das Element des 'aktuellen' Arrays durch einen konstanten Index innerhalb einer Schleife. 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 .... } .... } .... }

Dieser Code analysiert und überprüft eine Zeichenfolge. Wenn das erste Zeichen der aktuellen Zeichenfolge (dh bei Index 0) als Zahl erkannt wurde, durchläuft die Schleife alle übrigen Zeichen, um sicherzustellen, dass es sich bei allen um Zahlen handelt. Zumindest ist das die Idee. Das Problem ist, dass der i- Zähler in der Schleife nicht verwendet wird. Die Bedingung sollte so umgeschrieben werden, dass sie aktuell [i] und nicht aktuell [0] prüft.

Bild 4

Speicherverlust


V773 Die Funktion wurde beendet, ohne den Zeiger 'Optionsliste' loszulassen. Ein Speicherverlust ist möglich. 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; .... }

Der optionList- Zeiger wird vor der Rückkehr von der Funktion nicht freigegeben. Ich weiß nicht, ob eine solche Freigabe in diesem speziellen Fall erforderlich ist, aber wenn wir solche Fehler melden, beheben Entwickler sie normalerweise. Es hängt alles davon ab, ob Ihr Programm im Fehlerfall weiter ausgeführt werden soll oder nicht. ROOT weist eine Reihe solcher Mängel auf, daher würde ich den Autoren raten, das Projekt selbst zu überprüfen.

wieder memset


V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'x'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. 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)); }

Viele denken, dass der Kommentar nach der Kompilierung nicht in die Binärdatei gelangt, und sie sind absolut korrekt: D. Einige wissen möglicherweise nicht, dass der Compiler auch die Memset- Funktion entfernt. Und das wird sicher passieren. Wenn der betreffende Puffer im Code nicht mehr weiter verwendet wird, optimiert der Compiler den Funktionsaufruf. Technisch gesehen ist es eine vernünftige Entscheidung, aber wenn der Puffer private Daten gespeichert hat, bleiben diese Daten dort. Dies ist eine klassische Sicherheitsschwäche CWE-14 .

Verschiedenes


V591 Non-void-Funktion sollte einen Wert zurückgeben. 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; }

Der überladene Operator hat keinen Rückgabewert. Dies ist ein weiterer aktueller Trend.

V596 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: 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."); } .... }

Das Problem ist, dass der Programmierer das Schlüsselwort throw versehentlich weggelassen hat, wodurch das Auslösen einer Ausnahme im Fehlerfall verhindert wird.

Es gab nur zwei Warnungen dieses Typs. Hier ist der zweite:

  • V596 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); Forest.hxx 137

V609 Durch Null teilen. Nennerbereich [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 { .... } .... }

Dieser ähnelt den zuvor diskutierten Beispielen für die Array-Behandlung. Die Variable n ist auf den Bereich von 0 bis 100 begrenzt. Aber dann gibt es einen Zweig, der eine Division durch die Variable n durchführt , die den Wert 0 haben kann. Ich denke, die Bereichsgrenzen von n sollten wie folgt festgelegt werden:

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

V646 Überprüfen Sie die Logik der Anwendung. Möglicherweise fehlt das Schlüsselwort "else". 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'); } } .... }

Der Analysator meldet eine seltsam formatierte if- Anweisung mit dem fehlenden else- Schlüsselwort. Die Art und Weise, wie dieser Code aussieht, deutet darauf hin, dass er behoben werden muss.

Noch ein paar Warnungen dieses Typs:

  • V646 Überprüfen Sie die Logik der Anwendung. Möglicherweise fehlt das Schlüsselwort "else". TFormula_v5.cxx 3702
  • V646 Überprüfen Sie die Logik der Anwendung. Möglicherweise fehlt das Schlüsselwort "else". RooAbsCategory.cxx 604

V663 Endlosschleife ist möglich. Die Bedingung 'cin.eof ()' reicht nicht aus, um die Schleife zu verlassen. Fügen Sie dem bedingten Ausdruck möglicherweise den Funktionsaufruf 'cin.fail ()' hinzu. 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; } .... } .... }

Wenn Sie mit der Klasse std :: istream arbeiten , reicht es nicht aus, die Funktion eof () aufzurufen , um die Schleife zu beenden. Die Funktion eof () gibt immer false zurück, wenn die Daten nicht gelesen werden können und dieser Code keine anderen Endpunkte enthält. Um die Beendigung der Schleife zu gewährleisten, ist eine zusätzliche Überprüfung des von der Funktion fail () zurückgegebenen Werts erforderlich:

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

Alternativ kann es wie folgt umgeschrieben werden:

 while (is) { .... }

V678 Ein Objekt wird als Argument für seine eigene Methode verwendet. Überprüfen Sie das erste tatsächliche Argument der Funktion 'Kopieren'. 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; }

Beenden wir den Artikel mit diesem netten kleinen Tippfehler. Die Kopierfunktion sollte mit orig.fSizes aufgerufen werden , nicht mit fSizes .

Fazit


Vor ungefähr einem Jahr haben wir das NCBI Genome Workbench- Projekt überprüft, ein weiteres Programm, das in der wissenschaftlichen Forschung zur Genomanalyse eingesetzt wird. Ich erwähne dies, weil die Qualität wissenschaftlicher Software äußerst wichtig ist, Entwickler sie jedoch tendenziell unterschätzen.

MacOS 10.15 Catalina wurde übrigens neulich veröffentlicht, wo die Unterstützung von 32-Bit-Anwendungen eingestellt wurde. Glücklicherweise bietet PVS-Studio eine Vielzahl von Diagnosen, die speziell zur Erkennung von Fehlern entwickelt wurden, die mit der Portierung von Programmen auf 64-Bit-Systeme einhergehen. Weitere Informationen finden Sie in diesem Beitrag des PVS-Studio-Teams.

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

More articles:


All Articles