オープンソース専用のDolphin Smalltalk 7

先日、ObjectArtsはソースを完全にオープンし、言語と開発環境Dolphin SmalltalkをオープンライセンスMITでリリースしました！ PVS-Studioコードアナライザーを使用してこのプロジェクトを検証することなく、私は通り過ぎることができませんでした。 開発者が高品質のコードを作成できることを祝福できます。 重大なエラーは見つかりませんでした。 ただし、いつものように、多くのバグと悪臭を放つコードがあります。 この記事でコードが少し良くなることを願っています。

プロジェクトについて

Dolphin Smalltalkは、Windows用の独自のSmalltalk方言の開発環境です。 主な機能は、COMやActiveXなどのネイティブウィジェットやオペレーティングシステムのサブシステムとの緊密な統合、見栄えの良いグラフィックデザインです。

長い間、Dolphin Smalltalkには2つのバージョンがありました。シェアウェア限定バージョン（コミュニティエディション）とプロフェッショナル開発用の有料パッケージです。 後者は、高度なエディターやスタンドアロンモードでの公開アプリケーションを含むすべての機能へのアクセスを提供しましたが、約400ドルの費用がかかりました。

PVS-Studio 6.00を使用して、オープンソースのDolphin Smalltalk Virtual Machineが検証されました。 以下は、静的アナライザーによる検証の結果です。 DolphinVMプロジェクトは非常に小さいという事実にもかかわらず、そのコードには依然として疑わしい場所があります。

検証結果

警告N1： V611メモリは 'new T []'演算子を使用して割り当てられましたが、 'delete'演算子を使用して解放されました。 このコードを調べることを検討してください。 「delete [] msg;」を使用する方がおそらく良いでしょう。 compiler.cpp 379

Compiler::StaticType Compiler::FindNameAsStatic(....) { .... char* msg = new char[strlen(szPrompt)+name.size()+32]; ::wsprintf(msg, szPrompt, name.c_str()); char szCaption[256]; ::LoadString(GetResLibHandle(), IDR_COMPILER, szCaption, ....); int answer = ::MessageBox(NULL, msg, szCaption, ....); delete msg; //<==?? .... } 

アナライザーは、メモリーが互換性のない方法で割り当てられ、解放されるという事実に関連するエラーを検出しました。

「new []」演算子を呼び出した後、「delete []」演算子を使用してメモリを解放する必要があります。

警告N2 ： V716 returnステートメントでの疑わしい型変換：BOOLを返しましたが、関数は実際にHRESULTを返します。 idolphinstart.cpp 78

 #define STDMETHODIMP HRESULT STDMETHODCALLTYPE STDMETHODIMP CDolphinSmalltalk::GetVersionInfo(LPVOID pvi) { extern BOOL __stdcall GetVersionInfo(VS_FIXEDFILEINFO* ....); return ::GetVersionInfo(static_cast<VS_FIXEDFILEINFO*>(pvi)); } 

このコードスニペットでは、タイプ「BOOL」がタイプ「HRESULT」に暗黙的にキャストされます。 このような操作は、C ++言語の観点からは十分に受け入れられますが、実際的な意味はありません。 HRESULT型はステータスの保存を目的としており、かなり複雑な形式を持ち、BOOL型とは関係ありません。

警告N3 ： V701 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインタ「elems」が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 compiler.cpp 2922

 POTE Compiler::ParseByteArray() { NextToken(); while (m_ok && !ThisTokenIsClosing()) { if (elemcount>=maxelemcount) { _ASSERTE(maxelemcount > 0); maxelemcount *= 2; elems = (BYTE*)realloc(elems, maxelemcount*sizeof(BYTE)); } .... } .... } 

このコードは潜在的に危険です。realloc（）関数の結果を別の変数に保存することをお勧めします。 realloc（）関数は、メモリブロックのサイズを変更します。 現在メモリブロックのサイズを変更できない場合、関数はNULLポインターを返します。 主な問題は、「ptr = realloc（ptr、...）」という形式の構造を使用すると、このデータブロックへのptrポインターが失われる可能性があることです。

同様の危険な場所：

• V701 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインター 'm_pAllocations'が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 alloc.cpp 436
• V701 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインタ「pUnmarked」が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 gc.cpp 217

警告N4 ： V547式 'i> = 0'は常に真です。 符号なしの型の値は常に> = 0です。compact.cpp 35

 // Answer the index of the last occuppied OT entry unsigned __stdcall ObjectMemory::lastOTEntry() { HARDASSERT(m_pOT); // HARDASSERT(m_nInCritSection > 0); unsigned i = m_nOTSize-1; const OTE* pOT = m_pOT; while (pOT[i].isFree()) { ASSERT(i >= 0); i--; } return i; } 

ほとんどの場合、ここにはエラーはありませんが、コードは疑わしいです。 配列の要素は、いずれかの要素のisFree（）関数がfalseを返すまで順番に検索されます。 ここで間違っているのはASSERTです。 彼は本当に何もチェックしません。 変数 'i'は符号なしです。つまり、常に0以上です。

符号なし型との別の比較 '> = 0'：

• V547式は常に真です。 符号なしの型の値は常に> = 0です。loadimage.cpp 343

警告N5 ： V730クラスのすべてのメンバーがコンストラクター内で初期化されるわけではありません。 検査を検討してください：m_dwSize。 imagefilemapping.h 13

 class ImageFileMapping { HANDLE m_hFile; HANDLE m_hMapping; LPVOID m_pData; DWORD m_dwSize; public: ImageFileMapping() : m_hFile(0), m_hMapping(0), m_pData(NULL){} ~ImageFileMapping() { Close(); } .... }; 

潜在的に危険なコードの別のケース。 ImageFileMappingクラスには4つのフィールドしか含まれていませんが、コンストラクターで初期値が割り当てられるのはそのうち3つだけです。 メンバー 'm_dwSize'は初期化されていないままです。

これは、配列へのポインタがまだゼロの場合、クラスが「サイズ」で動作しない場合のかなり一般的な方法です。 ただし、間違いを犯すのは非常に簡単なので、クラスのすべてのメンバーを初期化することをお勧めします。

同様のクラス：

• V730クラスのすべてのメンバーがコンストラクター内で初期化されるわけではありません。 検査を検討してください：m_flags、m_oopWorkspacePools、m_context、m_compiledMethodClass。 compiler.cpp 84
• V730クラスのすべてのメンバーがコンストラクター内で初期化されるわけではありません。 検査を検討してください：m_tokenType、m_integer、tp、m_cc、m_base。 lexer.cpp 40

警告N6 ： V665このコンテキストでは、「＃pragma warning（default：X）」の使用法が間違っている可能性があります。 代わりに「#pragma warning（push / pop）」を使用する必要があります。 行を確認：99、101。compact.cpp 101

 // Perform a compacting GC size_t ObjectMemory::compact() { .... #pragma warning (disable : 4127) while(true) #pragma warning (default : 4127) .... } 

プログラマーは、「プラグマ警告（デフォルト：X）」ディレクティブの後、「プラグマ警告（無効：X）」を使用して以前に無効にされた警告が再び動作を開始すると信じています。 そうではありません。 「プラグマ警告（デフォルト：X）」ディレクティブは、番号「X」の警告をデフォルトの状態に設定します。 これは同じことからはほど遠い。

コードの正しいバージョン：

 size_t ObjectMemory::compact() { .... #pragma warning(push) #pragma warning (disable : 4127) while(true) #pragma warning(pop) .... } 

このトピックに関する良い記事：「 だから、Visual C ++でこの警告をかき消したい 」

そのような場所の全リスト：

• V665おそらく、このコンテキストでは「#pragma warning（default：X）」の使用法が正しくありません。 代わりに「#pragma warning（push / pop）」を使用する必要があります。 行を確認してください：244、246。expire.cpp 246
• V665おそらく、このコンテキストでは「#pragma warning（default：X）」の使用法が正しくありません。 代わりに「#pragma warning（push / pop）」を使用する必要があります。 行を確認してください：226、241。expire.cpp 241
• V665おそらく、このコンテキストでは「#pragma warning（default：X）」の使用法が正しくありません。 代わりに「#pragma warning（push / pop）」を使用する必要があります。 チェック行：126、128。interfac.cpp 128
• V665おそらく、このコンテキストでは「#pragma warning（default：X）」の使用法が正しくありません。 代わりに「#pragma warning（push / pop）」を使用する必要があります。 行を確認してください：385、387。interprt.cpp 387

警告N7 ： V576形式が正しくありません 。 'wsprintfA'関数の4番目の実引数を確認することを検討してください。 ポインターの値を出力するには、「％p」を使用する必要があります。 interfac.cpp 679

 inline DWORD __stdcall Interpreter::GenericCallbackMain(SMALLINTEGER id, BYTE* lpArgs) { .... #ifdef _DEBUG { char buf[128]; wsprintf(buf, "WARNING: .... (%d, %x)\n", id, lpArgs); WarningWithStackTrace(buf); } #endif .... } 

多くの場合、 '％x'修飾子を使用してポインターの値を出力しようとします。

このコードは、ポインターのサイズが 'int'型のサイズと一致するシステムでのみ機能するため、誤りです。 また、たとえば、Win64では、このコードは既に「ptr」ポインターの下部のみを出力します。 この場合、修飾子 '％p'を使用する必要があります。

警告N8 ： V547式 'ch> 127'は常にfalseです。 char型の値の範囲：[-128、127]。 decode.cpp 55

 ostream& operator<<(ostream& stream, const VariantCharOTE* oteChars) { .... char ch = string->m_characters[i]; //if (ch = '\0') break; if (ch < 32 || ch > 127) //<== { static char hexChars[16+1] = "0123456789ABCDEF"; .... } .... } 

デフォルトでは、タイプ「char」の値の範囲は[-127; 127]です。 コンパイルフラグ/ Jを使用すると、範囲[0; 255]を使用するようコンパイラーに指示できます。 ただし、このソースファイルをコンパイルする場合、このようなフラグは指定されていないため、「ch> 127」をチェックしても意味がありません。

警告N9 ： V688 'prev'関数の引数は、クラスメンバの1つと同じ名前を持っているため、混乱を招く可能性があります。 thrdcall.h 126

 void LinkAfter(T* prev) { T* pThis = static_cast<T*>(this); this->next = prev->next; if (this->next) this->next->prev = pThis; this->prev = prev; prev->next = pThis; } 

ほとんどの場合、この関数に間違いはありませんが、クラス関数とクラスメンバーのパラメーターを同じ名前で呼び出すことは、コードを記述するのにあまり良いスタイルではありません。 これにより、タイプミスが発生する可能性があり、そのため、間違った変数の値が使用または変更されます。

警告N10 ： V601 「false」値は暗黙的に整数型にキャストされます。 compiler.cpp 1940

 int Compiler::ParseUnaryContinuation(...., int textPosition) { int continuationPointer = m_codePointer; MaybePatchLiteralMessage(); while (m_ok && (ThisToken()==NameConst)) { int specialCase=false; //<== .... if (!specialCase) //<== { int sendIP = GenMessage(ThisTokenText(), 0, textPosition); AddTextMap(sendIP, textPosition, ThisTokenRange().m_stop); } .... } .... } 

この場合、この警告は助言です。 どこでも変数 'specialCase'が論理変数のように機能する場合、これには標準型 'bool'を使用することをお勧めします。

おわりに

別のプロジェクトが、テストしたオープンプロジェクトのリストに追加されました。

このような記事を準備することにより、静的アナライザーが提供するすべての警告からはほど遠い情報を引用しています。 したがって、プロジェクトの作成者が独自に分析を実行し、アナライザーによって発行されたすべてのメッセージを調査することをお勧めします。

そしていつものように、アナライザーの価値は単一のチェックではなく、通常の使用にあることを読者に思い出させます。


英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Svyatoslav Razmyslov。 Dolphin Smalltalk 7のソースコードを公開するためのオマージュ 。