[PATCH v3 1/3] ntdll: Report NtReadFileScatter results asynchronously

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>
---

v3: Always SetLastError before checking GLE return.

 dlls/kernel32/tests/file.c | 58 +++++++++++++++++++++++++++++++++++++++-------
 dlls/ntdll/file.c          |  2 +-
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 0b8cef23f8..a8d229c887 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -4353,7 +4353,8 @@ static void test_WriteFileGather(void)
     FILE_SEGMENT_ELEMENT fse[2];
     OVERLAPPED ovl, *povl = NULL;
     SYSTEM_INFO si;
-    LPVOID buf = NULL;
+    LPVOID wbuf = NULL, rbuf1;
+    BOOL br;
 
     ret = GetTempPathA( MAX_PATH, temp_path );
     ok( ret != 0, "GetTempPathA error %d\n", GetLastError() );
@@ -4371,12 +4372,17 @@ static void test_WriteFileGather(void)
     ok( hiocp2 != 0, "CreateIoCompletionPort failed err %u\n", GetLastError() );
 
     GetSystemInfo( &si );
-    buf = VirtualAlloc( NULL, si.dwPageSize, MEM_COMMIT, PAGE_READWRITE );
-    ok( buf != NULL, "VirtualAlloc failed err %u\n", GetLastError() );
+    wbuf = VirtualAlloc( NULL, si.dwPageSize, MEM_COMMIT, PAGE_READWRITE );
+    ok( wbuf != NULL, "VirtualAlloc failed err %u\n", GetLastError() );
+
+    rbuf1 = VirtualAlloc( NULL, si.dwPageSize, MEM_COMMIT, PAGE_READWRITE );
+    ok( rbuf1 != NULL, "VirtualAlloc failed err %u\n", GetLastError() );
 
     memset( &ovl, 0, sizeof(ovl) );
     memset( fse, 0, sizeof(fse) );
-    fse[0].Buffer = buf;
+    memset( wbuf, 0x42, si.dwPageSize );
+    fse[0].Buffer = wbuf;
+    SetLastError(0xdeadbeef);
     if (!WriteFileGather( hfile, fse, si.dwPageSize, NULL, &ovl ))
         ok( GetLastError() == ERROR_IO_PENDING, "WriteFileGather failed err %u\n", GetLastError() );
 
@@ -4384,20 +4390,56 @@ static void test_WriteFileGather(void)
     ok( ret, "GetQueuedCompletionStatus failed err %u\n", GetLastError());
     ok( povl == &ovl, "wrong ovl %p\n", povl );
 
+    /* read exact size */
     memset( &ovl, 0, sizeof(ovl) );
     memset( fse, 0, sizeof(fse) );
-    fse[0].Buffer = buf;
-    if (!ReadFileScatter( hfile, fse, si.dwPageSize, NULL, &ovl ))
-        ok( GetLastError() == ERROR_IO_PENDING, "ReadFileScatter failed err %u\n", GetLastError() );
+    fse[0].Buffer = rbuf1;
+    memset( rbuf1, 0, si.dwPageSize );
+    SetLastError(0xdeadbeef);
+    br = ReadFileScatter( hfile, fse, si.dwPageSize, NULL, &ovl );
+    ok( br == FALSE, "ReadFileScatter should be asynchronous\n" );
+    ok( GetLastError() == ERROR_IO_PENDING, "ReadFileScatter failed err %u\n", GetLastError() );
 
     ret = GetQueuedCompletionStatus( hiocp2, &size, &key, &povl, 1000 );
     ok( ret, "GetQueuedCompletionStatus failed err %u\n", GetLastError());
     ok( povl == &ovl, "wrong ovl %p\n", povl );
 
+    ok( memcmp( rbuf1, wbuf, si.dwPageSize ) == 0,
+            "data was not read into buffer\n" );
+
+    /* start read at EOF */
+    memset( &ovl, 0, sizeof(ovl) );
+    S(U(ovl)).OffsetHigh = 0;
+    S(U(ovl)).Offset = si.dwPageSize;
+    memset( fse, 0, sizeof(fse) );
+    fse[0].Buffer = rbuf1;
+    SetLastError(0xdeadbeef);
+    br = ReadFileScatter( hfile, fse, si.dwPageSize, NULL, &ovl );
+    ok( br == FALSE, "ReadFileScatter should have failed\n" );
+    ok( GetLastError() == ERROR_HANDLE_EOF ||
+            GetLastError() == ERROR_IO_PENDING, "ReadFileScatter gave wrong error %u\n", GetLastError() );
+    if (GetLastError() == ERROR_IO_PENDING)
+    {
+        SetLastError(0xdeadbeef);
+        ret = GetQueuedCompletionStatus( hiocp2, &size, &key, &povl, 1000 );
+        ok( !ret, "GetQueuedCompletionStatus should have returned failure\n" );
+        ok( GetLastError() == ERROR_HANDLE_EOF, "Got wrong error: %u\n", GetLastError() );
+        ok( povl == &ovl, "wrong ovl %p\n", povl );
+    }
+    else
+    {
+        SetLastError(0xdeadbeef);
+        ret = GetQueuedCompletionStatus( hiocp2, &size, &key, &povl, 100 );
+        ok( !ret, "GetQueuedCompletionStatus failed err %u\n", GetLastError() );
+        ok( GetLastError() == WAIT_TIMEOUT, "GetQueuedCompletionStatus gave wrong error %u\n", GetLastError() );
+        ok( povl == NULL, "wrong ovl %p\n", povl );
+    }
+
     CloseHandle( hfile );
     CloseHandle( hiocp1 );
     CloseHandle( hiocp2 );
-    VirtualFree( buf, 0, MEM_RELEASE );
+    VirtualFree( wbuf, 0, MEM_RELEASE );
+    VirtualFree( rbuf1, 0, MEM_RELEASE );
     DeleteFileA( filename );
 }
 
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
index ca2afa0e89..12b1d0f6c5 100644
--- a/dlls/ntdll/file.c
+++ b/dlls/ntdll/file.c
@@ -1105,7 +1105,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
 
     if (send_completion) NTDLL_AddCompletion( file, cvalue, status, total );
 
-    return status;
+    return STATUS_PENDING;
 }
 
 
-- 
2.15.1

[Alexandre Julliard]

Andrew Eikum <aeikum(a)codeweavers.com> writes:
> @@ -1105,7 +1105,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
>
> if (send_completion) NTDLL_AddCompletion( file, cvalue, status, total );
>
> -    return status;
> +    return STATUS_PENDING;

Is it really correct to do this for error cases too?  In particular,
returning STATUS_PENDING for the non-overlapped case doesn't seem right.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Andrew Eikum]

On Tue, Dec 05, 2017 at 05:43:00PM +0100, Alexandre Julliard wrote:
> Andrew Eikum <aeikum(a)codeweavers.com> writes:
> > @@ -1105,7 +1105,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
> >
> > if (send_completion) NTDLL_AddCompletion( file, cvalue, status, total );
> >
> > -    return status;
> > +    return STATUS_PENDING;
>
> Is it really correct to do this for error cases too?  In particular,
> returning STATUS_PENDING for the non-overlapped case doesn't seem right.

I see what you mean. I'll try to come up with some tests for that,
thanks.

Andrew