Geekly Articles each Day

吐き出し、チェックされたVisual C ++ 2013ライブラリ（更新3）

PVS-StudioおよびVisual Studio 2013 Visual Studio 2013に含まれているライブラリを確認するように求められました。特に注目すべき点は見つかりませんでした。 わずかな小さな間違いと欠点のみ。 興味深い記事を作成することはできませんが、私が気づいた欠点については引き続き説明します。 これにより、ライブラリが少し改善され、作成者がより徹底的なチェックを行うことをお勧めします。 ライブラリを構築するためのプロジェクトファイルがありません。 だから何とかファイルをチェックしましたが、多くのことをスキップすることができました。

これは、Visual C ++に含まれるライブラリのチェックに関する2番目の記事です。 以前のチェックの結果は、記事「 Visual C ++ 2012ライブラリでエラーが検出されました」で確認できます 。

ライブラリ全体を確認できません。 私は非常に不器用に行動しました。 新しいプロジェクトには、フォルダー「crt \ src」と「atlmfc \ src」に含まれるすべてのファイルが含まれています。 さらに、新しいtest.cppファイルを作成しました。このファイルには、標準ライブラリ（ベクター、マップ、セットなど）に関連するすべてのヘッダーファイルが含まれています。

その後、プロジェクトの設定を少し思い起こして、ファイルの約80％がコンパイルされることを実現しました。 これで十分だと思います。 ファイルがコンパイルされない場合でも、完全ではないとしても、ほとんどの場合PVS-Studioによってチェックされます。

この記事が図書館の開発者にとって興味深いものであれば、彼らはより徹底的な分析を行うことができると思います。 アセンブリがエキゾチックな方法で実行されたとしても、今では問題になりません。 コンパイラー起動追跡メカニズムを使用できます。

検証には、 PVS-Studioバージョン5.19を使用しました。 Visual Studio 2013（アップデート3）に含まれるC / C ++ライブラリのソースコードを確認しました。

チェック結果


Visual Studio 2012の以前のバージョンで見つかったいくつかの欠点に遭遇しました。たとえば、proj（）関数はまだ奇妙に見えますが、〜single_link_registry（）デストラクタは危険です。 しかし、繰り返すことは面白くありません。 何か新しいものを見つけてみましょう。

誤ったインデックスチェック


void _Initialize_order_node(...., size_t _Index, ....) { if (_Index < 0) { throw std::invalid_argument("_Index"); } .... }

PVS-Studio 警告 ： V547式 '_Index <0'は常にfalseです。 符号なしの型の値が<0になることはありません。

引数は_Indexであり、符号なしの型を持ちます。 したがって、検証は意味がありません。 例外は決してスローされません。 これは間違いではなく、余分なコードだと思います。

フォーマットエラー


 int _tfpecode; /* float point exception code */ void __cdecl _print_tiddata1 ( _ptiddata ptd ) { .... printf("\t_gmtimebuf = %p\n", ptd->_gmtimebuf); printf("\t_initaddr = %p\n", ptd->_initaddr); printf("\t_initarg = %p\n", ptd->_initarg); printf("\t_pxcptacttab = %p\n", ptd->_pxcptacttab); printf("\t_tpxcptinfoptrs = %p\n", ptd->_tpxcptinfoptrs); printf("\t_tfpecode = %p\n\n", ptd->_tfpecode); .... }

警告PVS-Studio： V576の形式が正しくありません 。 'printf'関数の2番目の実引数を確認することを検討してください。 ポインターは引数として期待されています。 tidprint.c 133

ここでは、最後の行の効果を扱っています 。 同じタイプの行のブロックの最後でエラーが発生しました。 どこでもポインタの値を印刷する必要があります。 しかし、最後に、変数「_tfpecode」はポインターではなく、単なる整数値です。 それは書かれるべきです：
 printf("\t_tfpecode = %i\n\n", ptd->_tfpecode);

奇妙な繰り返し計算


 unsigned int SchedulerProxy::AdjustAllocationIncrease(....) const { .... unsigned int remainingConcurrency = m_maxConcurrency - m_currentConcurrency; remainingConcurrency = m_maxConcurrency - m_currentConcurrency; .... }

PVS-Studio警告： V519 「remainingConcurrency」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：1136、1137。schedulerproxy.cpp 1137

変数には、同じ式の結果が2回割り当てられます。 このコードは冗長であり、リファクタリングの失敗が原因である可能性が高いです。

タイプミスの疑い


 double HillClimbing::CalculateThroughputSlope(....) { .... MeasuredHistory * lastHistory = GetHistory(fromSetting); MeasuredHistory * currentHistory = GetHistory(toSetting); .... double varianceOfcurrentHistory = currentHistory->VarianceMean(); double varianceOflastHistory = currentHistory->VarianceMean(); .... }

PVS-Studio警告： V656変数 'varianceOfcurrentHistory'、 'varianceOflastHistory'は、同じ関数の呼び出しによって初期化されます。 おそらくエラーまたは最適化されていないコードです。 「currentHistory-> VarianceMean（）」式の検査を検討してください。 行を確認してください：412、413。hillclimbing.cpp 413

変数varianceOfcurrentHistoryとvarianceOflastHistoryに同じ値が割り当てられているのは疑わしいです。 次のようにvarianceOflastHistoryを初期化することは論理的です：
 double varianceOflastHistory = varianceOfcurrentHistory;

さらに、「lastHistory」へのポインタがまだあります。 コードにタイプミスが含まれていることを提案させてください。 おそらく、コードは次のようになります。
 double varianceOfcurrentHistory = currentHistory->VarianceMean(); double varianceOflastHistory = lastHistory->VarianceMean();

そして、これはまさにタイプミスです


 BOOL CPropertySheet::SetActivePage(CPropertyPage* pPage) { ASSERT_VALID(this); ENSURE_VALID(pPage); ASSERT_KINDOF(CPropertyPage, pPage); int nPage = GetPageIndex(pPage); ASSERT(pPage >= 0); return SetActivePage(nPage); }

PVS-Studio警告： V503これは無意味な比較です：pointer> =0。dlgprop.cpp 1206

ポインター値がゼロ以上であることを確認するのは奇妙です。 これはタイプミスであり、実際、変数「nPage」を確認したかったのです。
 int nPage = GetPageIndex(pPage); ASSERT(nPage >= 0);

もちろん、これは単なるアサートであり、エラーがマイナスの結果をもたらすことはありません。 それでも、これは間違いです。

条件に関係なく同じアクション


 void CMFCVisualManager::OnDrawTasksGroupCaption(....) { .... if (pGroup->m_bIsSpecial) { if (!pGroup->m_bIsCollapsed) { CMenuImages::Draw(pDC, CMenuImages::IdArrowUp, rectButton.TopLeft()); } else { CMenuImages::Draw(pDC, CMenuImages::IdArrowDown, rectButton.TopLeft()); } } else { if (!pGroup->m_bIsCollapsed) { CMenuImages::Draw(pDC, CMenuImages::IdArrowUp, rectButton.TopLeft()); } else { CMenuImages::Draw(pDC, CMenuImages::IdArrowDown, rectButton.TopLeft()); } } .... }

PVS-Studio警告： V523 「then」ステートメントは「else」ステートメントと同等です。 afxvisualmanager.cpp 2118

条件に関係なく（pGroup-> m_bIsSpecial）、同じアクションが実行されます。 これは奇妙です。

誤ったポート番号の検証


 typedef WORD ATL_URL_PORT; ATL_URL_PORT m_nPortNumber; inline BOOL Parse(_In_z_ LPCTSTR lpszUrl) { .... m_nPortNumber = (ATL_URL_PORT) _ttoi(tmpBuf); if (m_nPortNumber < 0) goto error; .... }

PVS-Studio 警告 ： V547式 'm_nPortNumber <0'は常にfalseです。 符号なしの型の値が<0になることはありませんatlutil.h 2773

変数 'm_nPortNumber'には、符号なしのWORDタイプがあります。

仮想デストラクタなし


 class CDataSourceControl { .... ~CDataSourceControl(); .... virtual IUnknown* GetCursor(); virtual void BindProp(....); virtual void BindProp(....); .... } CDataSourceControl* m_pDataSourceControl; COleControlSite::~COleControlSite() { .... delete m_pDataSourceControl; .... }

警告PVS_Studio： V599 「CDataSourceControl」クラスには仮想関数が含まれていますが、デストラクタは仮想デストラクタとして宣言されていません。 occsite.cpp 77

CDataSourceControlクラスには仮想メソッドが含まれていますが、デストラクタは仮想ではありません。 これは危険です。 クラスXがCDataSourceControlクラスから継承されている場合、基本クラスへのポインターを使用して、タイプXのオブジェクトを破棄することはできません。

不完全なコード


 BOOL CMFCWindowsManagerDialog::OnHelpInfo(HELPINFO* pHelpInfo) { pHelpInfo->iCtrlId; CWnd* pParentFrame = AfxGetMainWnd(); pParentFrame->SendMessage(AFX_WM_WINDOW_HELP, 0, (LPARAM) this); return FALSE; }

警告PVS_Studio： V607所有者なしの式 'pHelpInfo-> iCtrlId'。 afxwindowsmanagerdialog.cpp 472

「pHelpInfo-> iCtrlId;」とは何ですか？ それはどういう意味ですか？

疑わしい二重初期化


 CMFCStatusBar::CMFCStatusBar() { m_hFont = NULL; // setup correct margins m_cxRightBorder = m_cxDefaultGap; //<<-- m_cxSizeBox = 0; m_cxLeftBorder = 4; m_cyTopBorder = 2; m_cyBottomBorder = 0; m_cxRightBorder = 0; //<<-- .... }

PVS-Studio警告： V519 「m_cxRightBorder」変数には、連続して2回値が割り当てられます。 おそらくこれは間違いです。 行をチェック：74、80。afxstatusbar.cpp 80

最初に、別の変数の値が変数「m_cxRightBorder」に書き込まれます。 そして、その値は突然ゼロで上書きされます。

疑わしいステータスの確認


 #define S_OK ((HRESULT)0L) #define E_NOINTERFACE _HRESULT_TYPEDEF_(0x80004002L) HRESULT GetDocument(IHTMLDocument2** ppDoc) const { const T* pT = static_cast<const T*>(this); return pT->GetDHtmlDocument(ppDoc) ? S_OK : E_NOINTERFACE; } HRESULT GetEvent(IHTMLEventObj **ppEventObj) const { .... if (GetDocument(&sphtmlDoc)) .... }

PVS-Studio警告： V545このような「if」演算子の条件式は、HRESULT型の値「GetDocument（＆sphtmlDoc）」に対して正しくありません。 代わりにSUCCEEDEDまたはFAILEDマクロを使用する必要があります。 afxhtml.h 593

コードの設計は、その作業ロジックに対応していない場合があります。 条件 'GetDocument（...）'が真の場合、ドキュメントを「取得」できたようです。 しかし、実際にはそうではありません。 GetDocument（）関数は、HRESULT型の値を返します。 このタイプでは、逆のことが当てはまります。 たとえば、ステータスS_OKは0としてエンコードされ、ステータスE_NOINTERFACEは0x80004002Lとしてエンコードされます。 タイプHRESULTの値を確認するには、特別なマクロSUCCEEDED、FAILEDを使用する必要があります。

ここにエラーがあるかどうかはわかりませんが、このコードはわかりにくいので確認する必要があります。

マクロMAKE_HRESULTの引数が無効です


 #define MAKE_HRESULT(sev,fac,code) \ ((HRESULT) \ (((unsigned long)(sev)<<31) | \ ((unsigned long)(fac)<<16) | \ ((unsigned long)(code))) ) ATLINLINE ATLAPI AtlSetErrorInfo(....) { .... hRes = MAKE_HRESULT(3, FACILITY_ITF, nID); .... }

PVS-Studio 警告 ： V673 「（unsigned long）（3）<< 31」式は6442450944に評価されます。値を保存するには33ビットが必要ですが、式は「32」のみを保持できる「unsigned」タイプに評価されますビット。 atlcom.h 6650

すべてが正しく機能しますが、間違いがあります。 説明します。

関数は、HRESULT型の変数にエラー情報を生成する必要があります。 これを行うには、マクロMAKE_HRESULTを使用します。 しかし、それは誤って使用されます。 プログラマーは、最初のパラメーター「重大度」が0から3までの通路にあると考えました。どうやら、これをGetLastError（）/ SetLastError（）関数を操作するときに使用されるエラーコードを生成する方法と混同したようです。

MAKE_HRESULTマクロは、最初の引数として0（成功）または1（失敗）のみを受け入れます。 この問題は、CodeGuruのWebサイトフォーラムで詳細に説明されています：警告！ MAKE_HRESULTマクロは機能しません。

数値3が最初の実引数として使用されるため、オーバーフローが発生します。 番号3は1に変わります。このランダム性のため、エラーはプログラムに影響しません。

条件が常に真であるASSERT


ASSERTの条件が次のように見える状況はかなりあります：（X> = 0）。 この場合、変数Xは符号なし整数型として宣言されます。 条件は常に真であることがわかります。

場合によっては、そのようなASSERTの存在は合理的です。 突然、リファクタリングプロセス中に変数が重要になり、アルゴリズムは負の数を処理する準備ができていません。 ただし、それらのいくつかの存在はほとんど無意味です。 コードから削除するか、別の便利なチェックに置き換える必要があります。 したがって、私はそれらを記事で言及することにしました。

例を考えてみましょう：
 DWORD m_oversubscribeCount; void ExternalContextBase::Oversubscribe(....) { if (beginOversubscription) { ASSERT(m_oversubscribeCount >= 0); ++m_oversubscribeCount; } .... }

PVS-Studioの警告 ： V547式 'm_oversubscribeCount> = 0'は常にtrueです。 符号なしの型の値は常に> = 0です。externalcontextbase.cpp 204

残りの警告は単なるリストです。

余分なキャスト


不要なだけでなく、値を台無しにする可能性がある明示的な型変換がいくつかありました。

最初の例を考えてみましょう：
 size_t __cdecl strnlen(const char *str, size_t maxsize); size_t __cdecl _mbstrnlen_l(const char *s, size_t sizeInBytes, _locale_t plocinfo) { .... if ( _loc_update.GetLocaleT()->locinfo->mb_cur_max == 1 ) /* handle single byte character sets */ return (int)strnlen(s, sizeInBytes); .... }

PVS-Studio 警告 ： V220型キャストの不審なシーケンス：memsize-> 32ビット整数-> memsize。 キャストされる値： 'strnlen（s、sizeInBytes）'。 _mbslen_s.c 67

strnlen（）関数は、 'size_t'型の値を返します。 その後、突然明示的に 'int'型になります。 その後、値は暗黙的に再びsize_t型に展開されます。

このコードには、潜在的な64ビットエラーが含まれています。 突然、64ビットプログラムで_mbstrnlen_l（）関数を使用して非常に長い行の文字数をカウントしたい場合、間違った結果が返されます。

この明示的なキャストは偶然にコードに残っていたため、単に削除する必要があるように思えます。

2番目の例を考えてみましょう。
 WINBASEAPI SIZE_T WINAPI GlobalSize (_In_ HGLOBAL hMem); inline void __cdecl memcpy_s( _Out_writes_bytes_to_(_S1max,_N) void *_S1, _In_ size_t _S1max, _In_reads_bytes_(_N) const void *_S2, _In_ size_t _N); AFX_STATIC HGLOBAL AFXAPI _AfxCopyGlobalMemory(....) { ULONG_PTR nSize = ::GlobalSize(hSource); .... Checked::memcpy_s(lpDest, (ULONG)::GlobalSize(hDest), lpSource, (ULONG)nSize); .... }

PVS-Studio警告：V220疑わしい型キャスト：memsize-> 32ビット整数-> memsize。 キャストされる値： 'nSize'。 olemisc.cpp 684。

GlobalSize（）関数は、タイプSIZE_Tを返します。 memcpy_s（）の引数もsize_t型です。

では、なぜ「（ULONG）:: GlobalSize（hDest）」のような明示的なキャストが行われたのですか？

4Gbより大きいバッファーを使用する場合、memcpy_s（）関数は配列の一部のみをコピーします。

さらに疑わしい型変換がいくつかあります。

検証前に使用


 CMFCPopupMenu* CMFCCustomizeButton::CreatePopupMenu() { .... if (m_pWndParentToolbar->IsLocked()) { pMenu->GetMenuBar()->m_pRelatedToolbar = m_pWndParentToolbar; } pMenu->m_bRightAlign = m_bMenuRightAlign && (m_pWndParentToolbar->GetExStyle() & WS_EX_LAYOUTRTL) == 0; BOOL bIsLocked = (m_pWndParentToolbar == NULL || m_pWndParentToolbar->IsLocked()); .... }

PVS-Studio 警告 ： V595 nullptrに対して検証される前に、「m_pWndParentToolbar」ポインターが使用されました。 行を確認してください：192、199。afxcustomizebutton.cpp 192

最初は、式「m_pWndParentToolbar-> IsLocked（）」でポインター「m_pWndParentToolbar」が逆参照されます。 そして、ゼロと等しいかどうかがチェックされます： 'm_pWndParentToolbar == NULL'。

このようなコードは危険であり、その理由を説明する価値はないと思います。

別のそのような場合：
 void COleControlSite::BindDefaultProperty(....) { .... if (pDSCWnd != NULL) { .... m_pDSCSite = pDSCWnd->m_pCtrlSite; .... m_pDSCSite->m_pDataSourceControl->BindProp(this, TRUE); if (m_pDSCSite != NULL) m_pDSCSite->m_pDataSourceControl->BindColumns(); } .... }

PVS-Studio警告：V595 nullptrに対して検証される前に、「m_pDSCSite」ポインターが使用されました。 チェック行：1528、1528。occsite.cpp 1528

追加変数


余分な変数は間違いではありません。 しかし、余分な、彼らは余分であり、それらを取り除く価値があります。 例：
 int GetImageCount() const { CRect rectImage(m_Params.m_rectImage); if (m_Bitmap.GetCount() == 1) { HBITMAP hBmp = m_Bitmap.GetImageWell(); BITMAP bmp; if (::GetObject(hBmp, sizeof(BITMAP), &bmp) == sizeof(BITMAP)) { return bmp.bmHeight / m_Params.m_rectImage.Height(); } return 0; } return m_Bitmap.GetCount(); }

PVS-Studio警告：「CRect」タイプのV808 「rectImage」オブジェクトが作成されましたが、使用されませんでした。 afxcontrolrenderer.h 89

四角形「rectImage」が作成されますが、後で使用されません。 プログラム内の余分な行と、デバッグバージョンの作業中のいくつかの余分なサイクル。

見つかった不要な変数のリストを示します： vs2003_V808.txt

ささいなこと


非常に多くの警告は、エラーではなく、プログラミングスタイルの失敗に起因する可能性があります。 Visual C ++ライブラリのソースコードは、ロールモデルとして他のプログラマーに役立つはずです。 したがって、彼らに悪いことを教えないでください。

改善できるいくつかのフラグメントをリストします。

TRUEとの危険な比較


 _PHNDLR __cdecl signal(int signum, _PHNDLR sigact) { .... if ( SetConsoleCtrlHandler(ctrlevent_capture, TRUE) == TRUE ) .... }

PVS-Studio警告： V676 BOOL型の変数をTRUEと比較するのは正しくありません。 winsig.c 255

MSDNを含むあらゆる場所で、何かをTRUEと比較するのは良くないと言われています。 この関数は0以外の値を返すことができ、これは真と見なされます。 TRUE、これは1です。比較するのは常に正しいです：Foo（）！= FALSE。

同様のチェックはここにあります：

インクリメント


 void _To_array( ::Concurrency::details::_Dynamic_array<_EType>& _Array) { _LockHolder _Lock(_M_lock); _M_iteratorCount++; for(_LinkRegistry::iterator _Link = _M_links.begin(); *_Link != NULL; _Link++) { _Array._Push_back(*_Link); } }

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

もちろん些細なことですが、どこでも++イテレータを使用することをお勧めします。 可能であれば、他の人に教えるための良いスタイルを示すために接頭辞演算子を使用するのが最善です。

ご注意 このトピックに関する注：ライブラリの作成者が、増分で運動する価値があると考えている場合は、この場所のリストをvs2003_V803.txtに示します。

誤った警告レベルの回復


 #pragma warning (disable : 4311) SetClassLongPtr(m_hWnd, GCLP_HBRBACKGROUND, PtrToLong(reinterpret_cast<void*>( ::GetSysColorBrush(COLOR_BTNFACE)))); #pragma warning (default : 4311)

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

前の警告状態に戻る正しい方法は、「＃pragma warning（push [、n]）」および「#pragma warning（pop）」を使用することです。

その他の場所： vs2003_V665.txt

検証（this == NULL）


ジャンルの古典：
 _AFXWIN_INLINE CWnd::operator HWND() const { return this == NULL ? NULL : m_hWnd; }

PVS-Studio 警告 ： V704 'this == 0'式は避ける必要があります-'this 'ポインタがNULLになることはないため、この式は新しいコンパイラでは常にfalseです afxwin2.inl 19

残念ながら、これは非常に一般的なパターンです。 特にMFCで。 しかし、そのようなデザインを使用することから人々を徐々に引き離し、良い例を設定することは価値があります。

なぜこれが悪いのかまだわからない読者のために、 V704診断のドキュメントに精通することを提案します。 そこにはすべてが詳細に説明されています。

演算子HWND（）を修正する方法がないことを理解しています。 下位互換性が重要です。 しかし、突然どこかで簡単にできます。 そのようなチェックのリスト： vs2003_V704.txt

おわりに


ご覧のとおり、かなり大きな記事が見つかりました。 しかし、実際には、重要なものは発見されていません。 Visual C ++ライブラリコードは、独自の品質でデバッグされています。

この記事が将来的にVisual C ++ライブラリを少し良くするのに役立ってくれたら嬉しいです。 チェックが不適切に実行されたことを改めて指摘します。 Visual C ++ライブラリの開発者は、ライブラリを構築するためのスクリプト/プロジェクトがあるため、これをより効率的に行うことができます。 何か問題があれば、私は助ける準備ができています-サポートで私たちに書いてください。

PS「 ゲームを遊ぼう 」の投稿を見逃した人のために、あなたの注意力をチェックし、提案されたテストに合格することをお勧めします。 同時に、なぜテストが時間通りに行われるのかという質問に答えたいと思います。 これは、PVS-Studioが検出したエラーが15秒で目で発見されたと主張する人々のトローリングです。

この記事は英語です。


この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 Visual C ++ 2013ライブラリのSliphodチェック（更新3） 。

記事を読んで質問がありますか？
多くの場合、記事には同じ質問が寄せられます。 ここでそれらに対する回答を収集しました： PVS-StudioおよびCppCatバージョン2014に関する記事の読者からの質問への回答 。 リストをご覧ください。

Source: https://habr.com/ru/post/J240143/

More articles:


All Articles