From ae2151de842c2a69614bc88882187b81f4cc9b1b Mon Sep 17 00:00:00 2001 From: Jovibor Date: Wed, 24 May 2023 16:59:14 +1000 Subject: [PATCH] Avoid double file loading in the OnOpenDocument. Removed unused old stuff from CFileLoader. Initialize m_stFileInfo.fHasIAT. g_mapLibpeErrors from std::wstring to std::wstring_view. --- .clang-tidy | 1 + Pepper/CFileLoader.cpp | 98 ++++++++++++++++++++++-------------------- Pepper/CFileLoader.h | 24 +++++------ Pepper/CPepperDoc.cpp | 21 ++++----- Pepper/CPepperDoc.h | 2 +- Pepper/Utility.ixx | 18 ++++---- 6 files changed, 84 insertions(+), 80 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index f519397..65577da 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -8,6 +8,7 @@ Checks: '*, -llvm-namespace-comment, -llvm-qualified-auto, -misc-non-private-member-variables-in-classes, +-misc-no-recursion, -misc-use-after-move, -modernize-use-trailing-return-type, -modernize-avoid-c-arrays, diff --git a/Pepper/CFileLoader.cpp b/Pepper/CFileLoader.cpp index 3745930..2a9021b 100644 --- a/Pepper/CFileLoader.cpp +++ b/Pepper/CFileLoader.cpp @@ -5,44 +5,20 @@ * Pepper is a PE32 (x86) and PE32+ (x64) binares viewer/editor. * ****************************************************************************************************/ #include "stdafx.h" +#include "res/resource.h" #include "CFileLoader.h" #include "CPepperDoc.h" -#include "res/resource.h" #include import Utility; -void CFileLoader::CreateHexCtrlWnd() +auto CFileLoader::GetData()const->std::span { - m_pHex->Create(m_hcs); - - const auto hWndHex = m_pHex->GetWindowHandle(EHexWnd::WND_MAIN); - const auto iWidthActual = m_pHex->GetActualWidth() + GetSystemMetrics(SM_CXVSCROLL); - CRect rcHex(0, 0, iWidthActual, iWidthActual); //Square window. - AdjustWindowRectEx(rcHex, m_dwStyle, FALSE, m_dwExStyle); - const auto iWidth = rcHex.Width(); - const auto iHeight = rcHex.Height() - rcHex.Height() / 3; - const auto iPosX = GetSystemMetrics(SM_CXSCREEN) / 2 - iWidth / 2; - const auto iPosY = GetSystemMetrics(SM_CYSCREEN) / 2 - iHeight / 2; - ::SetWindowPos(hWndHex, nullptr, iPosX, iPosY, iWidth, iHeight, SWP_SHOWWINDOW); - - const auto hIconSmall = static_cast(LoadImageW(AfxGetInstanceHandle(), MAKEINTRESOURCEW(IDI_HEXCTRL_LOGO), IMAGE_ICON, 0, 0, 0)); - const auto hIconBig = static_cast(LoadImageW(AfxGetInstanceHandle(), MAKEINTRESOURCEW(IDI_HEXCTRL_LOGO), IMAGE_ICON, 96, 96, 0)); - if (hIconSmall != nullptr) { - ::SendMessageW(m_pHex->GetWindowHandle(EHexWnd::WND_MAIN), WM_SETICON, ICON_SMALL, reinterpret_cast(hIconSmall)); - ::SendMessageW(m_pHex->GetWindowHandle(EHexWnd::WND_MAIN), WM_SETICON, ICON_BIG, reinterpret_cast(hIconBig)); + if (!IsLoaded()) { + return { }; } -} - -bool CFileLoader::Flush() -{ - if (!IsLoaded()) - return false; - if (IsModified()) - FlushViewOfFile(m_lpBase, 0); - - return false; + return { static_cast(m_lpBase), static_cast(m_stFileSize.QuadPart) }; } HRESULT CFileLoader::LoadFile(LPCWSTR lpszFileName, CPepperDoc* pDoc) @@ -72,8 +48,8 @@ HRESULT CFileLoader::LoadFile(LPCWSTR lpszFileName, CPepperDoc* pDoc) MessageBoxW(L"CreateFileMappingW call in FileLoader::LoadFile failed.", L"Error", MB_ICONERROR); return E_ABORT; } - m_fLoaded = true; + m_fLoaded = true; m_lpBase = MapViewOfFile(m_hMapObject, fWritable ? FILE_MAP_WRITE : FILE_MAP_READ, 0, 0, 0); if (!CWnd::CreateEx(0, AfxRegisterWndClass(0), nullptr, 0, 0, 0, 0, 0, HWND_MESSAGE, nullptr)) @@ -92,8 +68,9 @@ HRESULT CFileLoader::LoadFile(LPCWSTR lpszFileName, CPepperDoc* pDoc) HRESULT CFileLoader::ShowOffsetInWholeFile(ULONGLONG ullOffset, ULONGLONG ullSelSize, IHexCtrl* pHexCtrl) { if (!pHexCtrl) { - if (!m_pHex->IsCreated()) + if (!m_pHex->IsCreated()) { CreateHexCtrlWnd(); + } pHexCtrl = m_pHex.get(); } @@ -102,7 +79,7 @@ HRESULT CFileLoader::ShowOffsetInWholeFile(ULONGLONG ullOffset, ULONGLONG ullSel if (const auto iter = std::find_if(m_vecCheck.begin(), m_vecCheck.end(), [pHexCtrl](const HEXTODATACHECK& ref) { return ref.pHexCtrl == pHexCtrl; }); iter == m_vecCheck.end() || !iter->fWhole) { m_hds.fMutable = m_pMainDoc->IsEditMode(); - m_hds.spnData = { static_cast(m_lpBase), static_cast(m_stFileSize.QuadPart) }; + m_hds.spnData = GetData(); pHexCtrl->SetData(m_hds); if (iter == m_vecCheck.end()) { @@ -114,16 +91,16 @@ HRESULT CFileLoader::ShowOffsetInWholeFile(ULONGLONG ullOffset, ULONGLONG ullSel } if (ullSelSize > 0) { - std::vector vecSel { { ullOffset, ullSelSize } }; - pHexCtrl->SetSelection(vecSel); + pHexCtrl->SetSelection({ { ullOffset, ullSelSize } }); if (!pHexCtrl->IsOffsetVisible(ullOffset)) { pHexCtrl->GoToOffset(ullOffset); } } //If floating HexCtrl is in use we bring it to the front. - if (pHexCtrl == m_pHex.get()) + if (pHexCtrl == m_pHex.get()) { ::SetForegroundWindow(pHexCtrl->GetWindowHandle(EHexWnd::WND_MAIN)); + } return S_OK; } @@ -134,8 +111,9 @@ HRESULT CFileLoader::ShowFilePiece(ULONGLONG ullOffset, ULONGLONG ullSize, IHexC return E_ABORT; if (!pHexCtrl) { - if (!m_pHex->IsCreated()) + if (!m_pHex->IsCreated()) { CreateHexCtrlWnd(); + } pHexCtrl = m_pHex.get(); } @@ -143,8 +121,10 @@ HRESULT CFileLoader::ShowFilePiece(ULONGLONG ullOffset, ULONGLONG ullSize, IHexC pHexCtrl->ClearData(); return E_ABORT; } - if (ullOffset + ullSize > static_cast(m_stFileSize.QuadPart)) //Overflow check. + + if (ullOffset + ullSize > static_cast(m_stFileSize.QuadPart)) { //Overflow check. ullSize = static_cast(m_stFileSize.QuadPart) - ullOffset; + } if (const auto iter = std::find_if(m_vecCheck.begin(), m_vecCheck.end(), [pHexCtrl](const HEXTODATACHECK& ref) { return ref.pHexCtrl == pHexCtrl; }); iter == m_vecCheck.end()) { @@ -155,7 +135,8 @@ HRESULT CFileLoader::ShowFilePiece(ULONGLONG ullOffset, ULONGLONG ullSize, IHexC } m_hds.fMutable = m_pMainDoc->IsEditMode(); - m_hds.spnData = { reinterpret_cast(reinterpret_cast(m_lpBase) + ullOffset), static_cast(ullSize) }; + m_hds.spnData = { reinterpret_cast(reinterpret_cast(m_lpBase) + ullOffset), + static_cast(ullSize) }; pHexCtrl->SetData(m_hds); return S_OK; @@ -163,15 +144,21 @@ HRESULT CFileLoader::ShowFilePiece(ULONGLONG ullOffset, ULONGLONG ullSize, IHexC HRESULT CFileLoader::UnloadFile() { - if (!m_fLoaded) + if (!IsLoaded()) return E_ABORT; - if (m_lpBase) + if (m_lpBase) { + FlushViewOfFile(m_lpBase, 0); UnmapViewOfFile(m_lpBase); - if (m_hMapObject) + } + + if (m_hMapObject) { CloseHandle(m_hMapObject); - if (m_hFile) + } + + if (m_hFile) { CloseHandle(m_hFile); + } m_lpBase = nullptr; m_hMapObject = nullptr; @@ -181,12 +168,31 @@ HRESULT CFileLoader::UnloadFile() return S_OK; } -bool CFileLoader::IsLoaded()const +//Private methods. + +void CFileLoader::CreateHexCtrlWnd() { - return m_fLoaded; + m_pHex->Create(m_hcs); + + const auto hWndHex = m_pHex->GetWindowHandle(EHexWnd::WND_MAIN); + const auto iWidthActual = m_pHex->GetActualWidth() + GetSystemMetrics(SM_CXVSCROLL); + CRect rcHex(0, 0, iWidthActual, iWidthActual); //Square window. + AdjustWindowRectEx(rcHex, m_dwStyle, FALSE, m_dwExStyle); + const auto iWidth = rcHex.Width(); + const auto iHeight = rcHex.Height() - rcHex.Height() / 3; + const auto iPosX = GetSystemMetrics(SM_CXSCREEN) / 2 - iWidth / 2; + const auto iPosY = GetSystemMetrics(SM_CYSCREEN) / 2 - iHeight / 2; + ::SetWindowPos(hWndHex, nullptr, iPosX, iPosY, iWidth, iHeight, SWP_SHOWWINDOW); + + const auto hIconSmall = static_cast(LoadImageW(AfxGetInstanceHandle(), MAKEINTRESOURCEW(IDI_HEXCTRL_LOGO), IMAGE_ICON, 0, 0, 0)); + const auto hIconBig = static_cast(LoadImageW(AfxGetInstanceHandle(), MAKEINTRESOURCEW(IDI_HEXCTRL_LOGO), IMAGE_ICON, 96, 96, 0)); + if (hIconSmall != nullptr) { + ::SendMessageW(m_pHex->GetWindowHandle(EHexWnd::WND_MAIN), WM_SETICON, ICON_SMALL, reinterpret_cast(hIconSmall)); + ::SendMessageW(m_pHex->GetWindowHandle(EHexWnd::WND_MAIN), WM_SETICON, ICON_BIG, reinterpret_cast(hIconBig)); + } } -BOOL CFileLoader::OnNotify(WPARAM wParam, LPARAM lParam, LRESULT* pResult) +bool CFileLoader::IsLoaded()const { - return CWnd::OnNotify(wParam, lParam, pResult); + return m_fLoaded; } \ No newline at end of file diff --git a/Pepper/CFileLoader.h b/Pepper/CFileLoader.h index e207f44..898e945 100644 --- a/Pepper/CFileLoader.h +++ b/Pepper/CFileLoader.h @@ -14,39 +14,35 @@ class CPepperDoc; class CFileLoader : public CWnd { public: + [[nodiscard]] auto GetData()const->std::span; HRESULT LoadFile(LPCWSTR lpszFileName, CPepperDoc* pDoc); //First function to call. [[nodiscard]] bool IsWritable()const { return m_fWritable; } //Shows arbitrary offset in already loaded file (LoadFile). If pHexCtrl == nullptr the inner IHexCtrl object is used. HRESULT ShowOffsetInWholeFile(ULONGLONG ullOffset, ULONGLONG ullSelSize, IHexCtrl* pHexCtrl = nullptr); - //Shows only a piece of the whole loaded file. If pHexCtrl == nullptr the inner CHexCtrl object is used. + //Shows only a piece of the whole loaded file. If pHexCtrl == nullptr the inner IHexCtrl object is used. HRESULT ShowFilePiece(ULONGLONG ullOffset, ULONGLONG ullSize, IHexCtrl* pHexCtrl = nullptr); - [[nodiscard]] bool IsModified()const { return m_fModified; } //Has file been modified in memory or not. - bool Flush(); //Writes memory mapped file on disk. HRESULT UnloadFile(); //Unloads loaded file and all pieces, if present. private: - [[nodiscard]] bool IsLoaded()const; - virtual BOOL OnNotify(WPARAM wParam, LPARAM lParam, LRESULT* pResult); void CreateHexCtrlWnd(); + [[nodiscard]] bool IsLoaded()const; private: - //Does given IHexCtrl set with a whole file data or only with a file piece. - struct HEXTODATACHECK { + static constexpr DWORD m_dwStyle { WS_POPUP | WS_OVERLAPPEDWINDOW }; + static constexpr DWORD m_dwExStyle { WS_EX_APPWINDOW }; //To force to the taskbar. + struct HEXTODATACHECK { //Was given IHexCtrl set with a whole file data or only with a file piece. IHexCtrl* pHexCtrl { }; bool fWhole { }; }; - static constexpr DWORD m_dwStyle { WS_POPUP | WS_OVERLAPPEDWINDOW }; - static constexpr DWORD m_dwExStyle { WS_EX_APPWINDOW }; //To force to the taskbar. CPepperDoc* m_pMainDoc { }; IHexCtrlPtr m_pHex { HEXCTRL::CreateHexCtrl() }; HEXCREATE m_hcs; HEXDATA m_hds; - LARGE_INTEGER m_stFileSize { }; //Size of the loaded PE file. - HANDLE m_hFile { }; - HANDLE m_hMapObject { }; //Returned by CreateFileMappingW. - LPVOID m_lpBase { }; + LARGE_INTEGER m_stFileSize { }; //Size of the loaded PE file. + HANDLE m_hFile { }; //Returned by CreateFileW. + HANDLE m_hMapObject { }; //Returned by CreateFileMappingW. + LPVOID m_lpBase { }; //Returned by MapViewOfFile. std::vector m_vecCheck; bool m_fLoaded { false }; - bool m_fModified { false }; bool m_fWritable { false }; }; \ No newline at end of file diff --git a/Pepper/CPepperDoc.cpp b/Pepper/CPepperDoc.cpp index 84e9a19..65fd9b4 100644 --- a/Pepper/CPepperDoc.cpp +++ b/Pepper/CPepperDoc.cpp @@ -31,15 +31,19 @@ BOOL CPepperDoc::OnOpenDocument(LPCTSTR lpszPathName) { m_wstrDocName = lpszPathName; m_wstrDocName = m_wstrDocName.substr(m_wstrDocName.find_last_of(L'\\') + 1); //Doc name with .extension. - libpe::Clibpe libPE; + const std::wstring wstrErrCaption = L"File load failed: " + m_wstrDocName; + + if (m_stFileLoader.LoadFile(lpszPathName, this) != S_OK) { + MessageBoxW(nullptr, L"File load failed.", wstrErrCaption.data(), MB_ICONERROR); + return FALSE; + } - if (const auto err = libPE.OpenFile(lpszPathName); err != libpe::PEOK) { - m_wstrDocName += L" File Load Failed."; + libpe::Clibpe libPE; + if (const auto err = libPE.OpenFile(m_stFileLoader.GetData()); err != libpe::PEOK) { const auto it = g_mapLibpeErrors.find(err); - MessageBoxW(nullptr, std::vformat(L"File load failed with libpe error code: 0x{:04X}\n{}", + MessageBoxW(nullptr, std::vformat(L"File load failed with the libpe error code: 0x{:04X}\n{}", std::make_wformat_args(err, it != g_mapLibpeErrors.end() ? it->second : L"N/A")).data(), - m_wstrDocName.data(), MB_ICONERROR); - + wstrErrCaption.data(), MB_ICONERROR); return FALSE; } @@ -56,7 +60,7 @@ BOOL CPepperDoc::OnOpenDocument(LPCTSTR lpszPathName) m_optExport = libPE.GetExport(); m_stFileInfo.fHasExport = m_optExport.has_value(); m_optImport = libPE.GetImport(); - m_stFileInfo.fHasImport = m_optImport.has_value(); + m_stFileInfo.fHasImport = m_stFileInfo.fHasIAT = m_optImport.has_value(); m_optResRoot = libPE.GetResources(); m_stFileInfo.fHasResource = m_optResRoot.has_value(); m_optExcept = libPE.GetExceptions(); @@ -104,8 +108,6 @@ BOOL CPepperDoc::OnOpenDocument(LPCTSTR lpszPathName) } } - libPE.CloseFile(); - m_stFileLoader.LoadFile(lpszPathName, this); UpdateAllViews(nullptr); return TRUE; @@ -113,7 +115,6 @@ BOOL CPepperDoc::OnOpenDocument(LPCTSTR lpszPathName) void CPepperDoc::OnCloseDocument() { - m_stFileLoader.Flush(); m_stFileLoader.UnloadFile(); CDocument::OnCloseDocument(); diff --git a/Pepper/CPepperDoc.h b/Pepper/CPepperDoc.h index 1a70c25..7b3b1f7 100644 --- a/Pepper/CPepperDoc.h +++ b/Pepper/CPepperDoc.h @@ -71,7 +71,7 @@ class CPepperDoc : public CDocument std::optional m_optBoundImp; std::optional m_optDelayImp; std::optional m_optComDescr; - Util::PEFILEINFO m_stFileInfo; + Util::PEFILEINFO m_stFileInfo { }; bool m_fEditMode { false }; bool m_fHasCur { false }; bool m_fHasIco { false }; diff --git a/Pepper/Utility.ixx b/Pepper/Utility.ixx index 13cd98c..4fe3b82 100644 --- a/Pepper/Utility.ixx +++ b/Pepper/Utility.ixx @@ -260,13 +260,13 @@ export namespace Util } } - //Errors, that might come from libpe. - inline const std::unordered_map g_mapLibpeErrors { + //Errors, that might come from the libpe. + inline const std::unordered_map g_mapLibpeErrors { TO_WSTR_MAP(libpe::PEOK), - TO_WSTR_MAP(libpe::ERR_FILE_OPEN), - TO_WSTR_MAP(libpe::ERR_FILE_MAPPING), - TO_WSTR_MAP(libpe::ERR_FILE_SIZESMALL), - TO_WSTR_MAP(libpe::ERR_FILE_NODOSHDR) + TO_WSTR_MAP(libpe::ERR_FILE_OPEN), + TO_WSTR_MAP(libpe::ERR_FILE_MAPPING), + TO_WSTR_MAP(libpe::ERR_FILE_SIZESMALL), + TO_WSTR_MAP(libpe::ERR_FILE_NODOSHDR) }; struct PEFILEINFO { @@ -580,8 +580,8 @@ export namespace Util //All HexCtrl dialogs' IDs for hiding/showing in Views, when tab is deactivated/activated. inline const std::vector g_vecHexDlgs { HEXCTRL::EHexWnd::DLG_BKMMANAGER, HEXCTRL::EHexWnd::DLG_DATAINTERP, HEXCTRL::EHexWnd::DLG_MODIFY, - HEXCTRL::EHexWnd::DLG_SEARCH, HEXCTRL::EHexWnd::DLG_ENCODING, - HEXCTRL::EHexWnd::DLG_GOTO, HEXCTRL::EHexWnd::DLG_TEMPLMGR }; + HEXCTRL::EHexWnd::DLG_SEARCH, HEXCTRL::EHexWnd::DLG_ENCODING, + HEXCTRL::EHexWnd::DLG_GOTO, HEXCTRL::EHexWnd::DLG_TEMPLMGR }; /***************************************************************** @@ -656,5 +656,5 @@ export namespace Util ********************************************************/ //Color of the list's "Offset" column - constexpr COLORREF g_clrOffset = RGB(150, 150, 150); + constexpr auto g_clrOffset = RGB(150, 150, 150); }; \ No newline at end of file