[PATCH 2/3] ntdll: Don't return an error when reading past EOF in NtReadFileScatter

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>
---
 dlls/kernel32/tests/file.c | 27 ++++++++++++++++++++++++++-
 dlls/ntdll/file.c          |  3 ---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 44fbf7a4b1..e1cf268817 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -4354,7 +4354,7 @@ static void test_WriteFileGather(void)
     OVERLAPPED ovl, *povl = NULL;
     SYSTEM_INFO si;
     BOOL br;
-    char *wbuf = NULL, *rbuf1;
+    char *wbuf = NULL, *rbuf1, *rbuf2;
 
     ret = GetTempPathA( MAX_PATH, temp_path );
     ok( ret != 0, "GetTempPathA error %d\n", GetLastError() );
@@ -4378,6 +4378,9 @@ static void test_WriteFileGather(void)
     rbuf1 = VirtualAlloc( NULL, si.dwPageSize, MEM_COMMIT, PAGE_READWRITE );
     ok( rbuf1 != NULL, "VirtualAlloc failed err %u\n", GetLastError() );
 
+    rbuf2 = VirtualAlloc( NULL, si.dwPageSize, MEM_COMMIT, PAGE_READWRITE );
+    ok( rbuf2 != NULL, "VirtualAlloc failed err %u\n", GetLastError() );
+
     memset( &ovl, 0, sizeof(ovl) );
     memset( fse, 0, sizeof(fse) );
     memset( wbuf, 0x42, si.dwPageSize );
@@ -4405,11 +4408,33 @@ static void test_WriteFileGather(void)
     ok( memcmp( rbuf1, wbuf, si.dwPageSize ) == 0,
             "data was not read into buffer\n" );
 
+    /* read past EOF */
+    memset( &ovl, 0, sizeof(ovl) );
+    memset( fse, 0, sizeof(fse) );
+    fse[0].Buffer = rbuf1;
+    fse[1].Buffer = rbuf2;
+    memset( rbuf1, 0, si.dwPageSize );
+    memset( rbuf2, 0x17, si.dwPageSize );
+    br = ReadFileScatter( hfile, fse, si.dwPageSize * 2, 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" );
+    memset( rbuf1, 0x17, si.dwPageSize );
+    ok( memcmp( rbuf2, rbuf1, si.dwPageSize ) == 0,
+            "data should not have been read into buffer\n" );
+
     CloseHandle( hfile );
     CloseHandle( hiocp1 );
     CloseHandle( hiocp2 );
     VirtualFree( wbuf, 0, MEM_RELEASE );
     VirtualFree( rbuf1, 0, MEM_RELEASE );
+    VirtualFree( rbuf2, 0, MEM_RELEASE );
     DeleteFileA( filename );
 }
 
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
index bdfc383d7e..f7db24cc8b 100644
--- a/dlls/ntdll/file.c
+++ b/dlls/ntdll/file.c
@@ -1071,10 +1071,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
             break;
         }
         if (!result)
-        {
-            status = STATUS_END_OF_FILE;
             break;
-        }
         total += result;
         length -= result;
         if ((pos += result) == page_size)
-- 
2.15.0

[Thomas Faber]

On 2017-11-29 19:52, Andrew Eikum wrote:
> diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> index bdfc383d7e..f7db24cc8b 100644
> --- a/dlls/ntdll/file.c
> +++ b/dlls/ntdll/file.c
> @@ -1071,10 +1071,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
>               break;
>           }
>           if (!result)
> -        {
> -            status = STATUS_END_OF_FILE;
>               break;
> -        }
>           total += result;
>           length -= result;
>           if ((pos += result) == page_size)

I think your conclusion from the test may be overly broad.
The regular NtReadFile only returns STATUS_END_OF_FILE if the file
pointer was already at EOF, i.e. if no data was read.
It seems likely the Scatter version would do the same, so another test
that starts reading at or beyond EOF should still return
STATUS_END_OF_FILE.

Best,
Thomas

[Andrew Eikum]

On Thu, Nov 30, 2017 at 10:43:41AM +0100, Thomas Faber wrote:
> On 2017-11-29 19:52, Andrew Eikum wrote:
> > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> > index bdfc383d7e..f7db24cc8b 100644
> > --- a/dlls/ntdll/file.c
> > +++ b/dlls/ntdll/file.c
> > @@ -1071,10 +1071,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
> >               break;
> >           }
> >           if (!result)
> > -        {
> > -            status = STATUS_END_OF_FILE;
> >               break;
> > -        }
> >           total += result;
> >           length -= result;
> >           if ((pos += result) == page_size)
>
> I think your conclusion from the test may be overly broad.
> The regular NtReadFile only returns STATUS_END_OF_FILE if the file
> pointer was already at EOF, i.e. if no data was read.
> It seems likely the Scatter version would do the same, so another test
> that starts reading at or beyond EOF should still return
> STATUS_END_OF_FILE.

Thanks for the pointer, I'll add a test for that.

Andrew