Notepad ++プロジェクトの再確認

PVS-Studioを使用してNotepad ++をテストしてから1年以上が経過しました。 PVS-Studioアナライザーがどのように改善され、Notepad ++で以前のエラーから何が修正されたかを見るのは興味深いです。

はじめに

そのため、2012年1月31日にリポジトリから取得したNotepad ++プロジェクトを確認しました。検証にはPVS-Studioアナライザーバージョン4.54を使用しました。



すでに述べたように、私たちは以前にこのプロジェクトをテストしました。 多くのエラーは見つかりませんでしたが、それでも何かが見つかりました。 プロジェクトの新しいバージョンでは、古いエラーの一部が修正され、一部は修正されていません。 これは変です。 どうやら、以前の記事はNotepad ++の作者に気付かれず、彼らはPVS-Studioを使用してプロジェクトを検証しなかったようです。 おそらくこのメモは、特に最近トライアルモードを変更し 、PVS-Studioを使用して検出されたすべてのエラーが表示されるため、Notepad ++の作成者を引き付けるでしょう。



バグ修正は明らかに他のテスト方法で発見されたか、ユーザーから報告されました。



たとえば、_iContMap配列の充填に関する以前の記事で説明したエラーは修正されました。 次のようなものでした。

memset(_iContMap, -1, CONT_MAP_MAX);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

修正されたオプション：

 memset(_iContMap, -1, CONT_MAP_MAX * sizeof(int));
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

しかし、この間違いは生き続けています。

 bool isPointValid() { return _isPointXValid && _isPointXValid; };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V501 '&&'演算子の左右に同じ副次式があります。_isPointXValid&& _isPointXValid Notepad ++ parameters.h 166



前の記事で説明したエラーには戻りません。 PVS-Studioアナライザーが過去に診断することを学んだ新しいことを考えてみましょう。



いつものように、これは発見されたすべての欠点ではなく、記事を書く上で私たちにとって興味深いと思われた欠点だけであることを思い出してほしい。 オープンソースプロジェクトのチェックにも最適な新しいトライアルモードを忘れないでください。

新たに見つかったエラー

エラーN1。 配列の境界を越える

 int encodings[] = { 1250, 1251, 1252, .... }; BOOL CALLBACK DefaultNewDocDlg::run_dlgProc( UINT Message, WPARAM wParam, LPARAM) { ... for (int i = 0 ; i <= sizeof(encodings)/sizeof(int) ; i++) { int cmdID = em->getIndexFromEncoding(encodings[i]); ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V557配列のオーバーランが可能です。 'i'インデックスの値は46に達する可能性があります。メモ帳++ preferencedlg.cpp 984



ループは、エンコーディング配列の要素を列挙します。 エラーは、サイクルを停止するための誤った条件です。 ループの最後の繰り返しで、配列の境界を超えてメモリにアクセスします。 エラーは、条件「<=」を「<」に置き換えることで修正できます。 正しいコードは次のとおりです。

 for (int i = 0 ; i < sizeof(encodings)/sizeof(int) ; i++)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーN2。 不適切なバッファサイズの計算

 typedef struct tagTVITEMA { ... LPSTR pszText; ... } TVITEMA, *LPTVITEMA; #define TVITEM TVITEMA HTREEITEM TreeView::addItem(...) { TVITEM tvi; ... tvi.cchTextMax = sizeof(tvi.pszText)/sizeof(tvi.pszText[0]); ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V514 sizeofポインター「sizeof（tvi.pszText）」を別の値で除算します。 論理エラーが存在する可能性があります。 メモ帳++ treeview.cpp 88



バッファサイズは、「sizeof（tvi.pszText）/ sizeof（tvi.pszText [0]）」という式を使用して計算されます。 この表現は意味がありません。 その中で、ポインターのサイズは1文字のサイズで除算されます。 プログラムのロジックに慣れていないため、コードの修正方法を言うのは困難です。

エラーN3。 文字列が空であることの誤ったチェック

 size_t Printer::doPrint(bool justDoIt) { ... TCHAR headerM[headerSize] = TEXT(""); ... if (headerM != '\0') ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V528「char」タイプへのポインターが「\ 0」値と比較されるのは奇妙です。 おそらく意味：* headerM！= '\ 0'。 メモ帳++ printer.cpp 380



ポインターはヌル値と比較されます。 ゼロは、「\ 0」という形式のリテラルで指定されます。 これは、ポインターの逆参照がここでは忘れられていることを示しています。 正しいコードは次のとおりです。

 if (*headerM != '\0')
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

同様の間違いが他の場所で行われました：



V528「char」タイプへのポインターが「\ 0」値と比較されるのは奇妙です。 おそらく意味：* headerR！= '\ 0'。 メモ帳++ printer.cpp 392

エラーN4。 条件のタイプミス

 DWORD WINAPI Notepad_plus::threadTextPlayer(void *params) { ... const char *text2display = ...; ... if (text2display[i] == ' ' && text2display[i] == '.') ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V547式は常に偽です。 おそらく '||' ここで演算子を使用する必要があります。 メモ帳++ notepad_plus.cpp 4967



条件（text2display [i] == '' && text2display [i] == '。'）は決して満たされない。 記号をスペースとピリオドの両方にすることはできません。 どうやらここでは簡単なタイプミスを扱っており、コードは次のようになっているはずです。

 if (text2display[i] == ' ' || text2display[i] == '.')
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

同様の間違いが他の場所で行われました：



V547式は常に偽です。 おそらく '||' ここで演算子を使用する必要があります。 メモ帳++ notepad_plus.cpp 5032

エラーN5。 条件のタイプミス

 int Notepad_plus::getHtmlXmlEncoding(....) const { ... if (langT != L_XML && langT != L_HTML && langT == L_PHP) return -1; ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V590この表現を調べることを検討してください。 表現が過剰であるか、誤植が含まれています。 メモ帳++ notepad_plus.cpp 853



コードで利用可能な検証を簡素化できます。 次に、コードは次のようになります。

 if (langT == L_PHP)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これは明らかにプログラマが望んでいたものではありません。 どうやらここで再びタイプミスを扱っています。 正しいコードは次のとおりです。

 if (langT != L_XML && langT != L_HTML && langT != L_PHP)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーN6。 間違ったビット処理

 TCHAR GetASCII(WPARAM wParam, LPARAM lParam) { ... result=ToAscii(wParam,(lParam >> 16) && 0xff, keys,&dwReturnedValue,0); ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V560条件式の一部は常に真です：0xff。 メモ帳++ babygrid.cpp 694



コードでは、変数「lParam」から3番目のバイトを抽出します。 タイプミスのため、コードはこれをまったく行いません。 エラーは、「＆」の代わりに演算子「&&」が使用されることです。 正しいコードは次のとおりです。

 result=ToAscii(wParam,(lParam >> 16) & 0xff, keys,&dwReturnedValue,0);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーN7。 不完全なコード

 #define SCE_T3_BRACE 20 static inline bool IsAnOperator(const int style) { return style == SCE_T3_OPERATOR || SCE_T3_BRACE; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V560条件式の一部は常に真です：20。lextads3.cxx 700



IsAnOperator（）関数は常に 'true'を返します。 正しいコードは次のとおりです。

 return style == SCE_T3_OPERATOR || style == SCE_T3_BRACE;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーN8。 決して実行されないコード

 static void ColouriseVHDLDoc(....) { ... } else if (sc.Match('-', '-')) { sc.SetState(SCE_VHDL_COMMENT); sc.Forward(); } else if (sc.Match('-', '-')) { if (sc.Match("--!")) sc.SetState(SCE_VHDL_COMMENTLINEBANG); else sc.SetState(SCE_VHDL_COMMENT); } ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V517「if（A）{...} else if（A）{...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 チェック行：130、133。lexvhdl.cxx 130



最初の条件（sc.Match（ '-'、 '-'））がtrueの場合、2番目のチェックは実行されません。 これにより、文字シーケンス「-！」が発生します。 正しく処理されることはありません。 どうやら、ここではコードの一部は不要であり、次のように記述する必要があります。

 static void ColouriseVHDLDoc(....) { ... } else if (sc.Match('-', '-')) { if (sc.Match("--!")) sc.SetState(SCE_VHDL_COMMENTLINEBANG); else sc.SetState(SCE_VHDL_COMMENT); } ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーN9。 追加コード

問題を引き起こさないが冗長なコードスニペットがあります。 このタイプの2つの例を示します。

 void Gripper::doTabReordering(POINT pt) { ... else if (_hTab == hTabOld) { /* delete item on switch between tabs */ ::SendMessage(_hTab, TCM_DELETEITEM, iItemOld, 0); } else { if (_hTab == hTabOld) { /* delete item on switch between tabs */ ::SendMessage(_hTab, TCM_DELETEITEM, iItemOld, 0); } } ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーの診断メッセージ：



V571定期的なチェック。 「if（_hTab == hTabOld）」条件は、478行目で既に検証されています。メモ帳++ gripper.cpp 485



コードの2番目の部分は意味をなさないため、削除できます。



また、追加のチェックがある別の例を次に示します。 ポインター「mainVerStr」および「auxVerStr」は常にゼロ以外です。これらはスタック上に作成された配列であるためです。

 LRESULT Notepad_plus::process(....) { ... TCHAR mainVerStr[16]; TCHAR auxVerStr[16]; ... if (mainVerStr) mainVer = generic_atoi(mainVerStr); if (auxVerStr) auxVer = generic_atoi(auxVerStr); ... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここで簡単に書くことができます：

 mainVer = generic_atoi(mainVerStr); auxVer = generic_atoi(auxVerStr);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

上記のチェックは、Notepad ++プロジェクトで複数回検出されます。



V600状態の検査を検討してください。 「mainVerStr」ポインターは常にNULLと等しくありません。 メモ帳++ nppbigswitch.cpp 938



V600状態の検査を検討してください。 「auxVerStr」ポインターは常にNULLと等しくありません。 メモ帳++ nppbigswitch.cpp 940



V600状態の検査を検討してください。 'intStr'ポインターは常にNULLと等しくありません。 メモ帳++ preferencedlg.cpp 1871



V600状態の検査を検討してください。 'intStr'ポインターは常にNULLと等しくありません。 メモ帳++ userdefinedialog.cpp 222



V600状態の検査を検討してください。 'intStr'ポインターは常にNULLと等しくありません。 メモ帳++ wordstyledlg.cpp 539

結論

静的コードアナライザーは、時々使用できるツールではありません。 それらを定期的に使用すると、エラーをすばやく見つけることができるため、エラーを10倍なくすことができます。



配列がクリーニングされていないために、プログラムの奇妙な動作を探すのに長い時間がかかるのはなぜですか？ コード「memset（_iContMap、-1、CONT_MAP_MAX）」は、静的アナライザーによって簡単かつ迅速に検出されます。



それでもこのエラーがPVS-Studioスタティックアナライザーによって検出された場合、ツールはとにかく誤って使用されていました。 第一に、他の診断メッセージは慎重に研究されていません。 次に、静的分析を定期的に使用する必要があります。 これにより、作成したコードのエラーをすばやく修正できます。 さらに、PVS-Studioには新しい診断ルールが常に表示されています。



繰り返しますが、静的解析は通常の使用のためのツールであるという考えを強調したいと思います。 これは、コンパイラが生成する警告のリストの拡張のようなものです。 無効になっている警告を処理し、気分があるときにたまにのみ有効にしますか？ いや 同様に、静的分析ツールによって発行される診断警告に適用する必要があります。



どうやってやるの？ サーバーで夜間に分析を実行できます。 増分分析を使用して、新しく変更されたファイルを確認できます。 分析がゆっくりと実行されており、多くのリソースを消費していると思われる場合は、作業の速度を上げるためのヒントを理解することをお勧めします。