Geekly articles weekly

NCBI Genome Workbench: Gefährdete Forschung

Moderne Computertechnologien, technische und Softwarelösungen - all dies erleichtert und beschleunigt die Umsetzung verschiedener wissenschaftlicher Forschungen erheblich. Oft ist die Computersimulation die einzige Möglichkeit, viele Theorien zu testen. Wissenschaftliche Software hat ihre eigenen Eigenschaften. Beispielsweise wird eine solche Software häufig sehr gründlichen Tests unterzogen, ist jedoch schlecht dokumentiert. Software wird jedoch von Menschen geschrieben, und Menschen machen Fehler. Fehler in wissenschaftlichen Programmen können die gesamte Forschung in Frage stellen. In diesem Artikel werden Dutzende von Problemen aufgelistet, die im Code des NCBI Genome Workbench-Softwarepakets enthalten sind.

Einführung


Die NCBI Genome Workbench bietet Forschern eine breite Palette von Werkzeugen zum Studieren und Analysieren genetischer Daten. Benutzer können Daten aus verschiedenen Quellen recherchieren und vergleichen, einschließlich der NCBI-Datenbanken (National Center for Biotechnology Information) oder ihrer eigenen persönlichen Daten.

Wie bereits erwähnt, wird wissenschaftliche Software in der Regel durch Unit-Tests gut abgedeckt. Bei der Überprüfung dieses Projekts wurden 85 Verzeichnisse mit Testdateien von der Analyse ausgeschlossen. Das sind ungefähr tausend Dateien. Möglicherweise liegt dies an den Anforderungen zum Testen verschiedener komplexer Algorithmen, die für verschiedene Studien erfunden wurden. Aber die Qualität des restlichen Codes (nicht des Tests) ist nicht so hoch, wie wir es gerne hätten. Wie in jedem Projekt, in dem sie sich noch nicht um die Einführung statischer Code-Analyse-Tools gekümmert haben :).

Daten zur Überprüfung (oder sogar Recherche) des Codes wurden vom statischen Code-Analysator für C / C ++ / C # / Java - PVS-Studio bereitgestellt.

Nur zwei Zahlen, die Ihr Projekt ruinieren




Basierend auf unserer Fehlerdatenbank, die derzeit mehr als 12.000 ausgewählte Beispiele umfasst, bemerken und beschreiben wir spezifische Muster zum Schreiben von Code, die zu zahlreichen Fehlern führen. Zum Beispiel haben wir folgende Studien durchgeführt:

  1. Die Wirkung der letzten Zeile ;
  2. Die gefährlichste Funktion in der Welt von C / C ++ ;
  3. Logische Ausdrücke in C / C ++. Wie falsch sind die Profis ;
  4. Das Böse lebt in Vergleichsfunktionen .

Dieses Projekt war der Beginn der Beschreibung des neuen Musters. Wir sprechen über die Nummern 1 und 2 in den Namen von Variablen, zum Beispiel Datei1 und Datei2 usw. Es ist sehr leicht, zwei solche Variablen zu verwechseln. Dies ist ein Sonderfall von Tippfehlern im Code, aber ein solcher Fehler führt zu dem Wunsch, mit gleichnamigen Variablen zu arbeiten, die sich nur in den Nummern 1 und 2 am Ende des Namens unterscheiden.

Mit Blick auf die Zukunft werde ich sagen, dass alle oben aufgeführten Studien im Code dieses Projekts bestätigt wurden: D.

Betrachten Sie das erste Beispiel aus dem Genome Workbench-Projekt:

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke '(! Loc1.IsInt () &&! Loc1.IsWhole ())'. Betreiber. nw_aligner.cpp 480

CRef<CSeq_align> CNWAligner::Run(CScope &scope, const CSeq_loc &loc1, const CSeq_loc &loc2, bool trim_end_gaps) { if ((!loc1.IsInt() && !loc1.IsWhole()) || (!loc1.IsInt() && !loc1.IsWhole())) { NCBI_THROW(CException, eUnknown, "Only whole and interval locations supported"); } .... }

Wir sehen zwei Variablen mit den Namen loc1 und loc2 . Und auch ein Fehler im Code: Die Variable loc2 wird nicht verwendet, da stattdessen wieder loc1 verwendet wird.

Ein weiteres Beispiel:

V560 Ein Teil des bedingten Ausdrucks ist immer falsch: s1.IsSet (). valid_biosource.cpp 3073

 static bool s_PCRPrimerSetLess(const CPCRPrimerSet& s1, const CPCRPrimerSet& s2) { if (!s1.IsSet() && s1.IsSet()) { return true; } else if (s1.IsSet() && !s2.IsSet()) { return false; } else if (!s1.IsSet() && !s2.IsSet()) { return false; } else if (s1.Get().size() < s2.Get().size()) { return true; } else if (s1.Get().size() > s2.Get().size()) { return false; } else { ..... }

In der ersten Codezeile wurden die Variablen s1 und s2 verwechselt . Basierend auf dem Namen ist dies eine Vergleichsfunktion. Ein solcher Fehler kann jedoch überall auftreten, da der Programmierer durch die Benennung der Variablen Nummer 1 und Nummer 2 mit ziemlicher Sicherheit in Zukunft einen Fehler machen wird. Und je häufiger solche Namen in einer Funktion verwendet werden, desto höher ist die Wahrscheinlichkeit eines Fehlers.

Andere Tippfehler und Copy-Paste




V501 Links und rechts vom Operator '! =' Gibt es identische Unterausdrücke: bd.bit_.bits [i]! = Bd.bit_.bits [i] bm.h 296

 bool compare_state(const iterator_base& ib) const { .... if (this->block_type_ == 0 { if (bd.bit_.ptr != ib_db.bit_.ptr) return false; if (bd.bit_.idx != ib_db.bit_.idx) return false; if (bd.bit_.cnt != ib_db.bit_.cnt) return false; if (bd.bit_.pos != ib_db.bit_.pos) return false; for (unsigned i = 0; i < bd.bit_.cnt; ++i) { if (bd.bit_.bits[i] != bd.bit_.bits[i]) return false; } } .... }

Ich glaube, dass nach all den Überprüfungen die Größen der Bit- Arrays der Objekte bd.bit_ und ib_db.bit_ gleich sind. Daher schrieb der Autor des Codes einen Zyklus für den elementweisen Vergleich des Bit- Arrays, machte jedoch einen Tippfehler im Namen eines der verglichenen Objekte. Infolgedessen können verglichene Objekte in einigen Situationen fälschlicherweise als gleich angesehen werden.

Dieses Beispiel verdient den Artikel "Das Böse lebt in Vergleichsfunktionen ".

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke 'CFieldHandler :: QualifierNamesAreEquivalent (Feld, kFieldTypeSeqId)'. Betreiber. field_handler.cpp 152

 bool CFieldHandlerFactory::s_IsSequenceIDField(const string& field) { if ( CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId) || CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId)) { return true; } else { return false; } }

Höchstwahrscheinlich ist eine der Prüfungen überflüssig. Ich habe in den Codevariablen , die kFieldTypeSeqId ähneln, keine gefunden . Trotzdem ist aufgrund des Operators "||" ein zusätzlicher Funktionsaufruf möglich, der die Leistung beeinträchtigt.

Ein paar weitere Orte derselben Art mit einer Analysatorwarnung, die überprüft werden muss:

  • V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'uf-> GetData (). IsBool ()'. Variation_utils.cpp 1711
  • V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'uf-> GetData (). IsBool ()'. Variation_utils.cpp 1735

V766 Ein Element mit demselben Schlüssel 'kArgRemote' wurde bereits hinzugefügt. blast_args.cpp 3262

 void CBlastAppArgs::x_IssueWarningsForIgnoredOptions(const CArgs& args) { set<string> can_override; .... can_override.insert(kArgOutputFormat); can_override.insert(kArgNumDescriptions); can_override.insert(kArgNumAlignments); can_override.insert(kArgMaxTargetSequences); can_override.insert(kArgRemote); // <= can_override.insert(kArgNumThreads); can_override.insert(kArgInputSearchStrategy); can_override.insert(kArgRemote); // <= can_override.insert("remote_verbose"); can_override.insert("verbose"); .... }

Der Analysator hat festgestellt, dass dem eingestellten Behälter 2 identische Werte hinzugefügt wurden. Denken Sie daran, dass dieser Container nur eindeutige Werte speichert, sodass ihm keine Duplikate hinzugefügt werden.

Code wie der obige wird häufig mit der Copy-Paste-Methode geschrieben. Möglicherweise gibt es einfach einen zusätzlichen Wert, oder der Autor hat beim Kopieren vergessen, eine der Variablen umzubenennen. Wenn Sie einen zusätzlichen Aufruf zum Einfügen entfernen , wird der Code leicht optimiert, was jedoch nicht von Bedeutung ist. Noch wichtiger ist, dass hier möglicherweise ein schwerwiegender Fehler aufgrund eines fehlenden Elements in der Gruppe ausgeblendet wird.

V523 Die Anweisung 'then' entspricht dem nachfolgenden Codefragment. vcf_reader.cpp 1105

 bool CVcfReader::xAssignFeatureLocationSet(....) { .... if (data.m_SetType == CVcfData::ST_ALL_DEL) { if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); //-1 for 0-based, //another -1 for inclusive end-point ( ie [], not [) ) pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; } //default: For MNV's we will use the single starting point //NB: For references of size >=2, this location will not //match the reference allele. Future Variation-ref //normalization code will address these issues, //and obviate the need for this code altogether. if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; }

Die Funktion enthält große und vollständig identische Codefragmente. Sie enthalten jedoch verschiedene begleitende Kommentare. Der Code ist nicht optimal, verwirrend geschrieben und enthält möglicherweise einen Fehler.

Die gesamte Liste der verdächtigen Orte mit der if-else-Anweisung sieht folgendermaßen aus:

  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. blk.c 2142
  • V523 Die Anweisung 'then' entspricht dem nachfolgenden Codefragment. odbc.c 379
  • V523 Die Anweisung 'then' entspricht dem nachfolgenden Codefragment. odbc.c 1414
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. seqdbvol.cpp 1922
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. seqdb_demo.cpp 466
  • V523 Die Anweisung 'then' entspricht dem nachfolgenden Codefragment. blast_engine.c 1917
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. blast_filter.c 420
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. blast_parameters.c 636
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. unordered_spliter.cpp 684
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. bme.cpp 333
  • V523 Die Anweisung 'then' entspricht der Anweisung 'else'. gme.cpp 484

/ * mit Sicherheit ist am besten pedantisch * /



V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'passwd_buf' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 366

 /** * Crypt a given password using schema required for NTLMv1 authentication * @param passwd clear text domain password * @param challenge challenge data given by server * @param flags NTLM flags from server side * @param answer buffer where to store crypted password */ void tds_answer_challenge(....) { #define MAX_PW_SZ 14 .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); memset(ntlm2_challenge, 0, sizeof(ntlm2_challenge)); } else { .... } }

Wie Sie wahrscheinlich bereits vermutet haben, wurde im Abschnittstitel ein lustiger Kommentar zur Sicherheit durch Code verwendet.

Kurz gesagt, die Memset- Funktion wird vom Compiler entfernt, da geleerte Puffer nicht mehr verwendet werden. Und Daten wie Hash oder passwd_buf sind eigentlich keine Nullen. Weitere Informationen zu diesem nicht offensichtlichen Compiler-Mechanismus finden Sie im Artikel " Sicheres Löschen privater Daten ".

V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'answer' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 561

 static TDSRET tds7_send_auth(....) { .... /* for security reason clear structure */ memset(&answer, 0, sizeof(TDSANSWER)); return tds_flush_packet(tds); }

Dies war nicht das einzige Beispiel mit Kommentaren zu „Sicherheit“. Den Kommentaren nach zu urteilen, kann davon ausgegangen werden, dass Sicherheit für das Projekt wirklich wichtig ist. Daher füge ich die gesamte nicht kleine Liste der identifizierten Probleme bei:

  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das 'heap'-Objekt geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. ncbi_heapmgr.c 1300
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das 'context'-Objekt geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 167
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ks' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 339
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'md5_ctx' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 353
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'Hash'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Challenge.c 365
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ks' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 406
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ntlm_v2_response' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. login.c 795
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'answer' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. login.c 801
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'Paket'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. numeric.c 256
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'Paket'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. numeric.c 110
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'pwd'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. getpassarg.c 50
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das 'context'-Objekt geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 188
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'buf'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 243
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'ntlm_v2_hash' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 309
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'md5_ctx' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 354
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'passwd_buf' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 380
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ks' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 393
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der 'Hash'-Puffer geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 394
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'ntlm2_challenge' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 395
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ks' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 419
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ntlm_v2_response' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. Herausforderung.c 556

Verdächtige Zyklen




V534 Es ist wahrscheinlich, dass eine falsche Variable innerhalb des ' for' -Operators verglichen wird. Betrachten Sie die Überprüfung von 'i'. taxFormat.cpp 569

 void CTaxFormat::x_LoadTaxTree(void) { .... for(size_t i = 0; i < alignTaxids.size(); i++) { int tax_id = alignTaxids[i]; .... for(size_t j = 0; i < taxInfo.seqInfoList.size(); j++) { SSeqInfo* seqInfo = taxInfo.seqInfoList[j]; seqInfo->taxid = newTaxid; } .... } .... }

Ich denke, im Zustand der inneren Schleife habe ich die Variable zufällig erhalten. Stattdessen sollte die Variable j verwendet werden.

V535 Die Variable 'i' wird für diese Schleife und für die äußere Schleife verwendet. Überprüfen Sie die Zeilen: 302, 309. sls_alp.cpp 309

 alp::~alp() { .... if(d_alp_states) { for(i=0;i<=d_nalp;i++) // <= { if(i<=d_alp_states->d_dim) { if(d_alp_states->d_elem[i]) { for(i=0;i<=d_nalp;i++) // <= { .... .... }

Zwei verschachtelte identische Zyklen, in denen auch der globale Zähler zurückgesetzt wird, sehen sehr verdächtig aus. Entwickler sollten überprüfen, was hier passiert.

Array-Indizierung abnormal




V520 Der Kommaoperator ',' im Array- Indexausdruck '[- i2, - k]'. nw_spliced_aligner16.cpp 564

 void CSplicedAligner16::x_DoBackTrace ( const Uint2* backtrace_matrix, CNWAligner::SAlignInOut* data, int i_global_max, int j_global_max) { .... while(intron_length < m_IntronMinSize || (Key & donor) == 0) { Key = backtrace_matrix[--i2, --k]; ++intron_length; data->m_transcript.push_back(eTS_Intron); } .... }

Ich muss sofort sagen, dass es keinen Fehler zu geben scheint (vorerst lol). Betrachten Sie die folgende Zeile:

 Key = backtrace_matrix[--i2, --k];

Das Wort "Matrix" und die doppelte Indizierung können darauf hinweisen, dass das Array zweidimensional ist, dies ist jedoch nicht der Fall. Dies ist ein regulärer Zeiger auf ein Array von Ganzzahlen. Die V520- Diagnose wurde jedoch nicht nur angezeigt . Programmierer sind wirklich verwirrt darüber, wie zweidimensionale Arrays indiziert werden sollen.

In diesem Fall hat der Autor einfach beschlossen, in einer Codezeile zu speichern, obwohl er folgendermaßen schreiben könnte:

 --i2; Key = backtrace_matrix[--k];

V661 Ein verdächtiger Ausdruck 'A [B == C]'. Bedeutete wahrscheinlich 'A [B] == C'. ncbi_service_connector.c 180

 static EHTTP_HeaderParse s_ParseHeader(const char* header, ....) { .... 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*/; } .... }

Ein weiteres Beispiel für Code, in dem ich lange versucht habe zu verstehen, was vor sich geht: D. Die Funktion isspace () überprüft das Zeichen mit dem Index m . Wenn dieses Zeichen jedoch '$' ist, wird das Zeichen mit dem Index m + 1 an die Funktion übergeben. Darüber hinaus war der Vergleich mit '$' bereits im Voraus. Vielleicht liegt hier kein Fehler vor, aber der Code kann definitiv klarer umgeschrieben werden.

V557 Array-Überlauf ist möglich. Der 'Zeilen'-Index zeigt über die Array-Grenze hinaus. aln_reader.cpp 412

 bool CAlnReader::x_IsGap(TNumrow row, TSeqPos pos, const string& residue) { if (m_MiddleSections.size() == 0) { x_CalculateMiddleSections(); } if (row > m_MiddleSections.size()) { return false; } if (pos < m_MiddleSections[row].first) { .... } .... }

Hier liegt ein schwerwiegender Fehler vor. Die korrekte Überprüfung des Zeilenindex sollte folgendermaßen aussehen:

 if (row >= m_MiddleSections.size()) { return false; }

Andernfalls ist es möglich, auf Daten außerhalb des MiddleSections- Vektors zuzugreifen .

Viele weitere solcher Orte:

  • V557 Array-Überlauf ist möglich. Der 'i'-Index zeigt über die Array-Grenze hinaus. resource_pool.hpp 388
  • V557 Array-Überlauf ist möglich. Der 'Zeilen'-Index zeigt über die Array-Grenze hinaus. aln_reader.cpp 418
  • V557 Array-Überlauf ist möglich. Der Index 'fmt_idx' zeigt über die Array-Grenze hinaus. seq_writer.cpp 384
  • V557 Array-Überlauf ist möglich. Der Index 'fmt_idx' zeigt über die Array-Grenze hinaus. blastdb_formatter.cpp 183
  • V557 Array-Überlauf ist möglich. Der 'num'-Index zeigt über die Array-Grenze hinaus. newcleanupp.cpp 13035

Wie man Misstrauen gegenüber Funktionen verdient




V570 Die Variable 'm_onClickFunction' wird sich selbst zugewiesen. alngraphic.hpp 103

 void SetOnClickFunctionName(string onClickFunction) { m_onClickFunction = m_onClickFunction; }

Es gibt nichts zu kommentieren. Sie können nur mit der Person sympathisieren, die auf etwas geklickt, geklickt, aber nichts geändert hat.

Zwei weitere Fälle, in denen mir Variablen zugewiesen werden, führen zu einer Liste:

  • V570 Die Variable 'iter-> level' wird sich selbst zugewiesen. align_format_util.cpp 189
  • V570 Die Variable 'd_elements_values ​​[ind]' wird sich selbst zugewiesen. sls_alp_data.cpp 1416

V763 Der Parameter 'w1' wird vor seiner Verwendung immer in den Funktionskörper umgeschrieben. bmfunc.h 5363

 /// Bit COUNT functor template<typename W> struct bit_COUNT { W operator()(W w1, W w2) { w1 = 0; BM_INCWORD_BITCOUNT(w1, w2); return w1; } };

Eine Funktion, bei der das Argument unmittelbar nach Eingabe der Funktion ausgefranst ist, kann für die Entwickler, die sie verwenden, irreführend sein. Der Code sollte doppelt überprüft werden.

Klassendesignfehler




V688 Das Funktionsargument 'm_qsrc' hat denselben Namen wie eines der Klassenmitglieder, was zu Verwirrung führen kann. compart_matching.cpp 873

 class CElementaryMatching: public CObject { .... ISequenceSource * m_qsrc; .... void x_CreateIndex (ISequenceSource *m_qsrc, EIndexMode index_more, ....); void x_CreateRemapData(ISequenceSource *m_qsrc, EIndexMode mode); void x_LoadRemapData (ISequenceSource *m_qsrc, const string& sdb); .... };

Sofort enthalten 3 Klassenfunktionen Argumente, deren Namen mit dem Klassenfeld übereinstimmen. Dies kann zu Fehlern in Funktionskörpern führen: Der Programmierer glaubt möglicherweise, mit einem Mitglied der Klasse zu arbeiten, und ändert tatsächlich den Wert der lokalen Variablen.

V614 Nicht initialisierte Variable 'm_BitSet' verwendet. SnpBitAttributes.hpp 187

 /// SNP bit attribute container. class CSnpBitAttributes { public: .... private: /// Internal storage for bits. Uint8 m_BitSet; }; inline CSnpBitAttributes::CSnpBitAttributes(Uint8 bits) : m_BitSet(bits) { } inline CSnpBitAttributes::CSnpBitAttributes(const vector<char>& octet_string) { auto count = sizeof(m_BitSet); auto byte = octet_string.end(); do m_BitSet = (m_BitSet << 8) | *--byte; while (--count > 0); }

Einer der Konstruktoren arbeitet schlampig mit der Variablen m_BitSet . Tatsache ist, dass die Variable nicht initialisiert ist. Sein "Garbage" -Wert wird bei der ersten Iteration der Schleife verwendet, wonach die Initialisierung erfolgt. Dies ist ein sehr schwerwiegender Fehler, der zu undefiniertem Programmverhalten führt.

V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> SIntervalComparisonResult :: SIntervalComparisonResult (....)' verwendet werden. compare_feats.hpp 100

 //This struct keeps the result of comparison of two exons struct SIntervalComparisonResult : CObject { public: SIntervalComparisonResult(unsigned pos1, unsigned pos2, FCompareLocs result, int pos_comparison = 0) : m_exon_ordinal1(pos1), m_exon_ordinal2(pos2), m_result(result), m_position_comparison(pos_comparison) {} SIntervalComparisonResult() { SIntervalComparisonResult(0, 0, fCmp_Unknown, 0); } .... };

Vor sehr langer Zeit bin ich bei der Überprüfung von Projekten nicht auf solche Fehler gestoßen. Das Problem ist aber immer noch relevant. Der Fehler besteht darin, dass beim Aufrufen des parametrisierten Konstruktors auf diese Weise ein temporäres Objekt erstellt und entfernt wird. Und die Klassenfelder bleiben nicht initialisiert. Ein anderer Konstruktor sollte über die Initialisierungsliste aufgerufen werden (siehe Delegieren des Konstruktors ).

V591 Non-void-Funktion sollte einen Wert zurückgeben. bio_tree.hpp 266

 /// Recursive assignment CBioNode& operator=(const CBioNode& tree) { TParent::operator=(tree); TBioTree* pt = (TBioTree*)tree.GetParentTree(); SetParentTree(pt); }

Der Analysator geht davon aus, dass die Zeile in der überladenen Anweisung fehlt:

 return *this;

V670 Das nicht initialisierte Klassenmitglied 'm_OutBlobIdOrData' wird zum Initialisieren des 'm_StdOut'-Mitglieds verwendet. Denken Sie daran, dass Mitglieder in der Reihenfolge ihrer Deklarationen innerhalb einer Klasse initialisiert werden. remote_app.hpp 215

 class NCBI_XCONNECT_EXPORT CRemoteAppResult { public: CRemoteAppResult(CNetCacheAPI::TInstance netcache_api, size_t max_inline_size = kMaxBlobInlineSize) : m_NetCacheAPI(netcache_api), m_RetCode(-1), m_StdOut(netcache_api, m_OutBlobIdOrData, m_OutBlobSize), m_OutBlobSize(0), m_StdErr(netcache_api, m_ErrBlobIdOrData, m_ErrBlobSize), m_ErrBlobSize(0), m_StorageType(eBlobStorage), m_MaxInlineSize(max_inline_size) { } .... };

3 Analysatorwarnungen werden sofort an dieses Codefragment ausgegeben. Klassenfelder werden nicht in der Reihenfolge initialisiert, in der sie in der Initialisierungsliste aufgeführt sind, sondern so, wie sie in der Klasse deklariert sind. Die klassische Fehlerursache ist, dass sich nicht alle Programmierer an diese Regel erinnern oder sie kennen. Hier und in der Initialisierungsliste ist nur die falsche Reihenfolge. Man hat das Gefühl, dass die Liste der Felder in zufälliger Reihenfolge eingegeben wurde.

V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. cobalt.cpp 247

 void CMultiAligner::SetQueries(const vector< CRef<objects::CBioseq> >& queries) { .... try { seq_loc->SetId(*it->GetSeqId()); } catch (objects::CObjMgrException e) { NCBI_THROW(CMultiAlignerException, eInvalidInput, (string)"Missing seq-id in bioseq. " + e.GetMsg()); } m_tQueries.push_back(seq_loc); .... }

Das Abfangen von Ausnahmen nach Wert kann zum Verlust einiger Informationen über die Ausnahme führen, da ein neues Objekt erstellt wird. Es ist viel besser und sicherer, eine Ausnahme als Referenz zu erfassen.

Ähnliche Orte:

  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. agp_validate_reader.cpp 562
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. aln_build_app.cpp 320
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. aln_test_app.cpp 458
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. cobalt.cpp 691
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. cobalt.cpp 719
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. cobalt.cpp 728
  • V746 Objektschneiden. Eine Ausnahme sollte eher durch Referenz als durch Wert abgefangen werden. cobalt.cpp 732

Informationen zu nicht erreichbarem Code und anderen Problemen bei der Codeausführung



V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. merge_tree_core.cpp 627

 bool CMergeTree::x_FindBefores_Up_Iter(....) { .... FirstFrame->Curr = StartCurr; FirstFrame->Returned = false; FirstFrame->VisitCount = 0; FrameStack.push_back(FirstFrame); while(!FrameStack.empty()) { .... if(Rel == CEquivRange::eAfter) { Frame->Returned = false; FrameStack.pop_back(); continue; } else if(Rel == CEquivRange::eBefore) { .... continue; } else { if(Frame->VisitCount == 0) { .... continue; } else { .... continue; } } Frame->Returned = false; // <= FrameStack.pop_back(); continue; } // end stack loop FirstFrame->ChildFrames.clear(); return FirstFrame->Returned; }

Der Code der bedingten Anweisung ist so geschrieben, dass absolut alle Zweige des Codes mit der continue- Anweisung enden. Dies führte dazu, dass sich in der while-Schleife mehrere Zeilen nicht erreichbaren Codes bildeten. Diese Zeilen sehen sehr verdächtig aus. Dieses Problem trat höchstwahrscheinlich nach der Überarbeitung des Codes auf und erfordert nun eine sorgfältige Codeüberprüfung.

V519 Der Variablen 'interval_width' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 454, 456. aln_writer.cpp 456

 void CAlnWriter::AddGaps(....) { .... switch(exon_chunk->Which()) { case CSpliced_exon_chunk::e_Match: interval_width = exon_chunk->GetMatch(); case CSpliced_exon_chunk::e_Mismatch: interval_width = exon_chunk->GetMismatch(); case CSpliced_exon_chunk::e_Diag: interval_width = exon_chunk->GetDiag(); genomic_string.append(....); product_string.append(....); genomic_pos += interval_width; product_pos += interval_width/res_width; break; .... } .... }

Die Variable interval_width wird mehrmals überschrieben, weil In Fallzweigen gibt es keine break- Anweisungen. Obwohl ein Klassiker, aber ein sehr schlimmer Fehler.

Noch ein paar verdächtige Orte:

  • V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. dbapi_driver_utils.cpp 351
  • V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. net.c 780
  • V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. bcp.c 1495
  • V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. remote_blast.cpp 1470
  • V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. remote_blast.cpp 1522

V571 Wiederkehrende Prüfung. Die Bedingung 'if (m_QueryOpts-> filter_options)' wurde bereits in Zeile 703 überprüft. Blast_options_local_priv.hpp 713

 inline void CBlastOptionsLocal::SetFilterString(const char* f) { .... if (m_QueryOpts->filtering_options) // <= { SBlastFilterOptions* old_opts = m_QueryOpts->filtering_options; m_QueryOpts->filtering_options = NULL; SBlastFilterOptionsMerge(&(m_QueryOpts->filtering_options), old_opts, new_opts); old_opts = SBlastFilterOptionsFree(old_opts); new_opts = SBlastFilterOptionsFree(new_opts); } else { if (m_QueryOpts->filtering_options) // <= m_QueryOpts->filtering_options = SBlastFilterOptionsFree(m_QueryOpts->filtering_options); m_QueryOpts->filtering_options = new_opts; new_opts = NULL; } .... }

Offensichtlich muss der Zweig else neu geschrieben werden. Ich habe einige Ideen, die ich mit dem Zeiger m_QueryOpts-> filtering_options machen wollte , aber der Code ist immer noch irgendwie verwirrend. Ich appelliere an die Autoren des Kodex.

Nun, das Problem kommt nicht alleine:

  • V571 Wiederkehrende Prüfung. Die Bedingung 'if (Schlafzeit)' wurde bereits in Zeile 205 überprüft. Request_control.cpp 208
  • V571 Wiederkehrende Prüfung. Die Bedingung 'if (assignValue.empty ())' wurde bereits in Zeile 712 überprüft. Classstr.cpp 718

Datenlesefehler



V739 EOF sollte nicht mit einem Wert vom Typ 'char' verglichen werden. Der 'linestring [0]' sollte vom Typ 'int' sein. alnread.c 3509

 static EBool s_AfrpInitLineData( .... char* linestring = readfunc (pfile); .... while (linestring != NULL && linestring [0] != EOF) { s_TrimSpace (&linestring); .... } .... }

Zeichen, die Sie mit EOF vergleichen möchten, sollten nicht in Zeichenvariablen gespeichert werden. Andernfalls besteht die Gefahr, dass aus einem Zeichen mit dem Wert 0xFF (255) -1 wird und wie am Ende einer Datei (EOF) interpretiert wird. Auch (nur für den Fall) lohnt es sich, die Implementierung der Readfunc- Funktion zu überprüfen.

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. ncbicgi.cpp 1564

 typedef std::istream CNcbiIstream; void CCgiRequest::Serialize(CNcbiOstream& os) const { .... CNcbiIstream* istrm = GetInputStream(); if (istrm) { char buf[1024]; while(!istrm->eof()) { istrm->read(buf, sizeof(buf)); os.write(buf, istrm->gcount()); } } }

Der Analysator hat einen möglichen Fehler festgestellt, aufgrund dessen eine Endlosschleife auftreten kann. Wenn beim Lesen von Daten ein Fehler auftritt, gibt der Aufruf der Funktion eof () immer false zurück . Um die Schleife in diesem Fall abzuschließen, ist eine zusätzliche Überprüfung des von der Funktion fail () zurückgegebenen Werts erforderlich .

Verschiedene Fehler



V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '&&'. ncbi_connutil.c 1135

 static const char* x_ClientAddress(const char* client_host, int/*bool*/ local_host) { .... if ((client_host == c && x_IsSufficientAddress(client_host)) || !(ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)) || SOCK_ntoa(ip, addr, sizeof(addr)) != 0 || !(s = (char*) malloc(strlen(client_host) + strlen(addr) + 3))) { return client_host/*least we can do :-/*/; } .... }

Achten Sie auf den Ausdruck:

 !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)


Es wird nicht wie vom Programmierer erwartet berechnet, da der gesamte Ausdruck folgendermaßen aussieht:

 ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(...)


Die && Operatorpriorität ist höher als ?: . Aus diesem Grund wird der Code nicht wie beabsichtigt ausgeführt.

V561 Es ist wahrscheinlich besser, der Variablen 'seq' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: validator.cpp, Zeile 490. validator.cpp 492

 bool CValidator::IsSeqLocCorrectlyOrdered(const CSeq_loc& loc, CScope& scope) { CBioseq_Handle seq; try { CBioseq_Handle seq = scope.GetBioseqHandle(loc); } catch (CObjMgrException& ) { // no way to tell return true; } catch (const exception& ) { // no way to tell return true; } if (seq && seq.GetInst_Topology() == CSeq_inst::eTopology_circular) { // no way to check if topology is circular return true; } return CheckConsecutiveIntervals(loc, scope, x_IsCorrectlyOrdered); }

Da der Programmierer eine neue seq- Variable im try / catch-Abschnitt deklariert , bleibt die andere seq- Variable nicht initialisiert und wird unten im Code verwendet.

V562 Es ist seltsam, einen Wert vom Typ Bool mit einem Wert von 0 zu vergleichen: (((Status) & 0x7f) == 0)! = 0. ncbi_process.cpp 111

 bool CProcess::CExitInfo::IsExited(void) const { EXIT_INFO_CHECK; if (state != eExitInfo_Terminated) { return false; } #if defined(NCBI_OS_UNIX) return WIFEXITED(status) != 0; #elif defined(NCBI_OS_MSWIN) // The process always terminates with exit code return true; #endif }

Nichts war schlecht, aber WIFEXITED stellte sich auf diese Weise als Makroöffnung heraus:

 return (((status) & 0x7f) == 0) != 0;

Es stellt sich heraus, dass die Funktion den entgegengesetzten Wert zurückgibt.

Im Code gab es eine andere solche Funktion, die eine Warnung ausgab:

  • V562 Es ist seltsam, einen Wert vom Typ Bool mit einem Wert von 0 zu vergleichen. Ncbi_process.cpp 126

V595 Der Zeiger 'dst_len' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 309, 315. zlib.cpp 309

 bool CZipCompression::CompressBuffer( const void* src_buf, size_t src_len, void* dst_buf, size_t dst_size, /* out */ size_t* dst_len) { *dst_len = 0; // Check parameters if (!src_len && !F_ISSET(fAllowEmptyData)) { src_buf = NULL; } if (!src_buf || !dst_buf || !dst_len) { SetError(Z_STREAM_ERROR, "bad argument"); ERR_COMPRESS(48, FormatErrorMessage("CZipCompression::CompressBuffer")); return false; } .... }

Der Zeiger dst_len wird ganz am Anfang der Funktion dereferenziert, während der Code weiter auf Gleichheit mit Null überprüft wird. Im Code ist ein Fehler aufgetreten , der zu undefiniertem Verhalten führt, wenn der Zeiger dst_len nullptr ist .

V590 Überprüfen Sie den Ausdruck 'ch! =' \ 0 '&& ch ==' ''. Der Ausdruck ist übertrieben oder enthält einen Druckfehler. cleanup_utils.cpp 580

 bool Asn2gnbkCompressSpaces(string& val) { .... while (ch != '\0' && ch == ' ') { ptr++; ch = *ptr; } .... }

Die Bedingung zum Stoppen der Schleife hängt nur davon ab, ob das Zeichen ch ein Leerzeichen ist oder nicht. Der Ausdruck kann wie folgt vereinfacht werden:

 while (ch == ' ') { .... }

Fazit


Die Verwendung von Computerprogrammen in der wissenschaftlichen Forschung hilft und wird helfen, Entdeckungen zu machen. Hoffen wir, dass besonders wichtige nicht durch Tippfehler übersehen werden.

Ich lade die Entwickler des NCBI Genome Workbench-Projekts ein, sich mit uns in Verbindung zu setzen, und wir werden einen vollständigen Bericht des PVS-Studio-Analysators bereitstellen.

Ich hoffe, diese kleine Code-Recherche hilft, viele Fehler zu beheben und die Zuverlässigkeit des Projekts im Allgemeinen zu verbessern. Versuchen Sie, PVS-Studio mit dem Code Ihrer Projekte auszuführen, falls Sie dies noch nicht getan haben. Es könnte dir gefallen :).



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Svyatoslav Razmyslov. NCBI Genome Workbench: Wissenschaftliche Forschung unter Bedrohung

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

More articles:


All Articles