From 14520719a1f9c0d05544e2655d2b21374d34bb9a Mon Sep 17 00:00:00 2001 From: Christophe Pichaud Date: Fri, 7 Jun 2019 02:36:26 +0200 Subject: [PATCH 1/5] Changes for Issue #1058 are done in VS2017. Works fine. --- src/host/CommandListPopup.cpp | 56 ++++++++--------- src/host/CommandListPopup.hpp | 4 +- src/host/CommandNumberPopup.cpp | 2 +- src/host/CopyToCharPopup.cpp | 2 +- src/host/cmdline.cpp | 12 ++-- src/host/cookedRead.cpp | 6 +- src/host/cookedRead.hpp | 6 +- src/host/history.cpp | 66 ++++++++++---------- src/host/history.h | 8 +-- src/host/stream.cpp | 2 +- src/host/ut_host/CommandLineTests.cpp | 4 +- src/host/ut_host/CommandListPopupTests.cpp | 26 ++++---- src/host/ut_host/CommandNumberPopupTests.cpp | 4 +- src/host/ut_host/CopyToCharPopupTests.cpp | 2 +- 14 files changed, 100 insertions(+), 100 deletions(-) diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index a41b7352b15..9f8645ddd0d 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -22,7 +22,7 @@ static constexpr size_t COMMAND_NUMBER_SIZE = 8; // size of command number buf // - history - the history to look through to measure command sizes // Return Value: // - the proposed size of the popup with the history list taken into account -static COORD calculatePopupSize(const CommandHistory& history) +static COORD calculatePopupSize(const std::shared_ptr& history) { // this is the historical size of the popup, so it is now used as a minimum const COORD minSize = { 40, 10 }; @@ -34,9 +34,9 @@ static COORD calculatePopupSize(const CommandHistory& history) // find the widest command history item and use it for the width size_t width = minSize.X; - for (size_t i = 0; i < history.GetNumberOfCommands(); ++i) + for (size_t i = 0; i < history->GetNumberOfCommands(); ++i) { - const auto& historyItem = history.GetNth(gsl::narrow(i)); + const auto& historyItem = history->GetNth(gsl::narrow(i)); width = std::max(width, historyItem.size() + padding); } if (width > SHRT_MAX) @@ -45,15 +45,15 @@ static COORD calculatePopupSize(const CommandHistory& history) } // calculate height, it can range up to 20 rows - short height = std::clamp(gsl::narrow(history.GetNumberOfCommands()), minSize.Y, 20i16); + short height = std::clamp(gsl::narrow(history->GetNumberOfCommands()), minSize.Y, 20i16); return { gsl::narrow(width), height }; } -CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const CommandHistory& history) : +CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const std::shared_ptr& history) : Popup(screenInfo, calculatePopupSize(history)), _history{ history }, - _currentCommand{ std::min(history.LastDisplayed, static_cast(history.GetNumberOfCommands() - 1)) } + _currentCommand{ std::min(history->LastDisplayed, static_cast(history->GetNumberOfCommands() - 1)) } { FAIL_FAST_IF(_currentCommand < 0); _setBottomIndex(); @@ -106,11 +106,11 @@ NTSTATUS CommandListPopup::_handlePopupKeys(CookedRead& cookedReadData, const wc break; case VK_END: // Move waaay forward, UpdateCommandListPopup() can handle it. - _update((SHORT)(cookedReadData.History().GetNumberOfCommands())); + _update((SHORT)(cookedReadData.History()->GetNumberOfCommands())); break; case VK_HOME: // Move waaay back, UpdateCommandListPopup() can handle it. - _update(-(SHORT)(cookedReadData.History().GetNumberOfCommands())); + _update(-(SHORT)(cookedReadData.History()->GetNumberOfCommands())); break; case VK_PRIOR: _update(-(SHORT)Height()); @@ -136,13 +136,13 @@ NTSTATUS CommandListPopup::_handlePopupKeys(CookedRead& cookedReadData, const wc void CommandListPopup::_setBottomIndex() { - if (_currentCommand < (SHORT)(_history.GetNumberOfCommands() - Height())) + if (_currentCommand < (SHORT)(_history->GetNumberOfCommands() - Height())) { _bottomIndex = std::max(_currentCommand, gsl::narrow(Height() - 1i16)); } else { - _bottomIndex = (SHORT)(_history.GetNumberOfCommands() - 1); + _bottomIndex = (SHORT)(_history->GetNumberOfCommands() - 1); } } @@ -151,18 +151,18 @@ NTSTATUS CommandListPopup::_deleteSelection(CookedRead& cookedReadData) noexcept { try { - auto& history = cookedReadData.History(); - history.Remove(static_cast(_currentCommand)); + auto history = cookedReadData.History(); + history->Remove(static_cast(_currentCommand)); _setBottomIndex(); - if (history.GetNumberOfCommands() == 0) + if (history->GetNumberOfCommands() == 0) { // close the popup return CONSOLE_STATUS_READ_COMPLETE; } - else if (_currentCommand >= static_cast(history.GetNumberOfCommands())) + else if (_currentCommand >= static_cast(history->GetNumberOfCommands())) { - _currentCommand = static_cast(history.GetNumberOfCommands() - 1); + _currentCommand = static_cast(history->GetNumberOfCommands() - 1); _bottomIndex = _currentCommand; } @@ -181,13 +181,13 @@ NTSTATUS CommandListPopup::_swapUp(CookedRead& cookedReadData) noexcept { try { - auto& history = cookedReadData.History(); + auto history = cookedReadData.History(); - if (history.GetNumberOfCommands() <= 1 || _currentCommand == 0) + if (history->GetNumberOfCommands() <= 1 || _currentCommand == 0) { return STATUS_SUCCESS; } - history.Swap(_currentCommand, _currentCommand - 1); + history->Swap(_currentCommand, _currentCommand - 1); _update(-1); _drawList(); } @@ -204,13 +204,13 @@ NTSTATUS CommandListPopup::_swapDown(CookedRead& cookedReadData) noexcept { try { - auto& history = cookedReadData.History(); + auto history = cookedReadData.History(); - if (history.GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history.GetNumberOfCommands()) - 1i16) + if (history->GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history->GetNumberOfCommands()) - 1i16) { return STATUS_SUCCESS; } - history.Swap(_currentCommand, _currentCommand + 1); + history->Swap(_currentCommand, _currentCommand + 1); _update(1); _drawList(); } @@ -229,7 +229,7 @@ void CommandListPopup::_handleReturn(CookedRead& cookedReadData) void CommandListPopup::_cycleSelectionToMatchingCommands(CookedRead& cookedReadData, const wchar_t wch) { short Index = 0; - if (cookedReadData.History().FindMatchingCommand({ &wch, 1 }, + if (cookedReadData.History()->FindMatchingCommand({ &wch, 1 }, _currentCommand, Index, CommandHistory::MatchOptions::JustLooking)) @@ -346,7 +346,7 @@ void CommandListPopup::_drawList() CommandNumberLength)); // write command to screen - auto command = _history.GetNth(i); + auto command = _history->GetNth(i); lStringLength = command.size(); { size_t lTmpStringLength = lStringLength; @@ -423,13 +423,13 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) if (wrap) { // Modulo the number of commands to "circle" around if we went off the end. - NewCmdNum %= _history.GetNumberOfCommands(); + NewCmdNum %= _history->GetNumberOfCommands(); } else { - if (NewCmdNum >= gsl::narrow(_history.GetNumberOfCommands())) + if (NewCmdNum >= gsl::narrow(_history->GetNumberOfCommands())) { - NewCmdNum = gsl::narrow(_history.GetNumberOfCommands()) - 1i16; + NewCmdNum = gsl::narrow(_history->GetNumberOfCommands()) - 1i16; } else if (NewCmdNum < 0) { @@ -452,9 +452,9 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) else if (NewCmdNum > _bottomIndex) { _bottomIndex += delta; - if (_bottomIndex >= gsl::narrow(_history.GetNumberOfCommands())) + if (_bottomIndex >= gsl::narrow(_history->GetNumberOfCommands())) { - _bottomIndex = gsl::narrow(_history.GetNumberOfCommands()) - 1i16; + _bottomIndex = gsl::narrow(_history->GetNumberOfCommands()) - 1i16; } Scroll = true; } diff --git a/src/host/CommandListPopup.hpp b/src/host/CommandListPopup.hpp index 71edd76a2ca..49d151681ac 100644 --- a/src/host/CommandListPopup.hpp +++ b/src/host/CommandListPopup.hpp @@ -21,7 +21,7 @@ Module Name: class CommandListPopup : public Popup { public: - CommandListPopup(SCREEN_INFORMATION& screenInfo, const CommandHistory& history); + CommandListPopup(SCREEN_INFORMATION& screenInfo, const std::shared_ptr& history); [[nodiscard]] NTSTATUS Process(CookedRead& cookedReadData) noexcept override; @@ -48,7 +48,7 @@ class CommandListPopup : public Popup SHORT _currentCommand; SHORT _bottomIndex; // number of command displayed on last line of popup - const CommandHistory& _history; + const std::shared_ptr& _history; #ifdef UNIT_TESTING friend class CommandListPopupTests; diff --git a/src/host/CommandNumberPopup.cpp b/src/host/CommandNumberPopup.cpp index 136e0e35064..c4f5a1f32fd 100644 --- a/src/host/CommandNumberPopup.cpp +++ b/src/host/CommandNumberPopup.cpp @@ -102,7 +102,7 @@ void CommandNumberPopup::_handleEscape(CookedRead& cookedReadData) noexcept void CommandNumberPopup::_handleReturn(CookedRead& cookedReadData) noexcept { const short commandNumber = gsl::narrow(std::min(static_cast(_parse()), - cookedReadData.History().GetNumberOfCommands() - 1)); + cookedReadData.History()->GetNumberOfCommands() - 1)); CommandLine::Instance().EndAllPopups(); SetCurrentCommandLine(cookedReadData, commandNumber); diff --git a/src/host/CopyToCharPopup.cpp b/src/host/CopyToCharPopup.cpp index 74046651fc9..4dcf7176883 100644 --- a/src/host/CopyToCharPopup.cpp +++ b/src/host/CopyToCharPopup.cpp @@ -70,7 +70,7 @@ NTSTATUS CopyToCharPopup::Process(CookedRead& cookedReadData) noexcept } // copy up to specified char - const auto lastCommand = cookedReadData.History().GetLastCommand(); + const auto lastCommand = cookedReadData.History()->GetLastCommand(); if (!lastCommand.empty()) { _copyToChar(cookedReadData, lastCommand, wch); diff --git a/src/host/cmdline.cpp b/src/host/cmdline.cpp index 37800a6d1f1..fc22446c1a7 100644 --- a/src/host/cmdline.cpp +++ b/src/host/cmdline.cpp @@ -254,7 +254,7 @@ void SetCurrentCommandLine(CookedRead& cookedReadData, _In_ SHORT Index) // inde NTSTATUS CommandLine::_startCommandListPopup(CookedRead& cookedReadData) { if (cookedReadData.HasHistory() && - cookedReadData.History().GetNumberOfCommands()) + cookedReadData.History()->GetNumberOfCommands()) { try { @@ -336,7 +336,7 @@ NTSTATUS CommandLine::_startCopyToCharPopup(CookedRead& cookedReadData) HRESULT CommandLine::StartCommandNumberPopup(CookedRead& cookedReadData) { if (cookedReadData.HasHistory() && - cookedReadData.History().GetNumberOfCommands() && + cookedReadData.History()->GetNumberOfCommands() && cookedReadData.ScreenInfo().GetBufferSize().Width() >= MINIMUM_COMMAND_PROMPT_SIZE + 2) { try @@ -380,12 +380,12 @@ void CommandLine::_processHistoryCycling(CookedRead& cookedReadData, return; } else if (searchDirection == CommandHistory::SearchDirection::Previous - && cookedReadData.History().AtFirstCommand()) + && cookedReadData.History()->AtFirstCommand()) { return; } else if (searchDirection == CommandHistory::SearchDirection::Next - && cookedReadData.History().AtLastCommand()) + && cookedReadData.History()->AtLastCommand()) { return; } @@ -543,8 +543,8 @@ void CommandLine::_deleteCommandHistory(CookedRead& cookedReadData) noexcept { if (cookedReadData.HasHistory()) { - cookedReadData.History().Empty(); - cookedReadData.History().Flags |= CLE_ALLOCATED; + cookedReadData.History()->Empty(); + cookedReadData.History()->Flags |= CLE_ALLOCATED; } } diff --git a/src/host/cookedRead.cpp b/src/host/cookedRead.cpp index dbd9ff6298c..d4565d3d088 100644 --- a/src/host/cookedRead.cpp +++ b/src/host/cookedRead.cpp @@ -17,7 +17,7 @@ CookedRead::CookedRead(InputBuffer* const pInputBuffer, INPUT_READ_HANDLE_DATA* const pInputReadHandleData, SCREEN_INFORMATION& screenInfo, - CommandHistory* pCommandHistory, + std::shared_ptr pCommandHistory, wchar_t* userBuffer, const size_t cchUserBuffer, const ULONG ctrlWakeupMask, @@ -55,9 +55,9 @@ bool CookedRead::HasHistory() const noexcept return _pCommandHistory != nullptr; } -CommandHistory& CookedRead::History() noexcept +std::shared_ptr CookedRead::History() noexcept { - return *_pCommandHistory; + return _pCommandHistory; } void CookedRead::Erase() diff --git a/src/host/cookedRead.hpp b/src/host/cookedRead.hpp index 03073f6f50d..02a48197043 100644 --- a/src/host/cookedRead.hpp +++ b/src/host/cookedRead.hpp @@ -12,7 +12,7 @@ class CookedRead final : public ReadData CookedRead(InputBuffer* const pInputBuffer, INPUT_READ_HANDLE_DATA* const pInputReadHandleData, SCREEN_INFORMATION& screenInfo, - CommandHistory* pCommandHistory, + std::shared_ptr pCommandHistory, wchar_t* userBuffer, const size_t cchUserBuffer, const ULONG ctrlWakeupMask, @@ -69,7 +69,7 @@ class CookedRead final : public ReadData SCREEN_INFORMATION& ScreenInfo(); - CommandHistory& History() noexcept; + std::shared_ptr History() noexcept; bool HasHistory() const noexcept; void BufferInput(const wchar_t wch); @@ -112,7 +112,7 @@ class CookedRead final : public ReadData COORD _promptStartLocation; // the location of the cursor before a popup is launched COORD _beforePopupCursorPosition; - CommandHistory* _pCommandHistory; // non-ownership pointer + std::shared_ptr _pCommandHistory; // shared pointer // mask of control keys that if pressed will end the cooked read early const ULONG _ctrlWakeupMask; // current state of the CookedRead diff --git a/src/host/history.cpp b/src/host/history.cpp index f8de3462ae2..020e98ffbe8 100644 --- a/src/host/history.cpp +++ b/src/host/history.cpp @@ -28,16 +28,16 @@ // (where other collections like deque do not.) // If CommandHistory::s_Allocate and friends stop shuffling elements // for maintaining LRU, then this datatype can be changed. -std::list CommandHistory::s_historyLists; +std::list> CommandHistory::s_historyLists; -CommandHistory* CommandHistory::s_Find(const HANDLE processHandle) +std::shared_ptr CommandHistory::s_Find(const HANDLE processHandle) { - for (auto& historyList : s_historyLists) + for (auto historyList : s_historyLists) { - if (historyList._processHandle == processHandle) + if (historyList->_processHandle == processHandle) { - FAIL_FAST_IF(WI_IsFlagClear(historyList.Flags, CLE_ALLOCATED)); - return &historyList; + FAIL_FAST_IF(WI_IsFlagClear(historyList->Flags, CLE_ALLOCATED)); + return historyList; } } @@ -50,7 +50,7 @@ CommandHistory* CommandHistory::s_Find(const HANDLE processHandle) // - processHandle - handle to client process. void CommandHistory::s_Free(const HANDLE processHandle) { - CommandHistory* const History = CommandHistory::s_Find(processHandle); + const std::shared_ptr& History = CommandHistory::s_Find(processHandle); if (History) { WI_ClearFlag(History->Flags, CLE_ALLOCATED); @@ -64,9 +64,9 @@ void CommandHistory::s_ResizeAll(const size_t commands) FAIL_FAST_IF(commands > SHORT_MAX); gci.SetHistoryBufferSize(gsl::narrow(commands)); - for (auto& historyList : s_historyLists) + for (auto historyList : s_historyLists) { - historyList.Realloc(commands); + historyList->Realloc(commands); } } @@ -297,10 +297,10 @@ void CommandHistory::s_ReallocExeToFront(const std::wstring_view appName, const { for (auto it = s_historyLists.begin(); it != s_historyLists.end(); it++) { - if (WI_IsFlagSet(it->Flags, CLE_ALLOCATED) && it->IsAppNameMatch(appName)) + if (WI_IsFlagSet((*it)->Flags, CLE_ALLOCATED) && (*it)->IsAppNameMatch(appName)) { - CommandHistory backup = *it; - backup.Realloc(commands); + std::shared_ptr backup = *it; + backup->Realloc(commands); s_historyLists.erase(it); s_historyLists.push_front(backup); @@ -310,13 +310,13 @@ void CommandHistory::s_ReallocExeToFront(const std::wstring_view appName, const } } -CommandHistory* CommandHistory::s_FindByExe(const std::wstring_view appName) +std::shared_ptr CommandHistory::s_FindByExe(const std::wstring_view appName) { - for (auto& historyList : s_historyLists) + for (auto historyList : s_historyLists) { - if (WI_IsFlagSet(historyList.Flags, CLE_ALLOCATED) && historyList.IsAppNameMatch(appName)) + if (WI_IsFlagSet(historyList->Flags, CLE_ALLOCATED) && historyList->IsAppNameMatch(appName)) { - return &historyList; + return historyList; } } return nullptr; @@ -333,20 +333,20 @@ size_t CommandHistory::s_CountOfHistories() // - Console - pointer to console. // Return Value: // - Pointer to command history buffer. if none are available, returns nullptr. -CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, const HANDLE processHandle) +std::shared_ptr CommandHistory::s_Allocate(const std::wstring_view appName, const HANDLE processHandle) { CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // Reuse a history buffer. The buffer must be !CLE_ALLOCATED. // If possible, the buffer should have the same app name. - std::optional BestCandidate; + std::shared_ptr BestCandidate = nullptr; bool SameApp = false; for (auto it = s_historyLists.cbegin(); it != s_historyLists.cend(); it++) { - if (WI_IsFlagClear(it->Flags, CLE_ALLOCATED)) + if (WI_IsFlagClear((*it)->Flags, CLE_ALLOCATED)) { // use LRU history buffer with same app name - if (it->IsAppNameMatch(appName)) + if ((*it)->IsAppNameMatch(appName)) { BestCandidate = *it; SameApp = true; @@ -360,21 +360,21 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons // command history buffers hasn't been allocated, allocate a new one. if (!SameApp && s_historyLists.size() < gci.GetNumberOfHistoryBuffers()) { - CommandHistory History; + std::shared_ptr History = std::make_shared(); - History._appName = appName; - History.Flags = CLE_ALLOCATED; - History.LastDisplayed = -1; - History._maxCommands = gsl::narrow(gci.GetHistoryBufferSize()); - History._processHandle = processHandle; - return &s_historyLists.emplace_front(History); + History->_appName = appName; + History->Flags = CLE_ALLOCATED; + History->LastDisplayed = -1; + History->_maxCommands = gsl::narrow(gci.GetHistoryBufferSize()); + History->_processHandle = processHandle; + return s_historyLists.emplace_front(History); } - else if (!BestCandidate.has_value() && s_historyLists.size() > 0) + else if (BestCandidate == nullptr && s_historyLists.size() > 0) { // If we have no candidate already and we need one, take the LRU (which is the back/last one) which isn't allocated. for (auto it = s_historyLists.crbegin(); it != s_historyLists.crend(); it++) { - if (WI_IsFlagClear(it->Flags, CLE_ALLOCATED)) + if (WI_IsFlagClear((*it)->Flags, CLE_ALLOCATED)) { BestCandidate = *it; s_historyLists.erase(std::next(it).base()); // trickery to turn reverse iterator into forward iterator for erase. @@ -385,7 +385,7 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons } // If the app name doesn't match, copy in the new app name and free the old commands. - if (BestCandidate.has_value()) + if (BestCandidate) { if (!SameApp) { @@ -397,7 +397,7 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons BestCandidate->_processHandle = processHandle; WI_SetFlag(BestCandidate->Flags, CLE_ALLOCATED); - return &s_historyLists.emplace_front(BestCandidate.value()); + return s_historyLists.emplace_front(BestCandidate); } return nullptr; @@ -678,7 +678,7 @@ HRESULT GetConsoleCommandHistoryLengthImplHelper(const std::wstring_view exeName LockConsole(); auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); - CommandHistory* const pCommandHistory = CommandHistory::s_FindByExe(exeName); + const std::shared_ptr pCommandHistory = CommandHistory::s_FindByExe(exeName); if (nullptr != pCommandHistory) { size_t cchNeeded = 0; @@ -783,7 +783,7 @@ HRESULT GetConsoleCommandHistoryWImplHelper(const std::wstring_view exeName, historyBuffer.at(0) = UNICODE_NULL; } - CommandHistory* const CommandHistory = CommandHistory::s_FindByExe(exeName); + const std::shared_ptr CommandHistory = CommandHistory::s_FindByExe(exeName); if (nullptr != CommandHistory) { diff --git a/src/host/history.h b/src/host/history.h index 64afda8f2b5..7d8e787fa0c 100644 --- a/src/host/history.h +++ b/src/host/history.h @@ -19,9 +19,9 @@ Module Name: class CommandHistory { public: - static CommandHistory* s_Allocate(const std::wstring_view appName, const HANDLE processHandle); - static CommandHistory* s_Find(const HANDLE processHandle); - static CommandHistory* s_FindByExe(const std::wstring_view appName); + static std::shared_ptr s_Allocate(const std::wstring_view appName, const HANDLE processHandle); + static std::shared_ptr s_Find(const HANDLE processHandle); + static std::shared_ptr s_FindByExe(const std::wstring_view appName); static void s_ReallocExeToFront(const std::wstring_view appName, const size_t commands); static void s_Free(const HANDLE processHandle); static void s_ResizeAll(const size_t commands); @@ -93,7 +93,7 @@ class CommandHistory std::wstring _appName; HANDLE _processHandle; - static std::list s_historyLists; + static std::list> s_historyLists; public: DWORD Flags; diff --git a/src/host/stream.cpp b/src/host/stream.cpp index bef99798022..581d8c68757 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -474,7 +474,7 @@ static HRESULT _ReadLineInput(InputBuffer& inputBuffer, RETURN_HR_IF(E_FAIL, !gci.HasActiveOutputBuffer()); SCREEN_INFORMATION& screenInfo = gci.GetActiveOutputBuffer(); - CommandHistory* const pCommandHistory = CommandHistory::s_Find(processData); + const std::shared_ptr pCommandHistory = CommandHistory::s_Find(processData); try { diff --git a/src/host/ut_host/CommandLineTests.cpp b/src/host/ut_host/CommandLineTests.cpp index f6761788ab3..58ac13e38dd 100644 --- a/src/host/ut_host/CommandLineTests.cpp +++ b/src/host/ut_host/CommandLineTests.cpp @@ -21,7 +21,7 @@ constexpr size_t PROMPT_SIZE = 512; class CommandLineTests { std::unique_ptr m_state; - CommandHistory* m_pHistory; + std::shared_ptr m_pHistory; TEST_CLASS(CommandLineTests); @@ -72,7 +72,7 @@ class CommandLineTests } void InitCookedReadData(CookedRead& cookedReadData, - CommandHistory* pHistory, + std::shared_ptr pHistory, const std::wstring prompt) { cookedReadData._pCommandHistory = pHistory; diff --git a/src/host/ut_host/CommandListPopupTests.cpp b/src/host/ut_host/CommandListPopupTests.cpp index 44793476235..fdb0fdc999b 100644 --- a/src/host/ut_host/CommandListPopupTests.cpp +++ b/src/host/ut_host/CommandListPopupTests.cpp @@ -25,7 +25,7 @@ class CommandListPopupTests TEST_CLASS(CommandListPopupTests); std::unique_ptr m_state; - CommandHistory* m_pHistory; + std::shared_ptr m_pHistory; TEST_CLASS_SETUP(ClassSetup) { @@ -86,7 +86,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -127,7 +127,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -164,7 +164,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -203,7 +203,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -241,7 +241,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -277,7 +277,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitLongHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -313,7 +313,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitLongHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -342,7 +342,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -373,7 +373,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -413,7 +413,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -450,7 +450,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -493,7 +493,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData diff --git a/src/host/ut_host/CommandNumberPopupTests.cpp b/src/host/ut_host/CommandNumberPopupTests.cpp index 1d70c2e7daf..7184e556f69 100644 --- a/src/host/ut_host/CommandNumberPopupTests.cpp +++ b/src/host/ut_host/CommandNumberPopupTests.cpp @@ -25,7 +25,7 @@ class CommandNumberPopupTests TEST_CLASS(CommandNumberPopupTests); std::unique_ptr m_state; - CommandHistory* m_pHistory; + std::shared_ptr m_pHistory; TEST_CLASS_SETUP(ClassSetup) { @@ -122,7 +122,7 @@ class CommandNumberPopupTests // add popups to CommandLine auto& commandLine = CommandLine::Instance(); - commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer(), *m_pHistory)); + commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer(), m_pHistory)); commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer())); auto& numberPopup = *commandLine._popups.front(); numberPopup.SetUserInputFunction(fn); diff --git a/src/host/ut_host/CopyToCharPopupTests.cpp b/src/host/ut_host/CopyToCharPopupTests.cpp index d5f44a779b5..4804a0f3a15 100644 --- a/src/host/ut_host/CopyToCharPopupTests.cpp +++ b/src/host/ut_host/CopyToCharPopupTests.cpp @@ -24,7 +24,7 @@ class CopyToCharPopupTests TEST_CLASS(CopyToCharPopupTests); std::unique_ptr m_state; - CommandHistory* m_pHistory; + std::shared_ptr m_pHistory; TEST_CLASS_SETUP(ClassSetup) { From 4e4b965246c0467b52bcd8ae4a6f5b7036761057 Mon Sep 17 00:00:00 2001 From: Christophe Pichaud Date: Fri, 7 Jun 2019 20:43:22 +0200 Subject: [PATCH 2/5] calculatePopupSize use a const CommandHistory& and is called from a shared_ptr. --- src/host/CommandListPopup.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index 9f8645ddd0d..00632772f67 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -22,7 +22,7 @@ static constexpr size_t COMMAND_NUMBER_SIZE = 8; // size of command number buf // - history - the history to look through to measure command sizes // Return Value: // - the proposed size of the popup with the history list taken into account -static COORD calculatePopupSize(const std::shared_ptr& history) +static COORD calculatePopupSize(const CommandHistory& history) { // this is the historical size of the popup, so it is now used as a minimum const COORD minSize = { 40, 10 }; @@ -34,9 +34,9 @@ static COORD calculatePopupSize(const std::shared_ptr& history) // find the widest command history item and use it for the width size_t width = minSize.X; - for (size_t i = 0; i < history->GetNumberOfCommands(); ++i) + for (size_t i = 0; i < history.GetNumberOfCommands(); ++i) { - const auto& historyItem = history->GetNth(gsl::narrow(i)); + const auto& historyItem = history.GetNth(gsl::narrow(i)); width = std::max(width, historyItem.size() + padding); } if (width > SHRT_MAX) @@ -45,13 +45,13 @@ static COORD calculatePopupSize(const std::shared_ptr& history) } // calculate height, it can range up to 20 rows - short height = std::clamp(gsl::narrow(history->GetNumberOfCommands()), minSize.Y, 20i16); + short height = std::clamp(gsl::narrow(history.GetNumberOfCommands()), minSize.Y, 20i16); return { gsl::narrow(width), height }; } CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const std::shared_ptr& history) : - Popup(screenInfo, calculatePopupSize(history)), + Popup(screenInfo, calculatePopupSize(*history)), _history{ history }, _currentCommand{ std::min(history->LastDisplayed, static_cast(history->GetNumberOfCommands() - 1)) } { From 3f19be4ea2cb1a3fdf77d6f691f9e96b95554008 Mon Sep 17 00:00:00 2001 From: Christophe Pichaud Date: Tue, 11 Jun 2019 01:30:14 +0200 Subject: [PATCH 3/5] Some shared_ptr are converted into refrences. Better appropriated. --- src/host/CommandListPopup.cpp | 44 ++++++++++---------- src/host/CommandListPopup.hpp | 4 +- src/host/CommandNumberPopup.cpp | 2 +- src/host/CopyToCharPopup.cpp | 2 +- src/host/cmdline.cpp | 12 +++--- src/host/cookedRead.cpp | 4 +- src/host/cookedRead.hpp | 4 +- src/host/ut_host/CommandListPopupTests.cpp | 24 +++++------ src/host/ut_host/CommandNumberPopupTests.cpp | 2 +- 9 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index 00632772f67..a89763f5404 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -50,10 +50,10 @@ static COORD calculatePopupSize(const CommandHistory& history) return { gsl::narrow(width), height }; } -CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const std::shared_ptr& history) : - Popup(screenInfo, calculatePopupSize(*history)), +CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, CommandHistory& history) : + Popup(screenInfo, calculatePopupSize(history)), _history{ history }, - _currentCommand{ std::min(history->LastDisplayed, static_cast(history->GetNumberOfCommands() - 1)) } + _currentCommand{ std::min(history.LastDisplayed, static_cast(history.GetNumberOfCommands() - 1)) } { FAIL_FAST_IF(_currentCommand < 0); _setBottomIndex(); @@ -106,11 +106,11 @@ NTSTATUS CommandListPopup::_handlePopupKeys(CookedRead& cookedReadData, const wc break; case VK_END: // Move waaay forward, UpdateCommandListPopup() can handle it. - _update((SHORT)(cookedReadData.History()->GetNumberOfCommands())); + _update((SHORT)(cookedReadData.History().GetNumberOfCommands())); break; case VK_HOME: // Move waaay back, UpdateCommandListPopup() can handle it. - _update(-(SHORT)(cookedReadData.History()->GetNumberOfCommands())); + _update(-(SHORT)(cookedReadData.History().GetNumberOfCommands())); break; case VK_PRIOR: _update(-(SHORT)Height()); @@ -136,13 +136,13 @@ NTSTATUS CommandListPopup::_handlePopupKeys(CookedRead& cookedReadData, const wc void CommandListPopup::_setBottomIndex() { - if (_currentCommand < (SHORT)(_history->GetNumberOfCommands() - Height())) + if (_currentCommand < (SHORT)(_history.GetNumberOfCommands() - Height())) { _bottomIndex = std::max(_currentCommand, gsl::narrow(Height() - 1i16)); } else { - _bottomIndex = (SHORT)(_history->GetNumberOfCommands() - 1); + _bottomIndex = (SHORT)(_history.GetNumberOfCommands() - 1); } } @@ -152,17 +152,17 @@ NTSTATUS CommandListPopup::_deleteSelection(CookedRead& cookedReadData) noexcept try { auto history = cookedReadData.History(); - history->Remove(static_cast(_currentCommand)); + history.Remove(static_cast(_currentCommand)); _setBottomIndex(); - if (history->GetNumberOfCommands() == 0) + if (history.GetNumberOfCommands() == 0) { // close the popup return CONSOLE_STATUS_READ_COMPLETE; } - else if (_currentCommand >= static_cast(history->GetNumberOfCommands())) + else if (_currentCommand >= static_cast(history.GetNumberOfCommands())) { - _currentCommand = static_cast(history->GetNumberOfCommands() - 1); + _currentCommand = static_cast(history.GetNumberOfCommands() - 1); _bottomIndex = _currentCommand; } @@ -183,11 +183,11 @@ NTSTATUS CommandListPopup::_swapUp(CookedRead& cookedReadData) noexcept { auto history = cookedReadData.History(); - if (history->GetNumberOfCommands() <= 1 || _currentCommand == 0) + if (history.GetNumberOfCommands() <= 1 || _currentCommand == 0) { return STATUS_SUCCESS; } - history->Swap(_currentCommand, _currentCommand - 1); + history.Swap(_currentCommand, _currentCommand - 1); _update(-1); _drawList(); } @@ -206,11 +206,11 @@ NTSTATUS CommandListPopup::_swapDown(CookedRead& cookedReadData) noexcept { auto history = cookedReadData.History(); - if (history->GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history->GetNumberOfCommands()) - 1i16) + if (history.GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history.GetNumberOfCommands()) - 1i16) { return STATUS_SUCCESS; } - history->Swap(_currentCommand, _currentCommand + 1); + history.Swap(_currentCommand, _currentCommand + 1); _update(1); _drawList(); } @@ -229,7 +229,7 @@ void CommandListPopup::_handleReturn(CookedRead& cookedReadData) void CommandListPopup::_cycleSelectionToMatchingCommands(CookedRead& cookedReadData, const wchar_t wch) { short Index = 0; - if (cookedReadData.History()->FindMatchingCommand({ &wch, 1 }, + if (cookedReadData.History().FindMatchingCommand({ &wch, 1 }, _currentCommand, Index, CommandHistory::MatchOptions::JustLooking)) @@ -346,7 +346,7 @@ void CommandListPopup::_drawList() CommandNumberLength)); // write command to screen - auto command = _history->GetNth(i); + auto command = _history.GetNth(i); lStringLength = command.size(); { size_t lTmpStringLength = lStringLength; @@ -423,13 +423,13 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) if (wrap) { // Modulo the number of commands to "circle" around if we went off the end. - NewCmdNum %= _history->GetNumberOfCommands(); + NewCmdNum %= _history.GetNumberOfCommands(); } else { - if (NewCmdNum >= gsl::narrow(_history->GetNumberOfCommands())) + if (NewCmdNum >= gsl::narrow(_history.GetNumberOfCommands())) { - NewCmdNum = gsl::narrow(_history->GetNumberOfCommands()) - 1i16; + NewCmdNum = gsl::narrow(_history.GetNumberOfCommands()) - 1i16; } else if (NewCmdNum < 0) { @@ -452,9 +452,9 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) else if (NewCmdNum > _bottomIndex) { _bottomIndex += delta; - if (_bottomIndex >= gsl::narrow(_history->GetNumberOfCommands())) + if (_bottomIndex >= gsl::narrow(_history.GetNumberOfCommands())) { - _bottomIndex = gsl::narrow(_history->GetNumberOfCommands()) - 1i16; + _bottomIndex = gsl::narrow(_history.GetNumberOfCommands()) - 1i16; } Scroll = true; } diff --git a/src/host/CommandListPopup.hpp b/src/host/CommandListPopup.hpp index 49d151681ac..bebe7567563 100644 --- a/src/host/CommandListPopup.hpp +++ b/src/host/CommandListPopup.hpp @@ -21,7 +21,7 @@ Module Name: class CommandListPopup : public Popup { public: - CommandListPopup(SCREEN_INFORMATION& screenInfo, const std::shared_ptr& history); + CommandListPopup(SCREEN_INFORMATION& screenInfo, CommandHistory& history); [[nodiscard]] NTSTATUS Process(CookedRead& cookedReadData) noexcept override; @@ -48,7 +48,7 @@ class CommandListPopup : public Popup SHORT _currentCommand; SHORT _bottomIndex; // number of command displayed on last line of popup - const std::shared_ptr& _history; + const CommandHistory& _history; #ifdef UNIT_TESTING friend class CommandListPopupTests; diff --git a/src/host/CommandNumberPopup.cpp b/src/host/CommandNumberPopup.cpp index c4f5a1f32fd..136e0e35064 100644 --- a/src/host/CommandNumberPopup.cpp +++ b/src/host/CommandNumberPopup.cpp @@ -102,7 +102,7 @@ void CommandNumberPopup::_handleEscape(CookedRead& cookedReadData) noexcept void CommandNumberPopup::_handleReturn(CookedRead& cookedReadData) noexcept { const short commandNumber = gsl::narrow(std::min(static_cast(_parse()), - cookedReadData.History()->GetNumberOfCommands() - 1)); + cookedReadData.History().GetNumberOfCommands() - 1)); CommandLine::Instance().EndAllPopups(); SetCurrentCommandLine(cookedReadData, commandNumber); diff --git a/src/host/CopyToCharPopup.cpp b/src/host/CopyToCharPopup.cpp index 4dcf7176883..74046651fc9 100644 --- a/src/host/CopyToCharPopup.cpp +++ b/src/host/CopyToCharPopup.cpp @@ -70,7 +70,7 @@ NTSTATUS CopyToCharPopup::Process(CookedRead& cookedReadData) noexcept } // copy up to specified char - const auto lastCommand = cookedReadData.History()->GetLastCommand(); + const auto lastCommand = cookedReadData.History().GetLastCommand(); if (!lastCommand.empty()) { _copyToChar(cookedReadData, lastCommand, wch); diff --git a/src/host/cmdline.cpp b/src/host/cmdline.cpp index fc22446c1a7..37800a6d1f1 100644 --- a/src/host/cmdline.cpp +++ b/src/host/cmdline.cpp @@ -254,7 +254,7 @@ void SetCurrentCommandLine(CookedRead& cookedReadData, _In_ SHORT Index) // inde NTSTATUS CommandLine::_startCommandListPopup(CookedRead& cookedReadData) { if (cookedReadData.HasHistory() && - cookedReadData.History()->GetNumberOfCommands()) + cookedReadData.History().GetNumberOfCommands()) { try { @@ -336,7 +336,7 @@ NTSTATUS CommandLine::_startCopyToCharPopup(CookedRead& cookedReadData) HRESULT CommandLine::StartCommandNumberPopup(CookedRead& cookedReadData) { if (cookedReadData.HasHistory() && - cookedReadData.History()->GetNumberOfCommands() && + cookedReadData.History().GetNumberOfCommands() && cookedReadData.ScreenInfo().GetBufferSize().Width() >= MINIMUM_COMMAND_PROMPT_SIZE + 2) { try @@ -380,12 +380,12 @@ void CommandLine::_processHistoryCycling(CookedRead& cookedReadData, return; } else if (searchDirection == CommandHistory::SearchDirection::Previous - && cookedReadData.History()->AtFirstCommand()) + && cookedReadData.History().AtFirstCommand()) { return; } else if (searchDirection == CommandHistory::SearchDirection::Next - && cookedReadData.History()->AtLastCommand()) + && cookedReadData.History().AtLastCommand()) { return; } @@ -543,8 +543,8 @@ void CommandLine::_deleteCommandHistory(CookedRead& cookedReadData) noexcept { if (cookedReadData.HasHistory()) { - cookedReadData.History()->Empty(); - cookedReadData.History()->Flags |= CLE_ALLOCATED; + cookedReadData.History().Empty(); + cookedReadData.History().Flags |= CLE_ALLOCATED; } } diff --git a/src/host/cookedRead.cpp b/src/host/cookedRead.cpp index d4565d3d088..5c2bb319d3d 100644 --- a/src/host/cookedRead.cpp +++ b/src/host/cookedRead.cpp @@ -55,9 +55,9 @@ bool CookedRead::HasHistory() const noexcept return _pCommandHistory != nullptr; } -std::shared_ptr CookedRead::History() noexcept +CommandHistory& CookedRead::History() noexcept { - return _pCommandHistory; + return *_pCommandHistory; } void CookedRead::Erase() diff --git a/src/host/cookedRead.hpp b/src/host/cookedRead.hpp index 02a48197043..86c69d7dbdd 100644 --- a/src/host/cookedRead.hpp +++ b/src/host/cookedRead.hpp @@ -69,7 +69,7 @@ class CookedRead final : public ReadData SCREEN_INFORMATION& ScreenInfo(); - std::shared_ptr History() noexcept; + CommandHistory& History() noexcept; bool HasHistory() const noexcept; void BufferInput(const wchar_t wch); @@ -112,7 +112,7 @@ class CookedRead final : public ReadData COORD _promptStartLocation; // the location of the cursor before a popup is launched COORD _beforePopupCursorPosition; - std::shared_ptr _pCommandHistory; // shared pointer + std::shared_ptr _pCommandHistory; // CommandHistory pointer // mask of control keys that if pressed will end the cooked read early const ULONG _ctrlWakeupMask; // current state of the CookedRead diff --git a/src/host/ut_host/CommandListPopupTests.cpp b/src/host/ut_host/CommandListPopupTests.cpp index fdb0fdc999b..1d98e28de44 100644 --- a/src/host/ut_host/CommandListPopupTests.cpp +++ b/src/host/ut_host/CommandListPopupTests.cpp @@ -86,7 +86,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -127,7 +127,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -164,7 +164,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -203,7 +203,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -241,7 +241,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -277,7 +277,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitLongHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -313,7 +313,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitLongHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -342,7 +342,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // set the current command selection to the top of the list popup._currentCommand = 0; @@ -373,7 +373,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -413,7 +413,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -450,7 +450,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData @@ -493,7 +493,7 @@ class CommandListPopupTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // prepare popup PopupTestHelper::InitHistory(*m_pHistory); - CommandListPopup popup{ gci.GetActiveOutputBuffer(), m_pHistory }; + CommandListPopup popup{ gci.GetActiveOutputBuffer(), *m_pHistory }; popup.SetUserInputFunction(fn); // prepare cookedReadData diff --git a/src/host/ut_host/CommandNumberPopupTests.cpp b/src/host/ut_host/CommandNumberPopupTests.cpp index 7184e556f69..784b6706098 100644 --- a/src/host/ut_host/CommandNumberPopupTests.cpp +++ b/src/host/ut_host/CommandNumberPopupTests.cpp @@ -122,7 +122,7 @@ class CommandNumberPopupTests // add popups to CommandLine auto& commandLine = CommandLine::Instance(); - commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer(), m_pHistory)); + commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer(), *m_pHistory)); commandLine._popups.emplace_front(std::make_unique(gci.GetActiveOutputBuffer())); auto& numberPopup = *commandLine._popups.front(); numberPopup.SetUserInputFunction(fn); From 3cd6f1b55439edd538bfb0921dae4b17c1842d71 Mon Sep 17 00:00:00 2001 From: Christophe Pichaud Date: Wed, 12 Jun 2019 02:04:55 +0200 Subject: [PATCH 4/5] Add missing const stuff. Remove a comment. --- src/host/CommandListPopup.cpp | 2 +- src/host/CommandListPopup.hpp | 2 +- src/host/cookedRead.hpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index a89763f5404..d9b04c4735a 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -50,7 +50,7 @@ static COORD calculatePopupSize(const CommandHistory& history) return { gsl::narrow(width), height }; } -CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, CommandHistory& history) : +CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const CommandHistory& history) : Popup(screenInfo, calculatePopupSize(history)), _history{ history }, _currentCommand{ std::min(history.LastDisplayed, static_cast(history.GetNumberOfCommands() - 1)) } diff --git a/src/host/CommandListPopup.hpp b/src/host/CommandListPopup.hpp index bebe7567563..71edd76a2ca 100644 --- a/src/host/CommandListPopup.hpp +++ b/src/host/CommandListPopup.hpp @@ -21,7 +21,7 @@ Module Name: class CommandListPopup : public Popup { public: - CommandListPopup(SCREEN_INFORMATION& screenInfo, CommandHistory& history); + CommandListPopup(SCREEN_INFORMATION& screenInfo, const CommandHistory& history); [[nodiscard]] NTSTATUS Process(CookedRead& cookedReadData) noexcept override; diff --git a/src/host/cookedRead.hpp b/src/host/cookedRead.hpp index 86c69d7dbdd..9a43c191501 100644 --- a/src/host/cookedRead.hpp +++ b/src/host/cookedRead.hpp @@ -112,7 +112,7 @@ class CookedRead final : public ReadData COORD _promptStartLocation; // the location of the cursor before a popup is launched COORD _beforePopupCursorPosition; - std::shared_ptr _pCommandHistory; // CommandHistory pointer + std::shared_ptr _pCommandHistory; // mask of control keys that if pressed will end the cooked read early const ULONG _ctrlWakeupMask; // current state of the CookedRead From e8104ed1943123ab2f52e90f62f4f6f053f644b9 Mon Sep 17 00:00:00 2001 From: Christophe Pichaud Date: Thu, 13 Jun 2019 00:12:44 +0200 Subject: [PATCH 5/5] cookedReadData.History() returns a ref. Callers adapted. OK. --- src/host/CommandListPopup.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index d9b04c4735a..a41b7352b15 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -151,7 +151,7 @@ NTSTATUS CommandListPopup::_deleteSelection(CookedRead& cookedReadData) noexcept { try { - auto history = cookedReadData.History(); + auto& history = cookedReadData.History(); history.Remove(static_cast(_currentCommand)); _setBottomIndex(); @@ -181,7 +181,7 @@ NTSTATUS CommandListPopup::_swapUp(CookedRead& cookedReadData) noexcept { try { - auto history = cookedReadData.History(); + auto& history = cookedReadData.History(); if (history.GetNumberOfCommands() <= 1 || _currentCommand == 0) { @@ -204,7 +204,7 @@ NTSTATUS CommandListPopup::_swapDown(CookedRead& cookedReadData) noexcept { try { - auto history = cookedReadData.History(); + auto& history = cookedReadData.History(); if (history.GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history.GetNumberOfCommands()) - 1i16) {