Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert CommandHistory to a std::shared_ptr #1161

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/host/cookedRead.cpp
Expand Up @@ -17,7 +17,7 @@
CookedRead::CookedRead(InputBuffer* const pInputBuffer,
INPUT_READ_HANDLE_DATA* const pInputReadHandleData,
SCREEN_INFORMATION& screenInfo,
CommandHistory* pCommandHistory,
std::shared_ptr<CommandHistory> pCommandHistory,
wchar_t* userBuffer,
const size_t cchUserBuffer,
const ULONG ctrlWakeupMask,
Expand Down
4 changes: 2 additions & 2 deletions src/host/cookedRead.hpp
Expand Up @@ -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<CommandHistory> pCommandHistory,
wchar_t* userBuffer,
const size_t cchUserBuffer,
const ULONG ctrlWakeupMask,
Expand Down Expand Up @@ -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<CommandHistory> _pCommandHistory;
// mask of control keys that if pressed will end the cooked read early
const ULONG _ctrlWakeupMask;
// current state of the CookedRead
Expand Down
66 changes: 33 additions & 33 deletions src/host/history.cpp
Expand Up @@ -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> CommandHistory::s_historyLists;
std::list<std::shared_ptr<CommandHistory>> CommandHistory::s_historyLists;

CommandHistory* CommandHistory::s_Find(const HANDLE processHandle)
std::shared_ptr<CommandHistory> 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;
}
}

Expand All @@ -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<CommandHistory>& History = CommandHistory::s_Find(processHandle);
if (History)
{
WI_ClearFlag(History->Flags, CLE_ALLOCATED);
Expand All @@ -64,9 +64,9 @@ void CommandHistory::s_ResizeAll(const size_t commands)
FAIL_FAST_IF(commands > SHORT_MAX);
gci.SetHistoryBufferSize(gsl::narrow<UINT>(commands));

for (auto& historyList : s_historyLists)
for (auto historyList : s_historyLists)
{
historyList.Realloc(commands);
historyList->Realloc(commands);
}
}

Expand Down Expand Up @@ -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<CommandHistory> backup = *it;
backup->Realloc(commands);

s_historyLists.erase(it);
s_historyLists.push_front(backup);
Expand All @@ -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> 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;
Expand All @@ -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> 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<CommandHistory> BestCandidate;
std::shared_ptr<CommandHistory> 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;
Expand All @@ -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<CommandHistory> History = std::make_shared<CommandHistory>();

History._appName = appName;
History.Flags = CLE_ALLOCATED;
History.LastDisplayed = -1;
History._maxCommands = gsl::narrow<SHORT>(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<SHORT>(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.
Expand All @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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<CommandHistory> pCommandHistory = CommandHistory::s_FindByExe(exeName);
if (nullptr != pCommandHistory)
{
size_t cchNeeded = 0;
Expand Down Expand Up @@ -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 = CommandHistory::s_FindByExe(exeName);

if (nullptr != CommandHistory)
{
Expand Down
8 changes: 4 additions & 4 deletions src/host/history.h
Expand Up @@ -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<CommandHistory> s_Allocate(const std::wstring_view appName, const HANDLE processHandle);
static std::shared_ptr<CommandHistory> s_Find(const HANDLE processHandle);
static std::shared_ptr<CommandHistory> 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);
Expand Down Expand Up @@ -93,7 +93,7 @@ class CommandHistory
std::wstring _appName;
HANDLE _processHandle;

static std::list<CommandHistory> s_historyLists;
static std::list<std::shared_ptr<CommandHistory>> s_historyLists;

public:
DWORD Flags;
Expand Down
2 changes: 1 addition & 1 deletion src/host/stream.cpp
Expand Up @@ -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<CommandHistory> pCommandHistory = CommandHistory::s_Find(processData);

try
{
Expand Down
4 changes: 2 additions & 2 deletions src/host/ut_host/CommandLineTests.cpp
Expand Up @@ -21,7 +21,7 @@ constexpr size_t PROMPT_SIZE = 512;
class CommandLineTests
{
std::unique_ptr<CommonState> m_state;
CommandHistory* m_pHistory;
std::shared_ptr<CommandHistory> m_pHistory;

TEST_CLASS(CommandLineTests);

Expand Down Expand Up @@ -72,7 +72,7 @@ class CommandLineTests
}

void InitCookedReadData(CookedRead& cookedReadData,
CommandHistory* pHistory,
std::shared_ptr<CommandHistory> pHistory,
const std::wstring prompt)
{
cookedReadData._pCommandHistory = pHistory;
Expand Down
2 changes: 1 addition & 1 deletion src/host/ut_host/CommandListPopupTests.cpp
Expand Up @@ -25,7 +25,7 @@ class CommandListPopupTests
TEST_CLASS(CommandListPopupTests);

std::unique_ptr<CommonState> m_state;
CommandHistory* m_pHistory;
std::shared_ptr<CommandHistory> m_pHistory;

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down
2 changes: 1 addition & 1 deletion src/host/ut_host/CommandNumberPopupTests.cpp
Expand Up @@ -25,7 +25,7 @@ class CommandNumberPopupTests
TEST_CLASS(CommandNumberPopupTests);

std::unique_ptr<CommonState> m_state;
CommandHistory* m_pHistory;
std::shared_ptr<CommandHistory> m_pHistory;

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down
2 changes: 1 addition & 1 deletion src/host/ut_host/CopyToCharPopupTests.cpp
Expand Up @@ -24,7 +24,7 @@ class CopyToCharPopupTests
TEST_CLASS(CopyToCharPopupTests);

std::unique_ptr<CommonState> m_state;
CommandHistory* m_pHistory;
std::shared_ptr<CommandHistory> m_pHistory;

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down