Ros-diffs

ros-diffs@reactos.org

81390 discussions

[reactos] 12/12: [HALX86] Fix the "ASSERT(j < 32); " problem in HalpStoreAndClearIopm() encountered from time to time. CORE-11921 CORE-13715
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=5887b170052db6af1ee86… commit 5887b170052db6af1ee86dc68f053d0c79d15028 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 22:58:12 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:55 2019 +0100 [HALX86] Fix the "ASSERT(j < 32);" problem in HalpStoreAndClearIopm() encountered from time to time. CORE-11921 CORE-13715 (Regression introduced by commit 2e1b82cf, r44841.) In some cases the number of valid (!= 0xFFFF) entries in the IOPM can be larger than the assumed size (32) of the entries cache. The maximum possible number of entries is equal to IOPM_SIZE / sizeof(USHORT). A way to reproduce the problem is as follows: start ReactOS in debugging mode using '/DEBUG /DEBUGPORT=SCREEN' . Then manage to break into the debugger exactly during the execution of Ke386CallBios() triggered by display initialization (for example in my case, while a video driver was being initialized via the HwInitialize() call done by videoport inside IntVideoPortDispatchOpen() ). When this happens, a "concurrent" execution between Ke386CallBios() and the HAL function HalpStoreAndClearIopm() takes place. This is due to the fact that when entering the debugger in SCREEN mode, the following call-chain holds: InbvResetDisplay() -> VidResetDisplay() -> HalResetDisplay() -> HalpBiosDisplayReset() -> HalpSetupRealModeIoPermissionsAndTask() -> HalpStoreAndClearIopm(). However, the code of Ke386CallBios() has reset the IOPM contents with all zeroes instead of 0xFFFF, and this triggers the caching of all the entries of the IOPM by HalpStoreAndClearIopm(), whose number is greater than the wrongly assumed number of '32'. As Thomas explained to me, "Windows supports [the maximum number of IOPM entries], it just makes a full copy of the table instead of this indexed partial copy." And I agree that this overengineered so-called "optimization" committed in 2e1b82cf contributed in introducing an unnecessary bug and making the code less clear. Also it makes the IOPM cache larger than the necessary size by twice as much. Finally, Ke386CallBios() also caches IOPM entries before doing a 16-bit call, and obviously uses the more straightforward way of doing a direct copy of the IOPM table (using RtlCopyMemory()). I wonder what kind of "optimization" this tried to achieve, knowing that we are not doing like thousands of 32->16bit BIOS interrupt calls per second in ReactOS... --- hal/halx86/generic/bios.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hal/halx86/generic/bios.c b/hal/halx86/generic/bios.c index cc8e57d1721..a922084ca8d 100644 --- a/hal/halx86/generic/bios.c +++ b/hal/halx86/generic/bios.c @@ -41,7 +41,7 @@ USHORT HalpSavedTss; // USHORT HalpSavedIopmBase; PUSHORT HalpSavedIoMap; -USHORT HalpSavedIoMapData[32][2]; +USHORT HalpSavedIoMapData[IOPM_SIZE / sizeof(USHORT)][2]; ULONG HalpSavedIoMapEntries; /* Where the protected mode stack is */ @@ -400,7 +400,7 @@ HalpStoreAndClearIopm(VOID) // // Save it // - ASSERT(j < 32); + ASSERT(j < IOPM_SIZE / sizeof(USHORT)); HalpSavedIoMapData[j][0] = i; HalpSavedIoMapData[j][1] = *Entry; j++;

1 0

[reactos] 11/12: [HALX86] Trailing whitespace fixes only.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=00cb464d9d1129b8d6de2… commit 00cb464d9d1129b8d6de253b055092eae837819d Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 22:05:37 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:55 2019 +0100 [HALX86] Trailing whitespace fixes only. --- hal/halx86/generic/bios.c | 70 +++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/hal/halx86/generic/bios.c b/hal/halx86/generic/bios.c index 329f0d227f7..cc8e57d1721 100644 --- a/hal/halx86/generic/bios.c +++ b/hal/halx86/generic/bios.c @@ -81,35 +81,35 @@ HalpPushInt(IN PHAL_BIOS_FRAME BiosFrame, { PUSHORT Stack; ULONG Eip; - + /* Calculate stack address (SP) */ Stack = (PUSHORT)(BiosFrame->SsBase + (BiosFrame->Esp & 0xFFFF)); - + /* Push EFlags */ Stack--; *Stack = BiosFrame->EFlags & 0xFFFF; - + /* Push CS */ Stack--; *Stack = BiosFrame->SegCs & 0xFFFF; - + /* Push IP */ Stack--; *Stack = BiosFrame->Eip & 0xFFFF; - + /* Compute new CS:IP from the IVT address for this interrupt entry */ Eip = *(PULONG)(Interrupt * 4); BiosFrame->Eip = Eip & 0xFFFF; BiosFrame->SegCs = Eip >> 16; - + /* Update stack address */ BiosFrame->Esp = (ULONG_PTR)Stack & 0xFFFF; - + /* Update CS to linear */ BiosFrame->CsBase = BiosFrame->SegCs << 4; BiosFrame->CsLimit = 0xFFFF; BiosFrame->CsFlags = 0; - + /* We're done */ return TRUE; } @@ -120,19 +120,19 @@ HalpOpcodeINTnn(IN PHAL_BIOS_FRAME BiosFrame) { UCHAR Interrupt; PKTRAP_FRAME TrapFrame; - + /* Convert SS to linear */ BiosFrame->SsBase = BiosFrame->SegSs << 4; BiosFrame->SsLimit = 0xFFFF; BiosFrame->SsFlags = 0; - + /* Increase EIP and validate */ BiosFrame->Eip++; if (BiosFrame->Eip > BiosFrame->CsLimit) return FALSE; - + /* Read interrupt number */ Interrupt = *(PUCHAR)(BiosFrame->CsBase + BiosFrame->Eip); - + /* Increase EIP and push the interrupt */ BiosFrame->Eip++; if (HalpPushInt(BiosFrame, Interrupt)) @@ -143,11 +143,11 @@ HalpOpcodeINTnn(IN PHAL_BIOS_FRAME BiosFrame) TrapFrame->HardwareEsp = BiosFrame->Esp; TrapFrame->SegCs = BiosFrame->SegCs; TrapFrame->EFlags = BiosFrame->EFlags; - + /* Success */ return TRUE; } - + /* Failure */ return FALSE; } @@ -158,7 +158,7 @@ HalpDispatchV86Opcode(IN PKTRAP_FRAME TrapFrame) { UCHAR Instruction; HAL_BIOS_FRAME BiosFrame; - + /* Fill out the BIOS frame */ BiosFrame.TrapFrame = TrapFrame; BiosFrame.SegSs = TrapFrame->HardwareSegSs; @@ -167,15 +167,15 @@ HalpDispatchV86Opcode(IN PKTRAP_FRAME TrapFrame) BiosFrame.SegCs = TrapFrame->SegCs; BiosFrame.Eip = TrapFrame->Eip; BiosFrame.Prefix = 0; - + /* Convert CS to linear */ BiosFrame.CsBase = BiosFrame.SegCs << 4; BiosFrame.CsLimit = 0xFFFF; BiosFrame.CsFlags = 0; - + /* Validate IP */ if (BiosFrame.Eip > BiosFrame.CsLimit) return FALSE; - + /* Read IP */ Instruction = *(PUCHAR)(BiosFrame.CsBase + BiosFrame.Eip); if (Instruction != 0xCD) @@ -184,17 +184,17 @@ HalpDispatchV86Opcode(IN PKTRAP_FRAME TrapFrame) HalpOpcodeInvalid(&BiosFrame); return FALSE; } - + /* Handle the interrupt */ if (HalpOpcodeINTnn(&BiosFrame)) { /* Update EIP */ TrapFrame->Eip = BiosFrame.Eip; - + /* We're done */ return TRUE; } - + /* Failure */ return FALSE; } @@ -209,7 +209,7 @@ HalpTrap0DHandler(IN PKTRAP_FRAME TrapFrame) { /* Enter the trap */ KiEnterTrap(TrapFrame); - + /* Check if this is a V86 trap */ if (TrapFrame->EFlags & EFLAGS_V86_MASK) { @@ -217,7 +217,7 @@ HalpTrap0DHandler(IN PKTRAP_FRAME TrapFrame) HalpDispatchV86Opcode(TrapFrame); KiEoiHelper(TrapFrame); } - + /* Strange, it isn't! This can happen during NMI */ DPRINT1("HAL: Trap0D while not in V86 mode\n"); KiDumpTrapFrame(TrapFrame); @@ -235,7 +235,7 @@ HalpTrap06(VOID) Ke386SetDs(KGDT_R3_DATA | RPL_MASK); Ke386SetFs(KGDT_R0_PCR); - /* Restore the stack */ + /* Restore the stack */ KeGetPcr()->TSS->Esp0 = HalpSavedEsp0; /* Return back to where we left */ @@ -252,24 +252,24 @@ HalpBiosCall(VOID) /* Must be volatile so it doesn't get optimized away! */ volatile KTRAP_FRAME V86TrapFrame; ULONG_PTR StackOffset, CodeOffset; - + /* Save the context, check for return */ if (_setjmp(HalpSavedContext)) { /* Returned from v86 */ return; } - + /* Kill alignment faults */ __writecr0(__readcr0() & ~CR0_AM); - + /* Set new stack address */ KeGetPcr()->TSS->Esp0 = (ULONG)&V86TrapFrame - 0x20 - sizeof(FX_SAVE_AREA); /* Compute segmented IP and SP offsets */ StackOffset = (ULONG_PTR)&HalpRealModeEnd - 4 - (ULONG_PTR)HalpRealModeStart; CodeOffset = (ULONG_PTR)HalpRealModeStart & 0xFFF; - + /* Now build the V86 trap frame */ V86TrapFrame.V86Es = 0; V86TrapFrame.V86Ds = 0; @@ -280,7 +280,7 @@ HalpBiosCall(VOID) V86TrapFrame.EFlags = __readeflags() | EFLAGS_V86_MASK | EFLAGS_IOPL; V86TrapFrame.SegCs = 0x2000; V86TrapFrame.Eip = CodeOffset; - + /* Exit to V86 mode */ HalpExitToV86((PKTRAP_FRAME)&V86TrapFrame); } @@ -339,7 +339,7 @@ HalpBorrowTss(VOID) TssGdt->HighWord.Bits.Type = I386_TSS; TssGdt->HighWord.Bits.Pres = 1; TssGdt->HighWord.Bits.Dpl = 0; - + // // Load new TSS and return old one // @@ -353,7 +353,7 @@ HalpReturnTss(VOID) { PKGDTENTRY TssGdt; PKTSS TssBase; - + // // Get the original TSS // @@ -497,7 +497,7 @@ HalpMapRealModeMemory(VOID) // Map the physical address into our real-mode region // Pte->PageFrameNumber = V86Pte->PageFrameNumber; - + // // Keep going until we've reached the end of our region // @@ -557,7 +557,7 @@ HalpSetupRealModeIoPermissionsAndTask(VOID) // // Save our stack pointer // - HalpSavedEsp0 = KeGetPcr()->TSS->Esp0; + HalpSavedEsp0 = KeGetPcr()->TSS->Esp0; } VOID @@ -661,7 +661,7 @@ HalpBiosDisplayReset(VOID) // HalpMapRealModeMemory(); - // + // // On P5, the first 7 entries of the IDT are write protected to work around // the cmpxchg8b lock errata. Unprotect them here so we can set our custom // invalid op-code handler. @@ -699,7 +699,7 @@ HalpBiosDisplayReset(VOID) // Restore TSS and IOPM // HalpRestoreIoPermissionsAndTask(); - + // // Restore low memory mapping //

1 0

[reactos] 10/12: [NTOS:KE] Explicitly use IOPM_SIZE for initializing the IO map, that is indeed also equal to PAGE_SIZE * 2 but is self-documented.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=03a9ccce651b2803f5866… commit 03a9ccce651b2803f5866ba3b8b2a4746a9c5ccf Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 22:56:55 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:54 2019 +0100 [NTOS:KE] Explicitly use IOPM_SIZE for initializing the IO map, that is indeed also equal to PAGE_SIZE * 2 but is self-documented. --- ntoskrnl/ke/amd64/kiinit.c | 6 +++--- ntoskrnl/ke/i386/kiinit.c | 4 ++-- ntoskrnl/ke/i386/v86vdm.c | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/ke/amd64/kiinit.c b/ntoskrnl/ke/amd64/kiinit.c index adeac8fdf5a..74623c1a2ca 100644 --- a/ntoskrnl/ke/amd64/kiinit.c +++ b/ntoskrnl/ke/amd64/kiinit.c @@ -75,12 +75,12 @@ KiInitMachineDependent(VOID) // /* Allocate the IOPM save area */ // Ki386IopmSaveArea = ExAllocatePoolWithTag(PagedPool, -// PAGE_SIZE * 2, -// TAG('K', 'e', ' ', ' ')); +// IOPM_SIZE, +// ' eK'); // if (!Ki386IopmSaveArea) // { // /* Bugcheck. We need this for V86/VDM support. */ -// KeBugCheckEx(NO_PAGES_AVAILABLE, 2, PAGE_SIZE * 2, 0, 0); +// KeBugCheckEx(NO_PAGES_AVAILABLE, 2, IOPM_SIZE, 0, 0); // } } diff --git a/ntoskrnl/ke/i386/kiinit.c b/ntoskrnl/ke/i386/kiinit.c index 230ee5235fb..7fc38067730 100644 --- a/ntoskrnl/ke/i386/kiinit.c +++ b/ntoskrnl/ke/i386/kiinit.c @@ -590,12 +590,12 @@ KiInitializeKernel(IN PKPROCESS InitProcess, /* Allocate the IOPM save area */ Ki386IopmSaveArea = ExAllocatePoolWithTag(PagedPool, - PAGE_SIZE * 2, + IOPM_SIZE, ' eK'); if (!Ki386IopmSaveArea) { /* Bugcheck. We need this for V86/VDM support. */ - KeBugCheckEx(NO_PAGES_AVAILABLE, 2, PAGE_SIZE * 2, 0, 0); + KeBugCheckEx(NO_PAGES_AVAILABLE, 2, IOPM_SIZE, 0, 0); } } diff --git a/ntoskrnl/ke/i386/v86vdm.c b/ntoskrnl/ke/i386/v86vdm.c index d1c030acc0c..e629f8bb44e 100644 --- a/ntoskrnl/ke/i386/v86vdm.c +++ b/ntoskrnl/ke/i386/v86vdm.c @@ -668,8 +668,8 @@ Ke386CallBios(IN ULONG Int, /* Make sure there's space for two IOPMs, then copy & clear the current */ ASSERT(((PKIPCR)KeGetPcr())->GDT[KGDT_TSS / 8].LimitLow >= (0x2000 + IOPM_OFFSET - 1)); - RtlCopyMemory(Ki386IopmSaveArea, &Tss->IoMaps[0].IoMap, PAGE_SIZE * 2); - RtlZeroMemory(&Tss->IoMaps[0].IoMap, PAGE_SIZE * 2); + RtlCopyMemory(Ki386IopmSaveArea, &Tss->IoMaps[0].IoMap, IOPM_SIZE); + RtlZeroMemory(&Tss->IoMaps[0].IoMap, IOPM_SIZE); /* Save the old offset and base, and set the new ones */ OldOffset = Process->IopmOffset; @@ -681,7 +681,7 @@ Ke386CallBios(IN ULONG Int, Ki386SetupAndExitToV86Mode(VdmTeb); /* Restore IOPM */ - RtlCopyMemory(&Tss->IoMaps[0].IoMap, Ki386IopmSaveArea, PAGE_SIZE * 2); + RtlCopyMemory(&Tss->IoMaps[0].IoMap, Ki386IopmSaveArea, IOPM_SIZE); Process->IopmOffset = OldOffset; Tss->IoMapBase = OldBase;

1 0

[reactos] 09/12: [NTOS:KE] Minor formatting fixes.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b2ca7d3ed989558b3bb81… commit b2ca7d3ed989558b3bb81a5d6f5a61d8d94f665f Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 22:08:35 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:54 2019 +0100 [NTOS:KE] Minor formatting fixes. --- ntoskrnl/ke/amd64/kiinit.c | 2 +- ntoskrnl/ke/i386/kiinit.c | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/ke/amd64/kiinit.c b/ntoskrnl/ke/amd64/kiinit.c index 3afeaf88582..adeac8fdf5a 100644 --- a/ntoskrnl/ke/amd64/kiinit.c +++ b/ntoskrnl/ke/amd64/kiinit.c @@ -73,7 +73,7 @@ KiInitMachineDependent(VOID) DPRINT("PAT support detected but not yet taken advantage of!\n"); } - /* Allocate the IOPM save area. */ +// /* Allocate the IOPM save area */ // Ki386IopmSaveArea = ExAllocatePoolWithTag(PagedPool, // PAGE_SIZE * 2, // TAG('K', 'e', ' ', ' ')); diff --git a/ntoskrnl/ke/i386/kiinit.c b/ntoskrnl/ke/i386/kiinit.c index 6d437a05815..230ee5235fb 100644 --- a/ntoskrnl/ke/i386/kiinit.c +++ b/ntoskrnl/ke/i386/kiinit.c @@ -588,7 +588,7 @@ KiInitializeKernel(IN PKPROCESS InitProcess, if (!DpcStack) KeBugCheckEx(NO_PAGES_AVAILABLE, 1, 0, 0, 0); Prcb->DpcStack = DpcStack; - /* Allocate the IOPM save area. */ + /* Allocate the IOPM save area */ Ki386IopmSaveArea = ExAllocatePoolWithTag(PagedPool, PAGE_SIZE * 2, ' eK'); @@ -600,8 +600,7 @@ KiInitializeKernel(IN PKPROCESS InitProcess, } /* Raise to Dispatch */ - KeRaiseIrql(DISPATCH_LEVEL, - &DummyIrql); + KeRaiseIrql(DISPATCH_LEVEL, &DummyIrql); /* Set the Idle Priority to 0. This will jump into Phase 1 */ KeSetPriorityThread(InitThread, 0); @@ -612,8 +611,7 @@ KiInitializeKernel(IN PKPROCESS InitProcess, KiReleasePrcbLock(Prcb); /* Raise back to HIGH_LEVEL and clear the PRCB for the loader block */ - KeRaiseIrql(HIGH_LEVEL, - &DummyIrql); + KeRaiseIrql(HIGH_LEVEL, &DummyIrql); LoaderBlock->Prcb = 0; } @@ -835,8 +833,7 @@ AppCpuInit: } /* Raise to HIGH_LEVEL */ - KeRaiseIrql(HIGH_LEVEL, - &DummyIrql); + KeRaiseIrql(HIGH_LEVEL, &DummyIrql); /* Switch to new kernel stack and start kernel bootstrapping */ KiSwitchToBootStack(InitialStack & ~3);

1 0

[reactos] 08/12: [NTOS:INBV] InbvEnableBootDriver() is an export, thus can be called at any time, therefore it must NOT be an INIT_FUNCTION.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=592f01a5941c595c0707a… commit 592f01a5941c595c0707a2a2c380cba35e82ba80 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 21:59:30 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:54 2019 +0100 [NTOS:INBV] InbvEnableBootDriver() is an export, thus can be called at any time, therefore it must NOT be an INIT_FUNCTION. --- ntoskrnl/inbv/inbv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ntoskrnl/inbv/inbv.c b/ntoskrnl/inbv/inbv.c index f5cd5652493..0681b99e4bd 100644 --- a/ntoskrnl/inbv/inbv.c +++ b/ntoskrnl/inbv/inbv.c @@ -495,7 +495,6 @@ InbvReleaseLock(VOID) if (InbvOldIrql <= DISPATCH_LEVEL) KeLowerIrql(OldIrql); } -INIT_FUNCTION VOID NTAPI InbvEnableBootDriver(IN BOOLEAN Enable)

1 0

[reactos] 07/12: [VIDEOPRT] Implement VideoPortReadStateEvent().
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=66909fa9d550ce11572b3… commit 66909fa9d550ce11572b348b65579da328a5a678 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 21:26:26 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:53 2019 +0100 [VIDEOPRT] Implement VideoPortReadStateEvent(). --- win32ss/drivers/videoprt/event.c | 13 +++++++++++++ win32ss/drivers/videoprt/stubs.c | 10 ---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/win32ss/drivers/videoprt/event.c b/win32ss/drivers/videoprt/event.c index 77f48817676..85e4d8b8c60 100644 --- a/win32ss/drivers/videoprt/event.c +++ b/win32ss/drivers/videoprt/event.c @@ -110,6 +110,19 @@ VideoPortClearEvent( KeClearEvent(Event->pKEvent); } +/* + * @implemented + */ +LONG +NTAPI +VideoPortReadStateEvent( + _In_ PVOID HwDeviceExtension, + _In_ PEVENT Event) +{ + ASSERT(Event); + return KeReadStateEvent(Event->pKEvent); +} + /* * @implemented */ diff --git a/win32ss/drivers/videoprt/stubs.c b/win32ss/drivers/videoprt/stubs.c index 5aa47a49dc2..ff1ba6c1ac9 100644 --- a/win32ss/drivers/videoprt/stubs.c +++ b/win32ss/drivers/videoprt/stubs.c @@ -63,16 +63,6 @@ VideoPortGetMdl( return 0; } -LONG -NTAPI -VideoPortReadStateEvent( - IN PVOID HwDeviceExtension, - IN PEVENT pEvent) -{ - UNIMPLEMENTED; - return 0; -} - VOID NTAPI VideoPortSetBytesUsed(

1 0

[reactos] 06/12: [VIDEOPRT] Use ExFreePoolWithTag() + Add some ASSERTs.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=bf74f101c258665268a42… commit bf74f101c258665268a428e9a788bc8e68c17b3c Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Mon Nov 25 00:32:28 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:53 2019 +0100 [VIDEOPRT] Use ExFreePoolWithTag() + Add some ASSERTs. --- win32ss/drivers/videoprt/agp.c | 10 +++++----- win32ss/drivers/videoprt/event.c | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/win32ss/drivers/videoprt/agp.c b/win32ss/drivers/videoprt/agp.c index e149a201375..2ad191e088b 100644 --- a/win32ss/drivers/videoprt/agp.c +++ b/win32ss/drivers/videoprt/agp.c @@ -176,7 +176,7 @@ IntAgpReleasePhysical( } /* Free resources */ - ExFreePool(AgpMapping); + ExFreePoolWithTag(AgpMapping, TAG_VIDEO_PORT); } PHYSICAL_ADDRESS NTAPI @@ -230,7 +230,7 @@ IntAgpReservePhysical( &AgpMapping->PhysicalAddress); if (!NT_SUCCESS(Status) || AgpMapping->MapHandle == NULL) { - ExFreePool(AgpMapping); + ExFreePoolWithTag(AgpMapping, TAG_VIDEO_PORT); WARN_(VIDEOPRT, "Warning: AgpBusInterface->ReserveMemory failed (Status = 0x%x)\n", Status); return ZeroAddress; @@ -412,7 +412,7 @@ IntAgpReleaseVirtual( } /* Free resources */ - ExFreePool(VirtualMapping); + ExFreePoolWithTag(VirtualMapping, TAG_VIDEO_PORT); } PVOID NTAPI @@ -447,7 +447,7 @@ IntAgpReserveVirtual( if (ProcessHandle == NULL) { /* FIXME: What to do in this case? */ - ExFreePool(VirtualMapping); + ExFreePoolWithTag(VirtualMapping, TAG_VIDEO_PORT); return NULL; } else /* ProcessHandle != NULL */ @@ -459,7 +459,7 @@ IntAgpReserveVirtual( MEM_RESERVE, PAGE_NOACCESS); if (!NT_SUCCESS(Status)) { - ExFreePool(VirtualMapping); + ExFreePoolWithTag(VirtualMapping, TAG_VIDEO_PORT); WARN_(VIDEOPRT, "ZwAllocateVirtualMemory() failed: Status = 0x%x\n", Status); return NULL; } diff --git a/win32ss/drivers/videoprt/event.c b/win32ss/drivers/videoprt/event.c index 16153397f8a..77f48817676 100644 --- a/win32ss/drivers/videoprt/event.c +++ b/win32ss/drivers/videoprt/event.c @@ -78,7 +78,7 @@ VideoPortDeleteEvent( if (!Event->pKEvent) return ERROR_INVALID_PARAMETER; /* Free storage */ - ExFreePool(Event); + ExFreePoolWithTag(Event, TAG_VIDEO_PORT); /* Indicate success */ return NO_ERROR; @@ -93,6 +93,7 @@ VideoPortSetEvent( _In_ PVOID HwDeviceExtension, _In_ PEVENT Event) { + ASSERT(Event); return KeSetEvent(Event->pKEvent, IO_NO_INCREMENT, FALSE); } @@ -105,6 +106,7 @@ VideoPortClearEvent( _In_ PVOID HwDeviceExtension, _In_ PEVENT Event) { + ASSERT(Event); KeClearEvent(Event->pKEvent); }

1 0

[reactos] 05/12: [VIDEOPRT] Code formatting only.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=e930b868a00c682fd5540… commit e930b868a00c682fd554088b0453489179f6293f Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Mon Nov 25 00:29:36 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:52 2019 +0100 [VIDEOPRT] Code formatting only. --- win32ss/drivers/videoprt/event.c | 43 ++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/win32ss/drivers/videoprt/event.c b/win32ss/drivers/videoprt/event.c index c4cf2d272f5..16153397f8a 100644 --- a/win32ss/drivers/videoprt/event.c +++ b/win32ss/drivers/videoprt/event.c @@ -21,10 +21,11 @@ */ VP_STATUS NTAPI -VideoPortCreateEvent(IN PVOID HwDeviceExtension, - IN ULONG EventFlag, - IN PVOID Unused, - OUT PEVENT *Event) +VideoPortCreateEvent( + _In_ PVOID HwDeviceExtension, + _In_ ULONG EventFlag, + _In_ PVOID Unused, + _Out_ PEVENT* pEvent) { VP_STATUS Result = NO_ERROR; PVIDEO_PORT_EVENT EngEvent; @@ -40,7 +41,7 @@ VideoPortCreateEvent(IN PVOID HwDeviceExtension, /* Set KEVENT pointer */ EngEvent->pKEvent = EngEvent + 1; - + /* Initialize the kernel event */ KeInitializeEvent(EngEvent->pKEvent, (EventFlag & EVENT_TYPE_MASK) ? @@ -48,13 +49,13 @@ VideoPortCreateEvent(IN PVOID HwDeviceExtension, EventFlag & INITIAL_EVENT_STATE_MASK); /* Pass pointer to our structure to the caller */ - *Event = (PEVENT)EngEvent; + *pEvent = (PEVENT)EngEvent; DPRINT("VideoPortCreateEvent() created %p\n", EngEvent); } else { /* Out of memory */ - DPRINT("VideoPortCreateEvent() failed\n"); + DPRINT("VideoPortCreateEvent() failed\n"); Result = ERROR_NOT_ENOUGH_MEMORY; } @@ -67,8 +68,9 @@ VideoPortCreateEvent(IN PVOID HwDeviceExtension, */ VP_STATUS NTAPI -VideoPortDeleteEvent(IN PVOID HwDeviceExtension, - IN PEVENT Event) +VideoPortDeleteEvent( + _In_ PVOID HwDeviceExtension, + _In_ PEVENT Event) { /* Handle error cases */ if (!Event) return ERROR_INVALID_PARAMETER; @@ -87,8 +89,9 @@ VideoPortDeleteEvent(IN PVOID HwDeviceExtension, */ LONG NTAPI -VideoPortSetEvent(IN PVOID HwDeviceExtension, - IN PEVENT Event) +VideoPortSetEvent( + _In_ PVOID HwDeviceExtension, + _In_ PEVENT Event) { return KeSetEvent(Event->pKEvent, IO_NO_INCREMENT, FALSE); } @@ -98,8 +101,9 @@ VideoPortSetEvent(IN PVOID HwDeviceExtension, */ VOID NTAPI -VideoPortClearEvent(IN PVOID HwDeviceExtension, - IN PEVENT Event) +VideoPortClearEvent( + _In_ PVOID HwDeviceExtension, + _In_ PEVENT Event) { KeClearEvent(Event->pKEvent); } @@ -109,18 +113,19 @@ VideoPortClearEvent(IN PVOID HwDeviceExtension, */ VP_STATUS NTAPI -VideoPortWaitForSingleObject(IN PVOID HwDeviceExtension, - IN PVOID Event, - IN PLARGE_INTEGER Timeout OPTIONAL) +VideoPortWaitForSingleObject( + _In_ PVOID HwDeviceExtension, + _In_ PVOID Event, + _In_opt_ PLARGE_INTEGER Timeout) { PVIDEO_PORT_EVENT EngEvent = Event; NTSTATUS Status; - + /* Handle error cases */ if (!EngEvent) return ERROR_INVALID_PARAMETER; if (!EngEvent->pKEvent) return ERROR_INVALID_PARAMETER; if (EngEvent->fFlags & ENG_EVENT_USERMAPPED) return ERROR_INVALID_PARAMETER; - + /* Do the actual wait */ Status = KeWaitForSingleObject(EngEvent->pKEvent, Executive, @@ -137,7 +142,7 @@ VideoPortWaitForSingleObject(IN PVOID HwDeviceExtension, /* All other success codes are Win32 success */ return NO_ERROR; } - + /* Otherwise, return default Win32 failure */ return ERROR_INVALID_PARAMETER; }

1 0

[reactos] 04/12: [WIN32K] Add some validity checks in PDEVOBJ_vReference() and use it instead of explicitly manipulating ppdev->cPdevRefs.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=148e137b421587abfa0ca… commit 148e137b421587abfa0ca987cb12a51d267a62d7 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 21:22:21 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:51 2019 +0100 [WIN32K] Add some validity checks in PDEVOBJ_vReference() and use it instead of explicitly manipulating ppdev->cPdevRefs. --- win32ss/gdi/eng/pdevobj.c | 2 +- win32ss/gdi/eng/pdevobj.h | 25 ++++++++++++++++++------- win32ss/gdi/ntgdi/dclife.c | 2 +- win32ss/gdi/ntgdi/dcstate.c | 2 +- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/win32ss/gdi/eng/pdevobj.c b/win32ss/gdi/eng/pdevobj.c index 450d37f6785..4147f0fa749 100644 --- a/win32ss/gdi/eng/pdevobj.c +++ b/win32ss/gdi/eng/pdevobj.c @@ -630,7 +630,7 @@ EngpGetPDEV( if (ppdev) { /* Yes, reference the PDEV */ - InterlockedIncrement(&ppdev->cPdevRefs); + PDEVOBJ_vReference(ppdev); } else { diff --git a/win32ss/gdi/eng/pdevobj.h b/win32ss/gdi/eng/pdevobj.h index 9b986acde13..a232a2e057c 100644 --- a/win32ss/gdi/eng/pdevobj.h +++ b/win32ss/gdi/eng/pdevobj.h @@ -162,6 +162,24 @@ NTAPI EngpGetPDEV( _In_opt_ PUNICODE_STRING pustrDevice); +FORCEINLINE +VOID +PDEVOBJ_vReference( + _In_ PPDEVOBJ ppdev) +{ + ASSERT(ppdev); + + /* Fail if the PDEV is being destroyed */ + if (ppdev->cPdevRefs == 0) + { + ASSERT(FALSE); + return; + } + ASSERT(ppdev->cPdevRefs > 0); + + InterlockedIncrement(&ppdev->cPdevRefs); +} + VOID NTAPI PDEVOBJ_vRelease( @@ -201,11 +219,4 @@ PDEVOBJ_pdmMatchDevMode( PPDEVOBJ ppdev, PDEVMODEW pdm); -FORCEINLINE -VOID -PDEVOBJ_vReference(PPDEVOBJ ppdev) -{ - InterlockedIncrement(&ppdev->cPdevRefs); -} - #endif /* !__WIN32K_PDEVOBJ_H */ diff --git a/win32ss/gdi/ntgdi/dclife.c b/win32ss/gdi/ntgdi/dclife.c index 99d872a9c2f..52236c037c3 100644 --- a/win32ss/gdi/ntgdi/dclife.c +++ b/win32ss/gdi/ntgdi/dclife.c @@ -836,7 +836,7 @@ GreCreateCompatibleDC(HDC hdc, BOOL bAltDc) /* Get the pdev from the DC */ ppdev = pdc->ppdev; - InterlockedIncrement(&ppdev->cPdevRefs); + PDEVOBJ_vReference(ppdev); /* Unlock the source DC */ DC_UnlockDc(pdc); diff --git a/win32ss/gdi/ntgdi/dcstate.c b/win32ss/gdi/ntgdi/dcstate.c index ccf7c134e7f..314eaa6c380 100644 --- a/win32ss/gdi/ntgdi/dcstate.c +++ b/win32ss/gdi/ntgdi/dcstate.c @@ -282,7 +282,7 @@ NtGdiSaveDC( } hdcSave = pdcSave->BaseObject.hHmgr; - InterlockedIncrement(&pdc->ppdev->cPdevRefs); + PDEVOBJ_vReference(pdc->ppdev); DC_vInitDc(pdcSave, DCTYPE_MEMORY, pdc->ppdev); /* Handle references here correctly */

1 0

[reactos] 03/12: [WIN32K] Use RtlString* routines.
by Hermès Bélusca-Maïto 24 Nov '19

24 Nov '19

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=acf689a97365ae9cebdd6… commit acf689a97365ae9cebdd653f3edcfab4a13f8d65 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun Nov 24 21:51:22 2019 +0100 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon Nov 25 00:41:49 2019 +0100 [WIN32K] Use RtlString* routines. --- win32ss/gdi/eng/device.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/win32ss/gdi/eng/device.c b/win32ss/gdi/eng/device.c index e5f30aa0bba..7691aad92f5 100644 --- a/win32ss/gdi/eng/device.c +++ b/win32ss/gdi/eng/device.c @@ -83,8 +83,7 @@ EngpPopulateDeviceModeList( /* Some drivers like the VBox driver don't fill the dmDeviceName with the name of the display driver. So fix that here. */ - wcsncpy(pdm->dmDeviceName, pwsz, CCHDEVICENAME); - pdm->dmDeviceName[CCHDEVICENAME - 1] = 0; + RtlStringCbCopyW(pdm->dmDeviceName, sizeof(pdm->dmDeviceName), pwsz); } // FIXME: release the driver again until it's used? @@ -205,14 +204,17 @@ EngpRegisterGraphicsDevice( pGraphicsDevice->DeviceObject = pDeviceObject; pGraphicsDevice->FileObject = pFileObject; - /* Copy device name */ + /* Copy the device name */ RtlStringCbCopyNW(pGraphicsDevice->szNtDeviceName, - sizeof(pGraphicsDevice->szNtDeviceName), - pustrDeviceName->Buffer, - pustrDeviceName->Length); - - /* Create a win device name (FIXME: virtual devices!) */ - swprintf(pGraphicsDevice->szWinDeviceName, L"\\\\.\\DISPLAY%d", (int)giDevNum); + sizeof(pGraphicsDevice->szNtDeviceName), + pustrDeviceName->Buffer, + pustrDeviceName->Length); + + /* Create a Win32 device name (FIXME: virtual devices!) */ + RtlStringCbPrintfW(pGraphicsDevice->szWinDeviceName, + sizeof(pGraphicsDevice->szWinDeviceName), + L"\\\\.\\DISPLAY%d", + (int)giDevNum); /* Allocate a buffer for the strings */ cj = pustrDiplayDrivers->Length + pustrDescription->Length + sizeof(WCHAR);

1 0

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs