[PATCH 0/2] MR2560: dinput: Add test and fix EnumObjects callback return value handling.

[Florian Will (＠w-flo)]

First commit adds a "todo_wine" test, second commit fixes the behavior and marks the test as no longer "todo". It looks like the same logic is already implemented (and tested) for the EnumDevices method.

Commit message for the fix commit:

---

This solves an issue in ZUSI 3 settings for DirectInput devices. Delphi
defines the True value of the "C-compatible" LongBool type as -1, which
wine interpreted to mean DIENUM_STOP because it is != DIENUM_CONTINUE.
Change that logic so only an explicit DIENUM_STOP (= 0) return value
stops the enumeration of objects.

[PATCH 1/2] dinput/tests: Add EnumObjects callback return value test.

[Florian Will]

From: Florian Will

---
 dlls/dinput/tests/device8.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/dlls/dinput/tests/device8.c b/dlls/dinput/tests/device8.c
index ea0ec8db837..128fb2992f6 100644
--- a/dlls/dinput/tests/device8.c
+++ b/dlls/dinput/tests/device8.c
@@ -991,6 +991,13 @@ static BOOL CALLBACK check_object_count( const DIDEVICEOBJECTINSTANCEW *obj, voi
     return DIENUM_CONTINUE;
 }

+static BOOL CALLBACK check_object_count_bad_retval( const DIDEVICEOBJECTINSTANCEW *obj, void *args )
+{
+    DWORD *count = args;
+    *count = *count + 1;
+    return -1; /* Invalid, but should CONTINUE. Only explicit DIENUM_STOP will stop enumeration. */
+}
+
 static void test_sys_mouse( DWORD version )
 {
     const DIDEVCAPS expect_caps =
@@ -1336,6 +1343,11 @@ static void test_sys_mouse( DWORD version )
     ok( check_objects_params.index >= check_objects_params.expect_count, "missing %u objects\n",
         check_objects_params.expect_count - check_objects_params.index );

+    res = 0;
+    hr = IDirectInputDevice8_EnumObjects( device, check_object_count_bad_retval, &res, DIDFT_AXIS );
+    ok( hr == DI_OK, "EnumObjects returned %#lx\n", hr );
+    todo_wine ok( res == 3, "got %lu expected 3\n", res );
+
     objinst.dwSize = sizeof(DIDEVICEOBJECTINSTANCEW);
     res = MAKELONG( HID_USAGE_GENERIC_X, HID_USAGE_PAGE_GENERIC );
     hr = IDirectInputDevice8_GetObjectInfo( device, &objinst, res, DIPH_BYUSAGE );

[PATCH 2/2] dinput: Fix EnumObjects callback return value handling.

[Florian Will]

From: Florian Will

This solves an issue in ZUSI 3 settings for DirectInput devices. Delphi
defines the True value of the "C-compatible" LongBool type as -1, which
wine interpreted to mean DIENUM_STOP because it is != DIENUM_CONTINUE.
Change that logic so only an explicit DIENUM_STOP (= 0) return value
stops the enumeration of objects.
---
 dlls/dinput/device.c        | 4 +++-
 dlls/dinput/tests/device8.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c
index 19c3241e51f..4db8bd943cb 100644
--- a/dlls/dinput/device.c
+++ b/dlls/dinput/device.c
@@ -730,7 +730,9 @@ static BOOL enum_objects_callback( struct dinput_device *impl, UINT index, struc
     struct enum_objects_params *params = data;
     if (instance->wUsagePage == HID_USAGE_PAGE_PID && !(instance->dwType & DIDFT_NODATA))
         return DIENUM_CONTINUE;
-    return params->callback( instance, params->context );
+
+    /* Applications may return non-zero values instead of DIENUM_CONTINUE. */
+    return params->callback( instance, params->context ) ? DIENUM_CONTINUE : DIENUM_STOP;
 }

static HRESULT WINAPI dinput_device_EnumObjects( IDirectInputDevice8W *iface, LPDIENUMDEVICEOBJECTSCALLBACKW callback,
diff --git a/dlls/dinput/tests/device8.c b/dlls/dinput/tests/device8.c
index 128fb2992f6..a649fe53b41 100644
--- a/dlls/dinput/tests/device8.c
+++ b/dlls/dinput/tests/device8.c
@@ -1346,7 +1346,7 @@ static void test_sys_mouse( DWORD version )
     res = 0;
     hr = IDirectInputDevice8_EnumObjects( device, check_object_count_bad_retval, &res, DIDFT_AXIS );
     ok( hr == DI_OK, "EnumObjects returned %#lx\n", hr );
-    todo_wine ok( res == 3, "got %lu expected 3\n", res );
+    ok( res == 3, "got %lu expected 3\n", res );

objinst.dwSize = sizeof(DIDEVICEOBJECTINSTANCEW);
     res = MAKELONG( HID_USAGE_GENERIC_X, HID_USAGE_PAGE_GENERIC );

[PATCH 2/2] dinput: Fix EnumObjects callback return value handling.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=131412

Your paranoid android.

=== w7u_adm (32 bit report) ===

dinput:
device8: Timeout

[Florian Will (＠w-flo)]

On Mon Apr  3 06:49:14 2023 +0000, **** wrote:
> Marvin replied on the mailing list:
> Hi,
> It looks like your patch introduced the new failures shown below.
> Please investigate and fix them before resubmitting your patch.
> If they are not new, fixing them anyway would help a lot. Otherwise
> please ask for the known failures list to be updated.
> The tests also ran into some preexisting test failures. If you know how
> to fix them that would be helpful. See the TestBot job for the details:
> The full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=131412
> Your paranoid android.
> === w7u_adm (32 bit report) ===
> dinput:
> device8: Timeout

I'm not sure if my patch to the test could have possibly introduced this new failure? I mean, sure, the behavior when returning -1 from the callback instead of DIENUM_CONTINUE / DIENUM_STOP is not specified in the docs. So I guess win7 would be free to do something that would end up e.g. blocking the thread. I suspect that behavior would be pretty unlikely though, and [it didn't reproduce when I manually resubmitted the test](https://testbot.winehq.org/JobDetails.pl?Key=131414).

[Rémi Bernon (＠rbernon)]

On Mon Apr  3 07:07:22 2023 +0000, Florian Will wrote:
> I'm not sure if my patch to the test could have possibly introduced this
> new failure? I mean, sure, the behavior when returning -1 from the
> callback instead of DIENUM_CONTINUE / DIENUM_STOP is not specified in
> the docs. So I guess win7 would be free to do something that would end
> up e.g. blocking the thread. I suspect that behavior would be pretty
> unlikely though, and [it didn't reproduce when I manually resubmitted
> the test](https://testbot.winehq.org/JobDetails.pl?Key=131414).

Don't worry, this sometimes happens for various reasons.

[PATCH 0/2] MR2560: dinput: Add test and fix EnumObjects callback return value handling. - approved

[Rémi Bernon (＠rbernon)]

This merge request was approved by Rémi Bernon.