Appleseedソースコードの確認

記事で説明されているプロジェクトのほとんどには、PVS-Studioアナライザーからの多数の警告が含まれています。 もちろん、これは記事のために選択されたアナライザーのレポートのごく一部ですが、警告がそれほど多くないプロジェクトがあり、これらの記事には興味深い「間違い」はありません。 通常、これらは小さなプロジェクトまたはすでに開発されていないプロジェクトです。 この記事では、PVS-Studioの観点から見るとコードの品質が非常に高いAppleseedプロジェクトのチェックについて説明します。

はじめに

Appleseedは、フォトリアリスティックな画像、アニメーション、視覚効果を再生するように設計された物理的なサウンドレンダリングを備えた最先端のオープンソースエンジンです。 個々のユーザーと小規模スタジオの両方に対して、信頼性の高いオープンテクノロジー上に構築されたツールセットを提供します。



ソースコード付きの約700個のファイルで構成されるAppleseedプロジェクトのPVS-Studioアナライザーを使用すると、第1レベルと第2レベルの興味深い警告がわずかしか見つかりませんでした。

検証結果

V670初期化されていないクラスメンバー「m_s0_cache」は、「m_s1_element_swapper」メンバーを初期化するために使用されます。 メンバーは、クラス内の宣言の順序で初期化されることに注意してください。 animatecamera cache.h 1009

class DualStageCache : public NonCopyable { .... S1ElementSwapper m_s1_element_swapper; //<==Line 679 S1Cache m_s1_cache; S0ElementSwapper m_s0_element_swapper; S0Cache m_s0_cache; //<==Line 683 }; FOUNDATION_DSCACHE_TEMPLATE_DEF(APPLESEED_EMPTY) DualStageCache( KeyHasherType& key_hasher, ElementSwapperType& element_swapper, const KeyType& invalid_key, AllocatorType allocator) : m_s1_element_swapper(m_s0_cache, element_swapper)//warning... // warning: referring to an uninitialized member , m_s1_cache(m_s1_element_swapper, allocator) , m_s0_element_swapper(m_s1_cache) , m_s0_cache(key_hasher, m_s0_element_swapper, invalid_key) { }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

アナライザーは、クラスコンストラクターの初期化リストでエラーの可能性を検出しました。 開発者は、コードに既に存在する「警告：初期化されていないメンバーの参照」というコメントから判断すると、初期化されていないフィールド「m_s0_cache」が「m_s1_element_swapper」フィールドの初期化に使用されることを知っていますが、修正していません。 言語標準によれば、コンストラクターでのクラスメンバーの初期化の順序は、クラスでの宣言の順序で発生します。



V605式m_variation_aov_index <〜0の検証を検討してください。 符号なしの値は、数値-1と比較されます。 appleseed adaptivepixelrenderer.cpp 154

 size_t m_variation_aov_index; size_t m_samples_aov_index; virtual void on_tile_end( const Frame& frame, Tile& tile, TileStack& aov_tiles) APPLESEED_OVERRIDE { .... if (m_variation_aov_index < ~0) //<== aov_tiles.set_pixel(x, y, m_variation_aov_index, ....); if (m_samples_aov_index != ~0) //<== aov_tiles.set_pixel(x, y, m_samples_aov_index, ....); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

'〜0'の反転の結果は-1で、int型です。 次に、この数値は符号なしの型size_tに変わります。 怖くないが、見た目が良いわけではない。 このような式では、定数SIZE_MAXをすぐに指定することをお勧めします。



一見、明らかなエラーはありませんが、2つの異なる条件演算子の使用に注意が向けられましたが、両方の条件が同じことをチェックします。 変数がsize_t型の最大許容値（SIZE_MAX）に等しくない場合、条件は真です。 これらのチェックはさまざまな方法で記録されます。 そのようなコードは非常に疑わしく、おそらく論理エラーがあります。



V668 「new」演算子を使用してメモリが割り当てられたため、「result」ポインターをnullに対してテストしても意味がありません。 メモリ割り当てエラーの場合、例外が生成されます。 appleseed string.cpp 58

 char* duplicate_string(const char* s) { assert(s); char* result = new char[strlen(s) + 1]; if (result) strcpy(result, s); return result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

アナライザーは、 'new'オペレーターによって返されたポインターの値がゼロと比較される状況を検出しました。 'new'演算子がメモリを割り当てることができなかった場合、C ++言語標準に従って、例外std :: bad_alloc（）が生成されることに注意してください。



したがって、Visual Studio 2013でコンパイルするAppleseedプロジェクトでは、ほとんどの場合、ゼロへのポインターをチェックしても意味がありません。 そして、いつかこの関数を使用すると、予期しない結果になる可能性があります。 duplicate_string（）関数は、文字列を複製できない場合はnullptrを返すことになっています。 代わりに、プログラムの他の部分の準備ができていない可能性があるという例外をスローします。



V719 switchステートメントは、 'InputFormat'列挙型のすべての値をカバーしていません：InputFormatEntity。 appleseed inputarray.cpp 92

 enum InputFormat { InputFormatScalar, InputFormatSpectralReflectance, InputFormatSpectralIlluminance, InputFormatSpectralReflectanceWithAlpha, InputFormatSpectralIlluminanceWithAlpha, InputFormatEntity }; size_t add_size(size_t size) const { switch (m_format) { case InputFormatScalar: .... case InputFormatSpectralReflectance: case InputFormatSpectralIlluminance: .... case InputFormatSpectralReflectanceWithAlpha: case InputFormatSpectralIlluminanceWithAlpha: .... } return size; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そして、InputFormatEntityの場合はどこにありますか？ このswitch（）ブロックには、デフォルトセクションも値「InputFormatEntity」の変数のアクションも含まれていません。 これは間違いですか、著者はこの値をスキップする必要があると考えましたか？



さらに2つの場所が見つかりました。

• V719 switchステートメントは、 'InputFormat'列挙型のすべての値をカバーしていません：InputFormatEntity。 appleseed inputarray.cpp 121
• V719 switchステートメントは、 'InputFormat'列挙型のすべての値をカバーしていません：InputFormatEntity。 appleseed inputarray.cpp 182

「デフォルト」セクションがなく、変数のすべての値が処理されていない場合、「InputFormat」の新しい値のコードをどこかで追加することをスキップできます。



V205ポインター型から32ビット整数型への明示的な変換：（unsigned long int）strvalue appleseed snprintf.cpp 885

 #define UINTPTR_T unsigned long int int portable_vsnprintf(char *str, size_t size, const char *format, va_list args) { const char *strvalue; .... fmtint(str, &len, size, (UINTPTR_T)strvalue, 16, width, //<== precision, flags); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

最後に、プログラムの64ビットバージョンに現れる重大なエラーがAppleseedで見つかりました。 このプロジェクトはクロスプラットフォームであり、WindowsおよびLinux用にコンパイルされます。 Cmakeはプロジェクトファイルを取得するために使用されます。 Windowsのアセンブリドキュメントでは、「Visual Studio 12 Win64」の使用が推奨されているため、汎用診断（GA、一般分析）に加えて、PVS-Studioアナライザーの64ビットエラー（64、Viva64）の診断も調べました。



「UINTPTR_T」の完全なマクロ定義コードは次のとおりです。

 /* Support for uintptr_t. */ #ifndef UINTPTR_T #if HAVE_UINTPTR_T || defined(uintptr_t) #define UINTPTR_T uintptr_t #else #define UINTPTR_T unsigned long int #endif /* HAVE_UINTPTR_T || defined(uintptr_t) */ #endif /* !defined(UINTPTR_T) */
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

uintptr_t型は符号なし整数memsize型であり、プラットフォームのビット深度に関係なくそれ自体に安全にポインターを格納できますが、Windowsでのアセンブリには「unsigned long int」型が定義されています。 型のサイズはデータモデルに依存し、 Linuxとは異なり、Windowsでは 'long'型は常に32ビットであるため、Win64プラットフォームではポインターはこの型の変数に収まりません。

おわりに

PVS-Studioを使用して検証されたプロジェクトには、そのサイズに関して、アナライザーからの重大な警告がほとんど含まれていません。











英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Svyatoslav Razmyslov。 Appleseedソースコードの確認 。