From 7b5b96dbf97be9b2e5579fe837f5926c7a18c4d9 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 27 Jan 2023 17:19:30 +0100 Subject: [PATCH 01/10] Excise TranslateUnicodeToOem and everything it touched --- src/host/dbcs.cpp | 74 -------- src/host/dbcs.h | 7 - src/host/directio.cpp | 38 +++-- src/host/inputBuffer.cpp | 151 ++++++++++++----- src/host/inputBuffer.hpp | 17 +- src/host/misc.h | 32 ++++ src/host/readDataCooked.cpp | 195 +++------------------ src/host/readDataDirect.cpp | 168 +++++++++--------- src/host/readDataDirect.hpp | 2 - src/host/readDataRaw.cpp | 131 ++------------ src/host/stream.cpp | 311 +++++++++------------------------- src/host/stream.h | 6 + src/inc/til/at.h | 27 +-- src/inc/til/bytes.h | 57 +++++++ src/inc/til/type_traits.h | 55 ++++++ src/server/ApiDispatchers.cpp | 7 +- 16 files changed, 496 insertions(+), 782 deletions(-) create mode 100644 src/inc/til/bytes.h create mode 100644 src/inc/til/type_traits.h diff --git a/src/host/dbcs.cpp b/src/host/dbcs.cpp index 0d5b51e21a4..4a526c5a64f 100644 --- a/src/host/dbcs.cpp +++ b/src/host/dbcs.cpp @@ -177,77 +177,3 @@ BOOL IsAvailableEastAsianCodePage(const UINT uiCodePage) return false; } } - -_Ret_range_(0, cbAnsi) - ULONG TranslateUnicodeToOem(_In_reads_(cchUnicode) PCWCHAR pwchUnicode, - const ULONG cchUnicode, - _Out_writes_bytes_(cbAnsi) PCHAR pchAnsi, - const ULONG cbAnsi, - _Out_ std::unique_ptr& partialEvent) -{ - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto TmpUni = new (std::nothrow) WCHAR[cchUnicode]; - if (TmpUni == nullptr) - { - return 0; - } - - memcpy(TmpUni, pwchUnicode, cchUnicode * sizeof(WCHAR)); - - BYTE AsciiDbcs[2]; - AsciiDbcs[1] = 0; - - ULONG i, j; - for (i = 0, j = 0; i < cchUnicode && j < cbAnsi; i++, j++) - { - if (IsGlyphFullWidth(TmpUni[i])) - { - const auto NumBytes = sizeof(AsciiDbcs); - ConvertToOem(gci.CP, &TmpUni[i], 1, (LPSTR)&AsciiDbcs[0], NumBytes); - if (IsDBCSLeadByteConsole(AsciiDbcs[0], &gci.CPInfo)) - { - if (j < cbAnsi - 1) - { // -1 is safe DBCS in buffer - pchAnsi[j] = AsciiDbcs[0]; - j++; - pchAnsi[j] = AsciiDbcs[1]; - AsciiDbcs[1] = 0; - } - else - { - pchAnsi[j] = AsciiDbcs[0]; - break; - } - } - else - { - pchAnsi[j] = AsciiDbcs[0]; - AsciiDbcs[1] = 0; - } - } - else - { - ConvertToOem(gci.CP, &TmpUni[i], 1, &pchAnsi[j], 1); - } - } - - if (AsciiDbcs[1]) - { - try - { - auto keyEvent = std::make_unique(); - if (keyEvent.get()) - { - keyEvent->SetCharData(AsciiDbcs[1]); - partialEvent.reset(static_cast(keyEvent.release())); - } - } - catch (...) - { - LOG_HR(wil::ResultFromCaughtException()); - } - } - - delete[] TmpUni; - return j; -} diff --git a/src/host/dbcs.h b/src/host/dbcs.h index 53762d11880..7974021d0d2 100644 --- a/src/host/dbcs.h +++ b/src/host/dbcs.h @@ -29,10 +29,3 @@ bool IsDBCSLeadByteConsole(const CHAR ch, const CPINFO* const pCPInfo); BYTE CodePageToCharSet(const UINT uiCodePage); BOOL IsAvailableEastAsianCodePage(const UINT uiCodePage); - -_Ret_range_(0, cbAnsi) - ULONG TranslateUnicodeToOem(_In_reads_(cchUnicode) PCWCHAR pwchUnicode, - const ULONG cchUnicode, - _Out_writes_bytes_(cbAnsi) PCHAR pchAnsi, - const ULONG cbAnsi, - _Out_ std::unique_ptr& partialEvent); diff --git a/src/host/directio.cpp b/src/host/directio.cpp index 382aa2d840c..651a32561d8 100644 --- a/src/host/directio.cpp +++ b/src/host/directio.cpp @@ -169,20 +169,30 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, LockConsole(); auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); + auto amountToRead = eventReadCount; + std::deque> partialEvents; if (!IsUnicode) { - if (inputBuffer.IsReadPartialByteSequenceAvailable()) + std::string buffer(eventReadCount, '\0'); + gsl::span writer{ buffer }; + + if (IsPeek) { - partialEvents.push_back(inputBuffer.FetchReadPartialByteSequence(IsPeek)); + inputBuffer.PeekCachedA(writer); + } + else + { + inputBuffer.ConsumeCachedA(writer); } - } - size_t amountToRead; - if (FAILED(SizeTSub(eventReadCount, partialEvents.size(), &amountToRead))) - { - return STATUS_INTEGER_OVERFLOW; + const auto read = buffer.size() - writer.size(); + std::string_view str{ buffer.data(), read }; + StringToInputEvents(str, partialEvents); + + amountToRead -= read; } + std::deque> readEvents; auto Status = inputBuffer.Read(readEvents, amountToRead, @@ -193,7 +203,6 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, if (CONSOLE_STATUS_WAIT == Status) { - FAIL_FAST_IF(!(readEvents.empty())); // If we're told to wait until later, move all of our context // to the read data object and send it back up to the server. waiter = std::make_unique(&inputBuffer, @@ -206,11 +215,7 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, // split key events to oem chars if necessary if (!IsUnicode) { - try - { - SplitToOem(readEvents); - } - CATCH_LOG(); + SplitToOem(readEvents); } // combine partial and readEvents @@ -231,12 +236,11 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, readEvents.pop_front(); } - // store partial event if necessary if (!readEvents.empty()) { - inputBuffer.StoreReadPartialByteSequence(std::move(readEvents.front())); - readEvents.pop_front(); - FAIL_FAST_IF(!(readEvents.empty())); + std::string buffer; + InputEventsToString(readEvents, buffer); + inputBuffer.CacheA(buffer); } } return Status; diff --git a/src/host/inputBuffer.cpp b/src/host/inputBuffer.cpp index 2427d4c19ed..ffdd6ba835d 100644 --- a/src/host/inputBuffer.cpp +++ b/src/host/inputBuffer.cpp @@ -7,8 +7,9 @@ #include "stream.h" #include "../types/inc/GlyphWidth.hpp" -#include +#include +#include "misc.h" #include "../interactivity/inc/ServiceLocator.hpp" #define INPUT_BUFFER_DEFAULT_INPUT_MODE (ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT | ENABLE_ECHO_INPUT | ENABLE_MOUSE_INPUT) @@ -36,59 +37,129 @@ InputBuffer::InputBuffer() : fInComposition = false; } -// Routine Description: -// - This routine frees the resources associated with an input buffer. -// Arguments: -// - None -// Return Value: -InputBuffer::~InputBuffer() = default; - -// Routine Description: -// - checks if any partial char data is available for reading operation -// Arguments: -// - None -// Return Value: -// - true if partial char data is available, false otherwise -bool InputBuffer::IsReadPartialByteSequenceAvailable() +// Transfer as many `wchar_t`s from source over to the `wchar_t` buffer `target`. After it returns, +// the start of the `source` and `target` slices will be offset by as many bytes as have been copied +// over, so that if you call this function again it'll continue copying from wherever it left off. +// This function is intended for use with our W APIs. +void InputBuffer::ConsumeW(std::wstring_view& source, gsl::span& target) { - return _readPartialByteSequence.get() != nullptr; + if (!_aBuffer.empty()) + { + _resetBufferA(); + } + + if (source.empty() || target.empty()) + { + return; + } + + til::bytes_transfer(target, source); } -// Routine Description: -// - reads any read partial char data available -// Arguments: -// - peek - if true, data will not be removed after being fetched -// Return Value: -// - the partial char data. may be nullptr if no data is available -std::unique_ptr InputBuffer::FetchReadPartialByteSequence(_In_ bool peek) +// Transfer as many `wchar_t`s from source over to the `char` buffer `target`. After it returns, +// the start of the `source` and `target` slices will be offset by as many bytes as have been copied +// over, so that if you call this function again it'll continue copying from wherever it left off. +// This function is intended for use with our A APIs and performs the necessary `WideCharToMultiByte` +// conversion. Since not all converted `char`s might fit into `target` it'll cache the remainder. +// The next time this function is called those cached `char`s will then be the first to be copied over. +void InputBuffer::ConsumeA(std::wstring_view& source, gsl::span& target) { - if (!IsReadPartialByteSequenceAvailable()) + // `_aBufferReader` might still contain target data from a previous invocation. + ConsumeCachedA(target); + + if (source.empty() || target.empty()) { - return std::unique_ptr(); + return; } - if (peek) + // The above block should either leave `target` or `_aBufferReader` empty (or both). + // If we're here, `_aBufferReader` should be empty. + assert(_aBufferReader.empty()); + + const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().CP; + + // Fast path: Batch convert all data in case the user provided buffer is large enough. { - return IInputEvent::Create(_readPartialByteSequence->ToInputRecord()); + const auto wideLength = gsl::narrow(source.size()); + const auto narrowLength = gsl::narrow(target.size()); + + const auto length = WideCharToMultiByte(cp, 0, source.data(), wideLength, target.data(), narrowLength, nullptr, nullptr); + if (length > 0) + { + source = {}; + til::bytes_advance(target, gsl::narrow_cast(length)); + return; + } + + const auto error = GetLastError(); + THROW_HR_IF(HRESULT_FROM_WIN32(error), error != ERROR_INSUFFICIENT_BUFFER); } - else + + // Slow path: Character-wise conversion otherwise. We do this in order to only + // consume as many characters from `source` as necessary to fill `target`. { - std::unique_ptr outEvent; - outEvent.swap(_readPartialByteSequence); - return outEvent; + size_t read = 0; + + for (const auto& wch : source) + { + char buffer[8]; + const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); + THROW_LAST_ERROR_IF(length <= 0); + + std::string_view slice{ &buffer[0], gsl::narrow_cast(length) }; + til::bytes_transfer(target, slice); + + ++read; + + if (!slice.empty()) + { + _aBuffer = slice; + _aBufferReader = _aBuffer; + break; + } + } + + source = source.substr(read); } } -// Routine Description: -// - stores partial read char data for a later read. will overwrite -// any previously stored data. -// Arguments: -// - event - The event to store -// Return Value: -// - None -void InputBuffer::StoreReadPartialByteSequence(std::unique_ptr event) +// Same as `ConsumeA`, but without any `source` characters. +void InputBuffer::ConsumeCachedA(gsl::span& target) +{ + if (_aBufferReader.empty()) + { + return; + } + + til::bytes_transfer(target, _aBufferReader); + + if (_aBufferReader.empty()) + { + // This is just so that we release memory eagerly. + _aBuffer = std::string{}; + } +} + +// Same as `ConsumeCachedA`, but as the name indicates it doesn't "consume" the amount of data that has been copied +// into `target`. Still, the `target` slice will have been offset the same way `ConsumeW` and `ConsumeA` do it. +void InputBuffer::PeekCachedA(gsl::span& target) +{ + auto reader = _aBufferReader; + til::bytes_transfer(target, reader); +} + +// This function allows you to manually add some data into the internal buffer used by `ConsumeA`, etc. +void InputBuffer::CacheA(std::string_view source) +{ + const auto off = _aBufferReader.size() - _aBuffer.size(); + _aBuffer.append(source); + _aBufferReader = std::string_view{ _aBuffer }.substr(off); +} + +void InputBuffer::_resetBufferA() { - _readPartialByteSequence.swap(event); + _aBuffer = std::string{}; + _aBufferReader = {}; } // Routine Description: diff --git a/src/host/inputBuffer.hpp b/src/host/inputBuffer.hpp index 5c003faf03c..c210f6a154f 100644 --- a/src/host/inputBuffer.hpp +++ b/src/host/inputBuffer.hpp @@ -41,12 +41,13 @@ class InputBuffer final : public ConsoleObjectHeader bool fInComposition; // specifies if there's an ongoing text composition InputBuffer(); - ~InputBuffer(); - // storage API for partial dbcs bytes being read from the buffer - bool IsReadPartialByteSequenceAvailable(); - std::unique_ptr FetchReadPartialByteSequence(_In_ bool peek); - void StoreReadPartialByteSequence(std::unique_ptr event); + void ConsumeW(std::wstring_view& source, gsl::span& target); + + void ConsumeA(std::wstring_view& source, gsl::span& target); + void ConsumeCachedA(gsl::span& target); + void PeekCachedA(gsl::span& target); + void CacheA(std::string_view source); // storage API for partial dbcs bytes being written to the buffer bool IsWritePartialByteSequenceAvailable(); @@ -84,8 +85,10 @@ class InputBuffer final : public ConsoleObjectHeader void PassThroughWin32MouseRequest(bool enable); private: + std::string _aBuffer; + std::string_view _aBufferReader; + std::deque> _storage; - std::unique_ptr _readPartialByteSequence; std::unique_ptr _writePartialByteSequence; Microsoft::Console::VirtualTerminal::TerminalInput _termInput; Microsoft::Console::Render::VtEngine* _pTtyConnection; @@ -96,6 +99,8 @@ class InputBuffer final : public ConsoleObjectHeader // Otherwise, we should be calling them. bool _vtInputShouldSuppress{ false }; + void _resetBufferA(); + void _ReadBuffer(_Out_ std::deque>& outEvents, const size_t readCount, _Out_ size_t& eventsRead, diff --git a/src/host/misc.h b/src/host/misc.h index 6269353f5c0..f76e1099e9a 100644 --- a/src/host/misc.h +++ b/src/host/misc.h @@ -44,6 +44,38 @@ int ConvertToOem(const UINT uiCodePage, _Out_writes_(cchTarget) CHAR* const pchTarget, const UINT cchTarget) noexcept; +template +void StringToInputEvents(std::basic_string_view str, std::deque>& events) +{ + for (const auto& ch : str) + { + auto event = std::make_unique(); + event->SetKeyDown(true); + event->SetRepeatCount(1); + event->SetCharData(ch); + events.push_back(std::move(event)); + } +} + +template +void InputEventsToString(std::deque>& events, std::basic_string& str) +{ + for (const auto& event : events) + { + if (event->EventType() == InputEventType::KeyEvent) + { + const auto keyEvent = static_cast(event.get()); + // This keydown check prevents us from serializing ABC keypresses + // into AABBCC because each key-down is followed by a key-up. + if (keyEvent->IsKeyDown()) + { + const auto wch = keyEvent->GetCharData(); + str.push_back(static_cast(wch)); + } + } + } +} + void SplitToOem(std::deque>& events); int ConvertInputToUnicode(const UINT uiCodePage, diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 6ade6bf5f53..eb461509813 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -1003,211 +1003,62 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline // - Status code that indicates success, out of memory, etc. [[nodiscard]] NTSTATUS COOKED_READ_DATA::_handlePostCharInputLoop(const bool isUnicode, size_t& numBytes, ULONG& controlKeyState) noexcept { + gsl::span writer{ reinterpret_cast(_userBuffer), _userBufferSize }; + std::wstring_view input{ _backupLimit, _bytesRead / sizeof(wchar_t) }; DWORD LineCount = 1; if (_echoInput) { - // Figure out where real string ends (at carriage return or end of buffer). - auto StringPtr = _backupLimit; - auto StringLength = _bytesRead / sizeof(WCHAR); - auto FoundCR = false; - for (size_t i = 0; i < StringLength; i++) - { - if (*StringPtr++ == UNICODE_CARRIAGERETURN) - { - StringLength = i; - FoundCR = true; - break; - } - } - - if (FoundCR) + const auto idx = input.find(UNICODE_CARRIAGERETURN); + if (idx != decltype(input)::npos) { if (_commandHistory) { - // add to command line recall list if we have a history list. auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - LOG_IF_FAILED(_commandHistory->Add({ _backupLimit, StringLength }, - WI_IsFlagSet(gci.Flags, CONSOLE_HISTORY_NODUP))); + LOG_IF_FAILED(_commandHistory->Add({ _backupLimit, idx }, WI_IsFlagSet(gci.Flags, CONSOLE_HISTORY_NODUP))); } - Tracing::s_TraceCookedRead(_clientProcess, - _backupLimit, - base::saturated_cast(StringLength)); - - // check for alias + Tracing::s_TraceCookedRead(_clientProcess, _backupLimit, base::saturated_cast(idx)); ProcessAliases(LineCount); - } - } - - auto fAddDbcsLead = false; - size_t NumBytes = 0; - // at this point, a->NumBytes contains the number of bytes in - // the UNICODE string read. UserBufferSize contains the converted - // size of the app's buffer. - if (_bytesRead > _userBufferSize || LineCount > 1) - { - if (LineCount > 1) - { - PWSTR Tmp; - if (!isUnicode) - { - if (_pInputBuffer->IsReadPartialByteSequenceAvailable()) - { - fAddDbcsLead = true; - auto event = GetInputBuffer()->FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *_userBuffer = static_cast(pKeyEvent->GetCharData()); - _userBuffer++; - _userBufferSize -= sizeof(wchar_t); - } - - NumBytes = 0; - for (Tmp = _backupLimit; - *Tmp != UNICODE_LINEFEED && _userBufferSize / sizeof(WCHAR) > NumBytes; - Tmp++) - { - NumBytes += IsGlyphFullWidth(*Tmp) ? 2 : 1; - } - } - // clang-format off -#pragma prefast(suppress: __WARNING_BUFFER_OVERFLOW, "LineCount > 1 means there's a UNICODE_LINEFEED") - // clang-format on - for (Tmp = _backupLimit; *Tmp != UNICODE_LINEFEED; Tmp++) - { - FAIL_FAST_IF(!(Tmp < (_backupLimit + _bytesRead))); - } - - numBytes = (ULONG)(Tmp - _backupLimit + 1) * sizeof(*Tmp); - } - else - { - if (!isUnicode) + if (LineCount > 1) { - PWSTR Tmp; - - if (_pInputBuffer->IsReadPartialByteSequenceAvailable()) - { - fAddDbcsLead = true; - auto event = GetInputBuffer()->FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *_userBuffer = static_cast(pKeyEvent->GetCharData()); - _userBuffer++; - _userBufferSize -= sizeof(wchar_t); - } - NumBytes = 0; - auto NumToWrite = _bytesRead; - for (Tmp = _backupLimit; - NumToWrite && _userBufferSize / sizeof(WCHAR) > NumBytes; - Tmp++, NumToWrite -= sizeof(WCHAR)) - { - NumBytes += IsGlyphFullWidth(*Tmp) ? 2 : 1; - } + input = input.substr(0, idx + 1); } - numBytes = _userBufferSize; } + } - __analysis_assume(numBytes <= _userBufferSize); - memmove(_userBuffer, _backupLimit, numBytes); - - const auto pInputReadHandleData = GetInputReadHandleData(); - const std::wstring_view pending{ _backupLimit + (numBytes / sizeof(wchar_t)), (_bytesRead - numBytes) / sizeof(wchar_t) }; - if (LineCount > 1) - { - pInputReadHandleData->SaveMultilinePendingInput(pending); - } - else - { - pInputReadHandleData->SavePendingInput(pending); - } + if (isUnicode) + { + GetInputBuffer()->ConsumeW(input, writer); } else { - if (!isUnicode) - { - PWSTR Tmp; - - if (_pInputBuffer->IsReadPartialByteSequenceAvailable()) - { - fAddDbcsLead = true; - auto event = GetInputBuffer()->FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *_userBuffer = static_cast(pKeyEvent->GetCharData()); - _userBuffer++; - _userBufferSize -= sizeof(wchar_t); - - if (_userBufferSize == 0) - { - numBytes = 1; - return STATUS_SUCCESS; - } - } - NumBytes = 0; - auto NumToWrite = _bytesRead; - for (Tmp = _backupLimit; - NumToWrite && _userBufferSize / sizeof(WCHAR) > NumBytes; - Tmp++, NumToWrite -= sizeof(WCHAR)) - { - NumBytes += IsGlyphFullWidth(*Tmp) ? 2 : 1; - } - } - - numBytes = _bytesRead; - - if (numBytes > _userBufferSize) - { - return STATUS_BUFFER_OVERFLOW; - } - - memmove(_userBuffer, _backupLimit, numBytes); + GetInputBuffer()->ConsumeA(input, writer); } - controlKeyState = _controlKeyState; - if (!isUnicode) + if (!input.empty()) { - // if ansi, translate string. - std::unique_ptr tempBuffer; - try - { - tempBuffer = std::make_unique(NumBytes); - } - catch (...) - { - return STATUS_NO_MEMORY; - } - - std::unique_ptr partialEvent; - numBytes = TranslateUnicodeToOem(_userBuffer, - gsl::narrow(numBytes / sizeof(wchar_t)), - tempBuffer.get(), - gsl::narrow(NumBytes), - partialEvent); - - if (partialEvent.get()) - { - GetInputBuffer()->StoreReadPartialByteSequence(std::move(partialEvent)); - } - - if (numBytes > _userBufferSize) + if (LineCount > 1) { - return STATUS_BUFFER_OVERFLOW; + GetInputReadHandleData()->SaveMultilinePendingInput(input); } - - memmove(_userBuffer, tempBuffer.get(), numBytes); - if (fAddDbcsLead) + else { - numBytes++; + GetInputReadHandleData()->SavePendingInput(input); } } + + numBytes = _userBufferSize - writer.size(); + controlKeyState = _controlKeyState; return STATUS_SUCCESS; } void COOKED_READ_DATA::MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) { // See the comment in WaitBlock.cpp for more information. - if (_userBuffer == reinterpret_cast(oldBuffer)) + if (_userBuffer == static_cast(oldBuffer)) { - _userBuffer = reinterpret_cast(newBuffer); + _userBuffer = static_cast(newBuffer); } } diff --git a/src/host/readDataDirect.cpp b/src/host/readDataDirect.cpp index 21045a9ff4a..d2a44d2d54b 100644 --- a/src/host/readDataDirect.cpp +++ b/src/host/readDataDirect.cpp @@ -10,9 +10,7 @@ // Routine Description: // - Constructs direct read data class to hold context across sessions -// generally when there's not enough data to return. Also used a bit -// internally to just pass some information along the stack -// (regardless of wait necessity). +// generally when there's not enough data to return. // Arguments: // - pInputBuffer - Buffer that data will be read from. // - pInputReadHandleData - Context stored across calls from the same @@ -28,16 +26,10 @@ DirectReadData::DirectReadData(_In_ InputBuffer* const pInputBuffer, _In_ std::deque> partialEvents) : ReadData(pInputBuffer, pInputReadHandleData), _eventReadCount{ eventReadCount }, - _partialEvents{ std::move(partialEvents) }, - _outEvents{} + _partialEvents{ std::move(partialEvents) } { } -// Routine Description: -// - Destructs a read data class. -// - Decrements count of readers waiting on the given handle. -DirectReadData::~DirectReadData() = default; - // Routine Description: // - This routine is called to complete a direct read that blocked in // ReadInputBuffer. The context of the read was saved in the DirectReadData @@ -68,18 +60,18 @@ bool DirectReadData::Notify(const WaitTerminationReason TerminationReason, _Out_ size_t* const pNumBytes, _Out_ DWORD* const pControlKeyState, _Out_ void* const pOutputData) +try { FAIL_FAST_IF_NULL(pOutputData); - FAIL_FAST_IF(_pInputReadHandleData->GetReadCount() == 0); const auto& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); - FAIL_FAST_IF(!gci.IsConsoleLocked()); + assert(gci.IsConsoleLocked()); *pReplyStatus = STATUS_SUCCESS; *pControlKeyState = 0; *pNumBytes = 0; - auto retVal = true; + std::deque> readEvents; // If ctrl-c or ctrl-break was seen, ignore it. @@ -88,105 +80,111 @@ bool DirectReadData::Notify(const WaitTerminationReason TerminationReason, return false; } - // check if a partial byte is already stored that we should read - if (!fIsUnicode && - _pInputBuffer->IsReadPartialByteSequenceAvailable() && - _eventReadCount == 1) - { - _partialEvents.push_back(_pInputBuffer->FetchReadPartialByteSequence(false)); - } - // See if called by CsrDestroyProcess or CsrDestroyThread // via ConsoleNotifyWaitBlock. If so, just decrement the ReadCount and return. if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::ThreadDying)) { *pReplyStatus = STATUS_THREAD_IS_TERMINATING; + return true; } + // We must see if we were woken up because the handle is being // closed. If so, we decrement the read count. If it goes to // zero, we wake up the close thread. Otherwise, we wake up any // other thread waiting for data. - else if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::HandleClosing)) + if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::HandleClosing)) { *pReplyStatus = STATUS_ALERTED; + return true; + } + + // if we get to here, this routine was called either by the input + // thread or a write routine. both of these callers grab the + // current console lock. + + // calculate how many events we need to read + size_t amountAlreadyRead; + if (FAILED(SizeTAdd(_partialEvents.size(), _outEvents.size(), &amountAlreadyRead))) + { + *pReplyStatus = STATUS_INTEGER_OVERFLOW; + return true; } - else + size_t amountToRead; + if (FAILED(SizeTSub(_eventReadCount, amountAlreadyRead, &amountToRead))) { - // if we get to here, this routine was called either by the input - // thread or a write routine. both of these callers grab the - // current console lock. + *pReplyStatus = STATUS_INTEGER_OVERFLOW; + return true; + } - // calculate how many events we need to read - size_t amountAlreadyRead; - if (FAILED(SizeTAdd(_partialEvents.size(), _outEvents.size(), &amountAlreadyRead))) - { - *pReplyStatus = STATUS_INTEGER_OVERFLOW; - return retVal; - } - size_t amountToRead; - if (FAILED(SizeTSub(_eventReadCount, amountAlreadyRead, &amountToRead))) - { - *pReplyStatus = STATUS_INTEGER_OVERFLOW; - return retVal; - } + // check if partial bytes are already stored that we should read + if (!fIsUnicode && amountToRead > 0) + { + std::string buffer(amountToRead, '\0'); + gsl::span writer{ buffer }; + _pInputBuffer->ConsumeCachedA(writer); - *pReplyStatus = _pInputBuffer->Read(readEvents, - amountToRead, - false, - false, - fIsUnicode, - false); + const auto read = buffer.size() - writer.size(); + std::string_view str{ buffer.data(), read }; + StringToInputEvents(str, _partialEvents); - if (*pReplyStatus == CONSOLE_STATUS_WAIT) - { - retVal = false; - } + amountToRead -= read; } - if (*pReplyStatus != CONSOLE_STATUS_WAIT) + *pReplyStatus = _pInputBuffer->Read(readEvents, + amountToRead, + false, + false, + fIsUnicode, + false); + + if (*pReplyStatus == CONSOLE_STATUS_WAIT) { - // split key events to oem chars if necessary - if (*pReplyStatus == STATUS_SUCCESS && !fIsUnicode) - { - try - { - SplitToOem(readEvents); - } - CATCH_LOG(); - } + return false; + } - // combine partial and whole events - while (!_partialEvents.empty()) - { - readEvents.push_front(std::move(_partialEvents.back())); - _partialEvents.pop_back(); - } + // split key events to oem chars if necessary + if (!fIsUnicode && *pReplyStatus == STATUS_SUCCESS) + { + SplitToOem(readEvents); + } - // move read events to out storage - for (size_t i = 0; i < _eventReadCount; ++i) - { - if (readEvents.empty()) - { - break; - } - _outEvents.push_back(std::move(readEvents.front())); - readEvents.pop_front(); - } + // combine partial and whole events + while (!_partialEvents.empty()) + { + readEvents.push_front(std::move(_partialEvents.back())); + _partialEvents.pop_back(); + } - // store partial event if necessary - if (!readEvents.empty()) + // move read events to out storage + for (size_t i = 0; i < _eventReadCount; ++i) + { + if (readEvents.empty()) { - _pInputBuffer->StoreReadPartialByteSequence(std::move(readEvents.front())); - readEvents.pop_front(); - FAIL_FAST_IF(!(readEvents.empty())); + break; } + _outEvents.push_back(std::move(readEvents.front())); + readEvents.pop_front(); + } - // move events to pOutputData - const auto pOutputDeque = reinterpret_cast>* const>(pOutputData); - *pNumBytes = _outEvents.size() * sizeof(INPUT_RECORD); - pOutputDeque->swap(_outEvents); + if (!readEvents.empty()) + { + std::string buffer; + InputEventsToString(readEvents, buffer); + _pInputBuffer->CacheA(buffer); } - return retVal; + + // move events to pOutputData + const auto pOutputDeque = static_cast>* const>(pOutputData); + *pNumBytes = _outEvents.size() * sizeof(INPUT_RECORD); + *pOutputDeque = std::move(_outEvents); + + return true; +} +catch (...) +{ + // There doesn't seem to be a way to just get the NT exception code with WIL. + *pReplyStatus = NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); + return true; } void DirectReadData::MigrateUserBuffersOnTransitionToBackgroundWait(const void* /*oldBuffer*/, void* /*newBuffer*/) diff --git a/src/host/readDataDirect.hpp b/src/host/readDataDirect.hpp index d1ad2e50334..a12af8becf9 100644 --- a/src/host/readDataDirect.hpp +++ b/src/host/readDataDirect.hpp @@ -38,8 +38,6 @@ class DirectReadData final : public ReadData DirectReadData(DirectReadData&&) = default; - ~DirectReadData() override; - void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) override; bool Notify(const WaitTerminationReason TerminationReason, const bool fIsUnicode, diff --git a/src/host/readDataRaw.cpp b/src/host/readDataRaw.cpp index c1f5be05b85..c632d23bc4e 100644 --- a/src/host/readDataRaw.cpp +++ b/src/host/readDataRaw.cpp @@ -31,7 +31,6 @@ RAW_READ_DATA::RAW_READ_DATA(_In_ InputBuffer* const pInputBuffer, _BufferSize{ BufferSize }, _BufPtr{ THROW_HR_IF_NULL(E_INVALIDARG, BufPtr) } { - THROW_HR_IF(E_INVALIDARG, _BufferSize % sizeof(wchar_t) != 0); THROW_HR_IF(E_INVALIDARG, _BufferSize == 0); } @@ -82,146 +81,50 @@ bool RAW_READ_DATA::Notify(const WaitTerminationReason TerminationReason, *pControlKeyState = 0; *pNumBytes = 0; - size_t NumBytes = 0; - - PWCHAR lpBuffer; - auto RetVal = true; - auto fAddDbcsLead = false; - auto fSkipFinally = false; // If a ctrl-c is seen, don't terminate read. If ctrl-break is seen, terminate read. if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::CtrlC)) { return false; } - else if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::CtrlBreak)) + + if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::CtrlBreak)) { *pReplyStatus = STATUS_ALERTED; + return true; } + // See if we were called because the thread that owns this wait block is exiting. - else if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::ThreadDying)) + if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::ThreadDying)) { *pReplyStatus = STATUS_THREAD_IS_TERMINATING; + return true; } + // We must see if we were woken up because the handle is being // closed. If so, we decrement the read count. If it goes to zero, // we wake up the close thread. Otherwise, we wake up any other // thread waiting for data. - else if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::HandleClosing)) + if (WI_IsFlagSet(TerminationReason, WaitTerminationReason::HandleClosing)) { *pReplyStatus = STATUS_ALERTED; - } - else - { - // If we get to here, this routine was called either by the input - // thread or a write routine. Both of these callers grab the current - // console lock. - - lpBuffer = _BufPtr; - - if (!fIsUnicode && _pInputBuffer->IsReadPartialByteSequenceAvailable()) - { - auto event = _pInputBuffer->FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *lpBuffer = static_cast(pKeyEvent->GetCharData()); - _BufferSize -= sizeof(wchar_t); - *pReplyStatus = STATUS_SUCCESS; - fAddDbcsLead = true; - - if (_BufferSize == 0) - { - *pNumBytes = 1; - RetVal = false; - fSkipFinally = true; - } - } - else - { - // This call to GetChar may block. - *pReplyStatus = GetChar(_pInputBuffer, - lpBuffer, - true, - nullptr, - nullptr, - nullptr); - } - - if (!NT_SUCCESS(*pReplyStatus) || fSkipFinally) - { - if (*pReplyStatus == CONSOLE_STATUS_WAIT) - { - RetVal = false; - } - } - else - { - NumBytes += IsGlyphFullWidth(*lpBuffer) ? 2 : 1; - lpBuffer++; - *pNumBytes += sizeof(WCHAR); - while (*pNumBytes < _BufferSize) - { - // This call to GetChar won't block. - *pReplyStatus = GetChar(_pInputBuffer, - lpBuffer, - false, - nullptr, - nullptr, - nullptr); - if (!NT_SUCCESS(*pReplyStatus)) - { - *pReplyStatus = STATUS_SUCCESS; - break; - } - NumBytes += IsGlyphFullWidth(*lpBuffer) ? 2 : 1; - lpBuffer++; - *pNumBytes += sizeof(WCHAR); - } - } + return true; } - // If the read was completed (status != wait), free the raw read data. - if (*pReplyStatus != CONSOLE_STATUS_WAIT && - !fSkipFinally && - !fIsUnicode) - { - // It's ansi, so translate the string. - std::unique_ptr tempBuffer; - try - { - tempBuffer = std::make_unique(NumBytes); - } - catch (...) - { - return true; - } + // If we get to here, this routine was called either by the input + // thread or a write routine. Both of these callers grab the current + // console lock. - lpBuffer = _BufPtr; - std::unique_ptr partialEvent; - - *pNumBytes = TranslateUnicodeToOem(lpBuffer, - gsl::narrow(*pNumBytes / sizeof(wchar_t)), - tempBuffer.get(), - gsl::narrow(NumBytes), - partialEvent); - if (partialEvent.get()) - { - _pInputBuffer->StoreReadPartialByteSequence(std::move(partialEvent)); - } - - memmove(lpBuffer, tempBuffer.get(), *pNumBytes); - if (fAddDbcsLead) - { - (*pNumBytes)++; - } - } - return RetVal; + gsl::span buffer{ reinterpret_cast(_BufPtr), _BufferSize }; + *pReplyStatus = _ReadCharacterInput(*_pInputBuffer, buffer, *pNumBytes, *_pInputReadHandleData, fIsUnicode); + return *pReplyStatus != CONSOLE_STATUS_WAIT; } void RAW_READ_DATA::MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) { // See the comment in WaitBlock.cpp for more information. - if (_BufPtr == reinterpret_cast(oldBuffer)) + if (_BufPtr == static_cast(oldBuffer)) { - _BufPtr = reinterpret_cast(newBuffer); + _BufPtr = static_cast(newBuffer); } } diff --git a/src/host/stream.cpp b/src/host/stream.cpp index 23dd8002719..f818c9f0369 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -6,6 +6,8 @@ #include "_stream.h" #include "stream.h" +#include + #include "dbcs.h" #include "handle.h" #include "misc.h" @@ -285,151 +287,46 @@ til::CoordType RetrieveNumberOfSpaces(_In_ til::CoordType sOriginalCursorPositio size_t& bytesRead, INPUT_READ_HANDLE_DATA& readHandleState, const bool unicode) +try { - // TODO: MSFT: 18047766 - Correct this method to not play byte counting games. - auto fAddDbcsLead = FALSE; - size_t NumToWrite = 0; - size_t NumToBytes = 0; - auto pBuffer = reinterpret_cast(buffer.data()); - auto bufferRemaining = buffer.size_bytes(); bytesRead = 0; - if (buffer.size_bytes() < sizeof(wchar_t)) - { - return STATUS_BUFFER_TOO_SMALL; - } - - const auto pending = readHandleState.GetPendingInput(); - auto pendingBytes = pending.size() * sizeof(wchar_t); - auto Tmp = pending.cbegin(); + auto pending = readHandleState.GetPendingInput(); if (readHandleState.IsMultilineInput()) { - if (!unicode) + const auto idx = pending.find(UNICODE_LINEFEED); + if (idx != decltype(pending)::npos) { - if (inputBuffer.IsReadPartialByteSequenceAvailable()) - { - auto event = inputBuffer.FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *pBuffer = static_cast(pKeyEvent->GetCharData()); - ++pBuffer; - bufferRemaining -= sizeof(wchar_t); - pendingBytes -= sizeof(wchar_t); - fAddDbcsLead = TRUE; - } - - if (pendingBytes == 0 || bufferRemaining == 0) - { - readHandleState.CompletePending(); - bytesRead = 1; - return STATUS_SUCCESS; - } - else - { - for (NumToWrite = 0, Tmp = pending.cbegin(), NumToBytes = 0; - NumToBytes < pendingBytes && - NumToBytes < bufferRemaining / sizeof(wchar_t) && - *Tmp != UNICODE_LINEFEED; - Tmp++, NumToWrite += sizeof(wchar_t)) - { - NumToBytes += IsGlyphFullWidth(*Tmp) ? 2 : 1; - } - } + // +1 to include the newline. + pending = pending.substr(0, idx + 1); } + } - NumToWrite = 0; - Tmp = pending.cbegin(); - while (NumToWrite < pendingBytes && - *Tmp != UNICODE_LINEFEED) - { - ++Tmp; - NumToWrite += sizeof(wchar_t); - } + gsl::span writer{ buffer }; - NumToWrite += sizeof(wchar_t); - if (NumToWrite > bufferRemaining) - { - NumToWrite = bufferRemaining; - } + if (unicode) + { + inputBuffer.ConsumeW(pending, writer); } else { - if (!unicode) - { - if (inputBuffer.IsReadPartialByteSequenceAvailable()) - { - auto event = inputBuffer.FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *pBuffer = static_cast(pKeyEvent->GetCharData()); - ++pBuffer; - bufferRemaining -= sizeof(wchar_t); - pendingBytes -= sizeof(wchar_t); - fAddDbcsLead = TRUE; - } - - if (pendingBytes == 0) - { - readHandleState.CompletePending(); - bytesRead = 1; - return STATUS_SUCCESS; - } - else - { - for (NumToWrite = 0, Tmp = pending.cbegin(), NumToBytes = 0; - NumToBytes < pendingBytes && NumToBytes < bufferRemaining / sizeof(wchar_t); - Tmp++, NumToWrite += sizeof(wchar_t)) - { - NumToBytes += IsGlyphFullWidth(*Tmp) ? 2 : 1; - } - } - } - - NumToWrite = (bufferRemaining < pendingBytes) ? bufferRemaining : pendingBytes; + inputBuffer.ConsumeA(pending, writer); } - memmove(pBuffer, pending.data(), NumToWrite); - pendingBytes -= NumToWrite; - if (pendingBytes != 0) + if (pending.empty()) { - std::wstring_view remainingPending{ pending.data() + (NumToWrite / sizeof(wchar_t)), pendingBytes / sizeof(wchar_t) }; - readHandleState.UpdatePending(remainingPending); + readHandleState.CompletePending(); } else { - readHandleState.CompletePending(); + readHandleState.UpdatePending(pending); } - if (!unicode) - { - // if ansi, translate string. we allocated the capture buffer - // large enough to handle the translated string. - auto tempBuffer = std::make_unique(NumToBytes); - std::unique_ptr partialEvent; - - NumToWrite = TranslateUnicodeToOem(pBuffer, - gsl::narrow(NumToWrite / sizeof(wchar_t)), - tempBuffer.get(), - gsl::narrow(NumToBytes), - partialEvent); - if (partialEvent.get()) - { - inputBuffer.StoreReadPartialByteSequence(std::move(partialEvent)); - } - - // clang-format off -#pragma prefast(suppress: __WARNING_POTENTIAL_BUFFER_OVERFLOW_HIGH_PRIORITY, "This access is fine but prefast can't follow it, evidently") - // clang-format on - memmove(pBuffer, tempBuffer.get(), NumToWrite); - - if (fAddDbcsLead) - { - NumToWrite++; - } - } - - bytesRead = NumToWrite; + bytesRead = buffer.size() - writer.size(); return STATUS_SUCCESS; } +NT_CATCH_RETURN() // Routine Description: // - read in characters until the buffer is full or return is read. @@ -521,140 +418,80 @@ til::CoordType RetrieveNumberOfSpaces(_In_ til::CoordType sOriginalCursorPositio // populated. // - STATUS_SUCCESS on success // - Other NTSTATUS codes as necessary -[[nodiscard]] static NTSTATUS _ReadCharacterInput(InputBuffer& inputBuffer, - gsl::span buffer, - size_t& bytesRead, - INPUT_READ_HANDLE_DATA& readHandleState, - const bool unicode, - std::unique_ptr& waiter) +[[nodiscard]] NTSTATUS _ReadCharacterInput(InputBuffer& inputBuffer, + gsl::span buffer, + size_t& bytesRead, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool unicode) +try { - size_t NumToWrite = 0; - auto addDbcsLead = false; - auto Status = STATUS_SUCCESS; - auto pBuffer = reinterpret_cast(buffer.data()); - auto bufferRemaining = buffer.size_bytes(); + UNREFERENCED_PARAMETER(readHandleState); + bytesRead = 0; - if (buffer.size() < 1) + gsl::span writer{ buffer }; + const auto charSize = unicode ? sizeof(wchar_t) : sizeof(char); + + if (!unicode) { - return STATUS_BUFFER_TOO_SMALL; + inputBuffer.ConsumeCachedA(writer); } - if (bytesRead < bufferRemaining) + // This guards against us failing to put even a single character into `writer` below, + // but it also guards against `RAW_READ_DATA` not supporting empty buffers. + if (writer.size() < charSize) { - auto pwchBufferTmp = pBuffer; - - NumToWrite = 0; - - if (!unicode && inputBuffer.IsReadPartialByteSequenceAvailable()) - { - auto event = inputBuffer.FetchReadPartialByteSequence(false); - const auto pKeyEvent = static_cast(event.get()); - *pBuffer = static_cast(pKeyEvent->GetCharData()); - ++pBuffer; - bufferRemaining -= sizeof(wchar_t); - addDbcsLead = true; - - if (bufferRemaining == 0) - { - bytesRead = 1; - return STATUS_SUCCESS; - } - } - else - { - Status = GetChar(&inputBuffer, - pBuffer, - true, - nullptr, - nullptr, - nullptr); - } + bytesRead = buffer.size() - writer.size(); + // If we failed to even put a single character into the buffer + // we need to tell the client that the buffer was too small. + return bytesRead ? STATUS_SUCCESS : STATUS_BUFFER_TOO_SMALL; + } - if (Status == CONSOLE_STATUS_WAIT) + // We only need to wait for input if `ReadBufferToOem` hasn't filled in any cached text. + if (bytesRead == 0) + { + wchar_t wch; + const auto status = GetChar(&inputBuffer, &wch, true, nullptr, nullptr, nullptr); + if (!NT_SUCCESS(status)) { - waiter = std::make_unique(&inputBuffer, - &readHandleState, - gsl::narrow(buffer.size_bytes()), - reinterpret_cast(buffer.data())); + return status; } - if (!NT_SUCCESS(Status)) + if (unicode) { - bytesRead = 0; - return Status; + til::bytes_put(writer, wch); } - - if (!addDbcsLead) + else { - bytesRead += IsGlyphFullWidth(*pBuffer) ? 2 : 1; - NumToWrite += sizeof(wchar_t); - pBuffer++; + std::wstring_view wchView{ &wch, 1 }; + inputBuffer.ConsumeA(wchView, writer); } + } - while (NumToWrite < static_cast(bufferRemaining)) + while (writer.size() >= charSize) + { + wchar_t wch; + const auto status = GetChar(&inputBuffer, &wch, false, nullptr, nullptr, nullptr); + if (!NT_SUCCESS(status)) { - Status = GetChar(&inputBuffer, - pBuffer, - false, - nullptr, - nullptr, - nullptr); - if (!NT_SUCCESS(Status)) - { - break; - } - bytesRead += IsGlyphFullWidth(*pBuffer) ? 2 : 1; - NumToWrite += sizeof(wchar_t); - pBuffer++; + break; } - // if ansi, translate string. we allocated the capture buffer large enough to handle the translated string. - if (!unicode) + if (unicode) { - std::unique_ptr tempBuffer; - try - { - tempBuffer = std::make_unique(bytesRead); - } - catch (...) - { - return STATUS_NO_MEMORY; - } - - pBuffer = pwchBufferTmp; - std::unique_ptr partialEvent; - - bytesRead = TranslateUnicodeToOem(pBuffer, - gsl::narrow(NumToWrite / sizeof(wchar_t)), - tempBuffer.get(), - gsl::narrow(bytesRead), - partialEvent); - - if (partialEvent.get()) - { - inputBuffer.StoreReadPartialByteSequence(std::move(partialEvent)); - } - -#pragma prefast(suppress : 26053 26015, "PREfast claims read overflow. *pReadByteCount is the exact size of tempBuffer as allocated above.") - memmove(pBuffer, tempBuffer.get(), bytesRead); - - if (addDbcsLead) - { - ++bytesRead; - } + til::bytes_put(writer, wch); } else { - // We always return the byte count for A & W modes, so in - // the Unicode case where we didn't translate back, set - // the return to the byte count that we assembled while - // pulling characters from the internal buffers. - bytesRead = NumToWrite; + std::wstring_view wchView{ &wch, 1 }; + inputBuffer.ConsumeA(wchView, writer); } } + + bytesRead = buffer.size() - writer.size(); return STATUS_SUCCESS; } +NT_CATCH_RETURN() // Routine Description: // - This routine reads in characters for stream input and does the @@ -730,12 +567,16 @@ til::CoordType RetrieveNumberOfSpaces(_In_ til::CoordType sOriginalCursorPositio } else { - return _ReadCharacterInput(inputBuffer, - buffer, - bytesRead, - readHandleState, - unicode, - waiter); + const auto status = _ReadCharacterInput(inputBuffer, + buffer, + bytesRead, + readHandleState, + unicode); + if (status == CONSOLE_STATUS_WAIT) + { + waiter = std::make_unique(&inputBuffer, &readHandleState, gsl::narrow(buffer.size()), reinterpret_cast(buffer.data())); + } + return status; } } CATCH_RETURN(); diff --git a/src/host/stream.h b/src/host/stream.h index 7ecc0595f23..429d01e2a00 100644 --- a/src/host/stream.h +++ b/src/host/stream.h @@ -29,6 +29,12 @@ Revision History: _Out_opt_ bool* const pPopupKeys, _Out_opt_ DWORD* const pdwKeyState) noexcept; +[[nodiscard]] NTSTATUS _ReadCharacterInput(InputBuffer& inputBuffer, + gsl::span buffer, + size_t& bytesRead, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool unicode); + // Routine Description: // - This routine returns the total number of screen spaces the characters up to the specified character take up. til::CoordType RetrieveTotalNumberOfSpaces(const til::CoordType sOriginalCursorPositionX, diff --git a/src/inc/til/at.h b/src/inc/til/at.h index 28a715be86b..8f7cba02623 100644 --- a/src/inc/til/at.h +++ b/src/inc/til/at.h @@ -3,29 +3,10 @@ #pragma once +#include "type_traits.h" + namespace til { - namespace details - { - // This was lifted from gsl::details::is_span. - template - struct is_span_oracle : std::false_type - { - }; - -#ifdef GSL_SPAN_H - template - struct is_span_oracle> : std::true_type - { - }; -#endif - - template - struct is_span : public is_span_oracle> - { - }; - } - // The at function declares that you've already sufficiently checked that your array access // is in range before retrieving an item inside it at an offset. // This is to save double/triple/quadruple testing in circumstances where you are already @@ -37,8 +18,7 @@ namespace til template constexpr auto at(T&& cont, const I i) noexcept -> decltype(auto) { -#ifdef GSL_SPAN_H - if constexpr (details::is_span::value) + if constexpr (ContiguousView) { #pragma warning(suppress : 26481) // Suppress bounds.1 check for doing pointer arithmetic #pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions @@ -46,7 +26,6 @@ namespace til return cont.data()[i]; } else -#endif { #pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions #pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. diff --git a/src/inc/til/bytes.h b/src/inc/til/bytes.h new file mode 100644 index 00000000000..43dba44282f --- /dev/null +++ b/src/inc/til/bytes.h @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "type_traits.h" + +namespace til +{ + template + constexpr void bytes_advance(Target& target, size_t count) + { + if (count > target.size()) + { + throw std::length_error{ "insufficient space left" }; + } + + target = { target.data() + count, target.size() - count }; + } + + template + constexpr bool bytes_can_put(const Target& target) + { + return target.size() >= sizeof(T); + } + + template + constexpr void bytes_put(Target& target, const T& value) + { + using TargetType = typename Target::value_type; + constexpr auto size = sizeof(value); + + if (size > target.size()) + { + throw std::length_error{ "insufficient space left" }; + } + + std::copy_n(reinterpret_cast(&value), size, target.data()); + target = { target.data() + size, target.size() - size }; + } + + template + requires TriviallyCopyable + constexpr void bytes_transfer(Target& target, Source& source) + { + using TargetType = typename Target::value_type; + constexpr auto elementSize = sizeof(typename Source::value_type); + + const auto sourceCount = std::min(source.size(), target.size() / elementSize); + const auto targetCount = sourceCount * elementSize; + + std::copy_n(reinterpret_cast(source.data()), targetCount, target.data()); + + target = { target.data() + targetCount, target.size() - targetCount }; + source = { source.data() + sourceCount, source.size() - sourceCount }; + } +} diff --git a/src/inc/til/type_traits.h b/src/inc/til/type_traits.h new file mode 100644 index 00000000000..2957963fc0f --- /dev/null +++ b/src/inc/til/type_traits.h @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +namespace til +{ + namespace details + { + template + struct is_contiguous_view : std::false_type + { + }; + // std::span remains largely unused in this code base so far. + //template + //struct is_contiguous_view> : std::true_type + //{ + //}; + template + struct is_contiguous_view> : std::true_type + { + }; +#ifdef GSL_SPAN_H + template + struct is_contiguous_view> : std::true_type + { + }; +#endif + + template + struct is_byte : std::false_type + { + }; + template<> + struct is_byte : std::true_type + { + }; + template<> + struct is_byte : std::true_type + { + }; + } + + template + concept Byte = details::is_byte>::value; + + template + concept ContiguousView = details::is_contiguous_view>::value; + + template + concept ContiguousBytes = ContiguousView && Byte; + + template + concept TriviallyCopyable = std::is_trivially_copyable_v; +} diff --git a/src/server/ApiDispatchers.cpp b/src/server/ApiDispatchers.cpp index ed3c815eade..9a1f456a79b 100644 --- a/src/server/ApiDispatchers.cpp +++ b/src/server/ApiDispatchers.cpp @@ -270,13 +270,8 @@ // Get output parameter buffer. PVOID pvBuffer; ULONG cbBufferSize; - // TODO: This is dumb. We should find out how much we need, not guess. - // If the request is not in Unicode mode, we must allocate an output buffer that is twice as big as the actual caller buffer. - RETURN_IF_FAILED(m->GetAugmentedOutputBuffer((a->Unicode != FALSE) ? 1 : 2, - &pvBuffer, - &cbBufferSize)); + RETURN_IF_FAILED(m->GetOutputBuffer(&pvBuffer, &cbBufferSize)); - // TODO: This is also rather strange and will also probably make more sense if we stop guessing that we need 2x buffer to convert. // This might need to go on the other side of the fence (inside host) because the server doesn't know what we're going to do with initial num bytes. // (This restriction exists because it's going to copy initial into the final buffer, but we don't know that.) RETURN_HR_IF(E_INVALIDARG, a->InitialNumBytes > cbBufferSize); From 819febe0ca8b96bb1d9dc94048afc3489358fcfb Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 27 Jan 2023 19:03:07 +0100 Subject: [PATCH 02/10] Fix build --- src/host/readDataDirect.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/host/readDataDirect.cpp b/src/host/readDataDirect.cpp index d2a44d2d54b..d26911271e8 100644 --- a/src/host/readDataDirect.cpp +++ b/src/host/readDataDirect.cpp @@ -65,8 +65,7 @@ try FAIL_FAST_IF_NULL(pOutputData); FAIL_FAST_IF(_pInputReadHandleData->GetReadCount() == 0); - const auto& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); - assert(gci.IsConsoleLocked()); + assert(Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked()); *pReplyStatus = STATUS_SUCCESS; *pControlKeyState = 0; From caba56d4cb2cab7ca8bbb3890235a5f9035005fa Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 3 Feb 2023 15:30:31 +0100 Subject: [PATCH 03/10] Rewrite half InputBuffer --- src/host/directio.cpp | 72 +-- src/host/inputBuffer.cpp | 469 +++++++++--------- src/host/inputBuffer.hpp | 41 +- src/host/misc.cpp | 48 +- src/host/misc.h | 2 - src/host/readDataCooked.cpp | 27 +- src/host/readDataCooked.hpp | 4 +- src/host/readDataDirect.cpp | 59 +-- src/host/readDataDirect.hpp | 1 - src/host/stream.cpp | 70 +-- src/host/ut_host/Host.UnitTests.vcxproj | 1 - .../ut_host/Host.UnitTests.vcxproj.filters | 3 - src/host/ut_host/ReadWaitTests.cpp | 129 ----- src/types/inc/IInputEvent.hpp | 2 + 14 files changed, 305 insertions(+), 623 deletions(-) delete mode 100644 src/host/ut_host/ReadWaitTests.cpp diff --git a/src/host/directio.cpp b/src/host/directio.cpp index 1ebfedc9c8f..496470865e2 100644 --- a/src/host/directio.cpp +++ b/src/host/directio.cpp @@ -169,37 +169,12 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, LockConsole(); auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); - auto amountToRead = eventReadCount; - - std::deque> partialEvents; - if (!IsUnicode) - { - std::string buffer(eventReadCount, '\0'); - std::span writer{ buffer }; - - if (IsPeek) - { - inputBuffer.PeekCachedA(writer); - } - else - { - inputBuffer.ConsumeCachedA(writer); - } - - const auto read = buffer.size() - writer.size(); - std::string_view str{ buffer.data(), read }; - StringToInputEvents(str, partialEvents); - - amountToRead -= read; - } - - std::deque> readEvents; - auto Status = inputBuffer.Read(readEvents, - amountToRead, - IsPeek, - true, - IsUnicode, - false); + const auto Status = inputBuffer.Read(outEvents, + eventReadCount, + IsPeek, + true, + IsUnicode, + false); if (CONSOLE_STATUS_WAIT == Status) { @@ -208,40 +183,7 @@ void EventsToUnicode(_Inout_ std::deque>& inEvents, waiter = std::make_unique(&inputBuffer, &readHandleState, eventReadCount, - std::move(partialEvents)); - } - else if (NT_SUCCESS(Status)) - { - // split key events to oem chars if necessary - if (!IsUnicode) - { - SplitToOem(readEvents); - } - - // combine partial and readEvents - while (!partialEvents.empty()) - { - readEvents.push_front(std::move(partialEvents.back())); - partialEvents.pop_back(); - } - - // move events over - for (size_t i = 0; i < eventReadCount; ++i) - { - if (readEvents.empty()) - { - break; - } - outEvents.push_back(std::move(readEvents.front())); - readEvents.pop_front(); - } - - if (!readEvents.empty()) - { - std::string buffer; - InputEventsToString(readEvents, buffer); - inputBuffer.CacheA(buffer); - } + std::move(outEvents)); } return Status; } diff --git a/src/host/inputBuffer.cpp b/src/host/inputBuffer.cpp index 2efbb23886b..786d7898e0c 100644 --- a/src/host/inputBuffer.cpp +++ b/src/host/inputBuffer.cpp @@ -3,7 +3,7 @@ #include "precomp.h" #include "inputBuffer.hpp" -#include "dbcs.h" + #include "stream.h" #include "../types/inc/GlyphWidth.hpp" @@ -37,129 +37,199 @@ InputBuffer::InputBuffer() : fInComposition = false; } -// Transfer as many `wchar_t`s from source over to the `wchar_t` buffer `target`. After it returns, +// Transfer as many `wchar_t`s from source over to the `char`/`wchar_t` buffer `target`. After it returns, // the start of the `source` and `target` slices will be offset by as many bytes as have been copied // over, so that if you call this function again it'll continue copying from wherever it left off. -// This function is intended for use with our W APIs. -void InputBuffer::ConsumeW(std::wstring_view& source, std::span& target) +// +// It performs the necessary `WideCharToMultiByte` conversion if `isUnicode` is `false`. +// Since not all converted `char`s might fit into `target` it'll cache the remainder. The next +// time this function is called those cached `char`s will then be the first to be copied over. +void InputBuffer::Consume(bool isUnicode, std::wstring_view& source, std::span& target) { - if (!_aBuffer.empty()) - { - _resetBufferA(); - } + // `_cachedTextReaderA` might still contain target data from a previous invocation. + // `ConsumeCached` calls `_switchReadingMode` for us. + ConsumeCached(isUnicode, target); if (source.empty() || target.empty()) { return; } - til::bytes_transfer(target, source); -} - -// Transfer as many `wchar_t`s from source over to the `char` buffer `target`. After it returns, -// the start of the `source` and `target` slices will be offset by as many bytes as have been copied -// over, so that if you call this function again it'll continue copying from wherever it left off. -// This function is intended for use with our A APIs and performs the necessary `WideCharToMultiByte` -// conversion. Since not all converted `char`s might fit into `target` it'll cache the remainder. -// The next time this function is called those cached `char`s will then be the first to be copied over. -void InputBuffer::ConsumeA(std::wstring_view& source, std::span& target) -{ - // `_aBufferReader` might still contain target data from a previous invocation. - ConsumeCachedA(target); - - if (source.empty() || target.empty()) + if (isUnicode) { - return; + // The above block should either leave `target` or `_cachedTextReaderW` empty (or both). + // If we're here, `_cachedTextReaderW` should be empty. + assert(_cachedTextReaderW.empty()); + + til::bytes_transfer(target, source); } + else + { + // The above block should either leave `target` or `_cachedTextReaderA` empty (or both). + // If we're here, `_cachedTextReaderA` should be empty. + assert(_cachedTextReaderA.empty()); - // The above block should either leave `target` or `_aBufferReader` empty (or both). - // If we're here, `_aBufferReader` should be empty. - assert(_aBufferReader.empty()); + const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().CP; - const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().CP; + // Fast path: Batch convert all data in case the user provided buffer is large enough. + { + const auto wideLength = gsl::narrow(source.size()); + const auto narrowLength = gsl::narrow(target.size()); - // Fast path: Batch convert all data in case the user provided buffer is large enough. - { - const auto wideLength = gsl::narrow(source.size()); - const auto narrowLength = gsl::narrow(target.size()); + const auto length = WideCharToMultiByte(cp, 0, source.data(), wideLength, target.data(), narrowLength, nullptr, nullptr); + if (length > 0) + { + source = {}; + til::bytes_advance(target, gsl::narrow_cast(length)); + return; + } - const auto length = WideCharToMultiByte(cp, 0, source.data(), wideLength, target.data(), narrowLength, nullptr, nullptr); - if (length > 0) - { - source = {}; - til::bytes_advance(target, gsl::narrow_cast(length)); - return; + const auto error = GetLastError(); + THROW_HR_IF(HRESULT_FROM_WIN32(error), error != ERROR_INSUFFICIENT_BUFFER); } - const auto error = GetLastError(); - THROW_HR_IF(HRESULT_FROM_WIN32(error), error != ERROR_INSUFFICIENT_BUFFER); - } + // Slow path: Character-wise conversion otherwise. We do this in order to only + // consume as many characters from `source` as necessary to fill `target`. + { + size_t read = 0; - // Slow path: Character-wise conversion otherwise. We do this in order to only - // consume as many characters from `source` as necessary to fill `target`. - { - size_t read = 0; + for (const auto& wch : source) + { + char buffer[8]; + const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); + THROW_LAST_ERROR_IF(length <= 0); - for (const auto& wch : source) - { - char buffer[8]; - const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); - THROW_LAST_ERROR_IF(length <= 0); + std::string_view slice{ &buffer[0], gsl::narrow_cast(length) }; + til::bytes_transfer(target, slice); + + ++read; + + if (!slice.empty()) + { + _cachedTextA = slice; + _cachedTextReaderA = _cachedTextA; + break; + } + } + + source = source.substr(read); + } + } +} - std::string_view slice{ &buffer[0], gsl::narrow_cast(length) }; - til::bytes_transfer(target, slice); +// Same as `Consume`, but without any `source` characters. +void InputBuffer::ConsumeCached(bool isUnicode, std::span& target) +{ + _switchReadingMode(isUnicode ? ReadingMode::StringW : ReadingMode::StringA); - ++read; + if (isUnicode) + { + if (!_cachedTextReaderW.empty()) + { + til::bytes_transfer(target, _cachedTextReaderW); - if (!slice.empty()) + if (_cachedTextReaderW.empty()) { - _aBuffer = slice; - _aBufferReader = _aBuffer; - break; + // This is just so that we release memory eagerly. + _cachedTextW = std::wstring{}; } } + } + else + { + if (!_cachedTextReaderA.empty()) + { + til::bytes_transfer(target, _cachedTextReaderA); - source = source.substr(read); + if (_cachedTextReaderA.empty()) + { + // This is just so that we release memory eagerly. + _cachedTextA = std::string{}; + } + } } } -// Same as `ConsumeA`, but without any `source` characters. -void InputBuffer::ConsumeCachedA(std::span& target) +void InputBuffer::Cache(std::wstring_view source) { - if (_aBufferReader.empty()) + const auto off = _cachedTextW.empty() ? 0 : _cachedTextReaderW.data() - _cachedTextW.data(); + _cachedTextW.append(source); + _cachedTextReaderW = std::wstring_view{ _cachedTextW }.substr(off); +} + +// Moves up to `count`, previously cached events into `target`. +size_t InputBuffer::ConsumeCached(bool isUnicode, size_t count, InputEventQueue& target) +{ + _switchReadingMode(isUnicode ? ReadingMode::InputEventsW : ReadingMode::InputEventsA); + + size_t i = 0; + + while (i < count && !_cachedInputEvents.empty()) { - return; + target.push_back(std::move(_cachedInputEvents.front())); + _cachedInputEvents.pop_front(); + i++; } - til::bytes_transfer(target, _aBufferReader); + return i; +} + +// Copies up to `count`, previously cached events into `target`. +size_t InputBuffer::PeekCached(bool isUnicode, size_t count, InputEventQueue& target) +{ + _switchReadingMode(isUnicode ? ReadingMode::InputEventsW : ReadingMode::InputEventsA); + + size_t i = 0; - if (_aBufferReader.empty()) + for (const auto& e : _cachedInputEvents) { - // This is just so that we release memory eagerly. - _aBuffer = std::string{}; + if (i >= count) + { + break; + } + + target.push_back(IInputEvent::Create(e->ToInputRecord())); + i++; } + + return i; } -// Same as `ConsumeCachedA`, but as the name indicates it doesn't "consume" the amount of data that has been copied -// into `target`. Still, the `target` slice will have been offset the same way `ConsumeW` and `ConsumeA` do it. -void InputBuffer::PeekCachedA(std::span& target) +// Trims `source` to have a size below or equal to `expectedSourceSize` by +// storing any extra events in `_cachedInputEvents` for later retrieval. +void InputBuffer::Cache(bool isUnicode, InputEventQueue& source, size_t expectedSourceSize) { - auto reader = _aBufferReader; - til::bytes_transfer(target, reader); + _switchReadingMode(isUnicode ? ReadingMode::InputEventsW : ReadingMode::InputEventsA); + + if (source.size() > expectedSourceSize) + { + _cachedInputEvents.insert( + _cachedInputEvents.end(), + std::make_move_iterator(source.begin() + expectedSourceSize), + std::make_move_iterator(source.end())); + source.resize(expectedSourceSize); + } } -// This function allows you to manually add some data into the internal buffer used by `ConsumeA`, etc. -void InputBuffer::CacheA(std::string_view source) +void InputBuffer::_switchReadingMode(ReadingMode mode) { - const auto off = _aBufferReader.size() - _aBuffer.size(); - _aBuffer.append(source); - _aBufferReader = std::string_view{ _aBuffer }.substr(off); + if (_readingMode != mode) + { + _switchReadingModeSlowPath(mode); + } } -void InputBuffer::_resetBufferA() +void InputBuffer::_switchReadingModeSlowPath(ReadingMode mode) { - _aBuffer = std::string{}; - _aBufferReader = {}; + _cachedTextA = std::string{}; + _cachedTextReaderA = {}; + + _cachedTextW = std::wstring{}; + _cachedTextReaderW = {}; + + _cachedInputEvents = std::deque>{}; + + _readingMode = mode; } // Routine Description: @@ -332,47 +402,110 @@ void InputBuffer::PassThroughWin32MouseRequest(bool enable) const bool WaitForData, const bool Unicode, const bool Stream) +try { - try + assert(OutEvents.empty()); + + const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().CP; + + if (Peek) + { + PeekCached(Unicode, AmountToRead, OutEvents); + } + else { - if (_storage.empty()) + ConsumeCached(Unicode, AmountToRead, OutEvents); + } + + auto it = _storage.begin(); + const auto end = _storage.end(); + + while (it != end && OutEvents.size() < AmountToRead) + { + std::unique_ptr event; + { - if (!WaitForData) + decltype(_storage)::difference_type advance = 1; + auto& e = *it; + + // for stream reads we need to split any key events that have been coalesced + if (Stream && e->EventType() == InputEventType::KeyEvent) { - return STATUS_SUCCESS; + const auto keyEvent = static_cast(e.get()); + const auto repeatCount = keyEvent->GetRepeatCount(); + + if (repeatCount > 1) + { + auto tempEvent = std::make_unique(*keyEvent); + tempEvent->SetRepeatCount(1); + event = std::move(tempEvent); + + // By setting the advance to 0, the next time we loop around we'll + // process the same event again, up until the repeat count is 1. + keyEvent->SetRepeatCount(repeatCount - 1); + advance = 0; + } + } + + if (!event) + { + if (Peek) + { + event = IInputEvent::Create(event->ToInputRecord()); + } + else + { + event = std::move(e); + } } - return CONSOLE_STATUS_WAIT; + + it += advance; } - // read from buffer - std::deque> events; - size_t eventsRead; - bool resetWaitEvent; - _ReadBuffer(events, - AmountToRead, - eventsRead, - Peek, - resetWaitEvent, - Unicode, - Stream); - - // copy events to outEvents - while (!events.empty()) + if (!Unicode && event->EventType() == InputEventType::KeyEvent) { - OutEvents.push_back(std::move(events.front())); - events.pop_front(); - } + const auto keyEvent = static_cast(event.get()); + const auto wch = keyEvent->GetCharData(); + + char buffer[8]; + const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); + THROW_LAST_ERROR_IF(length <= 0); + + const std::string_view str{ &buffer[0], gsl::narrow_cast(length) }; - if (resetWaitEvent) + for (const auto& ch : str) + { + auto tempEvent = std::make_unique(*keyEvent); + tempEvent->SetCharData(ch); + OutEvents.push_back(std::move(tempEvent)); + } + } + else { - ServiceLocator::LocateGlobals().hInputEvent.ResetEvent(); + OutEvents.push_back(std::move(event)); } - return STATUS_SUCCESS; } - catch (...) + + if (!Peek) + { + _storage.erase(_storage.begin(), it); + } + + Cache(Unicode, OutEvents, AmountToRead); + + if (OutEvents.empty()) + { + return WaitForData ? CONSOLE_STATUS_WAIT : STATUS_SUCCESS; + } + if (_storage.empty()) { - return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); + ServiceLocator::LocateGlobals().hInputEvent.ResetEvent(); } + return STATUS_SUCCESS; +} +catch (...) +{ + return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); } // Routine Description: @@ -420,130 +553,6 @@ void InputBuffer::PassThroughWin32MouseRequest(bool enable) return Status; } -// Routine Description: -// - This routine reads from a buffer. It does the buffer manipulation. -// Arguments: -// - outEvents - where read events are placed -// - readCount - amount of events to read -// - eventsRead - where to store number of events read -// - peek - if true , don't remove data from buffer, just copy it. -// - resetWaitEvent - on exit, true if buffer became empty. -// - unicode - true if read should be done in unicode mode -// - streamRead - true if read should unpack KeyEvents that have a >1 repeat count. readCount must be 1 if streamRead is true. -// Return Value: -// - -// Note: -// - The console lock must be held when calling this routine. -void InputBuffer::_ReadBuffer(_Out_ std::deque>& outEvents, - const size_t readCount, - _Out_ size_t& eventsRead, - const bool peek, - _Out_ bool& resetWaitEvent, - const bool unicode, - const bool streamRead) -{ - // when stream reading, the previous behavior was to only allow reading of a single - // event at a time. - FAIL_FAST_IF(streamRead && readCount != 1); - - resetWaitEvent = false; - - std::deque> readEvents; - // we need another var to keep track of how many we've read - // because dbcs records count for two when we aren't doing a - // unicode read but the eventsRead count should return the number - // of events actually put into outRecords. - size_t virtualReadCount = 0; - - while (!_storage.empty() && virtualReadCount < readCount) - { - auto performNormalRead = true; - // for stream reads we need to split any key events that have been coalesced - if (streamRead) - { - if (_storage.front()->EventType() == InputEventType::KeyEvent) - { - const auto pKeyEvent = static_cast(_storage.front().get()); - if (pKeyEvent->GetRepeatCount() > 1) - { - // split the key event - auto streamKeyEvent = std::make_unique(*pKeyEvent); - streamKeyEvent->SetRepeatCount(1); - readEvents.push_back(std::move(streamKeyEvent)); - pKeyEvent->SetRepeatCount(pKeyEvent->GetRepeatCount() - 1); - performNormalRead = false; - } - } - } - - if (performNormalRead) - { - readEvents.push_back(std::move(_storage.front())); - _storage.pop_front(); - } - - ++virtualReadCount; - if (!unicode) - { - if (readEvents.back()->EventType() == InputEventType::KeyEvent) - { - const auto pKeyEvent = static_cast(readEvents.back().get()); - if (IsGlyphFullWidth(pKeyEvent->GetCharData())) - { - ++virtualReadCount; - } - } - } - } - - // the amount of events that were actually read - eventsRead = readEvents.size(); - - // copy the events back if we were supposed to peek - if (peek) - { - if (streamRead) - { - // we need to check and see if the event was split from a coalesced key event - // or if it was unrelated to the current front event in storage - if (!readEvents.empty() && - !_storage.empty() && - readEvents.back()->EventType() == InputEventType::KeyEvent && - _storage.front()->EventType() == InputEventType::KeyEvent && - _CanCoalesce(static_cast(*readEvents.back()), - static_cast(*_storage.front()))) - { - auto& keyEvent = static_cast(*_storage.front()); - keyEvent.SetRepeatCount(keyEvent.GetRepeatCount() + 1); - } - else - { - _storage.push_front(IInputEvent::Create(readEvents.back()->ToInputRecord())); - } - } - else - { - for (auto it = readEvents.crbegin(); it != readEvents.crend(); ++it) - { - _storage.push_front(IInputEvent::Create((*it)->ToInputRecord())); - } - } - } - - // move events read to proper deque - while (!readEvents.empty()) - { - outEvents.push_back(std::move(readEvents.front())); - readEvents.pop_front(); - } - - // signal if we emptied the buffer - if (_storage.empty()) - { - resetWaitEvent = true; - } -} - // Routine Description: // - Writes events to the beginning of the input buffer. // Arguments: diff --git a/src/host/inputBuffer.hpp b/src/host/inputBuffer.hpp index e2670156ab0..f5103ea9f98 100644 --- a/src/host/inputBuffer.hpp +++ b/src/host/inputBuffer.hpp @@ -42,12 +42,14 @@ class InputBuffer final : public ConsoleObjectHeader InputBuffer(); - void ConsumeW(std::wstring_view& source, std::span& target); - - void ConsumeA(std::wstring_view& source, std::span& target); - void ConsumeCachedA(std::span& target); - void PeekCachedA(std::span& target); - void CacheA(std::string_view source); + // String oriented APIs + void Consume(bool isUnicode, std::wstring_view& source, std::span& target); + void ConsumeCached(bool isUnicode, std::span& target); + void Cache(std::wstring_view source); + // INPUT_RECORD oriented APIs + size_t ConsumeCached(bool isUnicode, size_t count, InputEventQueue& target); + size_t PeekCached(bool isUnicode, size_t count, InputEventQueue& target); + void Cache(bool isUnicode, InputEventQueue& source, size_t expectedSourceSize); // storage API for partial dbcs bytes being written to the buffer bool IsWritePartialByteSequenceAvailable(); @@ -85,8 +87,20 @@ class InputBuffer final : public ConsoleObjectHeader void PassThroughWin32MouseRequest(bool enable); private: - std::string _aBuffer; - std::string_view _aBufferReader; + enum class ReadingMode : uint8_t + { + StringA, + StringW, + InputEventsA, + InputEventsW, + }; + + std::string _cachedTextA; + std::string_view _cachedTextReaderA; + std::wstring _cachedTextW; + std::wstring_view _cachedTextReaderW; + std::deque> _cachedInputEvents; + ReadingMode _readingMode = ReadingMode::StringA; std::deque> _storage; std::unique_ptr _writePartialByteSequence; @@ -99,15 +113,8 @@ class InputBuffer final : public ConsoleObjectHeader // Otherwise, we should be calling them. bool _vtInputShouldSuppress{ false }; - void _resetBufferA(); - - void _ReadBuffer(_Out_ std::deque>& outEvents, - const size_t readCount, - _Out_ size_t& eventsRead, - const bool peek, - _Out_ bool& resetWaitEvent, - const bool unicode, - const bool streamRead); + void _switchReadingMode(ReadingMode mode); + void _switchReadingModeSlowPath(ReadingMode mode); void _WriteBuffer(_Inout_ std::deque>& inRecords, _Out_ size_t& eventsWritten, diff --git a/src/host/misc.cpp b/src/host/misc.cpp index 671da86d7a6..531ec2a9a63 100644 --- a/src/host/misc.cpp +++ b/src/host/misc.cpp @@ -2,13 +2,12 @@ // Licensed under the MIT license. #include "precomp.h" - #include "misc.h" -#include "dbcs.h" -#include "../types/inc/convert.hpp" -#include "../types/inc/GlyphWidth.hpp" +#include +#include "dbcs.h" +#include "../types/inc/GlyphWidth.hpp" #include "../interactivity/inc/ServiceLocator.hpp" #pragma hdrstop @@ -182,47 +181,6 @@ BOOL CheckBisectProcessW(const SCREEN_INFORMATION& ScreenInfo, } } -// Routine Description: -// - Converts all key events in the deque to the oem char data and adds -// them back to events. -// Arguments: -// - events - on input the IInputEvents to convert. on output, the -// converted input events -// Note: may throw on error -void SplitToOem(std::deque>& events) -{ - const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().CP; - std::deque> convertedEvents; - - for (auto& currentEvent : events) - { - if (currentEvent->EventType() == InputEventType::KeyEvent) - { - const auto pKeyEvent = static_cast(currentEvent.get()); - const auto wch = pKeyEvent->GetCharData(); - - char buffer[8]; - const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); - THROW_LAST_ERROR_IF(length <= 0); - - const std::string_view str{ &buffer[0], gsl::narrow_cast(length) }; - - for (const auto& ch : str) - { - auto tempEvent = std::make_unique(*pKeyEvent); - tempEvent->SetCharData(ch); - convertedEvents.push_back(std::move(tempEvent)); - } - } - else - { - convertedEvents.push_back(std::move(currentEvent)); - } - } - - events = std::move(convertedEvents); -} - // Routine Description: // - Converts unicode characters to ANSI given a destination codepage // Arguments: diff --git a/src/host/misc.h b/src/host/misc.h index f76e1099e9a..13c4f42d3d2 100644 --- a/src/host/misc.h +++ b/src/host/misc.h @@ -76,8 +76,6 @@ void InputEventsToString(std::deque>& events, std:: } } -void SplitToOem(std::deque>& events); - int ConvertInputToUnicode(const UINT uiCodePage, _In_reads_(cchSource) const CHAR* const pchSource, const UINT cchSource, diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 6289d91f140..c4c5aec6112 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -47,7 +47,7 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _In_ INPUT_READ_HANDLE_DATA* const pInputReadHandleData, SCREEN_INFORMATION& screenInfo, _In_ size_t UserBufferSize, - _In_ PWCHAR UserBuffer, + _In_ char* UserBuffer, _In_ ULONG CtrlWakeupMask, _In_ const std::wstring_view exeName, _In_ const std::string_view initialData, @@ -62,7 +62,7 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _exeName{ exeName }, _pdwNumBytes{ nullptr }, - _commandHistory{ CommandHistory::s_Find((HANDLE)pClientProcess) }, + _commandHistory{ CommandHistory::s_Find(pClientProcess) }, _controlKeyState{ 0 }, _ctrlWakeupMask{ CtrlWakeupMask }, _visibleCharCount{ 0 }, @@ -881,10 +881,10 @@ size_t COOKED_READ_DATA::SavePromptToUserBuffer(const size_t cch) try { const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto wstr = ConvertToW(gci.CP, { reinterpret_cast(_userBuffer), cch }); - const auto copyAmount = std::min(wstr.size(), _userBufferSize / sizeof(wchar_t)); - std::copy_n(wstr.begin(), copyAmount, _userBuffer); - return copyAmount * sizeof(wchar_t); + const auto wstr = ConvertToW(gci.CP, { _userBuffer, cch }); + const auto copyAmount = std::min(wstr.size() * sizeof(wchar_t), _userBufferSize); + std::copy_n(reinterpret_cast(wstr.data()), copyAmount, _userBuffer); + return copyAmount; } CATCH_LOG(); } @@ -1003,7 +1003,7 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline // - Status code that indicates success, out of memory, etc. [[nodiscard]] NTSTATUS COOKED_READ_DATA::_handlePostCharInputLoop(const bool isUnicode, size_t& numBytes, ULONG& controlKeyState) noexcept { - std::span writer{ reinterpret_cast(_userBuffer), _userBufferSize }; + std::span writer{ _userBuffer, _userBufferSize }; std::wstring_view input{ _backupLimit, _bytesRead / sizeof(wchar_t) }; DWORD LineCount = 1; @@ -1028,14 +1028,7 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline } } - if (isUnicode) - { - GetInputBuffer()->ConsumeW(input, writer); - } - else - { - GetInputBuffer()->ConsumeA(input, writer); - } + GetInputBuffer()->Consume(isUnicode, input, writer); if (!input.empty()) { @@ -1057,8 +1050,8 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline void COOKED_READ_DATA::MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) { // See the comment in WaitBlock.cpp for more information. - if (_userBuffer == static_cast(oldBuffer)) + if (_userBuffer == oldBuffer) { - _userBuffer = static_cast(newBuffer); + _userBuffer = static_cast(newBuffer); } } diff --git a/src/host/readDataCooked.hpp b/src/host/readDataCooked.hpp index 23096e42b51..a263303e45d 100644 --- a/src/host/readDataCooked.hpp +++ b/src/host/readDataCooked.hpp @@ -37,7 +37,7 @@ class COOKED_READ_DATA final : public ReadData _In_ INPUT_READ_HANDLE_DATA* const pInputReadHandleData, SCREEN_INFORMATION& screenInfo, _In_ size_t UserBufferSize, - _In_ PWCHAR UserBuffer, + _In_ char* UserBuffer, _In_ ULONG CtrlWakeupMask, _In_ const std::wstring_view exeName, _In_ const std::string_view initialData, @@ -129,7 +129,7 @@ class COOKED_READ_DATA final : public ReadData wchar_t* _backupLimit; size_t _userBufferSize; // doubled size in ansi case - wchar_t* _userBuffer; + char* _userBuffer; size_t* _pdwNumBytes; diff --git a/src/host/readDataDirect.cpp b/src/host/readDataDirect.cpp index 6d750dfb0b5..b9a9c4934ac 100644 --- a/src/host/readDataDirect.cpp +++ b/src/host/readDataDirect.cpp @@ -25,8 +25,7 @@ DirectReadData::DirectReadData(_In_ InputBuffer* const pInputBuffer, const size_t eventReadCount, _In_ std::deque> partialEvents) : ReadData(pInputBuffer, pInputReadHandleData), - _eventReadCount{ eventReadCount }, - _partialEvents{ std::move(partialEvents) } + _eventReadCount{ eventReadCount } { } @@ -101,35 +100,14 @@ try // thread or a write routine. both of these callers grab the // current console lock. - // calculate how many events we need to read - size_t amountAlreadyRead; - if (FAILED(SizeTAdd(_partialEvents.size(), _outEvents.size(), &amountAlreadyRead))) - { - *pReplyStatus = STATUS_INTEGER_OVERFLOW; - return true; - } size_t amountToRead; - if (FAILED(SizeTSub(_eventReadCount, amountAlreadyRead, &amountToRead))) + if (FAILED(SizeTSub(_eventReadCount, _outEvents.size(), &amountToRead))) { *pReplyStatus = STATUS_INTEGER_OVERFLOW; return true; } - // check if partial bytes are already stored that we should read - if (!fIsUnicode && amountToRead > 0) - { - std::string buffer(amountToRead, '\0'); - std::span writer{ buffer }; - _pInputBuffer->ConsumeCachedA(writer); - - const auto read = buffer.size() - writer.size(); - std::string_view str{ buffer.data(), read }; - StringToInputEvents(str, _partialEvents); - - amountToRead -= read; - } - - *pReplyStatus = _pInputBuffer->Read(readEvents, + *pReplyStatus = _pInputBuffer->Read(_outEvents, amountToRead, false, false, @@ -141,37 +119,6 @@ try return false; } - // split key events to oem chars if necessary - if (!fIsUnicode && *pReplyStatus == STATUS_SUCCESS) - { - SplitToOem(readEvents); - } - - // combine partial and whole events - while (!_partialEvents.empty()) - { - readEvents.push_front(std::move(_partialEvents.back())); - _partialEvents.pop_back(); - } - - // move read events to out storage - for (size_t i = 0; i < _eventReadCount; ++i) - { - if (readEvents.empty()) - { - break; - } - _outEvents.push_back(std::move(readEvents.front())); - readEvents.pop_front(); - } - - if (!readEvents.empty()) - { - std::string buffer; - InputEventsToString(readEvents, buffer); - _pInputBuffer->CacheA(buffer); - } - // move events to pOutputData const auto pOutputDeque = static_cast>* const>(pOutputData); *pNumBytes = _outEvents.size() * sizeof(INPUT_RECORD); diff --git a/src/host/readDataDirect.hpp b/src/host/readDataDirect.hpp index a12af8becf9..820ddd59819 100644 --- a/src/host/readDataDirect.hpp +++ b/src/host/readDataDirect.hpp @@ -48,6 +48,5 @@ class DirectReadData final : public ReadData private: const size_t _eventReadCount; - std::deque> _partialEvents; std::deque> _outEvents; }; diff --git a/src/host/stream.cpp b/src/host/stream.cpp index b95e774f43e..1b43e4363a8 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -6,9 +6,6 @@ #include "_stream.h" #include "stream.h" -#include - -#include "dbcs.h" #include "handle.h" #include "misc.h" #include "readDataRaw.hpp" @@ -303,16 +300,8 @@ try } } - std::span writer{ buffer }; - - if (unicode) - { - inputBuffer.ConsumeW(pending, writer); - } - else - { - inputBuffer.ConsumeA(pending, writer); - } + std::span writer{ buffer }; + inputBuffer.Consume(unicode, pending, writer); if (pending.empty()) { @@ -375,7 +364,7 @@ NT_CATCH_RETURN() &readHandleState, // pInputReadHandleData screenInfo, // pScreenInfo buffer.size_bytes(), // UserBufferSize - reinterpret_cast(buffer.data()), // UserBuffer + buffer.data(), // UserBuffer ctrlWakeupMask, // CtrlWakeupMask exeName, // exe name initialData, @@ -429,63 +418,34 @@ try bytesRead = 0; - std::span writer{ buffer }; const auto charSize = unicode ? sizeof(wchar_t) : sizeof(char); + std::span writer{ buffer }; - if (!unicode) - { - inputBuffer.ConsumeCachedA(writer); - } - - // This guards against us failing to put even a single character into `writer` below, - // but it also guards against `RAW_READ_DATA` not supporting empty buffers. if (writer.size() < charSize) { - bytesRead = buffer.size() - writer.size(); - // If we failed to even put a single character into the buffer - // we need to tell the client that the buffer was too small. - return bytesRead ? STATUS_SUCCESS : STATUS_BUFFER_TOO_SMALL; + return STATUS_BUFFER_TOO_SMALL; } - // We only need to wait for input if `ReadBufferToOem` hasn't filled in any cached text. - if (bytesRead == 0) - { - wchar_t wch; - const auto status = GetChar(&inputBuffer, &wch, true, nullptr, nullptr, nullptr); - if (!NT_SUCCESS(status)) - { - return status; - } + inputBuffer.ConsumeCached(unicode, writer); - if (unicode) - { - til::bytes_put(writer, wch); - } - else - { - std::wstring_view wchView{ &wch, 1 }; - inputBuffer.ConsumeA(wchView, writer); - } - } + // We don't need to wait for input if `ConsumeCached` read something already, which is + // indicated by the writer having been advanced (= it's shorter than the original buffer). + auto wait = writer.size() == buffer.size(); + auto status = STATUS_SUCCESS; while (writer.size() >= charSize) { wchar_t wch; - const auto status = GetChar(&inputBuffer, &wch, false, nullptr, nullptr, nullptr); + status = GetChar(&inputBuffer, &wch, wait, nullptr, nullptr, nullptr); if (!NT_SUCCESS(status)) { break; } - if (unicode) - { - til::bytes_put(writer, wch); - } - else - { - std::wstring_view wchView{ &wch, 1 }; - inputBuffer.ConsumeA(wchView, writer); - } + std::wstring_view wchView{ &wch, 1 }; + inputBuffer.Consume(unicode, wchView, writer); + + wait = false; } bytesRead = buffer.size() - writer.size(); diff --git a/src/host/ut_host/Host.UnitTests.vcxproj b/src/host/ut_host/Host.UnitTests.vcxproj index a20cf4d5ac0..31f0f8e5a95 100644 --- a/src/host/ut_host/Host.UnitTests.vcxproj +++ b/src/host/ut_host/Host.UnitTests.vcxproj @@ -35,7 +35,6 @@ - diff --git a/src/host/ut_host/Host.UnitTests.vcxproj.filters b/src/host/ut_host/Host.UnitTests.vcxproj.filters index 6e56f956f9f..2dcabcef1d4 100644 --- a/src/host/ut_host/Host.UnitTests.vcxproj.filters +++ b/src/host/ut_host/Host.UnitTests.vcxproj.filters @@ -54,9 +54,6 @@ Source Files - - Source Files - Source Files diff --git a/src/host/ut_host/ReadWaitTests.cpp b/src/host/ut_host/ReadWaitTests.cpp deleted file mode 100644 index 44c76579e14..00000000000 --- a/src/host/ut_host/ReadWaitTests.cpp +++ /dev/null @@ -1,129 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "precomp.h" -#include "WexTestClass.h" -#include "../../inc/consoletaeftemplates.hpp" - -#include "misc.h" -#include "dbcs.h" -#include "../../types/inc/IInputEvent.hpp" - -#include "../interactivity/inc/ServiceLocator.hpp" - -#include -#include - -using namespace WEX::Logging; -using Microsoft::Console::Interactivity::ServiceLocator; - -class InputRecordConversionTests -{ - TEST_CLASS(InputRecordConversionTests); - - static const size_t INPUT_RECORD_COUNT = 10; - UINT savedCodepage = 0; - - TEST_CLASS_SETUP(ClassSetup) - { - auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - savedCodepage = gci.CP; - gci.CP = CP_JAPANESE; - VERIFY_IS_TRUE(!!GetCPInfo(gci.CP, &gci.CPInfo)); - return true; - } - - TEST_CLASS_CLEANUP(ClassCleanup) - { - auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - gci.CP = savedCodepage; - VERIFY_IS_TRUE(!!GetCPInfo(gci.CP, &gci.CPInfo)); - return true; - } - - TEST_METHOD(SplitToOemLeavesNonKeyEventsAlone) - { - Log::Comment(L"nothing should happen to input events that aren't key events"); - - std::deque> inEvents; - INPUT_RECORD inRecords[INPUT_RECORD_COUNT] = { 0 }; - for (size_t i = 0; i < INPUT_RECORD_COUNT; ++i) - { - inRecords[i].EventType = MOUSE_EVENT; - inRecords[i].Event.MouseEvent.dwMousePosition.X = static_cast(i); - inRecords[i].Event.MouseEvent.dwMousePosition.Y = static_cast(i * 2); - inEvents.push_back(IInputEvent::Create(inRecords[i])); - } - - SplitToOem(inEvents); - VERIFY_ARE_EQUAL(INPUT_RECORD_COUNT, inEvents.size()); - - for (size_t i = 0; i < INPUT_RECORD_COUNT; ++i) - { - VERIFY_ARE_EQUAL(inRecords[i], inEvents[i]->ToInputRecord()); - } - } - - TEST_METHOD(SplitToOemLeavesNonDbcsCharsAlone) - { - Log::Comment(L"non-dbcs chars shouldn't be split"); - - std::deque> inEvents; - INPUT_RECORD inRecords[INPUT_RECORD_COUNT] = { 0 }; - for (size_t i = 0; i < INPUT_RECORD_COUNT; ++i) - { - inRecords[i].EventType = KEY_EVENT; - inRecords[i].Event.KeyEvent.uChar.UnicodeChar = static_cast(L'a' + i); - inEvents.push_back(IInputEvent::Create(inRecords[i])); - } - - SplitToOem(inEvents); - VERIFY_ARE_EQUAL(INPUT_RECORD_COUNT, inEvents.size()); - - for (size_t i = 0; i < INPUT_RECORD_COUNT; ++i) - { - VERIFY_ARE_EQUAL(inRecords[i], inEvents[i]->ToInputRecord()); - } - } - - TEST_METHOD(SplitToOemSplitsDbcsChars) - { - Log::Comment(L"dbcs chars should be split"); - - const auto codepage = ServiceLocator::LocateGlobals().getConsoleInformation().CP; - - INPUT_RECORD inRecords[INPUT_RECORD_COUNT * 2] = { 0 }; - std::deque> inEvents; - // U+3042 hiragana letter A - wchar_t hiraganaA = 0x3042; - wchar_t inChars[INPUT_RECORD_COUNT]; - for (size_t i = 0; i < INPUT_RECORD_COUNT; ++i) - { - auto currentChar = static_cast(hiraganaA + (i * 2)); - inRecords[i].EventType = KEY_EVENT; - inRecords[i].Event.KeyEvent.uChar.UnicodeChar = currentChar; - inChars[i] = currentChar; - inEvents.push_back(IInputEvent::Create(inRecords[i])); - } - - SplitToOem(inEvents); - VERIFY_ARE_EQUAL(INPUT_RECORD_COUNT * 2, inEvents.size()); - - // create the data to compare the output to - char dbcsChars[INPUT_RECORD_COUNT * 2] = { 0 }; - auto writtenBytes = WideCharToMultiByte(codepage, - 0, - inChars, - INPUT_RECORD_COUNT, - dbcsChars, - INPUT_RECORD_COUNT * 2, - nullptr, - FALSE); - VERIFY_ARE_EQUAL(writtenBytes, static_cast(INPUT_RECORD_COUNT * 2)); - for (size_t i = 0; i < INPUT_RECORD_COUNT * 2; ++i) - { - const auto pKeyEvent = static_cast(inEvents[i].get()); - VERIFY_ARE_EQUAL(static_cast(pKeyEvent->GetCharData()), dbcsChars[i]); - } - } -}; diff --git a/src/types/inc/IInputEvent.hpp b/src/types/inc/IInputEvent.hpp index edda9846a55..50626cdda7e 100644 --- a/src/types/inc/IInputEvent.hpp +++ b/src/types/inc/IInputEvent.hpp @@ -64,6 +64,8 @@ class IInputEvent inline IInputEvent::~IInputEvent() = default; +using InputEventQueue = std::deque>; + #ifdef UNIT_TESTING std::wostream& operator<<(std::wostream& stream, const IInputEvent* pEvent); #endif From eac0aeb2350c1dac0c3726d4135b03aca7108842 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 3 Feb 2023 15:37:11 +0100 Subject: [PATCH 04/10] Remove previously added code --- src/host/misc.h | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/src/host/misc.h b/src/host/misc.h index 13c4f42d3d2..ca90ac23ac8 100644 --- a/src/host/misc.h +++ b/src/host/misc.h @@ -44,38 +44,6 @@ int ConvertToOem(const UINT uiCodePage, _Out_writes_(cchTarget) CHAR* const pchTarget, const UINT cchTarget) noexcept; -template -void StringToInputEvents(std::basic_string_view str, std::deque>& events) -{ - for (const auto& ch : str) - { - auto event = std::make_unique(); - event->SetKeyDown(true); - event->SetRepeatCount(1); - event->SetCharData(ch); - events.push_back(std::move(event)); - } -} - -template -void InputEventsToString(std::deque>& events, std::basic_string& str) -{ - for (const auto& event : events) - { - if (event->EventType() == InputEventType::KeyEvent) - { - const auto keyEvent = static_cast(event.get()); - // This keydown check prevents us from serializing ABC keypresses - // into AABBCC because each key-down is followed by a key-up. - if (keyEvent->IsKeyDown()) - { - const auto wch = keyEvent->GetCharData(); - str.push_back(static_cast(wch)); - } - } - } -} - int ConvertInputToUnicode(const UINT uiCodePage, _In_reads_(cchSource) const CHAR* const pchSource, const UINT cchSource, From 005bc2154c08dcb6559547c61d479af519a0f0f8 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 3 Feb 2023 15:38:42 +0100 Subject: [PATCH 05/10] Address feedback --- src/host/readDataRaw.cpp | 2 +- src/host/stream.cpp | 20 ++++++++++---------- src/host/stream.h | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/host/readDataRaw.cpp b/src/host/readDataRaw.cpp index 5b7b6480ff3..9351382c074 100644 --- a/src/host/readDataRaw.cpp +++ b/src/host/readDataRaw.cpp @@ -116,7 +116,7 @@ bool RAW_READ_DATA::Notify(const WaitTerminationReason TerminationReason, // console lock. std::span buffer{ reinterpret_cast(_BufPtr), _BufferSize }; - *pReplyStatus = _ReadCharacterInput(*_pInputBuffer, buffer, *pNumBytes, *_pInputReadHandleData, fIsUnicode); + *pReplyStatus = ReadCharacterInput(*_pInputBuffer, buffer, *pNumBytes, *_pInputReadHandleData, fIsUnicode); return *pReplyStatus != CONSOLE_STATUS_WAIT; } diff --git a/src/host/stream.cpp b/src/host/stream.cpp index 1b43e4363a8..9912c5eda17 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -407,11 +407,11 @@ NT_CATCH_RETURN() // populated. // - STATUS_SUCCESS on success // - Other NTSTATUS codes as necessary -[[nodiscard]] NTSTATUS _ReadCharacterInput(InputBuffer& inputBuffer, - std::span buffer, - size_t& bytesRead, - INPUT_READ_HANDLE_DATA& readHandleState, - const bool unicode) +[[nodiscard]] NTSTATUS ReadCharacterInput(InputBuffer& inputBuffer, + std::span buffer, + size_t& bytesRead, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool unicode) try { UNREFERENCED_PARAMETER(readHandleState); @@ -527,11 +527,11 @@ NT_CATCH_RETURN() } else { - const auto status = _ReadCharacterInput(inputBuffer, - buffer, - bytesRead, - readHandleState, - unicode); + const auto status = ReadCharacterInput(inputBuffer, + buffer, + bytesRead, + readHandleState, + unicode); if (status == CONSOLE_STATUS_WAIT) { waiter = std::make_unique(&inputBuffer, &readHandleState, gsl::narrow(buffer.size()), reinterpret_cast(buffer.data())); diff --git a/src/host/stream.h b/src/host/stream.h index 2d743849f3b..b4c37d82a45 100644 --- a/src/host/stream.h +++ b/src/host/stream.h @@ -29,11 +29,11 @@ Revision History: _Out_opt_ bool* const pPopupKeys, _Out_opt_ DWORD* const pdwKeyState) noexcept; -[[nodiscard]] NTSTATUS _ReadCharacterInput(InputBuffer& inputBuffer, - std::span buffer, - size_t& bytesRead, - INPUT_READ_HANDLE_DATA& readHandleState, - const bool unicode); +[[nodiscard]] NTSTATUS ReadCharacterInput(InputBuffer& inputBuffer, + std::span buffer, + size_t& bytesRead, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool unicode); // Routine Description: // - This routine returns the total number of screen spaces the characters up to the specified character take up. From 738ce5f0c17ed4aac6b216617c56128be27162da Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sun, 5 Feb 2023 14:17:40 +0100 Subject: [PATCH 06/10] Simplify InputBuffer::Read and fix KeyEvent repeats --- src/host/inputBuffer.cpp | 77 +++++++++++----------- src/host/stream.cpp | 2 +- src/host/ut_host/CommandLineTests.cpp | 2 +- src/host/ut_host/InputBufferTests.cpp | 92 +++++++++++++++------------ 4 files changed, 88 insertions(+), 85 deletions(-) diff --git a/src/host/inputBuffer.cpp b/src/host/inputBuffer.cpp index 786d7898e0c..215b49ceb69 100644 --- a/src/host/inputBuffer.cpp +++ b/src/host/inputBuffer.cpp @@ -422,68 +422,63 @@ try while (it != end && OutEvents.size() < AmountToRead) { - std::unique_ptr event; + auto event = IInputEvent::Create((*it)->ToInputRecord()); + if (event->EventType() == InputEventType::KeyEvent) { - decltype(_storage)::difference_type advance = 1; - auto& e = *it; + const auto keyEvent = static_cast(event.get()); + WORD repeat = 1; // for stream reads we need to split any key events that have been coalesced - if (Stream && e->EventType() == InputEventType::KeyEvent) + if (Stream) { - const auto keyEvent = static_cast(e.get()); - const auto repeatCount = keyEvent->GetRepeatCount(); - - if (repeatCount > 1) - { - auto tempEvent = std::make_unique(*keyEvent); - tempEvent->SetRepeatCount(1); - event = std::move(tempEvent); - - // By setting the advance to 0, the next time we loop around we'll - // process the same event again, up until the repeat count is 1. - keyEvent->SetRepeatCount(repeatCount - 1); - advance = 0; - } + repeat = keyEvent->GetRepeatCount(); + keyEvent->SetRepeatCount(1); } - if (!event) + if (Unicode) { - if (Peek) - { - event = IInputEvent::Create(event->ToInputRecord()); - } - else + do { - event = std::move(e); - } + OutEvents.push_back(std::make_unique(*keyEvent)); + repeat--; + } while (repeat > 0 && OutEvents.size() < AmountToRead); } + else + { + const auto wch = keyEvent->GetCharData(); - it += advance; - } - - if (!Unicode && event->EventType() == InputEventType::KeyEvent) - { - const auto keyEvent = static_cast(event.get()); - const auto wch = keyEvent->GetCharData(); + char buffer[8]; + const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); + THROW_LAST_ERROR_IF(length <= 0); - char buffer[8]; - const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr); - THROW_LAST_ERROR_IF(length <= 0); + const std::string_view str{ &buffer[0], gsl::narrow_cast(length) }; - const std::string_view str{ &buffer[0], gsl::narrow_cast(length) }; + do + { + for (const auto& ch : str) + { + auto tempEvent = std::make_unique(*keyEvent); + tempEvent->SetCharData(ch); + OutEvents.push_back(std::move(tempEvent)); + } + repeat--; + } while (repeat > 0 && OutEvents.size() < AmountToRead); + } - for (const auto& ch : str) + if (repeat && !Peek) { - auto tempEvent = std::make_unique(*keyEvent); - tempEvent->SetCharData(ch); - OutEvents.push_back(std::move(tempEvent)); + const auto originalKeyEvent = static_cast((*it).get()); + originalKeyEvent->SetRepeatCount(repeat); + break; } } else { OutEvents.push_back(std::move(event)); } + + ++it; } if (!Peek) diff --git a/src/host/stream.cpp b/src/host/stream.cpp index 9912c5eda17..a844777a562 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -449,7 +449,7 @@ try } bytesRead = buffer.size() - writer.size(); - return STATUS_SUCCESS; + return status; } NT_CATCH_RETURN() diff --git a/src/host/ut_host/CommandLineTests.cpp b/src/host/ut_host/CommandLineTests.cpp index 3248ee4d298..b003c37b5f2 100644 --- a/src/host/ut_host/CommandLineTests.cpp +++ b/src/host/ut_host/CommandLineTests.cpp @@ -77,7 +77,7 @@ class CommandLineTests const size_t cchBuffer) { cookedReadData._commandHistory = pHistory; - cookedReadData._userBuffer = pBuffer; + cookedReadData._userBuffer = reinterpret_cast(pBuffer); cookedReadData._userBufferSize = cchBuffer * sizeof(wchar_t); cookedReadData._bufferSize = cchBuffer * sizeof(wchar_t); cookedReadData._backupLimit = pBuffer; diff --git a/src/host/ut_host/InputBufferTests.cpp b/src/host/ut_host/InputBufferTests.cpp index 84e0c86f18b..353eaecc5a4 100644 --- a/src/host/ut_host/InputBufferTests.cpp +++ b/src/host/ut_host/InputBufferTests.cpp @@ -367,7 +367,7 @@ class InputBufferTests TEST_METHOD(EmptyingBufferDuringReadSetsResetWaitEvent) { - Log::Comment(L"ResetWaitEvent should be true if a read to the buffer completely empties it"); + Log::Comment(L"hInputEvent should be reset if a read to the buffer completely empties it"); InputBuffer inputBuffer; @@ -381,48 +381,62 @@ class InputBufferTests } VERIFY_IS_GREATER_THAN(inputBuffer.Write(inEvents), 0u); - // read one record, make sure ResetWaitEvent isn't set + auto& waitEvent = ServiceLocator::LocateGlobals().hInputEvent; + waitEvent.SetEvent(); + + // read one record, hInputEvent should still be signaled std::deque> outEvents; - size_t eventsRead = 0; - auto resetWaitEvent = false; - inputBuffer._ReadBuffer(outEvents, - 1, - eventsRead, - false, - resetWaitEvent, - true, - false); - VERIFY_ARE_EQUAL(eventsRead, 1u); - VERIFY_IS_FALSE(!!resetWaitEvent); - - // read the rest, resetWaitEvent should be set to true + VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, + 1, + false, + false, + true, + false)); + VERIFY_ARE_EQUAL(outEvents.size(), 1u); + VERIFY_IS_TRUE(waitEvent.is_signaled()); + + // read the rest, hInputEvent should be reset + waitEvent.SetEvent(); outEvents.clear(); - inputBuffer._ReadBuffer(outEvents, - RECORD_INSERT_COUNT - 1, - eventsRead, - false, - resetWaitEvent, - true, - false); - VERIFY_ARE_EQUAL(eventsRead, RECORD_INSERT_COUNT - 1); - VERIFY_IS_TRUE(!!resetWaitEvent); + VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, + RECORD_INSERT_COUNT - 1, + false, + false, + true, + false)); + VERIFY_ARE_EQUAL(outEvents.size(), RECORD_INSERT_COUNT - 1); + VERIFY_IS_FALSE(waitEvent.is_signaled()); } TEST_METHOD(ReadingDbcsCharsPadsOutputArray) { Log::Comment(L"During a non-unicode read, the input buffer should count twice for each dbcs key event"); + auto& codepage = ServiceLocator::LocateGlobals().getConsoleInformation().CP; + const auto restoreCP = wil::scope_exit([&, previous = codepage]() { + codepage = previous; + }); + + codepage = CP_JAPANESE; + // write a mouse event, key event, dbcs key event, mouse event InputBuffer inputBuffer; - const unsigned int recordInsertCount = 4; - INPUT_RECORD inRecords[recordInsertCount]; + + std::array inRecords; inRecords[0].EventType = MOUSE_EVENT; inRecords[1] = MakeKeyEvent(TRUE, 1, L'A', 0, L'A', 0); inRecords[2] = MakeKeyEvent(TRUE, 1, 0x3042, 0, 0x3042, 0); // U+3042 hiragana A inRecords[3].EventType = MOUSE_EVENT; + std::array outRecordsExpected; + outRecordsExpected[0].EventType = MOUSE_EVENT; + outRecordsExpected[1] = MakeKeyEvent(TRUE, 1, L'A', 0, L'A', 0); + outRecordsExpected[2] = MakeKeyEvent(TRUE, 1, 0x3042, 0, 0x82, 0); + outRecordsExpected[3] = MakeKeyEvent(TRUE, 1, 0x3042, 0, 0xa0, 0); + outRecordsExpected[4].EventType = MOUSE_EVENT; + std::deque> inEvents; - for (size_t i = 0; i < recordInsertCount; ++i) + for (size_t i = 0; i < inRecords.size(); ++i) { inEvents.push_back(IInputEvent::Create(inRecords[i])); } @@ -432,22 +446,16 @@ class InputBufferTests // read them out non-unicode style and compare std::deque> outEvents; - size_t eventsRead = 0; - auto resetWaitEvent = false; - inputBuffer._ReadBuffer(outEvents, - recordInsertCount, - eventsRead, - false, - resetWaitEvent, - false, - false); - // the dbcs record should have counted for two elements in - // the array, making it so that we get less events read - VERIFY_ARE_EQUAL(eventsRead, recordInsertCount - 1); - VERIFY_ARE_EQUAL(eventsRead, outEvents.size()); - for (size_t i = 0; i < eventsRead; ++i) + VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, + outRecordsExpected.size(), + false, + false, + false, + false)); + VERIFY_ARE_EQUAL(outEvents.size(), outRecordsExpected.size()); + for (size_t i = 0; i < outEvents.size(); ++i) { - VERIFY_ARE_EQUAL(outEvents[i]->ToInputRecord(), inRecords[i]); + VERIFY_ARE_EQUAL(outEvents[i]->ToInputRecord(), outRecordsExpected[i]); } } From a0f16372646cbeb9f0c62b815adfc476db4700ab Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sun, 5 Feb 2023 16:27:52 +0100 Subject: [PATCH 07/10] Fix zero initialization --- src/host/ut_host/InputBufferTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/host/ut_host/InputBufferTests.cpp b/src/host/ut_host/InputBufferTests.cpp index 353eaecc5a4..f619d75e01e 100644 --- a/src/host/ut_host/InputBufferTests.cpp +++ b/src/host/ut_host/InputBufferTests.cpp @@ -422,13 +422,13 @@ class InputBufferTests // write a mouse event, key event, dbcs key event, mouse event InputBuffer inputBuffer; - std::array inRecords; + std::array inRecords{}; inRecords[0].EventType = MOUSE_EVENT; inRecords[1] = MakeKeyEvent(TRUE, 1, L'A', 0, L'A', 0); inRecords[2] = MakeKeyEvent(TRUE, 1, 0x3042, 0, 0x3042, 0); // U+3042 hiragana A inRecords[3].EventType = MOUSE_EVENT; - std::array outRecordsExpected; + std::array outRecordsExpected{}; outRecordsExpected[0].EventType = MOUSE_EVENT; outRecordsExpected[1] = MakeKeyEvent(TRUE, 1, L'A', 0, L'A', 0); outRecordsExpected[2] = MakeKeyEvent(TRUE, 1, 0x3042, 0, 0x82, 0); From d3ca7470a89b42aea88b02ee497147f4ab4b3f56 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 6 Feb 2023 15:51:04 +0100 Subject: [PATCH 08/10] Fix CRLF --- src/inc/til/bytes.h | 114 ++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/src/inc/til/bytes.h b/src/inc/til/bytes.h index 43dba44282f..593a0cd435f 100644 --- a/src/inc/til/bytes.h +++ b/src/inc/til/bytes.h @@ -1,57 +1,57 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include "type_traits.h" - -namespace til -{ - template - constexpr void bytes_advance(Target& target, size_t count) - { - if (count > target.size()) - { - throw std::length_error{ "insufficient space left" }; - } - - target = { target.data() + count, target.size() - count }; - } - - template - constexpr bool bytes_can_put(const Target& target) - { - return target.size() >= sizeof(T); - } - - template - constexpr void bytes_put(Target& target, const T& value) - { - using TargetType = typename Target::value_type; - constexpr auto size = sizeof(value); - - if (size > target.size()) - { - throw std::length_error{ "insufficient space left" }; - } - - std::copy_n(reinterpret_cast(&value), size, target.data()); - target = { target.data() + size, target.size() - size }; - } - - template - requires TriviallyCopyable - constexpr void bytes_transfer(Target& target, Source& source) - { - using TargetType = typename Target::value_type; - constexpr auto elementSize = sizeof(typename Source::value_type); - - const auto sourceCount = std::min(source.size(), target.size() / elementSize); - const auto targetCount = sourceCount * elementSize; - - std::copy_n(reinterpret_cast(source.data()), targetCount, target.data()); - - target = { target.data() + targetCount, target.size() - targetCount }; - source = { source.data() + sourceCount, source.size() - sourceCount }; - } -} +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "type_traits.h" + +namespace til +{ + template + constexpr void bytes_advance(Target& target, size_t count) + { + if (count > target.size()) + { + throw std::length_error{ "insufficient space left" }; + } + + target = { target.data() + count, target.size() - count }; + } + + template + constexpr bool bytes_can_put(const Target& target) + { + return target.size() >= sizeof(T); + } + + template + constexpr void bytes_put(Target& target, const T& value) + { + using TargetType = typename Target::value_type; + constexpr auto size = sizeof(value); + + if (size > target.size()) + { + throw std::length_error{ "insufficient space left" }; + } + + std::copy_n(reinterpret_cast(&value), size, target.data()); + target = { target.data() + size, target.size() - size }; + } + + template + requires TriviallyCopyable + constexpr void bytes_transfer(Target& target, Source& source) + { + using TargetType = typename Target::value_type; + constexpr auto elementSize = sizeof(typename Source::value_type); + + const auto sourceCount = std::min(source.size(), target.size() / elementSize); + const auto targetCount = sourceCount * elementSize; + + std::copy_n(reinterpret_cast(source.data()), targetCount, target.data()); + + target = { target.data() + targetCount, target.size() - targetCount }; + source = { source.data() + sourceCount, source.size() - sourceCount }; + } +} From 7d9a213322866b11fb293869d256c7f1ebf9040b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 28 Feb 2023 15:22:58 +0100 Subject: [PATCH 09/10] Address feedback --- src/host/readDataDirect.cpp | 3 +-- src/inc/til/type_traits.h | 15 ++++----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/host/readDataDirect.cpp b/src/host/readDataDirect.cpp index b9a9c4934ac..c4f58cf3c64 100644 --- a/src/host/readDataDirect.cpp +++ b/src/host/readDataDirect.cpp @@ -128,8 +128,7 @@ try } catch (...) { - // There doesn't seem to be a way to just get the NT exception code with WIL. - *pReplyStatus = NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); + *pReplyStatus = wil::StatusFromCaughtException(); return true; } diff --git a/src/inc/til/type_traits.h b/src/inc/til/type_traits.h index c625ce411a8..a4a10173199 100644 --- a/src/inc/til/type_traits.h +++ b/src/inc/til/type_traits.h @@ -11,21 +11,14 @@ namespace til struct is_contiguous_view : std::false_type { }; - // std::span remains largely unused in this code base so far. - //template - //struct is_contiguous_view> : std::true_type - //{ - //}; - template - struct is_contiguous_view> : std::true_type - { - }; -#ifdef GSL_SPAN_H template struct is_contiguous_view> : std::true_type { }; -#endif + template + struct is_contiguous_view> : std::true_type + { + }; template struct is_byte : std::false_type From ee2850dae4c722abeb875aeb502900d8dae58086 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 28 Feb 2023 15:48:42 +0100 Subject: [PATCH 10/10] Fix bad merge --- src/host/ut_host/InputBufferTests.cpp | 36 +++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/host/ut_host/InputBufferTests.cpp b/src/host/ut_host/InputBufferTests.cpp index b96959f2a1f..904ca21f8ca 100644 --- a/src/host/ut_host/InputBufferTests.cpp +++ b/src/host/ut_host/InputBufferTests.cpp @@ -386,24 +386,24 @@ class InputBufferTests // read one record, hInputEvent should still be signaled std::deque> outEvents; - VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, - 1, - false, - false, - true, - false)); + VERIFY_NT_SUCCESS(inputBuffer.Read(outEvents, + 1, + false, + false, + true, + false)); VERIFY_ARE_EQUAL(outEvents.size(), 1u); VERIFY_IS_TRUE(waitEvent.is_signaled()); // read the rest, hInputEvent should be reset waitEvent.SetEvent(); outEvents.clear(); - VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, - RECORD_INSERT_COUNT - 1, - false, - false, - true, - false)); + VERIFY_NT_SUCCESS(inputBuffer.Read(outEvents, + RECORD_INSERT_COUNT - 1, + false, + false, + true, + false)); VERIFY_ARE_EQUAL(outEvents.size(), RECORD_INSERT_COUNT - 1); VERIFY_IS_FALSE(waitEvent.is_signaled()); } @@ -446,12 +446,12 @@ class InputBufferTests // read them out non-unicode style and compare std::deque> outEvents; - VERIFY_SUCCESS_NTSTATUS(inputBuffer.Read(outEvents, - outRecordsExpected.size(), - false, - false, - false, - false)); + VERIFY_NT_SUCCESS(inputBuffer.Read(outEvents, + outRecordsExpected.size(), + false, + false, + false, + false)); VERIFY_ARE_EQUAL(outEvents.size(), outRecordsExpected.size()); for (size_t i = 0; i < outEvents.size(); ++i) {