[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 at 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.