Hello Alistair,
On 1/11/20 9:28 PM, Alistair Leslie-Hughes wrote:
When a program calls SetProperty with DIPROP_BUFFERSIZE, dinput records this value for GetProperty but only uses it when the device can support that number of buffers otherwise a max value.
In the case of game "Far Cry 5", it passes through (DWORD)-1 which cause HeapAlloc to fail ((DWORD)-1 * sizeof(DIDEVICEOBJECTDATA)).
Since there is no real way of working out the max value, I've capped it at 20.
MSDN reference. https://docs.microsoft.com/en-us/previous-versions/windows/desktop/ee417908(...)
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45732
Signed-off-by: Alistair Leslie-Hughes leslie_alistair@hotmail.com
"Default" strikes me as a sort of odd way to characterize this. Maybe something like "dinput: Cap the buffer size to 100"?
dlls/dinput/device.c | 14 ++++++++++---- dlls/dinput/device_private.h | 3 ++- dlls/dinput/tests/device.c | 20 +++++++++++++++++--- 3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 28329d03b5..ab6f4e6896 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -1307,7 +1307,7 @@ HRESULT WINAPI IDirectInputDevice2WImpl_GetProperty(LPDIRECTINPUTDEVICE8W iface,
if (pdiph->dwSize != sizeof(DIPROPDWORD)) return DIERR_INVALIDPARAM;
pd->dwData = This->queue_len;
pd->dwData = This->buffersize; TRACE("buffersize = %d\n", pd->dwData); break; }@@ -1396,12 +1396,18 @@ HRESULT WINAPI IDirectInputDevice2WImpl_SetProperty( TRACE("buffersize = %d\n", pd->dwData);
EnterCriticalSection(&This->crit);
This->buffersize = pd->dwData;This->queue_len = This->buffersize > 20 ? 20 : This->buffersize;
"min(This->buffersize, 20)" might be more readable?
if (This->buffersize > 20)WARN("Trying to set large buffer size %d\n", pd->dwData);
Is this warning necessary? It seems like asking for a buffer "as big as possible" is a not unreasonable thing for an application to do, given that the maximum length is documented.
HeapFree(GetProcessHeap(), 0, This->data_queue);
This->data_queue = !pd->dwData ? NULL : HeapAlloc(GetProcessHeap(), 0,pd->dwData * sizeof(DIDEVICEOBJECTDATA));
This->data_queue = !This->queue_len ? NULL : HeapAlloc(GetProcessHeap(), 0,This->queue_len * sizeof(DIDEVICEOBJECTDATA)); This->queue_head = This->queue_tail = This->overflow = 0;
This->queue_len = pd->dwData; LeaveCriticalSection(&This->crit); break;diff --git a/dlls/dinput/device_private.h b/dlls/dinput/device_private.h index 27e9c26286..23d9e2eebc 100644 --- a/dlls/dinput/device_private.h +++ b/dlls/dinput/device_private.h @@ -71,10 +71,11 @@ struct IDirectInputDeviceImpl DI_EVENT_PROC event_proc; /* function to receive mouse & keyboard events */
LPDIDEVICEOBJECTDATA data_queue; /* buffer for 'GetDeviceData'. */
- int queue_len; /* size of the queue - set in 'SetProperty' */
int queue_len; /* valid size of the queue */ int queue_head; /* position to write new event into queue */ int queue_tail; /* next event to read from queue */ BOOL overflow; /* return DI_BUFFEROVERFLOW in 'GetDeviceData' */
DWORD buffersize; /* size of the queue - set in 'SetProperty' */
DataFormat data_format; /* user data format and wine to user format converter */
diff --git a/dlls/dinput/tests/device.c b/dlls/dinput/tests/device.c index a2a5a65686..5232a7b813 100644 --- a/dlls/dinput/tests/device.c +++ b/dlls/dinput/tests/device.c @@ -71,7 +71,7 @@ static BOOL CALLBACK enum_type_callback(const DIDEVICEOBJECTINSTANCEA *oi, void return DIENUM_CONTINUE; }
-static void test_object_info(IDirectInputDeviceA *device, HWND hwnd) +static void test_object_info(IDirectInputDeviceA *device, HWND hwnd, int size) { HRESULT hr; DIPROPDWORD dp; @@ -106,8 +106,22 @@ static void test_object_info(IDirectInputDeviceA *device, HWND hwnd) dp.diph.dwHeaderSize = sizeof(DIPROPHEADER); dp.diph.dwHow = DIPH_DEVICE; dp.diph.dwObj = 0;
- dp.dwData = -1;
Since "dwData" is unsigned, I feel like UINT_MAX is a bit more appropriate here.
hr = IDirectInputDevice_GetProperty(device, DIPROP_BUFFERSIZE, &dp.diph);
ok(hr == DI_OK, "Failed: %08x\n", hr);
ok(dp.dwData == size, "got %d\n", dp.dwData);
dp.dwData = -1;
hr = IDirectInputDevice_SetProperty(device, DIPROP_BUFFERSIZE, (LPCDIPROPHEADER)&dp.diph);
ok(hr == DI_OK, "SetProperty() failed: %08x\n", hr);
dp.dwData = 0;
hr = IDirectInputDevice_GetProperty(device, DIPROP_BUFFERSIZE, &dp.diph);
ok(hr == DI_OK, "Failed: %08x\n", hr);
ok(dp.dwData == -1, "got %d\n", dp.dwData);
dp.dwData = 0; hr = IDirectInputDevice_SetProperty(device, DIPROP_BUFFERSIZE, (LPCDIPROPHEADER)&dp.diph); ok(hr == DI_OK, "SetProperty() failed: %08x\n", hr); cnt = 5;
@@ -191,13 +205,13 @@ static BOOL CALLBACK enum_devices(const DIDEVICEINSTANCEA *lpddi, void *pvRef)
hr = IUnknown_QueryInterface(device, &IID_IDirectInputDevice2A, (LPVOID*)&obj); ok(SUCCEEDED(hr), "IUnknown_QueryInterface(IID_IDirectInputDevice2A) failed: %08x\n", hr);
test_object_info(obj, data->hwnd);
test_object_info(obj, data->hwnd, 0); IUnknown_Release(obj); obj = NULL; hr = IUnknown_QueryInterface(device, &IID_IDirectInputDevice2W, (LPVOID*)&obj); ok(SUCCEEDED(hr), "IUnknown_QueryInterface(IID_IDirectInputDevice2W) failed: %08x\n", hr);
test_object_info(obj, data->hwnd);
test_object_info(obj, data->hwnd, 20); IUnknown_Release(obj); IUnknown_Release(device);
I guess this is because the first test later sets the size to 20, but that's not obvious just from reading enum_devices()—it looks like an actual difference between the A and W interfaces. Maybe it'd be better to reset the size to 0 after the first test? Or create the device twice in enum_devices() instead of querying two interfaces from the same device?
Though for that matter, why exactly are we passing a W interface to a function that takes an A interface? This test seems kind of broken...