[PATCH v2 1/2] mfplat/tests: Test reading past the end of a file stream.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/mfplat/tests/mfplat.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c
index 541459c715b..b9aa4bbb68d 100644
--- a/dlls/mfplat/tests/mfplat.c
+++ b/dlls/mfplat/tests/mfplat.c
@@ -1947,8 +1947,10 @@ static void test_file_stream(void)
     WCHAR pathW[MAX_PATH];
     DWORD caps, count;
     WCHAR *filename;
+    BYTE data[8];
     HRESULT hr;
     WCHAR *str;
+    ULONG size;
     BOOL eos;
 
     filename = load_resource(L"test.mp4");
@@ -2023,6 +2025,32 @@ static void test_file_stream(void)
     ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
     ok(position == 2 * bytestream_length, "Unexpected position.\n");
 
+    hr = IMFByteStream_IsEndOfStream(bytestream, &eos);
+    ok(hr == S_OK, "Failed query end of stream, hr %#x.\n", hr);
+    ok(eos == TRUE, "Unexpected IsEndOfStream result, %u.\n", eos);
+
+    size = 0xdeadbeef;
+    hr = IMFByteStream_Read(bytestream, data, sizeof(data), &size);
+    todo_wine ok(hr == HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) || broken(hr == S_OK) /* win8 */,
+            "Unexpected hr %#x.\n", hr);
+    ok(!size || broken(size == 0xdeadbeef) /* win < 8 */, "Got size %u.\n", size);
+
+    hr = IMFByteStream_SetCurrentPosition(bytestream, bytestream_length - 4);
+    ok(hr == S_OK, "Failed to set bytestream position, hr %#x.\n", hr);
+
+    size = 0xdeadbeef;
+    hr = IMFByteStream_Read(bytestream, data, sizeof(data), &size);
+    ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
+    ok(size == 4, "Got size %u.\n", size);
+
+    hr = IMFByteStream_SetCurrentPosition(bytestream, bytestream_length);
+    ok(hr == S_OK, "Failed to set bytestream position, hr %#x.\n", hr);
+
+    size = 0xdeadbeef;
+    hr = IMFByteStream_Read(bytestream, data, sizeof(data), &size);
+    ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
+    ok(!size, "Got size %u.\n", size);
+
     hr = MFCreateFile(MF_ACCESSMODE_READ, MF_OPENMODE_FAIL_IF_NOT_EXIST,
                       MF_FILEFLAGS_NONE, filename, &bytestream2);
     ok(hr == S_OK, "got 0x%08x\n", hr);
-- 
2.33.0

[PATCH v2 2/2] mfplat: Return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) when reading past the end of a file stream.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/mfplat/main.c         | 15 ++++++++++++++-
 dlls/mfplat/tests/mfplat.c |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
index 7fd713261fc..fb55a8afb21 100644
--- a/dlls/mfplat/main.c
+++ b/dlls/mfplat/main.c
@@ -3843,7 +3843,7 @@ static HRESULT WINAPI bytestream_file_IsEndOfStream(IMFByteStream *iface, BOOL *
 static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, ULONG size, ULONG *read_len)
 {
     struct bytestream *stream = impl_from_IMFByteStream(iface);
-    LARGE_INTEGER position;
+    LARGE_INTEGER position, file_size;
     HRESULT hr = S_OK;
     BOOL ret;
 
@@ -3851,6 +3851,19 @@ static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, U
 
     EnterCriticalSection(&stream->cs);
 
+    if (!GetFileSizeEx(stream->hfile, &file_size))
+    {
+        LeaveCriticalSection(&stream->cs);
+        return HRESULT_FROM_WIN32(GetLastError());
+    }
+
+    if (stream->position > file_size.QuadPart)
+    {
+        LeaveCriticalSection(&stream->cs);
+        *read_len = 0;
+        return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF);
+    }
+
     position.QuadPart = stream->position;
     if ((ret = SetFilePointerEx(stream->hfile, position, NULL, FILE_BEGIN)))
     {
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c
index b9aa4bbb68d..1389975c2c2 100644
--- a/dlls/mfplat/tests/mfplat.c
+++ b/dlls/mfplat/tests/mfplat.c
@@ -2031,7 +2031,7 @@ static void test_file_stream(void)
 
     size = 0xdeadbeef;
     hr = IMFByteStream_Read(bytestream, data, sizeof(data), &size);
-    todo_wine ok(hr == HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) || broken(hr == S_OK) /* win8 */,
+    ok(hr == HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) || broken(hr == S_OK) /* win8 */,
             "Unexpected hr %#x.\n", hr);
     ok(!size || broken(size == 0xdeadbeef) /* win < 8 */, "Got size %u.\n", size);
 
-- 
2.33.0

[PATCH v2 2/2] mfplat: Return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) when reading past the end of a file stream.

[Nikolay Sivov]

On 10/8/21 7:06 AM, Zebediah Figura wrote:
> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> ---
>  dlls/mfplat/main.c         | 15 ++++++++++++++-
>  dlls/mfplat/tests/mfplat.c |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
> index 7fd713261fc..fb55a8afb21 100644
> --- a/dlls/mfplat/main.c
> +++ b/dlls/mfplat/main.c
> @@ -3843,7 +3843,7 @@ static HRESULT WINAPI bytestream_file_IsEndOfStream(IMFByteStream *iface, BOOL *
>  static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, ULONG size, ULONG *read_len)
>  {
>      struct bytestream *stream = impl_from_IMFByteStream(iface);
> -    LARGE_INTEGER position;
> +    LARGE_INTEGER position, file_size;
>      HRESULT hr = S_OK;
>      BOOL ret;
>
> @@ -3851,6 +3851,19 @@ static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, U
>
> EnterCriticalSection(&stream->cs);
>
> +    if (!GetFileSizeEx(stream->hfile, &file_size))
> +    {
> +        LeaveCriticalSection(&stream->cs);
> +        return HRESULT_FROM_WIN32(GetLastError());
> +    }
> +
> +    if (stream->position > file_size.QuadPart)
> +    {
> +        LeaveCriticalSection(&stream->cs);
> +        *read_len = 0;
> +        return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF);
> +    }
> +

I think this is too explicit. Can we rely on read length that ReadFile
returns? Doing (ret && size && !*read_len) -> EOF. Depending on how 0
reads work, when EOF is already reached.

[PATCH v2 2/2] mfplat: Return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) when reading past the end of a file stream.

[Zebediah Figura]

On 10/8/21 3:25 AM, Nikolay Sivov wrote:
> On 10/8/21 7:06 AM, Zebediah Figura wrote:
> > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> > ---
> >   dlls/mfplat/main.c         | 15 ++++++++++++++-
> >   dlls/mfplat/tests/mfplat.c |  2 +-
> >   2 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
> > index 7fd713261fc..fb55a8afb21 100644
> > --- a/dlls/mfplat/main.c
> > +++ b/dlls/mfplat/main.c
> > @@ -3843,7 +3843,7 @@ static HRESULT WINAPI bytestream_file_IsEndOfStream(IMFByteStream *iface, BOOL *
> >   static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, ULONG size, ULONG *read_len)
> >   {
> >       struct bytestream *stream = impl_from_IMFByteStream(iface);
> > -    LARGE_INTEGER position;
> > +    LARGE_INTEGER position, file_size;
> >       HRESULT hr = S_OK;
> >       BOOL ret;
> >
> > @@ -3851,6 +3851,19 @@ static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, U
> >
> > EnterCriticalSection(&stream->cs);
> >
> > +    if (!GetFileSizeEx(stream->hfile, &file_size))
> > +    {
> > +        LeaveCriticalSection(&stream->cs);
> > +        return HRESULT_FROM_WIN32(GetLastError());
> > +    }
> > +
> > +    if (stream->position > file_size.QuadPart)
> > +    {
> > +        LeaveCriticalSection(&stream->cs);
> > +        *read_len = 0;
> > +        return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF);
> > +    }
> > +
>
> I think this is too explicit. Can we rely on read length that ReadFile
> returns? Doing (ret && size && !*read_len) -> EOF. Depending on how 0
> reads work, when EOF is already reached.

Sorry for the late reply. Unfortunately that doesn't work. The tests 
from patch 1 show that the bytestream reader returns S_OK when reading 
at the end of the file, but ERROR_HANDLE_EOF when reading past it. 
ReadFile() however only returns ERROR_HANDLE_EOF when doing an 
overlapped read.

[PATCH v2 2/2] mfplat: Return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) when reading past the end of a file stream.

[Nikolay Sivov]

On 10/15/21 11:20 PM, Zebediah Figura wrote:
> On 10/8/21 3:25 AM, Nikolay Sivov wrote:
> > On 10/8/21 7:06 AM, Zebediah Figura wrote:
> > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> > > ---
> > >   dlls/mfplat/main.c         | 15 ++++++++++++++-
> > >   dlls/mfplat/tests/mfplat.c |  2 +-
> > >   2 files changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
> > > index 7fd713261fc..fb55a8afb21 100644
> > > --- a/dlls/mfplat/main.c
> > > +++ b/dlls/mfplat/main.c
> > > @@ -3843,7 +3843,7 @@ static HRESULT WINAPI
> > > bytestream_file_IsEndOfStream(IMFByteStream *iface, BOOL *
> > >   static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface,
> > > BYTE *buffer, ULONG size, ULONG *read_len)
> > >   {
> > >       struct bytestream *stream = impl_from_IMFByteStream(iface);
> > > -    LARGE_INTEGER position;
> > > +    LARGE_INTEGER position, file_size;
> > >       HRESULT hr = S_OK;
> > >       BOOL ret;
> > >   @@ -3851,6 +3851,19 @@ static HRESULT WINAPI
> > > bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, U
> > >         EnterCriticalSection(&stream->cs);
> > >   +    if (!GetFileSizeEx(stream->hfile, &file_size))
> > > +    {
> > > +        LeaveCriticalSection(&stream->cs);
> > > +        return HRESULT_FROM_WIN32(GetLastError());
> > > +    }
> > > +
> > > +    if (stream->position > file_size.QuadPart)
> > > +    {
> > > +        LeaveCriticalSection(&stream->cs);
> > > +        *read_len = 0;
> > > +        return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF);
> > > +    }
> > > +
> >
> > I think this is too explicit. Can we rely on read length that ReadFile
> > returns? Doing (ret && size && !*read_len) -> EOF. Depending on how 0
> > reads work, when EOF is already reached.
>
> Sorry for the late reply. Unfortunately that doesn't work. The tests
> from patch 1 show that the bytestream reader returns S_OK when reading
> at the end of the file, but ERROR_HANDLE_EOF when reading past it.
> ReadFile() however only returns ERROR_HANDLE_EOF when doing an
> overlapped read.

I was thinking not about the error that ReadFile would set, but a
combination of its return value and returned read_len.

[PATCH v2 2/2] mfplat: Return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF) when reading past the end of a file stream.

[Zebediah Figura (she/her)]

On 10/15/21 15:24, Nikolay Sivov wrote:
> On 10/15/21 11:20 PM, Zebediah Figura wrote:
> > On 10/8/21 3:25 AM, Nikolay Sivov wrote:
> > > On 10/8/21 7:06 AM, Zebediah Figura wrote:
> > > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> > > > ---
> > > >    dlls/mfplat/main.c         | 15 ++++++++++++++-
> > > >    dlls/mfplat/tests/mfplat.c |  2 +-
> > > >    2 files changed, 15 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
> > > > index 7fd713261fc..fb55a8afb21 100644
> > > > --- a/dlls/mfplat/main.c
> > > > +++ b/dlls/mfplat/main.c
> > > > @@ -3843,7 +3843,7 @@ static HRESULT WINAPI
> > > > bytestream_file_IsEndOfStream(IMFByteStream *iface, BOOL *
> > > >    static HRESULT WINAPI bytestream_file_Read(IMFByteStream *iface,
> > > > BYTE *buffer, ULONG size, ULONG *read_len)
> > > >    {
> > > >        struct bytestream *stream = impl_from_IMFByteStream(iface);
> > > > -    LARGE_INTEGER position;
> > > > +    LARGE_INTEGER position, file_size;
> > > >        HRESULT hr = S_OK;
> > > >        BOOL ret;
> > > >    @@ -3851,6 +3851,19 @@ static HRESULT WINAPI
> > > > bytestream_file_Read(IMFByteStream *iface, BYTE *buffer, U
> > > >          EnterCriticalSection(&stream->cs);
> > > >    +    if (!GetFileSizeEx(stream->hfile, &file_size))
> > > > +    {
> > > > +        LeaveCriticalSection(&stream->cs);
> > > > +        return HRESULT_FROM_WIN32(GetLastError());
> > > > +    }
> > > > +
> > > > +    if (stream->position > file_size.QuadPart)
> > > > +    {
> > > > +        LeaveCriticalSection(&stream->cs);
> > > > +        *read_len = 0;
> > > > +        return HRESULT_FROM_WIN32(ERROR_HANDLE_EOF);
> > > > +    }
> > > > +
> > >
> > > I think this is too explicit. Can we rely on read length that ReadFile
> > > returns? Doing (ret && size && !*read_len) -> EOF. Depending on how 0
> > > reads work, when EOF is already reached.
> >
> > Sorry for the late reply. Unfortunately that doesn't work. The tests
> > from patch 1 show that the bytestream reader returns S_OK when reading
> > at the end of the file, but ERROR_HANDLE_EOF when reading past it.
> > ReadFile() however only returns ERROR_HANDLE_EOF when doing an
> > overlapped read.
>
> I was thinking not about the error that ReadFile would set, but a
> combination of its return value and returned read_len.

Right, the point is that ReadFile() behaves identically between "seek to 
EOF and read nonzero bytes" and "seek past EOF and read nonzero bytes". 
In both cases it succeeds, doesn't set the last error, and returns a 
read size of zero.