[PATCH 0/1] MR4731: include: Fix boolean return value definition in IIterator<T> methods.
The practical effect of the change is that boolean is 1 byte while BOOL is 4 bytes, so we are wrongly filling more bytes in those methods and wrongly expect full 4 byte BOOL value to be filled from apps IIterator implemenation (and thus depend on uninitialized data in implementation). That fixes a hang on new game start in Phasmophobia (when it is configured to use windows.media.speech). That was originally attributed as a regression from commit 62173699c38453777c7d5638ed2e779790506b75 ("include: Don't align the stack for PE builds.") while before that commit it worked by chance only of course, the commit only changed stack data a bit so we were a bit more lucky in getting high bytes of BOOL as 0 in windows.media.speech/vector.c:vector_hstring_create_copy(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4731
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/dinput/tests/hotplug.c | 18 +++++++++--------- dlls/windows.gaming.input/vector.c | 4 ++-- dlls/windows.media.speech/tests/speech.c | 4 ++-- dlls/windows.media.speech/vector.c | 10 +++++----- include/windows.foundation.collections.idl | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index 7187cf7dc38..5b9455d1819 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -977,11 +977,11 @@ static void test_windows_gaming_input(void) HANDLE hwnd, thread, stop_event; IInspectable *tmp_inspectable; HDEVNOTIFY devnotify; + boolean boolret; HSTRING str; UINT32 size; HRESULT hr; DWORD res; - BOOL ret; MSG msg; if (!load_combase_functions()) return; @@ -1100,18 +1100,18 @@ static void test_windows_gaming_input(void) ok( hr == S_OK, "First returned %#lx\n", hr ); IIterable_RawGameController_Release( iterable ); - hr = IIterator_RawGameController_get_HasCurrent( iterator, &ret ); + hr = IIterator_RawGameController_get_HasCurrent( iterator, &boolret ); ok( hr == S_OK, "First returned %#lx\n", hr ); - ok( ret == TRUE, "got HasCurrent %u\n", ret ); - hr = IIterator_RawGameController_MoveNext( iterator, &ret ); + ok( boolret == TRUE, "got HasCurrent %u\n", boolret ); + hr = IIterator_RawGameController_MoveNext( iterator, &boolret ); ok( hr == S_OK, "First returned %#lx\n", hr ); - ok( ret == FALSE, "got MoveNext %u\n", ret ); - hr = IIterator_RawGameController_get_HasCurrent( iterator, &ret ); + ok( boolret == FALSE, "got MoveNext %u\n", boolret ); + hr = IIterator_RawGameController_get_HasCurrent( iterator, &boolret ); ok( hr == S_OK, "First returned %#lx\n", hr ); - ok( ret == FALSE, "got MoveNext %u\n", ret ); - hr = IIterator_RawGameController_MoveNext( iterator, &ret ); + ok( boolret == FALSE, "got MoveNext %u\n", boolret ); + hr = IIterator_RawGameController_MoveNext( iterator, &boolret ); ok( hr == S_OK, "First returned %#lx\n", hr ); - ok( ret == FALSE, "got MoveNext %u\n", ret ); + ok( boolret == FALSE, "got MoveNext %u\n", boolret ); IIterator_RawGameController_Release( iterator ); IVectorView_RawGameController_Release( controller_view ); diff --git a/dlls/windows.gaming.input/vector.c b/dlls/windows.gaming.input/vector.c index 8958b07c0f2..954d7df3552 100644 --- a/dlls/windows.gaming.input/vector.c +++ b/dlls/windows.gaming.input/vector.c @@ -108,7 +108,7 @@ static HRESULT WINAPI iterator_get_Current( IIterator_IInspectable *iface, IInsp return IVectorView_IInspectable_GetAt( impl->view, impl->index, value ); } -static HRESULT WINAPI iterator_get_HasCurrent( IIterator_IInspectable *iface, BOOL *value ) +static HRESULT WINAPI iterator_get_HasCurrent( IIterator_IInspectable *iface, boolean *value ) { struct iterator *impl = impl_from_IIterator_IInspectable( iface ); @@ -118,7 +118,7 @@ static HRESULT WINAPI iterator_get_HasCurrent( IIterator_IInspectable *iface, BO return S_OK; } -static HRESULT WINAPI iterator_MoveNext( IIterator_IInspectable *iface, BOOL *value ) +static HRESULT WINAPI iterator_MoveNext( IIterator_IInspectable *iface, boolean *value ) { struct iterator *impl = impl_from_IIterator_IInspectable( iface ); diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c index 49a4915332a..feeeda58be4 100644 --- a/dlls/windows.media.speech/tests/speech.c +++ b/dlls/windows.media.speech/tests/speech.c @@ -524,7 +524,7 @@ static HRESULT WINAPI iterator_hstring_get_Current( IIterator_HSTRING *iface, HS return hr; } -static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, BOOL *value ) +static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, boolean *value ) { struct iterator_hstring *impl = impl_from_IIterator_HSTRING(iface); @@ -532,7 +532,7 @@ static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, return S_OK; } -static HRESULT WINAPI iterator_hstring_MoveNext( IIterator_HSTRING *iface, BOOL *value ) +static HRESULT WINAPI iterator_hstring_MoveNext( IIterator_HSTRING *iface, boolean *value ) { struct iterator_hstring *impl = impl_from_IIterator_HSTRING(iface); diff --git a/dlls/windows.media.speech/vector.c b/dlls/windows.media.speech/vector.c index c23fb9dc56e..05bbaf89bef 100644 --- a/dlls/windows.media.speech/vector.c +++ b/dlls/windows.media.speech/vector.c @@ -113,7 +113,7 @@ static HRESULT WINAPI iterator_hstring_get_Current( IIterator_HSTRING *iface, HS return IVectorView_HSTRING_GetAt(impl->view, impl->index, value); } -static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, BOOL *value ) +static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, boolean *value ) { struct iterator_hstring *impl = impl_from_IIterator_HSTRING(iface); @@ -123,7 +123,7 @@ static HRESULT WINAPI iterator_hstring_get_HasCurrent( IIterator_HSTRING *iface, return S_OK; } -static HRESULT WINAPI iterator_hstring_MoveNext( IIterator_HSTRING *iface, BOOL *value ) +static HRESULT WINAPI iterator_hstring_MoveNext( IIterator_HSTRING *iface, boolean *value ) { struct iterator_hstring *impl = impl_from_IIterator_HSTRING(iface); @@ -735,7 +735,7 @@ HRESULT vector_hstring_create_copy( IIterable_HSTRING *iterable, IVector_HSTRING struct vector_hstring *impl; IIterator_HSTRING *iterator; UINT32 capacity = 0; - BOOL available; + boolean available; HRESULT hr; TRACE("iterable %p, out %p.\n", iterable, out); @@ -865,7 +865,7 @@ static HRESULT WINAPI iterator_inspectable_get_Current( IIterator_IInspectable * return IVectorView_IInspectable_GetAt(impl->view, impl->index, value); } -static HRESULT WINAPI iterator_inspectable_get_HasCurrent( IIterator_IInspectable *iface, BOOL *value ) +static HRESULT WINAPI iterator_inspectable_get_HasCurrent( IIterator_IInspectable *iface, boolean *value ) { struct iterator_inspectable *impl = impl_from_IIterator_IInspectable(iface); @@ -875,7 +875,7 @@ static HRESULT WINAPI iterator_inspectable_get_HasCurrent( IIterator_IInspectabl return S_OK; } -static HRESULT WINAPI iterator_inspectable_MoveNext( IIterator_IInspectable *iface, BOOL *value ) +static HRESULT WINAPI iterator_inspectable_MoveNext( IIterator_IInspectable *iface, boolean *value ) { struct iterator_inspectable *impl = impl_from_IIterator_IInspectable(iface); diff --git a/include/windows.foundation.collections.idl b/include/windows.foundation.collections.idl index 680d7803cc8..2a8fd55be7d 100644 --- a/include/windows.foundation.collections.idl +++ b/include/windows.foundation.collections.idl @@ -148,8 +148,8 @@ cpp_quote("#endif") interface IIterator<T> : IInspectable { [propget] HRESULT Current([out, retval] T *value); - [propget] HRESULT HasCurrent([out, retval] BOOL *value); - HRESULT MoveNext([out, retval] BOOL *value); + [propget] HRESULT HasCurrent([out, retval] boolean *value); + HRESULT MoveNext([out, retval] boolean *value); HRESULT GetMany([in] UINT32 items_size, [out] T *items, [out, retval] UINT32 *value); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4731
Paul Gofman <wine(a)gitlab.winehq.org> wrote:
diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index 7187cf7dc38..5b9455d1819 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -977,11 +977,11 @@ static void test_windows_gaming_input(void) HANDLE hwnd, thread, stop_event; IInspectable *tmp_inspectable; HDEVNOTIFY devnotify; + boolean boolret; HSTRING str; UINT32 size; HRESULT hr; DWORD res; - BOOL ret; MSG msg;
It should be possible to use Windows type BOOLEAN for the same purpose. Also avoiding to rename a variable would reduce the diff. -- Dmitry.
On 12/22/23 01:37, Dmitry Timoshkov wrote:
Paul Gofman <wine(a)gitlab.winehq.org> wrote:
diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index 7187cf7dc38..5b9455d1819 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -977,11 +977,11 @@ static void test_windows_gaming_input(void) HANDLE hwnd, thread, stop_event; IInspectable *tmp_inspectable; HDEVNOTIFY devnotify; + boolean boolret; HSTRING str; UINT32 size; HRESULT hr; DWORD res; - BOOL ret; MSG msg; It should be possible to use Windows type BOOLEAN for the same purpose.
That's true, although I saw some existing WinRT dlls already using boolean and it is used this way in Win headers and samples, so probably no particular reason to stick to winnt definitions?
Also avoiding to rename a variable would reduce the diff.
Well, I have touch the line anyway, I hope that more explicit naming won't hurt.
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4731
participants (5)
-
Dmitry Timoshkov -
Paul Gofman -
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon