[PATCH v4 0/1] MR11730: windowscodecs: Always pass a valid pcbRead pointer to IStream::Read().
Some non-conforming IStream implementations, including SharpDX, do not handle a NULL pcbRead for Read() [^1]. This causes the game "Surviving Deponia Playtest" to display a black main menu. [^1]: https://github.com/sharpdx/SharpDX/blob/ab36f12303e24aa60fe804866617716b6ded... -- v4: windowscodecs: Always pass a valid pcbRead pointer to IStream::Read(). https://gitlab.winehq.org/wine/wine/-/merge_requests/11730
From: Shaun Ren <sren@codeweavers.com> Some non-conforming IStream implementations, such as SharpDX, do not handle a NULL pcbRead for Read(). This causes the game "Surviving Deponia Playtest" to display a black main menu. Fixes: 2a167eb4ea844a090ca0247cf233633f697e3a85 --- dlls/windowscodecs/wincodecs_common.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/dlls/windowscodecs/wincodecs_common.c b/dlls/windowscodecs/wincodecs_common.c index 8da1bfed20c..67dafbf2ae1 100644 --- a/dlls/windowscodecs/wincodecs_common.c +++ b/dlls/windowscodecs/wincodecs_common.c @@ -188,7 +188,22 @@ HRESULT CDECL stream_getsize(IStream *stream, ULONGLONG *size) HRESULT CDECL stream_read(IStream *stream, void *buffer, ULONG read, ULONG *bytes_read) { - return IStream_Read(stream, buffer, read, bytes_read); + if (bytes_read) + { + return IStream_Read(stream, buffer, read, bytes_read); + } + else + { + ULONG nread; + HRESULT hr; + + /* Ensure that we always pass a non-NULL bytes_read pointer, as some + * implementations (e.g. SharpDX) can't handle it being NULL. */ + hr = IStream_Read(stream, buffer, read, &nread); + if (SUCCEEDED(hr) && nread != read) + return E_FAIL; + return hr; + } } HRESULT CDECL stream_seek(IStream *stream, LONGLONG ofs, DWORD origin, ULONGLONG *new_position) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11730
Nikolay Sivov (@nsivov) commented about dlls/windowscodecs/wincodecs_common.c:
+ if (bytes_read) + { + return IStream_Read(stream, buffer, read, bytes_read); + } + else + { + ULONG nread; + HRESULT hr; + + /* Ensure that we always pass a non-NULL bytes_read pointer, as some + * implementations (e.g. SharpDX) can't handle it being NULL. */ + hr = IStream_Read(stream, buffer, read, &nread); + if (SUCCEEDED(hr) && nread != read) + return E_FAIL; + return hr; + } If we do this, it should be consistent between two cases. Now for first path it won't return error code, and for second one it will. IMO it's more natural to keep whatever stream method returns.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11730#note_149725
On Mon Aug 24 17:59:56 2026 +0000, Nikolay Sivov wrote:
If we do this, it should be consistent between two cases. Now for first path it won't return error code, and for second one it will. IMO it's more natural to keep whatever stream method returns. We could also just make sure we return S_FALSE if it succeeds with fewer bytes read than requested. I just don't feel like we'd ever want to pass through a successful hr in the case where we read an unknown but non-zero number of bytes because we didn't pass in a variable to accept the amount.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11730#note_149726
On Mon Aug 24 18:03:13 2026 +0000, Esme Povirk wrote:
We could also just make sure we return S_FALSE if it succeeds with fewer bytes read than requested. I just don't feel like we'd ever want to pass through a successful hr in the case where we read an unknown but non-zero number of bytes because we didn't pass in a variable to accept the amount. I guess if we're doing it that way, we can *always* make that guarantee about S_FALSE and we won't need to check bytesread when we get S_OK.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11730#note_149727
participants (4)
-
Esme Povirk (＠madewokherd) -
Nikolay Sivov (＠nsivov)
-
Shaun Ren
-
Shaun Ren (＠shaunren)