コードを書く段階でエラーの可能性を減らす方法。 注N4

PVS-StudioとFirefox

これは、エラーパターンとその対処方法に関する有用な所見を共有したい4番目の記事です。 今回は、プログラムでのまれな緊急事態の処理などのトピックに触れます。 多くのプログラムを考慮すると、C / C ++プログラムのエラー処理コードは最も信頼性の低い場所の1つであるという結論に達しました。

そのような欠陥の原因は何ですか? プログラムは、「ファイルXが見つかりませんでした」というメッセージを表示する代わりにクラッシュし、ユーザーに何が間違っているのかを推測させます。 データベースを操作するプログラムは、フィールドの1つが誤って入力されていることを報告する代わりに、理解できないメッセージを表示します。 ユーザーを悩ますこの種のエラーと戦おう。





はじめに



最初は、以前のメモに詳しくない読者向けの情報です。 ここにあります:



いつものように、私は抽象的ではありませんが、例から始めます。 今回は、 Firefoxのソースコードから例を取り上げます。 エラー処理コードを備えた高品質で有名なアプリケーションであっても、すべてが最良の方法ではないことを実証しようとします。 PVS-Studio 4.50アナライザーを使用して、欠陥が見つかりました。



エラー例



例N1。 不完全なテーブル整合性チェック

  int AffixMgr :: parse_convtable(...、const char *キーワード)
 {
   ...
   if(strncmp(piece、keyword、sizeof(keyword))!= 0){
       HUNSPELL_WARNING(stderr、
                        「エラー:行%d:テーブルが破損しています\ n」、
                        af-> getlinenum());
      削除* rl;
       * rl = NULL;
       1を返します。
   }
   ...
 } 


PVS-Studio診断:V579 strncmp関数は、ポインターとそのサイズを引数として受け取ります。 間違いかもしれません。 3番目の引数を調べます。 affixmgr.cpp 3708



ここでは、テーブルの整合性を確認しようとしました。 残念ながら、このチェックは機能する場合と機能しない場合があります。 キーワードの長さを計算するには、sizeof()演算子を使用しますが、これは当然正しくありません。 その結果、コードのパフォーマンスは、状況の幸運な組み合わせ(キーワードの長さ、現在のデータモデルの「キーワード」ポインターのサイズ)に依存します。



例2.ファイルの読み取り中の操作不能チェック

  int PatchFile :: LoadSourceFile(FILE * ofile)
 {
   ...
   size_t c = fread(rb、1、r、ofile);
   if(c <0){
     LOG(( "LoadSourceFile:"
          「宛先ファイルの読み取りエラー:」LOG_S「\ n」、
          mFile));;
     return READ_ERROR;
   }
   ...
 } 


PVS-Studio診断:V547式 'c <0'は常にfalseです。 符号なしの型の値が<0になることはありませんupdater.cpp 1179



これは、エラー処理コードが「何」の原則に基づいて記述されている場合の例です。 プログラマーは、彼が実際に書いたもの、そしてそれがどのように機能するかについてさえ考えませんでした。 検証が正しくありません。 fread()関数は、符号なしの型を使用して読み取られたバイト数を返します。 関数プロトタイプ:

  size_t fread( 
    void *バッファ、
    size_tサイズ、
    size_tカウント、
    FILE *ストリーム 
 ); 


当然、結果を格納するために、size_t型の変数「c」が使用されます。 その結果、チェック結果(c <0)は常にfalseです。



これは良い例です。 一見すると何らかの検証があるように見えますが、実際には完全に役に立たないことがわかります。



同様のエラーは他の場所でも見られます:



V547式 'c <0'は常にfalseです。 符号なしの型の値が<0になることはありませんupdater.cpp 2373



V547式 'c <0'は常にfalseです。 符号なしの型の値が<0になることはありませんbspatch.cpp 107



例3.ポインターを使用した後にNULLを確認する

  nsresult
 nsFrameSelection :: MoveCaret(...)
 {
   ...
   mShell-> FlushPendingNotifications(Flush_Layout);
   if(!mShell){
     NS_OKを返します。
   }
   ...
 } 


PVS-Studioの診断:V595 nullptrに対して検証される前に、「mShell」ポインターが使用されました。 行を確認してください:1107、1109。nsselection.cpp 1107



ポインターがゼロの場合、この特殊なケースを処理し、関数からNS_OKを返す必要があります。 この時点までmShellポインターが既に使用されていることはわかりにくいです。



最も可能性が高いのは、mShellポインターが常にNULLでないためです。 非常に簡単なチェックでも間違いを犯す可能性があることを示す例を示します。 チェックがありますが、意味がありません。



例4.ポインターを使用した後にNULLを確認する

 コンパイルステータス
 mjit ::コンパイラ:: performCompilation(JITScript ** jitp)
 {
   ...
   JaegerSpew(JSpew_Scripts、
     「正常にコンパイルされました(コード\ "%p \")(サイズ\ "%u \")\ n "、
     (* jitp)-> code.m_code.executableAddress()、
    符号なし((* jitp)-> code.m_size));

   if(!* jitp)
       return Compile_Abort;
   ...
 } 


PVS-Studio診断:V595 nullptrに対して検証される前に、「* jitp」ポインターが使用されました。 チェック行:547、549。compiler.cpp 547



ところで、検証の前にポインターを使用することはよくある間違いです。 これは、この主題に関する別の例です。



例5.入力値の不完全な検証

  PRBool
 nsStyleAnimation :: AddWeighted(...)
 {
   ...
   if(unit [0] == eCSSUnit_Null || unit [1] == eCSSUnit_Null ||
      ユニット[0] == eCSSUnit_Null || ユニット[0] == eCSSUnit_URL){
     return PR_FALSE;
   }
   ...
 } 


PVS-Studio Diagnostics:V501「||」の左側と右側に同一のサブ式「ユニット[0] == eCSSUnit_Null」があります。 演算子。 nsstyleanimation.cpp 1767



すぐに2つのタイプミスがあるようです。 コードがどのように見えるかを正確に言うのは難しいですが、おそらく次のように書きたいと思いました。

  if(unit [0] == eCSSUnit_Null || unit [1] == eCSSUnit_Null ||
    ユニット[0] == eCSSUnit_URL || ユニット[1] == eCSSUnit_URL){ 


入力ミスにより、関数が無効な入力値の処理を開始する場合があります。



例6.入力値の不完全な検証

  nsresult PresShell :: SetResolution(float aXResolution、float aYResolution)
 {
   if(!(aXResolution> 0.0 && aXResolution> 0.0)){
     NS_ERROR_ILLEGAL_VALUEを返します。
   }
   ...
 } 


診断PVS-Studio:V501「&&」演算子の左右に同じ副次式があります:aXResolution> 0.0 && aXResolution> 0.0 nspresshell.cpp 5114



また、入力パラメータのチェックに失敗した別の例を次に示します。 今回はタイプミスのため、aYResolution引数の値はチェックされません。



例7.参照されていないポインター

  nsresult
 SVGNumberList :: SetValueFromString(const nsAString&aValue)
 {
   ...
   const char * token = str.get();
   if(token == '\ 0'){
     NS_ERROR_DOM_SYNTAX_ERRを返す;  //コンマの間に何もない
   }
   ...
 } 


PVS-Studio診断:V528「char」タイプへのポインターが「\ 0」値と比較されるのは奇妙です。 おそらく意味:*トークン== '\ 0'。 svgnumberlist.cpp 96



コンマの間に何もないことを確認しても機能しません。 空の文字列かどうかを調べるには、最初の文字を「\ 0」と比較できます。 しかし、ここでは、最初の文字がゼロと比較されるのではなく、ポインターと比較されます。 このポインターは常にゼロ以外です。 正しいチェックは次のようになります:(* token == '\ 0')。



例8.インデックスストレージの不適切なタイプ

  PRBool 
 nsIEProfileMigrator :: TestForIE7()
 {
   ...
   PRUint32インデックス= ieVersion.FindChar( '。'、0);
   if(インデックス<0)
     return PR_FALSE;
   ...
 } 


PVS-Studio診断:V547式 'index <0'は常にfalseです。 符号なしの型の値が<0になることはありません。nsieprofilemigrator.cpp622



この関数は、行にドットがない場合はPR_FALSEを返さず、不正なデータを処理し続けます。 エラーは、変数 'index'に符号なしデータ型が選択されていることです。 チェック(インデックス<0)は意味がありません。



例9.誤ったエラーメッセージの生成

  cairo_status_t
 _cairo_win32_print_gdi_error(const char *コンテキスト)
 {
   ...
   fwprintf(stderr、L "%s:%S"、コンテキスト、(wchar_t *)lpMsgBuf);
   ...
 } 


診断PVS-Studio:V576形式が正しくありません。 'fwprintf'関数の3番目の実引数を確認することを検討してください。 wchar_t型シンボルの文字列へのポインターが必要です。 cairo-win32-surface.c 129



エラーが正常に検出された場合でも、適切に処理できる必要があります。 また、誰もエラーハンドラをテストしていないため、多くの興味深いことがわかります。



_cairo_win32_print_gdi_error()関数は、意味のないものを出力します。 3番目の引数として、fwprintf()関数はUnicode文字列へのポインターを予期し、代わりに 'const char *'形式の文字列を受け取ります。



例10.ダンプの書き込みエラー

  bool ExceptionHandler :: WriteMinidumpForChild(...)
 {
   ...
   DWORD last_suspend_cnt = -1;
   ...
   //このスレッドはすでに終了している可能性があるため、開かない
   //ハンドルは致命的ではないエラーです
   if(NULL!= child_thread_handle){
     if(0 <=(last_suspend_cnt =
                 SuspendThread(child_thread_handle))){
   ...
 } 


PVS-Studio Diagnostics:V547式は常に真です。 符号なしの型の値は常に> = 0です。exception_handler.cc 846



これは、エラーハンドラーの別の例です。 SuspendThread関数によって返される結果は、ここでは正しく処理されません。 変数last_suspend_cntはDWORD型であり、常に0以上になります。



Firefoxのその他のエラー



私は少し余談し、Firefoxチェックの結果について全体的に説明します。 このプロジェクトは高品質であり、PVS-Studioはわずかなエラーを明らかにしました。 ただし、サイズが大きいため、定量的にかなりの誤差があります。 残念ながら、私はPVS-Studioツールによって発行されたレポートを完全に調べることができませんでした。 事実、Visual Studio for Firefoxのプロジェクトファイルはありません。 プロジェクトは、メイクファイルから呼び出されたPVS-Studioのコンソールバージョンによってチェックされました。 Visual Studioでレポートを開くと、すべての診断メッセージを表示できます。 ただし、プロジェクトがないため、Visual Studioはどの変数が宣言されているかを示唆せず、マクロが定義されている場所に移動することもできません。 その結果、未知のプロジェクトの分析には非常に時間がかかり、メッセージの一部のみを調査することができました。



エラーはさまざまです。 たとえば、配列の境界を超える出口があります。

 クラスnsBaseStatis:public nsStatis {
公開:
   ...
   PRUint32 mLWordLen [10]; 
   ...
   nsBaseStatis :: nsBaseStatis(...)
   {
     ...
     for(PRUint32 i = 0; i <20; i ++)
        mLWordLen [i] = 0;
     ...
   }
   ...
 }; 


診断PVS-Studio:V557アレイのオーバーランが可能です。 'i'インデックスの値は19に達する可能性があります。detectcharset.cpp89



このエラーや類似のエラーは興味深いものですが、この記事のトピックとは関係ありません。 したがって、興味があれば、このファイルのその他のエラーを見ることができます: mozilla-test.txt



エラーハンドラーのエラーに戻る



エラーハンドラーの欠陥の問題の緊急性を説得するために、カップルではなく10個の例を挙げることにしました。 もちろん、エラーハンドラはプログラムの最も重要で重要な部分ではありません。 しかし、結局のところ、プログラマーはそれらを作成します。つまり、彼らは彼らの助けを借りてプログラムの振る舞いを改善したいと考えています。 残念ながら、私の観察が示すように、非常に頻繁にチェックとエラーハンドラーが機能しません。 大規模なプロジェクトであっても、このタイプの多くのエラーを表示するには十分です。



それについて何をすべきか、どのような勧告を行うことができますか?



最初の推奨事項



確かに、単純なチェックでも間違いを犯す可能性があります。 これは最も困難で重要です。 これは、エラーハンドラーが非常に多くのタイプミスやその他の欠陥を含む単純なコードフラグメントと見なされるためです。 エラーハンドラはチェックもテストもしません。 彼らはテストを書きません。



もちろん、エラーハンドラーのテストを記述することは非常に難しく、多くの場合、経済的に実行可能ではありません。 しかし、プログラマが危険についてさえ知っていれば、それはたくさんあります。 知って、それから武装。 エラーハンドラを使用すると、このような類似性を引き出すことができます。



統計によると、登山者はほとんどの場合、登山の終わりに倒れます。 これは疲労が原因ではなく、人が自分の残りがほとんどないと考えているためです。 彼はリラックスし、注意を失い、その結果、しばしば間違いを犯します。 同様のことが、コードを書くときにプログラマーに起こります。 彼はアルゴリズムに多大な労力と注意を費やし、ミスを犯さないと確信しているため、焦点を合わせずにさまざまなチェックを作成します。



だから今、あなたは警告されます。 そして、これはすでに非常に便利で便利だと思います。



学生と経験の浅いプログラマだけがそのような愚かな間違いを犯すと言うなら、あなたは間違っています。 タイプミスはすべてを簡単にします。 このトピックでは、「 神話2-プロの開発者は愚かな間違いを犯さない 」という小さなメモを提案します。 これは、さまざまなプロジェクトの多くの例で確認できます。 しかし、ここに示したものは十分に反映できると思います。



第二の推奨事項



ダンプストレージメカニズム、ロギング機能、およびその他の同様の補助メカニズムは、それらのユニットテストを行うに値します。



アイドルダンプストレージメカニズムは役に立たないだけでなく、トラブルが発生した場合に使用できるように見えるだけです。 ユーザーが破損したダンプファイルを送信すると、助けになるだけでなく、誤解を招くだけでなく、ダンプファイルがまったくない場合よりもエラーの検索に時間がかかります。



推奨事項はシンプルで明白に見えます。 しかし、このノートを読んでいる人のうち、WriteMyDumpクラスをテストする単体テストを持っている人はどれくらいいますか?



第三の推奨事項



静的コードアナライザーを使用します。 エラーハンドラーで欠陥を見つける可能性は、 静的分析手法の長所の1つです。 静的分析は、実行中のアプリケーションでの使用頻度に関係なく、コードのすべてのブランチを対象としています。 非常にまれにしか現れないエラーを検出できます。



つまり、静的解析では、 コードカバレッジは100%です。 他のタイプのテストを使用してこのようなコードのカバレッジを達成することはほとんど不可能です。 単体テストと回帰テストのコードカバレッジは、通常80%未満です。 残りの20%はテストが非常に困難です。 これらの20%には、ほとんどのエラーハンドラーとまれな状況が含まれます。



4番目の推奨事項



障害入力方法を使用しみてください。 ポイントは、多くの関数が時々さまざまなエラーコードを返し始め、プログラムがそれらを正しく処理することです。 たとえば、独自のmalloc()関数を記述できます。この関数は、メモリが残っている場合でも、時々NULLを返します。 これにより、メモリが実際になくなったときにプログラムがどのように動作するかがわかります。 fopen()、CoCreateInstance()、CreateDC()などの関数でも同じことができます。



このプロセスを自動化し、独自の関数を作成せずに失敗することがある特別なプログラムがあります。 残念ながら、私は同様のシステムを使用していなかったため、それらについて詳しく説明するのは難しいと感じています。



おわりに



エラーハンドラーの欠陥は一般的です。 残念ながら、記事に記載されている推奨事項がそれらを回避するのに十分かどうかはわかりません。 しかし、あなたがこの問題に興味を持ち、プログラムの欠点の数を減らす方法を思いつくことができることを願っています。 また、検討中のタイプのエラーを回避するのに役立つアイデアと方法を共有していただければ、私と他の読者に感謝します。



All Articles