できるユニコーン

マイクロソフトの開発チームの1人がすでにPVS-Studioアナライザーを使用しています。 これは良いですが、十分ではありません。 したがって、Microsoftプロジェクトを使用した静的コード分析の利点を引き続き実証します。 3年前、カサブランカプロジェクトをチェックしましたが、プロジェクトには何も見つかりませんでした。 このプロジェクトでは、メダル「コードレスコード」が授与されました。 時間が経ち、プロジェクトは発展し成長しました。 また、PVS-Studioアナライザーは、コード分析の可能性が大幅に向上しました。 最後に、Casablancaプロジェクト（C ++ REST SDK）でアナライザーが特定したエラーに関する記事を作成できます。 エラーはほとんどありませんが、記事を書くのに十分であるという事実は、PVS-Studioの有効性を示しています。

カサブランカ

注釈で述べたように、カサブランカプロジェクトは既にテスト済みです。 これについては、 「 カサブランカプロジェクトに関する小さなメモ 」という記事で読むことができます。



Casablanca（C ++ REST SDK）は、最新のC ++で書かれた小さなプロジェクトです。 現代のC ++言語について言えば、コードはmoveセマンティクス、ラムダ関数、autoなどを積極的に使用しているということです。 C ++の新機能により、より短く信頼性の高いコードを作成できます。 これは、このプロジェクトでエラーを見つけることは簡単な作業ではないという事実によって確認されます。 通常は非常に簡単に行いますが。



私たちがチェックした他のMicrosoftプロジェクトに興味がある人のために、 Xamarin.Forms 、 CNTK 、 Microsoft Edge 、 CoreCLR 、 Windows 8 Driver Samples 、Visual C ++ 2012/2013ライブラリ、 CoreFX 、 Roslyn 、 Microsoft Code Contracts 、および間もなくWPFサンプルのチェックに関する記事が表示されます。



そのため、カサブランカプロジェクトは、良質で高品質なコードの例です。 PVS-Studio静的コードアナライザーを使用して、まだ何が見つかるか見てみましょう。

何が悪いとわかったのか

フラグメントN1：タイプミス



lowとhighの 2つのメンバーを含むNumericHandValues構造があります。 この構造の宣言は次のとおりです。

struct NumericHandValues { int low; int high; int Best() { return (high < 22) ? high : low; } };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここで、この構造が1か所で初期化される方法を見てみましょう。

 NumericHandValues GetNumericValues() { NumericHandValues res; res.low = 0; res.low = 0; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V519「res.low」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認：130、131。BlackJack_Client140 messagetypes.h 131



ご覧のように、誤ってlowメンバーを2回初期化しましたが、 highの初期化を忘れていました。 ここで思慮深いコメントを書くことは困難です。 誰もタイプミスに免疫がないというだけです。



フラグメントN2：メモリフリーエラー

 void DealerTable::FillShoe(size_t decks) { std::shared_ptr<int> ss(new int[decks * 52]); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V554 shared_ptrの誤った使用。 「new []」で割り当てられたメモリは、「delete」を使用して消去されます。 BlackJack_Server140 table.cpp 471



デフォルトでは、オブジェクトを破壊するshared_ptr型のスマートポインターは、角括弧[]なしで削除演算子を呼び出します。 この場合、それは間違っています。



コードの正しいバージョンは次のようになります。

 std::shared_ptr<int> ss(new int[decks * 52], std::default_delete<int[]>());
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

フラグメントN3：失われたポインター



静的メンバーs_server_apiはスマートポインターであり、次のように定義されます。

 std::unique_ptr<http_server> http_server_api::s_server_api((http_server*)nullptr);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

疑いは次の関数のコードを呼び出します：

 void http_server_api::unregister_server_api() { pplx::extensibility::scoped_critical_section_t lock(s_lock); if (http_server_api::has_listener()) { throw http_exception(_XPLATSTR("Server API ..... attached")); } s_server_api.release(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V530関数 'release'の戻り値を利用する必要があります。 cpprestsdk140 http_server_api.cpp 64



「s_server_api.release（）;」に注意してください。 リリース関数を呼び出した後、スマートポインターはオブジェクトを所有しなくなります。 オブジェクトへのポインタは「失われ」、オブジェクトはプログラムの寿命が終わるまで存在します。



ほとんどの場合、再びタイプミスに遭遇しました。 私は彼らがリセット機能を呼び出すことを望んでいたと思います。



フラグメントN4：間違った列挙型



プロジェクトには、次のように宣言された2つの列挙BJHandStateとBJHandResultがあります。

 enum BJHandState { HR_Empty, HR_BlackJack, HR_Active, HR_Held, HR_Busted }; enum BJHandResult { HR_None, HR_PlayerBlackJack, HR_PlayerWin, HR_ComputerWin, HR_Push };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PayUp関数のコードスニペットを見てみましょう。

 void DealerTable::PayUp(size_t idx) { .... if ( player.Hand.insurance > 0 && Players[0].Hand.state == HR_PlayerBlackJack ) { player.Balance += player.Hand.insurance*3; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V556異なる列挙型の値が比較されます。 タイプ：BJHandState、BJHandResult。 BlackJack_Server140 table.cpp 336



状態変数のタイプはBJHandStateです。 これは、プログラマーがリスト内で混乱していることを意味します。 どうやらコードは次のようになっているはずです。

 if ( player.Hand.insurance > 0 && Players[0].Hand.state == HR_BlackJack )
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

面白いことに、このエラーは実際にはプログラムに影響を与えません。 幸運な偶然のおかげで、現時点では、定数HR_BlackJackとHR_PlayerBlackJackの値は1に等しくなります。事実、列挙内のこれらの定数は両方ともリスト内の同じ位置にあります。 ただし、プログラム開発の過程でこれは変わる可能性があり、その後、奇妙で理解できないエラーが発生します。



フラグメントN5：奇妙なブレーク

 web::json::value AsJSON() const { .... int idx = 0; for (auto iter = cards.begin(); iter != cards.end();) { jCards[idx++] = iter->AsJSON(); break; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V612ループ内の無条件の「ブレーク」。 BlackJack_Client140 messagetypes.h 213



このコードのbreakステートメントは非常に疑わしく見えます。 ループは最大1回の反復を実行できます。 私はこのコードがどのように振る舞うべきかを言うのは難しいですが、ほとんどの場合、今では何かが間違っています。

その他のささいなこと

上記で説明し、エラーであると主張するコードフラグメントに加えて、アナライザーはいくつかの不正確な場所も発見しました。 たとえば、イテレータにポストインクリメントを使用します。

 inline web::json::value TablesAsJSON(...., std::shared_ptr<BJTable>> &tables) { web::json::value result = web::json::value::array(); size_t idx = 0; for (auto tbl = tables.begin(); tbl != tables.end(); tbl++) { result[idx++] = tbl->second->AsJSON(); } return result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio：V803パフォーマンスの低下。 'tbl'がイテレータの場合、プレフィックス形式のインクリメントを使用する方が効果的です。 イテレータ++を++イテレータに置き換えます。 BlackJack_Client140 messagetypes.h 356



もちろん、これは間違いではありません。 ただし、プリインクリメントを使用する場合、 ++ tblは良いスタイルと見なされます。 これが理にかなっていると思う人のために、私は次の2つの記事を参照します。

1. 接頭辞増分演算子++を反復子に使用するのは、接尾辞it ++ではなく実用的ですか？ http://www.viva64.com/en/b/0093/
2. プレvs. インクリメント後の演算子-ベンチマーク。 http://silviuardelean.ro/2011/04/20/pre-vs-post-increment-operator/

ライブラリコードにはループ内でイテレータのポストインクリメントを使用する場所が10個ありますが、それらをイテレータに含める理由はわかりません。



アナライザーがずさんなコードを指す別の例を考えてみましょう。

 struct _acquire_protector { _acquire_protector(....); ~_acquire_protector(); size_t m_size; private: _acquire_protector& operator=(const _acquire_protector&); uint8_t* m_ptr; concurrency::streams::streambuf<uint8_t>& m_buffer; };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioの警告：V690「=」演算子は「_acquire_protector」クラスでプライベートとして宣言されていますが、デフォルトのコピーコンストラクターはコンパイラーによって引き続き生成されます。 そのようなクラスを使用するのは危険です。 cpprestsdk140.uwp.staticlib fileio_winrt.cpp 825



ご覧のとおり、プログラマはコピー演算子の使用を禁止しています。 ただし、デフォルトのコンパイラーによって作成されたコピーコンストラクターを使用して、オブジェクトをコピーすることはできます。

おわりに

最後に、PVS-Studioアナライザーは何かの障害を見つけることができました。 エラーはほとんどありませんでしたが、まだ発生しています。 そして、これは、今のように一度だけではなく、定期的に静的分析を適用すれば、早期に多くのエラーを防ぐことができることを意味します。 テスト、デバッグ中などではなく、コードを記述した直後にエラーを修正することをお勧めします。ユーザーの1人が欠陥を報告した後は、さらに修正することをお勧めします。

サイトリンク

1. 記事のタイトルは、物語「 可能性のあるエンジン 」への参照です。
2. PVS-Studioアナライザーをダウンロードして、C、C ++、またはC＃でプロジェクトの1つをチェックしようとするリンク： http : //www.viva64.com/en/pvs-studio-download/

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 できる小さなユニコーン 。