cppcheckの後、キノコします

Big Calculatorについての激しい議論の後、研究に関連するプロジェクトから何か他のものをチェックしたかったのです。 最初に発見されたのは、タンパク質質量分析に関連するオープンプロジェクトであるOpenMSです。 このプロジェクトは、真剣なアプローチで書かれていることが判明しました。 開発では、少なくともCppcheckが使用されます。 したがって、センセーショナルなものは何も待つ必要がありませんでした。 ただし、Ppp-StudioがCppcheckの後に検出できるエラーには関心がありました。 興味のある方は、この記事を読み続けてください。







OpenMSプロジェクトがあります。 なぜそれが必要なのか、私は自分の言葉で言い直そうとは思わない。 まだ愚かさを吹き飛ばした。 ウィキペディアのウェブサイトから説明の断片を引用するだけです：



OpenMSは、 タンパク質質量分析のデータ分析および処理のためのオープンソースプロジェクトであり、2条項のBSDライセンスの下でリリースされています。 OpenMSには、プロテオミクスで使用される多くの一般的なデータ分析パイプライン用のツールがあり、信号処理、特徴発見（非アイソトープ化を含む）、1D（スペクトルまたはクロマトグラムレベル）、2Dおよび3Dでの視覚化、マップマッピング、ペプチド同定のアルゴリズムを提供します。 ラベルフリーおよび同位体ラベルベースの定量化（iTRAQ、TMT、SILACなど）をサポートしています。 さらに、メタボロミクスワークフローとDIA / SWATHターゲット分析もサポートしています。



出典：ウィキペディア。 Openms



このプロジェクトは中規模ですが、非常に複雑です。 ソースコードのサイズは20メガバイト以上です。 さらに、多数のサードパーティライブラリ（Boost、Qt、Zlibなど）。 このプロジェクトでは、テンプレートを非常に積極的に使用しています。 プロジェクトのソースコードはSourceForgeからダウンロードできます 。



OpenMSプロジェクトの開発時には、静的コード分析が使用されます。 「cppcheck.cmake」の存在と次の精神でのコメント：

if (i != peptide.size()) // added for cppcheck
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

少なくともCppcheckが使用されることを示します。 Cpplintも言及されており、「cpplint.py」というファイルがあります。 プロのアプローチ。 よくできました。



ここで、 PVS-Studioアナライザーを使用して見つけることができるものを見てみましょう。



ご注意 何らかの理由で、C ++ファイルには拡張子* .cが付いています。 したがって、「* .c」ファイルにあるC ++コードの例を参照しても、混乱しないでください。

1. OpenMPの欠点

OpenMPテクノロジーを使用するプロジェクトに出会うことはめったにありません。 一般的に、アナライザーからこれらの診断を削除するために時々考えが訪れます。 そのため、OpenMPに関連する警告を見たとき、本当に驚きました。 過去1年間、多数のプロジェクトをチェックしてきましたが、そのトピックに関する警告は一度も見たことがありません。 うわー、誰かがまだこの技術を使用しています。



発行された警告の中には、誤検知がありましたが、このケースにも警告がありました。

 DoubleReal ILPDCWrapper::compute(....) const { .... DoubleReal score = 0; .... #pragma omp parallel for schedule(dynamic, 1) for (SignedSize i = 0; i < (SignedSize)bins.size(); ++i) { score += computeSlice_(fm, pairs, bins[i].first, bins[i].second, verbose_level); } return score; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio：V1205データの競合リスク。 'score'変数を使用した保護されていない同時操作。 ilpdcwrapper.c 213



金額は間違っていると見なされます。 変数 'score'は、異なるスレッドでの同時使用から保護されていません。



他のコメントはそれほど重要ではありませんが、まだ注意する価値があります。 並列セクション内では、すべての例外をキャッチする必要があります。 並列セクションの制限を超えると、プログラムが異常終了する可能性があります。 このトピックの詳細については、「 OpenMPと例外 」、「 並列セクション内の例外の処理 」を参照してください。



throwステートメントを使用して、例外を明示的にスローできます。 new演算子（std :: bad_alloc）を呼び出すときにも発生する可能性があります。



最初のオプション。 getTheoreticalmaxPosition（）関数は例外をスローする場合があります。

 Size getTheoreticalmaxPosition() const { if (!this->size()) { throw Exception::Precondition(__FILE__, __LINE__, __PRETTY_FUNCTION__, "There must be at least one trace to ......"); } .... } virtual void run() { .... #pragma omp parallel for for (SignedSize i = 0; i < (SignedSize)seeds.size(); ++i) { .... f.setMZ( traces[traces.getTheoreticalmaxPosition()].getAvgMZ()); .... } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V1301 'throw'キーワードは、並列セクションのtry..catchブロックの外部では使用できません。 featurefinderalgorithmpickedhelperstructs.h 199



2番目のオプション。 「new」演算子を呼び出すと、例外が発生する場合があります。

 TraceFitter<PeakType>* chooseTraceFitter_(double& tau) { // choose fitter if (param_.getValue("feature:rt_shape") == "asymmetric") { LOG_DEBUG << "use asymmetric rt peak shape" << std::endl; tau = -1.0; return new EGHTraceFitter<PeakType>(); } .... } virtual void run() { .... #pragma omp parallel for for (SignedSize i = 0; i < (SignedSize)seeds.size(); ++i) { .... TraceFitter<PeakType>* fitter = chooseTraceFitter_(egh_tau); .... } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V1302 'new'演算子は、並列セクションのtry..catchブロックの外部では使用できません。 featurefinderalgorithmpicked.h 1926



他の同様の警告：

• V1301 featurefinderalgorithmpicked.h 1261
• V1301 mzmlfile.h 114
• V1301 rawmssignalsimulation.c 598
• V1301 rawmssignalsimulation.c 1152
• V1301 chromatogramextractor.h 103
• V1301 chromatogramextractor.h 118
• V1302 featurefinderalgorithmpicked.h 1931
• V1302 rawmssignalsimulation.c 592
• V1302 rawmssignalsimulation.c 601
• V1302 openswathanalyzer.c 246

2.タイプミス

 std::vector< std::pair<std::string, long> > spectra_offsets; std::vector< std::pair<std::string, long> > chromatograms_offsets; template <typename MapType> void MzMLHandler<MapType>::writeFooter_(std::ostream& os) { .... int indexlists; if (spectra_offsets.empty() && spectra_offsets.empty() ) { indexlists = 0; } else if (!spectra_offsets.empty() && !spectra_offsets.empty() ) { indexlists = 2; } else { indexlists = 1; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioの警告：



V501「&&」演算子の左側と右側には、同一のサブ式「spectra_offsets.empty（）」があります。 mzmlhandler.h 5288



V501「&&」演算子の左右には、同一の副次式「！Spectra_offsets.empty（）」があります。 mzmlhandler.h 5292



非常に奇妙なチェック。 コンテナ「spectra_offsets」は2回チェックされます。 ほとんどの場合、これはタイプミスであり、2つの異なるコンテナ（spectrum_offsetsおよびchromatograms_offsets）をチェックする必要があります。

 template <typename MapType> void MzMLHandler<MapType>::characters( const XMLCh* const chars, const XMLSize_t) { .... if (optionalAttributeAsString_(data_processing_ref, attributes, s_data_processing_ref)) { data_.back().meta.setDataProcessing( processing_[data_processing_ref]); } else { data_.back().meta.setDataProcessing( processing_[data_processing_ref]); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V523「then」ステートメントは「else」ステートメントと同等です。 mzmlhandler.h 534



同様のコードフラグメントを見ると、以下を使用する必要があると結論付けることができます。

• processing_ [data_processing_ref]
• processing_ [default_processing_]

多くのタイプミスは、例外の生成に関連していることが判明しました。 間違いはありふれています。 「スロー」キーワードは忘れられています。 一時オブジェクトは、ただちに作成および破棄されるだけです。 そのようなエラーの例：

 inline UInt asUInt_(const String & in) { UInt res = 0; try { Int tmp = in.toInt(); if (tmp < 0) { Exception::ConversionError( __FILE__, __LINE__, __PRETTY_FUNCTION__, ""); } res = UInt(tmp); } catch (Exception::ConversionError) { error(LOAD, String("UInt conversion error of \"") + in + "\""); } return res; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V596オブジェクトは作成されましたが、使用されていません。 「throw」キーワードが欠落している可能性があります：throw ConversionError（FOO）; xmlhandler.h 247



ここに同じタイプミス：

• lusionexclusionlist.c 281
• lusionexclusionlist.c 285
• precursionselectionpreprocessing.c 257
• modificationdb.c 419
• modificationdb.c 442
• svmtheoreticalspectrumgeneratorset.c 103
• logconfighandler.c 285
• logconfighandler.c 315
• suffixarraytrypticcompressed.c 488
• tooldescription.c 147
• tofcalibration.c 147

そして、私が気づいた最後のタイプミス：

 inline typename Value<Pipe>::Type const & operator*() { tmp.i1 = *in.in1; tmp.i2 = *in.in2; tmp.i3 = *in.in2; return tmp; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio：V525同様のブロックのコレクションを含むコード。 112行目、113行目、114行目の項目「in1」、「in2」、「in2」を確認します。pipe_joiner.h 112



それは書かれるべきです：

 tmp.i1 = *in.in1; tmp.i2 = *in.in2; tmp.i3 = *in.in3;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

3.奇妙な状態

 CompressedInputSource::CompressedInputSource( const String & file_path, const char * header, MemoryManager * const manager) : xercesc::InputSource(manager) { if (sizeof(header) / sizeof(char) > 1) { head_[0] = header[0]; head_[1] = header[1]; } else { head_[0] = '\0'; head_[1] = '\0'; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V514ポインターのサイズ 'sizeof（header）'を別の値で除算します。 論理エラーが存在する可能性があります。 compressedinputsource.c 52



ポインターのサイズをバイトのサイズで割ると、常に1より大きい値が得られます。 少なくとも、私はそれがどこにない派手なアーキテクチャを知りません。 だからここにある種の間違いがあります。



同様の奇妙なチェックは、compressedinputsource.c 104から入手できます。

4.ローカルオブジェクトへのリンクを返す

 template <typename TStringSet, typename TSpec> inline Iter<TStringSet, ConcatVirtual<TSpec> > const & operator++(Iter<TStringSet, ConcatVirtual<TSpec> > & me, int) { Iter<TStringSet, ConcatVirtual<TSpec> > before = me; goNext(me); return before; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V558関数は、一時ローカルオブジェクトへの参照を返します：before。 iter_concat_virtual.h 277



この関数は、一時変数「before」への参照を返します。 関数を終了すると、この変数は破棄されます。 破壊されたオブジェクトへの参照を使用すると、予測できない結果につながります。



新しいイテレータを作成する必要はないようですが、リンクを保存する必要があります：

 Iter<TStringSet, ConcatVirtual<TSpec> > &before = me;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

[PS間違っています。 コメントを参照してください。]



演算子 '-'にも同様の問題があります：iter_concat_virtual.h 310

5.ずさんなコンピューティング

 typedef size_t Size; typedef double DoubleReal; void updateMeanEstimate(const DoubleReal & x_t, DoubleReal & mean_t, Size t) { DoubleReal tmp(mean_t); tmp = mean_t + (1 / (t + 1)) * (x_t - mean_t); mean_t = tmp; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V636「1 /（t + 1）」式は、暗黙的に「int」型から「double」型にキャストされました。 分数部分の損失を避けるために、明示的な型キャストの使用を検討してください。 例：double A =（double）（X）/ Y;。 masstracedetection.c 129



式「（1 /（t + 1））」は常にゼロまたは1です。 これは、この式が整数であるためです。 おそらく、まったく別の結果が得られると考えられていたのかもしれません。 プログラムのロジックはわかりませんが、次のことを念頭に置いていたようです。

 tmp = mean_t + (1.0 / (t + 1)) * (x_t - mean_t);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

また、M_PI定数の代わりに、明示的な値が使用されており、あまり正確ではないことも気に入らなかった。 これは間違いではありませんが、あまり良くありません。 例：

 bool PosteriorErrorProbabilityModel::fit( std::vector<double> & search_engine_scores) { .... incorrectly_assigned_fit_param_.A = 1 / sqrt(2 * 3.14159 * pow(incorrectly_assigned_fit_param_.sigma, 2)); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio：V624定数3.14159が使用されています。 結果の値は不正確になる可能性があります。 <math.h>のM_PI定数の使用を検討してください。 posteriorerrorprobabilitymodel.c 92



同様に：

• posteriorerrorprobabilitymodel.c 101
• posteriorerrorprobabilitymodel.c 110
• posteriorerrorprobabilitymodel.c 155
• posteriorerrorprobabilitymodel.c 162

6.アレイを海外に行く

 static const Int CHANNELS_FOURPLEX[4][1]; static const Int CHANNELS_EIGHTPLEX[8][1]; ExitCodes main_(int, const char **) { .... if (itraq_type == ItraqQuantifier::FOURPLEX) { for (Size i = 0; i < 4; ++i) { std::vector<std::pair<String, DoubleReal> > one_label; one_label.push_back(std::make_pair<String, DoubleReal>( String("Channel ") + String(ItraqConstants::CHANNELS_FOURPLEX[i][0]), DoubleReal(ItraqConstants::CHANNELS_FOURPLEX[i][0]))); labels.push_back(one_label); } } else //ItraqQuantifier::EIGHTPLEX { for (Size i = 0; i < 8; ++i) { std::vector<std::pair<String, DoubleReal> > one_label; one_label.push_back(std::make_pair<String, DoubleReal>( String("Channel ") + String(ItraqConstants::CHANNELS_FOURPLEX[i][0]), DoubleReal(ItraqConstants::CHANNELS_FOURPLEX[i][0]))); labels.push_back(one_label); } } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V557アレイがオーバーランする可能性があります。 「i」インデックスの値は7に達する可能性があります。itraqanalyzer.c 232



実際、このエラーはコピーペーストエラーに起因する可能性があります。 しかし、「配列の境界を越えた出口」があるとします。 それはもっと怖いですね。 とにかく、この区分は非常にarbitrary意的です。 同じエラーをさまざまな方法で分類できます。



ここで、「else」ブランチでは、「CHANNELS_EIGHTPLEX」の配列を操作する必要がありました。 これはコメントによって証明されます：

 else //ItraqQuantifier::EIGHTPLEX
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ただし、コピーしたコードフラグメントは完全には変更されていません。 その結果、より小さな配列CHANNELS_FOURPLEXがそこで使用されます。



ここで同様のエラー（別のコピーペースト）：tmtanalyzer.c 225



別の例を考えてみましょう。

 DoubleReal masse_[255]; ///< mass table EdwardsLippertIterator::EdwardsLippertIterator(const EdwardsLippertIterator & source) : PepIterator(source), f_file_(source.f_file_), actual_pep_(source.actual_pep_), spec_(source.spec_), tol_(source.tol_), is_at_end_(source.is_at_end_), f_iterator_(source.f_iterator_), f_entry_(source.f_entry_), b_(source.b_), e_(source.e_), m_(source.m_), massMax_(source.massMax_) { for (Size i = 0; i < 256; i++) { masse_[i] = source.masse_[i]; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V557アレイがオーバーランする可能性があります。 「i」インデックスの値は255に達する可能性があります。edwardslippertiterator.c 134



コピーコンストラクターは、masse_配列では正しく機能しません。 255個の要素で構成されます。 そして、256個の要素がコピーされます。



正しいサイクル：

 for (Size i = 0; i < 255; i++) { masse_[i] = source.masse_[i]; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

さらに良いのは、魔法の定数をまったく使用しないことです。

7.非推奨のオペレーター呼び出し「new」

 svm_problem * LibSVMEncoder::encodeLibSVMProblem(....) { .... node_vectors = new svm_node *[problem->l]; if (node_vectors == NULL) { delete[] problem->y; delete problem; return NULL; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioの警告：V668メモリは「新しい」演算子を使用して割り当てられたため、「node_vectors」ポインターをnullに対してテストする意味はありません。 メモリ割り当てエラーの場合、例外が生成されます。 libsvmencoder.c 177



「if（node_vectors == NULL）」をチェックしても意味がありません。 メモリを割り当てることができない場合、例外がスローされます。 その結果、プログラムはプログラマが計画したとおりに動作しません。 たとえば、メモリリークが発生します。



同様の廃止されたチェック：

• file_page.h 728
• libsvmencoder.c 160

おわりに

Cppcheck、Cpplintだけでなく、PVS-Studioを使用すると便利だと思います。 特に定期的に行う場合。 プロジェクトの開発者にsupport@viva64.com宛てにご連絡ください 。 しばらくの間、より詳細なOpenMSテスト用のキーを割り当てます。