From e9c7111b66596f82f7885cf3a7c0986466676a1a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Mon, 28 Jul 2014 21:20:36 +0000 Subject: [PATCH] [KERNEL32][CONSRV] - Make kernel32 / winsrv console CSR structures Win2k3-compliant for Read/WriteConsoleInput functions. - Really fix some ASSERTS from r63108. Part 6/X CORE-7931 svn path=/branches/condrv_restructure/; revision=63754 --- dll/win32/kernel32/client/console/readwrite.c | 163 +++++++++++------- include/reactos/subsys/win/conmsg.h | 35 ++-- win32ss/user/winsrv/consrv/condrv/coninput.c | 18 +- win32ss/user/winsrv/consrv/condrv/text.c | 9 +- win32ss/user/winsrv/consrv/coninput.c | 104 ++++++++--- 5 files changed, 214 insertions(+), 115 deletions(-) diff --git a/dll/win32/kernel32/client/console/readwrite.c b/dll/win32/kernel32/client/console/readwrite.c index 57416b5c06e..9068ef36f58 100644 --- a/dll/win32/kernel32/client/console/readwrite.c +++ b/dll/win32/kernel32/client/console/readwrite.c @@ -127,39 +127,62 @@ IntGetConsoleInput(HANDLE hConsoleInput, { CONSOLE_API_MESSAGE ApiMessage; PCONSOLE_GETINPUT GetInputRequest = &ApiMessage.Data.GetInputRequest; - PCSR_CAPTURE_BUFFER CaptureBuffer; - ULONG Size; + PCSR_CAPTURE_BUFFER CaptureBuffer = NULL; if (lpBuffer == NULL) { - SetLastError(ERROR_INVALID_PARAMETER); + SetLastError(ERROR_INVALID_ACCESS); return FALSE; } - Size = nLength * sizeof(INPUT_RECORD); + if (!IsConsoleHandle(hConsoleInput)) + { + SetLastError(ERROR_INVALID_HANDLE); - DPRINT("IntGetConsoleInput: %lx %p\n", Size, lpNumberOfEventsRead); + if (lpNumberOfEventsRead != NULL) + *lpNumberOfEventsRead = 0; - /* Allocate a Capture Buffer */ - CaptureBuffer = CsrAllocateCaptureBuffer(1, Size); - if (CaptureBuffer == NULL) - { - DPRINT1("CsrAllocateCaptureBuffer failed!\n"); - SetLastError(ERROR_NOT_ENOUGH_MEMORY); return FALSE; } - /* Allocate space in the Buffer */ - CsrAllocateMessagePointer(CaptureBuffer, - Size, - (PVOID*)&GetInputRequest->InputRecord); + DPRINT("IntGetConsoleInput: %lx %p\n", nLength, lpNumberOfEventsRead); /* Set up the data to send to the Console Server */ - GetInputRequest->InputHandle = hConsoleInput; - GetInputRequest->InputsRead = 0; - GetInputRequest->Length = nLength; - GetInputRequest->wFlags = wFlags; - GetInputRequest->Unicode = bUnicode; + GetInputRequest->ConsoleHandle = NtCurrentPeb()->ProcessParameters->ConsoleHandle; + GetInputRequest->InputHandle = hConsoleInput; + GetInputRequest->NumRecords = nLength; + GetInputRequest->Flags = wFlags; + GetInputRequest->Unicode = bUnicode; + + /* + * For optimization purposes, Windows (and hence ReactOS, too, for + * compatibility reasons) uses a static buffer if no more than five + * input records are read. Otherwise a new buffer is allocated. + * This behaviour is also expected in the server-side. + */ + if (nLength <= sizeof(GetInputRequest->RecordStaticBuffer)/sizeof(INPUT_RECORD)) + { + GetInputRequest->RecordBufPtr = GetInputRequest->RecordStaticBuffer; + // CaptureBuffer = NULL; + } + else + { + ULONG Size = nLength * sizeof(INPUT_RECORD); + + /* Allocate a Capture Buffer */ + CaptureBuffer = CsrAllocateCaptureBuffer(1, Size); + if (CaptureBuffer == NULL) + { + DPRINT1("CsrAllocateCaptureBuffer failed!\n"); + SetLastError(ERROR_NOT_ENOUGH_MEMORY); + return FALSE; + } + + /* Allocate space in the Buffer */ + CsrAllocateMessagePointer(CaptureBuffer, + Size, + (PVOID*)&GetInputRequest->RecordBufPtr); + } /* Call the server */ CsrClientCallServer((PCSR_API_MESSAGE)&ApiMessage, @@ -171,16 +194,15 @@ IntGetConsoleInput(HANDLE hConsoleInput, if (NT_SUCCESS(ApiMessage.Status)) { /* Return the number of events read */ - DPRINT("Events read: %lx\n", GetInputRequest->InputsRead); + DPRINT("Events read: %lx\n", GetInputRequest->NumRecords); if (lpNumberOfEventsRead != NULL) - *lpNumberOfEventsRead = GetInputRequest->InputsRead; + *lpNumberOfEventsRead = GetInputRequest->NumRecords; /* Copy into the buffer */ - DPRINT("Copying to buffer\n"); RtlCopyMemory(lpBuffer, - GetInputRequest->InputRecord, - sizeof(INPUT_RECORD) * GetInputRequest->InputsRead); + GetInputRequest->RecordBufPtr, + GetInputRequest->NumRecords * sizeof(INPUT_RECORD)); } else { @@ -191,12 +213,11 @@ IntGetConsoleInput(HANDLE hConsoleInput, BaseSetLastNTError(ApiMessage.Status); } - /* Release the capture buffer */ - CsrFreeCaptureBuffer(CaptureBuffer); + /* Release the capture buffer if needed */ + if (CaptureBuffer) CsrFreeCaptureBuffer(CaptureBuffer); /* Return TRUE or FALSE */ - return (GetInputRequest->InputsRead > 0); - // return NT_SUCCESS(ApiMessage.Status); + return NT_SUCCESS(ApiMessage.Status); } @@ -216,7 +237,7 @@ IntReadConsoleOutput(HANDLE hConsoleOutput, if (lpBuffer == NULL) { - SetLastError(ERROR_INVALID_PARAMETER); + SetLastError(ERROR_INVALID_ACCESS); return FALSE; } @@ -454,38 +475,62 @@ IntWriteConsoleInput(HANDLE hConsoleInput, PINPUT_RECORD lpBuffer, DWORD nLength, LPDWORD lpNumberOfEventsWritten, - BOOL bUnicode, - BOOL bAppendToEnd) + BOOLEAN bUnicode, + BOOLEAN bAppendToEnd) { CONSOLE_API_MESSAGE ApiMessage; PCONSOLE_WRITEINPUT WriteInputRequest = &ApiMessage.Data.WriteInputRequest; - PCSR_CAPTURE_BUFFER CaptureBuffer; - DWORD Size; - - Size = nLength * sizeof(INPUT_RECORD); + PCSR_CAPTURE_BUFFER CaptureBuffer = NULL; - DPRINT("IntWriteConsoleInput: %lx %p\n", Size, lpNumberOfEventsWritten); - - /* Allocate a Capture Buffer */ - CaptureBuffer = CsrAllocateCaptureBuffer(1, Size); - if (CaptureBuffer == NULL) + if (lpBuffer == NULL) { - DPRINT1("CsrAllocateCaptureBuffer failed!\n"); - SetLastError(ERROR_NOT_ENOUGH_MEMORY); + SetLastError(ERROR_INVALID_ACCESS); return FALSE; } - /* Capture the user buffer */ - CsrCaptureMessageBuffer(CaptureBuffer, - lpBuffer, - Size, - (PVOID*)&WriteInputRequest->InputRecord); + DPRINT("IntWriteConsoleInput: %lx %p\n", nLength, lpNumberOfEventsWritten); /* Set up the data to send to the Console Server */ - WriteInputRequest->InputHandle = hConsoleInput; - WriteInputRequest->Length = nLength; - WriteInputRequest->Unicode = bUnicode; - WriteInputRequest->AppendToEnd = bAppendToEnd; + WriteInputRequest->ConsoleHandle = NtCurrentPeb()->ProcessParameters->ConsoleHandle; + WriteInputRequest->InputHandle = hConsoleInput; + WriteInputRequest->NumRecords = nLength; + WriteInputRequest->Unicode = bUnicode; + WriteInputRequest->AppendToEnd = bAppendToEnd; + + /* + * For optimization purposes, Windows (and hence ReactOS, too, for + * compatibility reasons) uses a static buffer if no more than five + * input records are written. Otherwise a new buffer is allocated. + * This behaviour is also expected in the server-side. + */ + if (nLength <= sizeof(WriteInputRequest->RecordStaticBuffer)/sizeof(INPUT_RECORD)) + { + WriteInputRequest->RecordBufPtr = WriteInputRequest->RecordStaticBuffer; + // CaptureBuffer = NULL; + + RtlCopyMemory(WriteInputRequest->RecordBufPtr, + lpBuffer, + nLength * sizeof(INPUT_RECORD)); + } + else + { + ULONG Size = nLength * sizeof(INPUT_RECORD); + + /* Allocate a Capture Buffer */ + CaptureBuffer = CsrAllocateCaptureBuffer(1, Size); + if (CaptureBuffer == NULL) + { + DPRINT1("CsrAllocateCaptureBuffer failed!\n"); + SetLastError(ERROR_NOT_ENOUGH_MEMORY); + return FALSE; + } + + /* Capture the user buffer */ + CsrCaptureMessageBuffer(CaptureBuffer, + lpBuffer, + Size, + (PVOID*)&WriteInputRequest->RecordBufPtr); + } /* Call the server */ CsrClientCallServer((PCSR_API_MESSAGE)&ApiMessage, @@ -493,14 +538,17 @@ IntWriteConsoleInput(HANDLE hConsoleInput, CSR_CREATE_API_NUMBER(CONSRV_SERVERDLL_INDEX, ConsolepWriteConsoleInput), sizeof(*WriteInputRequest)); + /* Release the capture buffer if needed */ + if (CaptureBuffer) CsrFreeCaptureBuffer(CaptureBuffer); + /* Check for success */ if (NT_SUCCESS(ApiMessage.Status)) { - /* Return the number of events read */ - DPRINT("Events read: %lx\n", WriteInputRequest->Length); + /* Return the number of events written */ + DPRINT("Events written: %lx\n", WriteInputRequest->NumRecords); if (lpNumberOfEventsWritten != NULL) - *lpNumberOfEventsWritten = WriteInputRequest->Length; + *lpNumberOfEventsWritten = WriteInputRequest->NumRecords; } else { @@ -511,9 +559,6 @@ IntWriteConsoleInput(HANDLE hConsoleInput, BaseSetLastNTError(ApiMessage.Status); } - /* Release the capture buffer */ - CsrFreeCaptureBuffer(CaptureBuffer); - /* Return TRUE or FALSE */ return NT_SUCCESS(ApiMessage.Status); } @@ -535,7 +580,7 @@ IntWriteConsoleOutput(HANDLE hConsoleOutput, if ((lpBuffer == NULL) || (lpWriteRegion == NULL)) { - SetLastError(ERROR_INVALID_PARAMETER); + SetLastError(ERROR_INVALID_ACCESS); return FALSE; } /* diff --git a/include/reactos/subsys/win/conmsg.h b/include/reactos/subsys/win/conmsg.h index d401dd54a13..032be0ac1b1 100644 --- a/include/reactos/subsys/win/conmsg.h +++ b/include/reactos/subsys/win/conmsg.h @@ -520,14 +520,26 @@ typedef struct typedef struct { - HANDLE InputHandle; - ULONG InputsRead; - PINPUT_RECORD InputRecord; - ULONG Length; - WORD wFlags; - BOOLEAN Unicode; + HANDLE ConsoleHandle; + HANDLE InputHandle; + INPUT_RECORD RecordStaticBuffer[5]; + PINPUT_RECORD RecordBufPtr; + ULONG NumRecords; + WORD Flags; + BOOLEAN Unicode; } CONSOLE_GETINPUT, *PCONSOLE_GETINPUT; +typedef struct +{ + HANDLE ConsoleHandle; + HANDLE InputHandle; + INPUT_RECORD RecordStaticBuffer[5]; + PINPUT_RECORD RecordBufPtr; + ULONG NumRecords; + BOOLEAN Unicode; + BOOLEAN AppendToEnd; +} CONSOLE_WRITEINPUT, *PCONSOLE_WRITEINPUT; + typedef struct { HANDLE OutputHandle; @@ -539,20 +551,11 @@ typedef struct PCHAR_INFO CharInfo; } CONSOLE_READOUTPUT, *PCONSOLE_READOUTPUT; -typedef struct -{ - HANDLE InputHandle; - DWORD Length; - INPUT_RECORD* InputRecord; - BOOL Unicode; - BOOL AppendToEnd; -} CONSOLE_WRITEINPUT, *PCONSOLE_WRITEINPUT; - typedef struct { HANDLE ConsoleHandle; HANDLE InputHandle; - DWORD NumberOfEvents; + ULONG NumberOfEvents; } CONSOLE_GETNUMINPUTEVENTS, *PCONSOLE_GETNUMINPUTEVENTS; diff --git a/win32ss/user/winsrv/consrv/condrv/coninput.c b/win32ss/user/winsrv/consrv/condrv/coninput.c index 3f3600091fe..c7b5e27b765 100644 --- a/win32ss/user/winsrv/consrv/condrv/coninput.c +++ b/win32ss/user/winsrv/consrv/condrv/coninput.c @@ -170,8 +170,7 @@ ConDrvReadConsole(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == InputBuffer->Header.Console); - ASSERT( (Buffer != NULL && NumCharsToRead > 0) || - (Buffer == NULL && NumCharsToRead == 0) ); + ASSERT((Buffer != NULL) || (Buffer == NULL && NumCharsToRead == 0)); /* We haven't read anything (yet) */ @@ -316,11 +315,9 @@ ConDrvGetConsoleInput(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == InputBuffer->Header.Console); - ASSERT( (InputRecord != NULL && NumEventsToRead > 0) || - (InputRecord == NULL && NumEventsToRead == 0) ); + ASSERT((InputRecord != NULL) || (InputRecord == NULL && NumEventsToRead == 0)); - // Do NOT do that !! Use the existing number of events already read, if any... - // if (NumEventsRead) *NumEventsRead = 0; + if (NumEventsRead) *NumEventsRead = 0; if (IsListEmpty(&InputBuffer->InputEvents)) { @@ -333,8 +330,7 @@ ConDrvGetConsoleInput(IN PCONSOLE Console, /* Only get input if there is any */ CurrentInput = InputBuffer->InputEvents.Flink; - if (NumEventsRead) i = *NumEventsRead; // We will read the remaining events... - + i = 0; while ((CurrentInput != &InputBuffer->InputEvents) && (i < NumEventsToRead)) { Input = CONTAINING_RECORD(CurrentInput, ConsoleInput, ListEntry); @@ -386,12 +382,10 @@ ConDrvWriteConsoleInput(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == InputBuffer->Header.Console); - ASSERT( (InputRecord != NULL && NumEventsToWrite > 0) || - (InputRecord == NULL && NumEventsToWrite == 0) ); + ASSERT((InputRecord != NULL) || (InputRecord == NULL && NumEventsToWrite == 0)); // if (NumEventsWritten) *NumEventsWritten = 0; - - /// Status = ConioAddInputEvents(Console, InputRecord, NumEventsToWrite, NumEventsWritten, AppendToEnd); + // Status = ConioAddInputEvents(Console, InputRecord, NumEventsToWrite, NumEventsWritten, AppendToEnd); for (i = 0; i < NumEventsToWrite && NT_SUCCESS(Status); ++i) { diff --git a/win32ss/user/winsrv/consrv/condrv/text.c b/win32ss/user/winsrv/consrv/condrv/text.c index 41b5ce72dc2..b906dc1a59e 100644 --- a/win32ss/user/winsrv/consrv/condrv/text.c +++ b/win32ss/user/winsrv/consrv/condrv/text.c @@ -829,8 +829,7 @@ ConDrvWriteConsole(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == ScreenBuffer->Header.Console); - ASSERT( (StringBuffer != NULL && NumCharsToWrite > 0) || - (StringBuffer == NULL && NumCharsToWrite == 0) ); + ASSERT((StringBuffer != NULL) || (StringBuffer == NULL && NumCharsToWrite == 0)); // if (Console->PauseFlags & (PAUSED_FROM_KEYBOARD | PAUSED_FROM_SCROLLBAR | PAUSED_FROM_SELECTION)) if (Console->PauseFlags && Console->UnpauseEvent != NULL) @@ -909,8 +908,7 @@ ConDrvReadConsoleOutputString(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == Buffer->Header.Console); - ASSERT( (StringBuffer != NULL && NumCodesToRead > 0) || - (StringBuffer == NULL && NumCodesToRead == 0) ); + ASSERT((StringBuffer != NULL) || (StringBuffer == NULL && NumCodesToRead == 0)); switch (CodeType) { @@ -1033,8 +1031,7 @@ ConDrvWriteConsoleOutputString(IN PCONSOLE Console, /* Validity checks */ ASSERT(Console == Buffer->Header.Console); - ASSERT( (StringBuffer != NULL && NumCodesToWrite > 0) || - (StringBuffer == NULL && NumCodesToWrite == 0) ); + ASSERT((StringBuffer != NULL) || (StringBuffer == NULL && NumCodesToWrite == 0)); switch (CodeType) { diff --git a/win32ss/user/winsrv/consrv/coninput.c b/win32ss/user/winsrv/consrv/coninput.c index 9671b50a808..dd3c5e00ce2 100644 --- a/win32ss/user/winsrv/consrv/coninput.c +++ b/win32ss/user/winsrv/consrv/coninput.c @@ -260,17 +260,39 @@ ReadInputBuffer(IN PGET_INPUT_INFO InputInfo, NTSTATUS Status; PCONSOLE_GETINPUT GetInputRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.GetInputRequest; PCONSOLE_INPUT_BUFFER InputBuffer = InputInfo->InputBuffer; + ULONG NumEventsRead; - // GetInputRequest->InputsRead = 0; + PINPUT_RECORD InputRecord; + /* + * For optimization purposes, Windows (and hence ReactOS, too, for + * compatibility reasons) uses a static buffer if no more than five + * input records are read. Otherwise a new buffer is used. + * The client-side expects that we know this behaviour. + */ + if (GetInputRequest->NumRecords <= sizeof(GetInputRequest->RecordStaticBuffer)/sizeof(INPUT_RECORD)) + { + /* + * Adjust the internal pointer, because its old value points to + * the static buffer in the original ApiMessage structure. + */ + // GetInputRequest->RecordBufPtr = GetInputRequest->RecordStaticBuffer; + InputRecord = GetInputRequest->RecordStaticBuffer; + } + else + { + InputRecord = GetInputRequest->RecordBufPtr; + } + + NumEventsRead = 0; Status = ConDrvGetConsoleInput(InputBuffer->Header.Console, InputBuffer, - (GetInputRequest->wFlags & CONSOLE_READ_KEEPEVENT) != 0, - (GetInputRequest->wFlags & CONSOLE_READ_CONTINUE ) == 0, + (GetInputRequest->Flags & CONSOLE_READ_KEEPEVENT) != 0, + (GetInputRequest->Flags & CONSOLE_READ_CONTINUE ) == 0, GetInputRequest->Unicode, - GetInputRequest->InputRecord, - GetInputRequest->Length, - &GetInputRequest->InputsRead); + InputRecord, + GetInputRequest->NumRecords, + &NumEventsRead); if (Status == STATUS_PENDING) { @@ -283,6 +305,7 @@ ReadInputBuffer(IN PGET_INPUT_INFO InputInfo, else { /* We read all what we wanted, we return the error code we were given */ + GetInputRequest->NumRecords = NumEventsRead; return Status; // return STATUS_SUCCESS; } @@ -345,22 +368,37 @@ CSR_API(SrvGetConsoleInput) DPRINT("SrvGetConsoleInput\n"); - if (GetInputRequest->wFlags & ~(CONSOLE_READ_KEEPEVENT | CONSOLE_READ_CONTINUE)) + if (GetInputRequest->Flags & ~(CONSOLE_READ_KEEPEVENT | CONSOLE_READ_CONTINUE)) return STATUS_INVALID_PARAMETER; - if (!CsrValidateMessageBuffer(ApiMessage, - (PVOID*)&GetInputRequest->InputRecord, - GetInputRequest->Length, - sizeof(INPUT_RECORD))) + /* + * For optimization purposes, Windows (and hence ReactOS, too, for + * compatibility reasons) uses a static buffer if no more than five + * input records are read. Otherwise a new buffer is used. + * The client-side expects that we know this behaviour. + */ + if (GetInputRequest->NumRecords <= sizeof(GetInputRequest->RecordStaticBuffer)/sizeof(INPUT_RECORD)) { - return STATUS_INVALID_PARAMETER; + /* + * Adjust the internal pointer, because its old value points to + * the static buffer in the original ApiMessage structure. + */ + // GetInputRequest->RecordBufPtr = &GetInputRequest->RecordStaticBuffer; + } + else + { + if (!CsrValidateMessageBuffer(ApiMessage, + (PVOID*)&GetInputRequest->RecordBufPtr, + GetInputRequest->NumRecords, + sizeof(INPUT_RECORD))) + { + return STATUS_INVALID_PARAMETER; + } } Status = ConSrvGetInputBufferAndHandleEntry(ProcessData, GetInputRequest->InputHandle, &InputBuffer, &HandleEntry, GENERIC_READ, TRUE); if (!NT_SUCCESS(Status)) return Status; - GetInputRequest->InputsRead = 0; - InputInfo.CallingThread = CsrGetClientThread(); InputInfo.HandleEntry = HandleEntry; InputInfo.InputBuffer = InputBuffer; @@ -389,14 +427,36 @@ CSR_API(SrvWriteConsoleInput) PCONSOLE_INPUT_BUFFER InputBuffer; ULONG NumEventsWritten; + PINPUT_RECORD InputRecord; + DPRINT("SrvWriteConsoleInput\n"); - if (!CsrValidateMessageBuffer(ApiMessage, - (PVOID*)&WriteInputRequest->InputRecord, - WriteInputRequest->Length, - sizeof(INPUT_RECORD))) + /* + * For optimization purposes, Windows (and hence ReactOS, too, for + * compatibility reasons) uses a static buffer if no more than five + * input records are written. Otherwise a new buffer is used. + * The client-side expects that we know this behaviour. + */ + if (WriteInputRequest->NumRecords <= sizeof(WriteInputRequest->RecordStaticBuffer)/sizeof(INPUT_RECORD)) { - return STATUS_INVALID_PARAMETER; + /* + * Adjust the internal pointer, because its old value points to + * the static buffer in the original ApiMessage structure. + */ + // WriteInputRequest->RecordBufPtr = WriteInputRequest->RecordStaticBuffer; + InputRecord = WriteInputRequest->RecordStaticBuffer; + } + else + { + if (!CsrValidateMessageBuffer(ApiMessage, + (PVOID*)&WriteInputRequest->RecordBufPtr, + WriteInputRequest->NumRecords, + sizeof(INPUT_RECORD))) + { + return STATUS_INVALID_PARAMETER; + } + + InputRecord = WriteInputRequest->RecordBufPtr; } Status = ConSrvGetInputBuffer(ConsoleGetPerProcessData(CsrGetClientThread()->Process), @@ -409,10 +469,10 @@ CSR_API(SrvWriteConsoleInput) InputBuffer, WriteInputRequest->Unicode, WriteInputRequest->AppendToEnd, - WriteInputRequest->InputRecord, - WriteInputRequest->Length, + InputRecord, + WriteInputRequest->NumRecords, &NumEventsWritten); - WriteInputRequest->Length = NumEventsWritten; + WriteInputRequest->NumRecords = NumEventsWritten; ConSrvReleaseInputBuffer(InputBuffer, TRUE); return Status; -- 2.17.1