🛅 🏹 🤯 PVS-Studioチームは技術的なブレークスルーを準備していますが、今のところ、Blenderを再確認しましょう 🧙🏿 🧑🏻 👨🏼‍⚖️

静的分析は、定期的なチェックに最も役立ちます。特にBlenderのような急速に発展しているプロジェクトの場合。もう一度チェックして、今度はどのような疑わしい場所が見つかるかを調べてみましょう。

はじめに

Blenderは、インタラクティブなゲームの作成にも使用される、音声付きのビデオのモデリング、アニメーション、レンダリング、後処理、編集のためのツールを含む、3次元コンピューターグラフィックスを作成するためのプロフェッショナルパッケージです。

このプロジェクトはすでにテスト済みです。バージョン2.62の検証結果は、記事「 PVS-Studioを使用したBlenderプロジェクトの検証」に記載されています。

最後のチェック以降、追加のライブラリとともにソースコードのサイズが77メガバイトに増加しました。そして、そのボリュームは2206 KLOCまで増加しました。前回の検証時、プロジェクトのサイズは68メガバイト（2105 KLOC）でした。

SourceMonitorユーティリティは、コードの量を計算するのに役立ちました。このユーティリティは、C ++、C、C＃、VB.NET、Java、Delphiのコードを分析し、さまざまなメトリックを計算できます。たとえば、プロジェクトの循環的な複雑さを判断したり、各プロジェクトファイルの詳細な統計を生成したりできます。結果を表または図の形式で表示します。

したがって、この記事では、バージョンBlender 2.77aで見つかったエラーと疑わしい場所について説明します。検証には、PVS-Studioアナライザーバージョン6.05が使用されました。

タイプミス

コピーメカニズムと自動コード補完を積極的に使用すると、さまざまな変数や定数の名前にエラーが発生することがよくあります。このようなエラーは、誤った計算結果やプログラムの予期しない動作を引き起こす可能性があります。 Blenderプロジェクトで、アナライザーはいくつかの例を同時に見つけました。それらを詳細に検討しましょう。

条件のタイプミス

CurvePoint::CurvePoint(CurvePoint *iA, CurvePoint *iB, float t3) { .... if ((iA->getPoint2D() - //<= iA->getPoint2D()).norm() < 1.0e-6) { //<= .... } .... }

V501 '-'演算子の左右に同じ副次式があります：iA-> getPoint2D（）-iA-> getPoint2D（）curve.cpp 136

CurvePoint関数内では、同じ名前の2つのオブジェクト、 iAとiBが機能します。これらのオブジェクトのさまざまなメソッドは、かなり長い条件ツリーのさまざまな操作で常に交差しています。条件ブロックの1つでは、タイプミスが許可されています。その結果、同じオブジェクトのプロパティ間で減算演算が行われます。コードの機能を知らないと、2つのオペランドのどちらでエラーが発生したかを正確に言うことはできません。私はその修正のために2つの可能なオプションを提案します：

 if ((iA->getPoint2D()-iB->getPoint2D()).norm()<1.0e-6)....

または

 if ((iB->getPoint2D()-iA->getPoint2D()).norm()<1.0e-6)....

次のエラーも条件ステートメント内に隠れています。

 template<typename MatrixType, int QRPreconditioner> void JacobiSVD<MatrixType, QRPreconditioner>::allocate(....) { .... if(m_cols>m_rows)m_qr_precond_morecols.allocate(*this); if(m_rows>m_cols)m_qr_precond_morerows.allocate(*this); if(m_cols!=m_cols)m_scaledMatrix.resize(rows,cols); //<= }

V501 '！='演算子の左右に同一の副次式があります：m_cols！= M_cols jacobisvd.h 819

特定のコードフラグメントでは、特定のマトリックス内の行と列の数の方程式が発生します。その数が等しくない場合、メモリは新しい要素またはその作成に割り当てられます。そして、新しいセルが追加された場合、マトリックスのサイズを変更する操作が発生します。ただし、条件演算子の式のエラーの結果、条件m_cols！= M_colsは常にfalseであるため、操作は発生しません。この場合、式の2つの部分のどちらを変更する必要があるかは関係ないため、このオプションを提案します。

 if(m_cols!=m_rows) m_scaledMatrix.resize(rows,cols)

V501診断で検出されたいくつかの問題領域：

V501「==」演算子の左右に同じ副次式があります。left.rows（）== left.rows（）numeric.cc 112
V501 '>'演算子の左右に同じ部分式があります：（from [0] [3]）>（from [0] [3]）stereoimbuf.c 120
V501 '>'演算子の左右に同じ部分式があります：（from [0] [3]）>（from [0] [3]）stereoimbuf.c 157
V501 '=='演算子の左右に同じ副次式があります。out-> y == out-> y filter.c 209

ヌルポインター操作

ここで、名前のタイプミスは、より深刻な間違いにつながりました。

 int QuantitativeInvisibilityF1D::operator()(....) { ViewEdge *ve = dynamic_cast<ViewEdge*>(&inter); if (ve) { result = ve->qi(); return 0; } FEdge *fe = dynamic_cast<FEdge*>(&inter); if (fe) { result = ve->qi(); //<= return 0; } .... }

V522 NULLポインター「ve」の逆参照が行われる場合があります。 functions1d.cpp 107

上記の関数は非常に短いですが、単純な関数であってもタイプミスは私たちを待っています。コードから、2つのオブジェクトが順番に作成および検証されることがわかります。しかし、2番目のオブジェクトをチェックした後、エラーが発生し、 feが正常に作成された場合、その代わりに、最初のオブジェクトからの関数の結果が結果に書き込まれます。これにより、例外が高レベルのハンドラをキャッチしない場合、プログラムがクラッシュする可能性が高くなります。

どうやら2番目のコードはCopy-Pasteを使用して書かれたようです。そして偶然にも、彼らは変数veの名前を変更するのを忘れていました。ほとんどの場合、正しいコードは次のようになっているはずです。

 FEdge *fe = dynamic_cast<FEdge*>(&inter); if (fe) { result = fe->qi(); return 0; }

NULLポインターを使用する

 static ImBuf *accessor_get_ibuf(....) { ImBuf *ibuf, *orig_ibuf, *final_ibuf; .... /* First try to get fully processed image from the cache. */ ibuf = accesscache_get(accessor, clip_index, frame, input_mode, downscale, transform_key); if (ibuf != NULL) { return ibuf; } /* And now we do postprocessing of the original frame. */ orig_ibuf = accessor_get_preprocessed_ibuf(accessor, clip_index, frame); if (orig_ibuf == NULL) { return NULL; } .... if (downscale > 0) { if (final_ibuf == orig_ibuf) { final_ibuf = IMB_dupImBuf(orig_ibuf); } IMB_scaleImBuf(final_ibuf, ibuf->x / (1 << downscale), //<= ibuf->y / (1 << downscale)); //<= } .... if (input_mode == LIBMV_IMAGE_MODE_RGBA) { BLI_assert(ibuf->channels == 3 || //<= ibuf->channels == 4); //<= } .... return final_ibuf; }

警告：

V522 NULLポインター「ibuf」の逆参照が行われる場合があります。 tracking_util.c 765
V522 NULLポインター「ibuf」の逆参照が行われる場合があります。 tracking_util.c 766
V522 NULLポインター「ibuf」の逆参照が行われる場合があります。 tracking_util.c 783

上記のスニペットは、オブジェクトが作成された場合、 ibuf変数をチェックすると、この変数が使用されるよりもずっと早く関数が終了することを示しています。おそらくこれにより、ポインターの逆参照の事実が停止および確認される可能性があります。 ただし、コードとそのコメントを詳しく見ると、エラーの本当の原因が明らかになります。実際、ここでもタイプミスが許可されています。アナライザーによって示された場所では、実際にはibufの代わりにorig_ibuf変数が使用されているはずです。

無効な変数タイプ

 typedef enum eOutlinerIdOpTypes { OUTLINER_IDOP_INVALID = 0, OUTLINER_IDOP_UNLINK, OUTLINER_IDOP_LOCAL, .... } eOutlinerIdOpTypes; typedef enum eOutlinerLibOpTypes { OL_LIB_INVALID = 0, OL_LIB_RENAME, OL_LIB_DELETE, } eOutlinerLibOpTypes; static int outliner_lib_operation_exec(....) { .... eOutlinerIdOpTypes event; //<= .... event = RNA_enum_get(op->ptr, "type"); switch (event) { case OL_LIB_RENAME: //<= { .... } case OL_LIB_DELETE: //<= { .... } default: /* invalid - unhandled */ break; } .... }

警告：

V556異なる列挙型の値が比較されます：switch（ENUM_TYPE_A）{case ENUM_TYPE_B：...}。 outliner_tools.c 1286
V556異なる列挙型の値が比較されます：switch（ENUM_TYPE_A）{case ENUM_TYPE_B：...}。 outliner_tools.c 1295

この例は、列挙である2つのタイプを示しています。そして、型でコードを書くときにタイプミスが行われたという事実は、名前でほとんど区別できないそのような型に対しては非常に期待されています。

実際、コードは正しく機能します。しかし同時に、彼は型の不一致に惑わされています。変数は、ある列挙の値を取得し、別の列挙の定数と比較されます。この場合のエラーを修正するには、 イベント 変数のタイプをeOutlinerLibOpTypesに変更するだけで十分です。

操作優先エラー

 static void blf_font_draw_buffer_ex(....) { .... cbuf[3] = (unsigned char)((alphatest = ((int)cbuf[3] + (int)(a * 255)) < 255) ? alphatest : 255); .... }

V593 「A = B <C」の種類の表現を見直すことを検討してください。式は次のように計算されます： 'A =（B <C）'。 blf_font.c 414

複雑な式を操作する場合、操作の優先順位の違反は、最も一般的なエラーの1つです。この場合、これは単なるタイプミスですが、三項演算子のロジックに違反することになりました。無効な括弧が原因で操作の優先順位エラーが発生しました。さらに、 alphatest変数の値が破損しています 。三項演算子が計算する値の代わりに、 alphatest変数には、比較演算の結果として取得されたブール型の値（<）が割り当てられます。その後、三項演算子はalphatest変数の値を処理し、その結果は保存されません。エラーを修正するには、次のように式を変更する必要があります。

 cbuf[3] = (unsigned char)(alphatest = (((int)cbuf[3] + (int)(a * 255)) < 255) ? alphatest : 255);

定数のエラー

 bool BKE_ffmpeg_alpha_channel_is_supported(RenderData *rd) { int codec = rd->ffcodecdata.codec; if (codec == AV_CODEC_ID_QTRLE) return true; if (codec == AV_CODEC_ID_PNG) return true; if (codec == AV_CODEC_ID_PNG) return true; .... }

V649同一の条件式を持つ2つの「if」ステートメントがあります。最初の「if」ステートメントには、関数の戻り値が含まれます。これは、2番目の「if」ステートメントが無意味であることを意味します。行を確認してください：1672、1675。writeffmpeg.c 1675

この関数は、単一行条件を使用して、フラグの順守について変数の値を順番にチェックします。タイプミスの結果、フラグの1つが2回チェックされます。確かに、再チェックする代わりに、別の定数をチェックする必要があります。しかし、これらの定数には多くのオプションがあるため、このコードを修正する方法を正確に推測することはできません。

外側のネストされたループで単一の変数を使用する

 bool BM_face_exists_overlap_subset(...., const int len) { int i; .... for (i = 0; i < len; i++) { BM_ITER_ELEM (f, &viter, varr[i], BM_FACES_OF_VERT) { if ((f->len <= len) && (....)) { BMLoop *l_iter, *l_first; if (is_init == false) { is_init = true; for (i = 0; i < len; i++) { //<= BM_ELEM_API_FLAG_ENABLE(varr[i], _FLAG_OVERLAP); } } .... } } } }

V535変数 'i'は、このループと外側のループに使用されています。行を確認してください：2204、2212。bmesh_queries.c 2212

外側のネストされたループで同じ変数を使用すると、外側のループが誤って実行される可能性があります。この場合、サイクルは明らかに目的の要素を検索して終了するため、これはエラーではない可能性が高く、2番目のサイクルはこの場合にのみ機能します。それでも、単一の変数を使用することは危険な場所であり、このコードを最適化する必要がある場合、実際のエラーにつながる可能性があります。

冗長コード

余分なコードフラグメントは、どのプログラムにもあります。これは、リファクタリングで残された古いコードである場合があります。そして、余分なフラグメントがプロジェクトコードのスタイルを維持するのに役立つことがあります。コードのこのような部分は危険な場合があります。言い換えれば、重複コードはしばしば論理エラーを示します。

再確認

 static void knife_add_single_cut(....) { .... if ((lh1->v && lh2->v) && //<= (lh1->v->v && lh2->v && lh2->v->v) && //<= (e_base = BM_edge_exists(lh1->v->v, lh2->v->v))) { .... return; } .... }

V501 「&&」演算子の左と右に同じ副次式「lh2-> v」があります。 editmesh_knife.c 781

これは、思いがけない状態のオプションの1つです。もちろん、これは間違いではなく、追加のチェックですが、コードを変更する必要がないという意味ではありません。条件は複数の式で構成されます。この場合、2番目の式の一部は、最初の式の変数の1つをチェックすることに完全に対応し、不要です。修正するには、2番目の式から余分なチェックlh2-> vを削除する必要があります。その後、コードはより明確になります。

別の例：

 static int edbm_rip_invoke__vert(....) { .... if (do_fill) { if (do_fill) { .... } } .... }

V571定期的なチェック。「if（do_fill）」条件は、751行目で既に検証されています。editmesh_rip.c 752

別の論理エラー。ここでは、外部条件とネストされた条件内で、2つの完全に同一の式がチェックされます。検証を繰り返しても常に同じ結果が得られ、意味がありません。もちろん、このコードはプログラムの動作には影響しません。しかし、コードが時間とともにどのように変化するかは不明であり、不必要なチェックは混乱を招く可能性があります。

プロジェクトの1か所で過剰なチェックは見つかりません。アナライザーによって検出された場所がさらにいくつかあります。

V571定期的なチェック。「but」条件は、行9587ですでに検証されています。interface_handlers.c 9590
V571定期的なチェック。 '！Me-> mloopcol'条件は、252行目で既に検証されています。paint_vertex.c 253
V571定期的なチェック。「constinv == 0」条件は、5256行目で既に検証されています。transform_conversions.c 5257
V571定期的なチェック。「vlr-> v4」条件は、4174行目で既に検証されています。convertblender.c 4176
V571定期的なチェック。 'ibuf ==（（void *）0）'条件は3557行目で既に検証されています。sequencer.c 3559

3番目の例は、明示的な冗長コードです。

 static void writedata_do_write(....) { if ((wd == NULL) || wd->error || (mem == NULL) || memlen < 1) return; if (wd->error) return; .... }

V649同一の条件式を持つ2つの「if」ステートメントがあります。最初の「if」ステートメントには、関数の戻り値が含まれます。これは、2番目の「if」ステートメントが無意味であることを意味します。行を確認：331、332。writefile.c 332

if（wd-> error）return; ただ余分であり、この条件が処理される前に関数は終了します。したがって、単に削除する必要があります。

条件ブロックの反対側

 static int select_less_exec(....) { .... if ((lastsel==0)&&(bp->hide==0)&&(bp->f1 & SELECT)){ if (lastsel != 0) sel = 1; else sel = 0; .... } .... }

V637反対の2つの条件が発生しました。 2番目の条件は常にfalseです。行をチェック：938、939。editcurve_select.c 938

フラグメントから、追加の条件が外部条件ブロック内にあることは事実です。ネストされた条件はメイン条件の反対であり、常に同じ結果を生成し、 sel変数は値1を取得しません。したがって、再チェックせずにsel = 0に設定するだけで十分です。ただし、いずれかの条件を変更することにより、おそらくエラーを修正する必要があります。私はプロジェクトの開発に参加していません。何が起こっているのかを判断するのは困難です。

冗長表現

 DerivedMesh *fluidsimModifier_do(....) { .... if (!fluidmd || (fluidmd && !fluidmd->fss)) return dm; .... }

V728過剰なチェックを簡素化できます。「||」演算子は、反対の表現「！fluidmd」と「fluidmd」に囲まれています。 mod_fluidsim_util.c 528

1つの条件のフレームワーク内で、同じ変数の反対の値がチェックされます。そのような状態は、しばしばさまざまな形やバリエーションで見られます。プログラムに害を与えることはありませんが、コードを無駄に複雑にします。この式は、次の形式に簡略化および縮小できます。

 if (!fluidmd || !fluidmd->fss)) ....

同様の場所：

V728過剰なチェックを簡素化できます。「||」演算子は反対の式「！render_only」と「render_only」に囲まれています。 drawobject.c 4663
V728過剰なチェックを簡素化できます。「||」演算子は反対の式 '！parent'と 'parent'に囲まれています。 kx_scene.cpp 1667

この条件の別のオプション：

 void ED_transverts_create_from_obedit(....) { .... if ((tipsel && rootsel) || (rootsel)) {....} .... }

V686パターンが検出されました：（rootsel）|| （（ルートセル）&& ...）。式が過剰であるか、論理エラーが含まれています。 ed_transverts.c 325

上記の例のように、同じ式内で同じ変数が2回チェックされます。そのような表現は誤りではありませんが、不必要な検証で明らかに過負荷になります。コードをよりコンパクトで明確にするためには、コードを単純化する必要があります。

 if ((tipsel || rootsel) {....}

プロジェクトの他の場所でも同様のエラーが見つかりました。

V686パターンが検出されました：（！Py_b_len）|| （（！py_b_len）&& ...）。式が過剰であるか、論理エラーが含まれています。 aud_pyapi.cpp 864
V686パターンが検出されました：（xn == 0.0f）|| （（xn == 0.0f）&& ...）。式が過剰であるか、論理エラーが含まれています。 renderdatabase.c 993
V686パターンが検出されました：（xn == 0.0f）|| （（xn == 0.0f）&& ...）。式が過剰であるか、論理エラーが含まれています。 renderdatabase.c 1115

再割り当て

 static bool find_prev_next_keyframes(....) { .... do { aknext = (ActKeyColumn *)BLI_dlrbTree_search_next( &keys, compare_ak_cfraPtr, &cfranext); if (aknext) { if (CFRA == (int)aknext->cfra) { cfranext = aknext->cfra; //<- } else { if (++nextcount == U.view_frame_keyframes) donenext = true; } cfranext = aknext->cfra; //<- } } while ((aknext != NULL) && (donenext == false)); .... }

V519 「cfranext」変数には、連続して2回値が割り当てられます。おそらくこれは間違いです。行を確認してください：447、454。anim_draw.c 454

条件ブロック内での割り当ては、その値が条件なしでループの最後に再び割り当てられるため、意味がありません。余分な行が正確に一番上にあると結論付けることは、特定のフラグメントの後のコードにあるループを助けます。 prev変数と条件にこの行がないことのみが異なります。さらに、下からの余分な行と条件CFRA ==（int）aknext-> cfraが偽であると仮定すると、サイクルは無限に変わります。このフラグメントは明らかに調整する必要がありますが、プロジェクト開発者のみがそれを変更する方法を知っています。

追加または未使用の変数

変数が初期化されているが、結果として使用されていない同様のフラグメントが多数あります。それらのいくつかは、論理エラーと過剰な再チェックの例に関連しています;同様の断片はすでに何度も言及されています。定数もありますが、これらはおそらく関数内で変更されるはずです。しかし、最終的に、それらはチェックの形でのみ残り、常に同じ結果を返しました。同様のフラグメントの例：

 static int rule_avoid_collision(....) { .... int n, neighbors = 0, nearest = 0; //<= .... if (ptn && nearest==0) //<= MEM_freeN(ptn); return ret; }

V560条件式の一部は常に真です：最も近い==0。boids.c 361

残りのフラグメントについては、リストを示します。おそらくそれらの多くは議論の余地があるが、注意を払う価値がある。

V560条件式の一部は常にtrueです：edit ==0。particle.c 3781
V560条件式の一部は常にtrue :!エラーです。 pointcache.c 154
V560条件式の一部は常にtrue :!エラーです。 pointcache.c 2742
V560条件式の一部は常にfalseです：col。 drawobject.c 7803
V560条件式の一部は常にfalse :! Canvas_vertsです。 dynamicpaint.c 4636
V560条件式の一部は常に真です：（！リーフ）。 octree.cpp 2513
V560条件式の一部は常に真です：（！リーフ）。 octree.cpp 2710
V560条件式の一部は常にfalseです：（1 == i）。 basicstrokeshaders.cpp 67
V560条件式の一部は常に真です：（0 == i）。 basicstrokeshaders.cpp 69
V560条件式の一部は常にfalseです：（1 == i）。 basicstrokeshaders.cpp 84
V560条件式の一部は常に真です：（0 == i）。 basicstrokeshaders.cpp 86
V560条件式の一部は常にfalseです：（1 == i）。 basicstrokeshaders.cpp 155
V560条件式の一部は常に真です：（0 == i）。 basicstrokeshaders.cpp 157
V560条件式の一部は常に真です：（！Radmod）。 solver_control.cpp 557
V560条件式の一部は常に真です：done！= 1. context.c 301
V560条件式の一部は常にtrueです：is_tablet == false。 ghost_systemwin32.cpp 665
V560条件式の一部は常に真です：mesh> =0。kx_gameobject.cpp 976

不要なリストクリーニング

 int TileManager::gen_tiles(bool sliced) { .... state.tiles.clear(); //<= .... int tile_index = 0; state.tiles.clear(); state.tiles.resize(num); .... }

V586同じリソースの割り振り解除のために、「クリア」機能が2回呼び出されます。行を確認：149、156。tile.cpp 156

この場合、それは単なる余分な行かもしれません。おそらく、リストの2つのクリーンアップの前にはまだ何らかのコードがありましたが、この場合は別の余分な部分であり、コードが乱雑にならないように削除する必要があります。この行は、コードの大まかな検査中に表示されなかった他のオブジェクトがその行でクリアされているはずであるという事実の結果である場合もあります。この場合、フラグメントは明らかな間違いとなり、プログラムに未知の結果をもたらす可能性があります。

非常に頻繁に、この一見冗長なコードは、本当に重大なエラーを引き起こしたり、コードを最終化する際に将来その外観を回避するのに役立ちます。そのため、このようなアナライザーメッセージに注意を払う価値があり、それらを重要でないものとして無視することはできません。

陰謀

PVS-Studioチームは現在、新しい方向に積極的に取り組んでいます。そして、私は後部をカバーし、いくつかの開いているプロジェクトの再チェックに関する記事で情報フィールドを埋めます。この方向は何ですか？私には言えません。誰もが自分のやり方で自由に解釈できるという写真を残しています。

写真2

おわりに

アナライザーは、プロジェクト内の多くの問題領域を示しました。ただし、Blenderは奇妙なコードスタイルを使用することがあり、エラーなどのフラグメントを正確に解釈できません。誤字が原因で危険なエラーが発生することがよくあります。 PVS-Studioアナライザーは、それらを見つけるのに適しています。しかし、記事に反映されているエラーは、単に著者の意見であり、非常に主観的です。また、アナライザの機能を完全に評価するには、ダウンロードして試してみる価値があります。

英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Alexander Chibisov。 PVS-Studioチームは技術的なブレークスルーを作成しようとしていますが、今はBlenderを再確認しましょう。

記事を読んで質問がありますか？

多くの場合、記事には同じ質問が寄せられます。ここで回答を集めました： PVS-Studioバージョン2015に関する記事の読者からの質問への回答。リストをご覧ください。

PVS-Studioチームは技術的なブレークスルーを準備していますが、今のところ、Blenderを再確認しましょう