On 9/18/22 10:11, Jinoh Kang wrote:
From: Jinoh Kang jinoh.kang.kr@gmail.com
Ensure IOSB Information (InternalHigh) is loaded after Status (Internal) to avoid reading stale value from the Information (InternalHigh) field.
dlls/kernelbase/file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
I don't think this should be a separate patch set as merge request 155, and arguably not a separate commit from it either.
diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index 5ba7e0be419..11dbcdeac4a 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -3133,7 +3133,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov
TRACE( "(%p %p %p %lu %d)\n", file, overlapped, result, timeout, alertable );
- status = overlapped->Internal;
- status = ReadAcquire( (volatile LONG *)&overlapped->Internal ); if (status == STATUS_PENDING) { if (!timeout)
In general when using barriers I think it's good practice to comment (on both sides) where the other half of the barrier is.
@@ -3150,7 +3150,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov return FALSE; }
status = overlapped->Internal;
/* Normally this does not require an atomic load, since the wait above* acts as a barrier as long as the caller supplied the correct handle* and nothing else interfered with signalling of I/O completion.* However, we do it anyway for consistency and extra safety in case* Windows takes the same precaution for misbehaving applications.*/status = ReadAcquire( (volatile LONG *)&overlapped->Internal ); if (status == STATUS_PENDING) status = STATUS_SUCCESS; }
I don't see how this can even make a difference?