音楽ソフトウェアコードの欠陥の概要。 パート5. Steinberg SDK

音楽アプリケーションのコードを引き続き確認しますが、これは商用ソフトウェアの最初の代表です。 以前の記事へのコメントで、Cubaseの人気に気付き、それについて読むことにしました。 これはSteinbergの製品で、いくつかのクローズドソースプログラムがあります。 偶然、私のサイトでサードパーティの開発者向けのSDKを見つけ、それを調べた後、多くの興味深いエラーを見つけました。

はじめに

Steinberg GmbH （Steinberg Media Technologies GmbH）は、音楽ソフトウェアとハ​​ードウェアを開発するハンブルグに拠点を置くドイツ企業です。 大部分は、デジタルオーディオワークステーションとVSTi形式のソフトウェアを備えたシンセサイザーの両方で使用するための音楽の録音、アレンジ、編集用のソフトウェアを生成します。 Steinbergはヤマハ株式会社の子会社であり、Steinbergはヤマハ株式会社が完全所有しています。

SDKの少数のソースについても、1つのレビュー記事では十分ではないため、完全なレポートを表示するには、作成者は一時キーのサポートをリクエストしてPVS-Studio静的アナライザーの機能を評価することにより、プロジェクトを個別に検証できます。 これは、C、C ++、C＃で書かれたプログラムのソースコードのエラーを検出するためのツールです。 WindowsおよびLinux環境で動作します。

コンマ演算子（、）

コンマ演算子（、）を使用して、その両側で式を左から右の順に実行し、 右式の値を取得します。 ほとんどの場合、演算子はforループのカウンター変更式で使用されます。 マクロのデバッグとテストに使用すると便利な場合があります。 しかし、ほとんどの場合、この演算子は悪用され、誤って使用されます。



V521 「、」演算子を使用したこのような式は危険です。 式 'i <temp、i <numParams'が正しいことを確認してください。 mdaBaseProcessor.cpp 309

tresult PLUGIN_API BaseProcessor::setState (IBStream* state) { .... // read each parameter for (uint32 i = 0; i < temp, i < numParams; i++) { state->read (&params[i], sizeof (ParamValue)); SWAP64_BE(params[i]) } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

カンマ演算子の誤用の小さな例。 コードの作者が何を言おうとしているかは明らかではありません。 コードは無害なようですので、次の例に進みましょう。



V521 「、」演算子を使用したこのような式は危険です。 式が正しいことを確認してください。 mdaBaseProcessor.cpp 142

 bool BaseProcessor::bypassProcessing (ProcessData& data) { .... for (int32 bus = 0; bus < data.numInputs, // <= bus < data.numOutputs; bus++) { .... if (data.numInputs <= bus || data.inputs[bus].numChannels <= channel) { memset(data.outputs[bus].channelBuffers32[channel], ....); data.outputs[bus].silenceFlags |= (uint64)1 << channel; } else { .... } .... } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

重大な間違いはすでにここで行われています。 ループはdata.inputsおよびdata.outputsの配列にアクセスしますが、条件式はエラーで書き込まれます。 式bus <data.numInputsは計算されますが、結果には影響しません。 したがって、 data.inputs配列の制限を超えてメモリにアクセスすることが可能です。



プログラマーの1人がこの演算子の使用を乱用し、ミスを犯すことを示すために、2つの例を特に引用しました。

その他のエラー

V567未定義の動作。 「p」変数は、シーケンスポイント間で2回使用されている間に変更されます。 mdaAmbienceProcessor.cpp 151

 void AmbienceProcessor::doProcessing (ProcessData& data) { .... ++p &= 1023; ++d1 &= 1023; ++d2 &= 1023; ++d3 &= 1023; ++d4 &= 1023; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

アナライザーは、未定義のプログラム動作につながる式を検出しました。 変数は2つのシーケンスポイント間で繰り返し使用され、その値は変化します。 結果として、そのような式の結果を予測することは不可能です。 合計で、約11のそのような場所が見つかりました。



V595 nullptrに対して検証される前に、「inputBitmap」ポインターが使用されました。 行を確認してください：409、410。cbitmapfilter.cpp 409

 bool run (bool replace) override { CBitmap* inputBitmap = getInputBitmap (); uint32_t radius = static_cast<uint32_t>(static_cast<double>( .... * inputBitmap->getPlatformBitmap()->getScaleFactor()); if (inputBitmap == nullptr || radius == UINT_MAX) return false; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

inputBitmapポインターは 、使用直後にnullptr と比較されます。 プログラマーはinputBitmapポインターとradius変数を1つの条件でチェックしたかったのですが、これは不可能です。 1つの値は別の値を使用して計算されます。 各変数を個別にチェックする必要があります。



V1004 nullptrに対して検証された後、「モジュール」ポインターが安全に使用されませんでした。 行を確認してください：76、84。audiohost.cpp 84

 void App::startAudioClient (....) { std::string error; module = VST3::Hosting::Module::create (path, error); if (!module) { std::string reason = "Could not create Module for file:"; reason += path; reason += "\nError: "; reason += error; // EditorHost::IPlatform::instance ().kill (-1, reason); } auto factory = module->getFactory (); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

以前は、 モジュールポインターがゼロの場合、 kill（）を呼び出すことで関数の実行が中断されました。 現在、この関数の呼び出しはコメント化されているため、nullポインターを逆参照するリスクがあります。



V766同じキー「0xff9b」を持つアイテムがすでに追加されています。 x11frame.cpp 51

 using VirtMap = std::unordered_map<guint, uint16_t>; const VirtMap keyMap = { {GDK_KEY_BackSpace, VKEY_BACK}, {GDK_KEY_Tab, VKEY_TAB}, {GDK_KEY_ISO_Left_Tab, VKEY_TAB}, {GDK_KEY_Clear, VKEY_CLEAR}, {GDK_KEY_Return, VKEY_RETURN}, {GDK_KEY_Pause, VKEY_PAUSE}, {GDK_KEY_Escape, VKEY_ESCAPE}, {GDK_KEY_space, VKEY_SPACE}, {GDK_KEY_KP_Next, VKEY_NEXT}, // <= {GDK_KEY_End, VKEY_END}, {GDK_KEY_Home, VKEY_HOME}, {GDK_KEY_Left, VKEY_LEFT}, {GDK_KEY_Up, VKEY_UP}, {GDK_KEY_Right, VKEY_RIGHT}, {GDK_KEY_Down, VKEY_DOWN}, {GDK_KEY_Page_Up, VKEY_PAGEUP}, {GDK_KEY_Page_Down, VKEY_PAGEDOWN}, {GDK_KEY_KP_Page_Up, VKEY_PAGEUP}, {GDK_KEY_KP_Page_Down, VKEY_PAGEDOWN}, // <= .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これは、アナライザーによって発見された明白なエラーです。 これは、プリプロセッサの出力を表示する場合にのみ表示されます。

 using VirtMap = std::unordered_map<guint, uint16_t>; const VirtMap keyMap = { {0xff08, VKEY_BACK}, {0xff09, VKEY_TAB}, {0xfe20, VKEY_TAB}, {0xff0b, VKEY_CLEAR}, {0xff0d, VKEY_RETURN}, {0xff13, VKEY_PAUSE}, {0xff1b, VKEY_ESCAPE}, {0x020, VKEY_SPACE}, {0xff9b, VKEY_NEXT}, // <= {0xff57, VKEY_END}, {0xff50, VKEY_HOME}, {0xff51, VKEY_LEFT}, {0xff52, VKEY_UP}, {0xff53, VKEY_RIGHT}, {0xff54, VKEY_DOWN}, {0xff55, VKEY_PAGEUP}, {0xff56, VKEY_PAGEDOWN}, {0xff9a, VKEY_PAGEUP}, {0xff9b, VKEY_PAGEDOWN}, // <= .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

実際、定数GDK_KEY_KP_NextとGDK_KEY_KP_PageDownの値は同じ0xff9bです。 どうすればいいかわからないだけです GDK3ライブラリから取得した定数。

テストからのいくつかの例

V571定期的なチェック。 「if（vstPlug）」条件は、行170ですでに検証されています。vsttestsuite.cpp 172

 bool VstTestBase::teardown () { if (vstPlug) { if (vstPlug) { vstPlug->activateBus (kAudio, kInput, 0, false); vstPlug->activateBus (kAudio, kOutput, 0, false); } plugProvider->releasePlugIn (vstPlug, controller); } return true; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

多くの場合、 V571診断は追加のチェックを見つけるだけですが、ここでは明らかに本当の間違いです。 ファイル内の同様のフラグメントを調べましたが、おそらく次のようなコードを修正する必要があります。

 bool VstTestBase::teardown () { if (plugProvider) // <= { if (vstPlug) { vstPlug->activateBus (kAudio, kInput, 0, false); vstPlug->activateBus (kAudio, kOutput, 0, false); } plugProvider->releasePlugIn (vstPlug, controller); } return true; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V773関数は、「paramIds」ポインターを解放せずに終了しました。 メモリリークが発生する可能性があります。 vsttestsuite.cpp 436

 bool PLUGIN_API VstScanParametersTest::run (....) { .... int32* paramIds = new int32[numParameters]; bool foundBypass = false; for (int32 i = 0; i < numParameters; ++i) { ParameterInfo paramInfo = {0}; tresult result = controller->getParameterInfo (i, paramInfo); if (result != kResultOk) { addErrorMessage (testResult, printf ("Param %03d: is missing!!!", i)); return false; // Memory Leak } int32 paramId = paramInfo.id; paramIds[i] = paramId; if (paramId < 0) { addErrorMessage (testResult, printf ("Param %03d: Invalid Id!!!", i)); return false; // Memory Leak } .... if (paramIds) delete[] paramIds; return true; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

run（）関数には、メモリリークが発生する出口ポイントが10個以上あります。 関数が最後まで実行された場合にのみ、 paramIdsポインターによってこの配列のメモリが解放されます。

コードノート

V523 「then」ステートメントは「else」ステートメントと同等です。 mdaJX10Processor.cpp 522

 void JX10Processor::noteOn (....) { .... if (!polyMode) //monophonic retriggering { voice[v].env += SILENCE + SILENCE; } else { //if (params[15] < 0.28f) //{ // voice[v].f0 = voice[v].f1 = voice[v].f2 = 0.0f; // voice[v].env = SILENCE + SILENCE; // voice[v].fenv = 0.0f; //} //else voice[v].env += SILENCE + SILENCE; //anti-glitching trick } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コードの一部にコメントを付けた後、条件ステートメントのブランチは同じアクションを実行し始めました。 これがエラーにつながるかどうかを判断するのは困難ですが、今ではチェックを取り除くことができます。 したがって、この場所はより明確に確認して書き換える価値があります。



V573初期化されていない変数「oldScrollSize」が使用されました。 変数は、それ自体を初期化するために使用されました。 cscrollview.cpp 482

 void CScrollView::setContainerSize (....) { CRect oldSize (containerSize); .... CRect oldScrollSize = vsb->getScrollSize (oldScrollSize); float oldValue = vsb->getValue (); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

アナライザーは、初期化されていない変数oldScrollSizeの潜在的な使用を検出しました 。 判明したとおり、エラーは発生しませんが、 getScrollSize（）関数の実装はひどいものです。

 CRect& getScrollSize (CRect& rect) const { rect = scrollSize; return rect; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

確かに、そのようなコードは見栄えがよかったでしょう。

 CRect oldScrollSize = vsb->getScrollSize(); .... CRect& getScrollSize () const { return scrollSize; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これらの初期化のいくつか：

• V573初期化されていない変数「oldScrollSize」が使用されました。 変数は、それ自体を初期化するために使用されました。 cscrollview.cpp 503
• V573初期化されていない変数「oldClip」が使用されました。 変数は、それ自体を初期化するために使用されました。 ctabview.cpp 359

V751パラメーター 'column'は関数本体内では使用されません。 pitchnamesdatabrowsersource.cpp 227

 void PitchNamesDataBrowserSource::dbCellTextChanged( int32_t row, int32_t column, ....) { if (pitchnames) { UString128 str (newText); if (str.getLength () == 0) pitchnames->removePitchName (0, (int16)row); else pitchnames->setPitchName (0, (int16)row, str); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

dbCellTextChanged（）関数は、関数に渡された列番号を使用しません。 エラーがあるかどうかを言うのは難しいので、プロジェクトの作成者はコードを再確認する必要があります。



V570同じ値が「lpf」変数に2回割り当てられます。 mdaComboProcessor.cpp 274

 void ComboProcessor::recalculate () { .... case 4: trim = 0.96f; lpf = filterFreq(1685.f); mix1 = -0.85f; mix2 = 0.41f; del1 = int (getSampleRate () / 6546.f); del2 = int (getSampleRate () / 3315.f); break; case 5: trim = 0.59f; lpf = lpf = filterFreq(2795.f); // <= mix1 = -0.29f; mix2 = 0.38f; del1 = int (getSampleRate () / 982.f); del2 = int (getSampleRate () / 2402.f); hpf = filterFreq(459.f); break; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コードに関する小さなコメント：コードには、変数lpfへの追加の割り当てがあります。 ほとんどの場合、これはエラーをランダムに引き起こさないタイプミスです。

おわりに

Steinberg SDKには、サンプルプラグインを含むさまざまなソースが含まれています。 見つかったエラーは、他のクローズドソース企業製品のコードステータスを反映している場合があります。



どちらのコードが良いか-オープンかクローズか-についての私の立場は非常に単純です。 コードの品質は、コードの公開性/公開性よりもプロジェクトマネージャーに依存しています。 もちろん、オープンソースは優れています。バグを報告するのは簡単です。誰かが機能を追加し、誰かがエラーを修正します。 使用可能なすべての無料ソリューションを使用し、可能であれば、有料ツールを追加する必要があります。



その他の音楽ソフトウェアのレビュー：

• パート1. MuseScore
• パート2. Audacity
• パート3. Rosegarden
• パート4。

音楽を扱うための興味深いソフトウェアを知っていて、レビューで見たい場合は、名前をメールで私に送ってください。



プロジェクトでPVS-Studioアナライザーを試すのは非常に簡単です。 ダウンロードページに進んでください。





英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Svyatoslav Razmyslov。 音楽ソフトウェアのコードの欠陥のレビューパート5. Steinberg SDK