[PATCH 0/1] MR4731: include: Fix boolean return value definition in IIterator<T> methods.

[Paul Gofman (＠gofman)]

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().

[PATCH 1/1] include: Fix boolean return value definition in IIterator<T> methods.

[Paul Gofman]

From: Paul Gofman

---
 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);
             }

[PATCH 1/1] include: Fix boolean return value definition in IIterator<T> methods.

[Dmitry Timoshkov]

Paul Gofman  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.

[PATCH 1/1] include: Fix boolean return value definition in IIterator<T> methods.

[Paul Gofman]

On 12/22/23 01:37, Dmitry Timoshkov wrote:
> Paul Gofman  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.

[PATCH 0/1] MR4731: include: Fix boolean return value definition in IIterator<T> methods. - approved

[Rémi Bernon (＠rbernon)]

This merge request was approved by Rémi Bernon.