Re: [v2 2/4] dinput: Handle username in EnumDevicesBySemantics.

[Andrew Eikum]

Thanks for fixing these up, they look good to me. However this patch
introduces a test failure:

dinput.c:573: Test succeeded inside todo block: No device should be available after action mapping available=0
dinput.c:596: Test succeeded inside todo block: This user should own no devices owned=0

This is obviously fixed by your 4/4 patch, but we like to keep the
tests clean in every commit. It's fine to squash the test changes into
the code changes to create a 2-patch sequence with no test failures.
Then I think it should be good to go in.

Thanks,
Andrew

On Thu, Mar 02, 2017 at 12:11:24AM +0200, Jetro Jormalainen wrote:
> EnumDevicesBySemantics enums only devices with given username when
> DIEDBSFL_THISUSER is set and only unowned devices when
> DIEDBSFL_AVAILABLEDEVICES is set.
>
> v2: Fixed problems noted by Andrew Eikum.
>
> Tested on Arch Linux and Rally Trophy Demo.
> Signed-off-by: Jetro Jormalainen
>
> dlls/dinput/dinput_main.c | 111 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 96 insertions(+), 15 deletions(-)
>
> diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c
> index 2bf9f38ba4..a59217472c 100644
> --- a/dlls/dinput/dinput_main.c
> +++ b/dlls/dinput/dinput_main.c
> @@ -923,6 +923,47 @@ static HRESULT WINAPI IDirectInput8WImpl_FindDevice(LPDIRECTINPUT8W iface, REFGU
>      return IDirectInput2WImpl_FindDevice( &This->IDirectInput7W_iface, rguid, pszName, pguidInstance );
>  }
>
> +static BOOL should_enumerate_device(const WCHAR *username, DWORD dwFlags,
> - struct list *device_players, REFGUID guid)
>
> +{
> - BOOL should_enumerate = TRUE;
> - struct DevicePlayer *device_player;
> -
> - /* Check if user owns this device */
> - if (dwFlags & DIEDBSFL_THISUSER && username && *username)
> - {
> -
>    should_enumerate = FALSE;
> -
>    LIST_FOR_EACH_ENTRY(device_player, device_players, struct DevicePlayer, entry)
> -
>    {
> -
>        if (IsEqualGUID(&device_player->instance_guid, guid))
> -
>        {
> -
>            if (*device_player->username && !lstrcmpW(username, device_player->username))
> -
>                return TRUE; /* Device username matches */
> -
>            break;
> -
>        }
> -
>    }
> - }
> -
> - /* Check if this device is not owned by anyone */
> - if (dwFlags & DIEDBSFL_AVAILABLEDEVICES) {
> -
>    BOOL found = FALSE;
> -
>    should_enumerate = FALSE;
> -
>    LIST_FOR_EACH_ENTRY(device_player, device_players, struct DevicePlayer, entry)
> -
>    {
> -
>        if (IsEqualGUID(&device_player->instance_guid, guid))
> -
>        {
> -
>            if (*device_player->username)
> -
>                found = TRUE;
> -
>            break;
> -
>        }
> -
>    }
> -
>    if (!found)
> -
>        return TRUE; /* Device does not have a username */
> - }
> -
> - return should_enumerate;
>
> +}
> -
>
> static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>        LPDIRECTINPUT8A iface, LPCSTR ptszUserName, LPDIACTIONFORMATA lpdiActionFormat,
>        LPDIENUMDEVICESBYSEMANTICSCBA lpCallback,
> @@ -939,6 +980,7 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>      int device_count = 0;
>      int remain;
>      DIDEVICEINSTANCEA *didevis = 0;
> -
>
> WCHAR *username_w = 0;
>
> FIXME("(this=%p,%s,%p,%p,%p,%04x): semi-stub\n", This, debugstr_a(ptszUserName), lpdiActionFormat,
>       lpCallback, pvRef, dwFlags);
>
> @@ -955,6 +997,14 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>  didevi.dwSize = sizeof(didevi);
>
> - if (ptszUserName)
> - {
> -
>    int len = MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, 0, 0);
> -
> -
>    username_w = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR)*len);
> -
>    MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, username_w, len);
> - }
> - /* Enumerate all the joysticks */
>    for (i = 0; i < NB_DINPUT_DEVICES; i++)
>    {
>
> @@ -968,7 +1018,8 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>          /* Default behavior is to enumerate attached game controllers */
>          enumSuccess = dinput_devices[i]->enum_deviceA(DI8DEVCLASS_GAMECTRL, DIEDFL_ATTACHEDONLY | dwFlags, &didevi, This->dwVersion, j);
> -
>        if (enumSuccess == S_OK)
> -
>        if (enumSuccess == S_OK &&
> -
>            should_enumerate_device(username_w, dwFlags, &This->device_players, &didevi.guidInstance))
>        {
>            if (device_count++)
>                didevis = HeapReAlloc(GetProcessHeap(), 0, didevis, sizeof(DIDEVICEINSTANCEA)*device_count);
>
> @@ -980,8 +1031,15 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>      }
>  remain = device_count;
> - /* Add keyboard and mouse to remaining device count */
> if (!(dwFlags & DIEDBSFL_FORCEFEEDBACK))
> -
>    remain += sizeof(guids)/sizeof(guids[0]);
> -
>
> {
> -
>    for (i = 0; i < sizeof(guids) / sizeof(guids[0]); i++)
> -
>    {
> -
>        if (should_enumerate_device(username_w, dwFlags, &This->device_players, guids[i]))
> -
>            remain++;
> -
>    }
> -
>
> }
>
> for (i = 0; i < device_count; i++)
> {
>
> @@ -991,26 +1049,38 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics(
>          if (lpCallback(&didevis[i], lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP)
>          {
>              HeapFree(GetProcessHeap(), 0, didevis);
> -
>        HeapFree(GetProcessHeap(), 0, username_w);
>        return DI_OK;
>    }
>
> }
>
> HeapFree(GetProcessHeap(), 0, didevis);
> - if (dwFlags & DIEDBSFL_FORCEFEEDBACK) return DI_OK;
> -
>
> if (dwFlags & DIEDBSFL_FORCEFEEDBACK)
> -
>
> {
> -
>    HeapFree(GetProcessHeap(), 0, username_w);
> -
>    return DI_OK;
> -
>
> }
>
> /* Enumerate keyboard and mouse */
> for(i=0; i < sizeof(guids)/sizeof(guids[0]); i++)
> {
> -
>    callbackFlags = diactionformat_priorityA(lpdiActionFormat, actionMasks[i]);
> -
>    if (should_enumerate_device(username_w, dwFlags, &This->device_players, guids[i]))
> -
>    {
> -
>        callbackFlags = diactionformat_priorityA(lpdiActionFormat, actionMasks[i]);
>
> -
>    IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL);
> -
>    IDirectInputDevice_GetDeviceInfo(lpdid, &didevi);
> -
>        IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL);
> -
>        IDirectInputDevice_GetDeviceInfo(lpdid, &didevi);
>
> -
>    if (lpCallback(&didevi, lpdid, callbackFlags, sizeof(guids)/sizeof(guids[0]) - (i+1), pvRef) == DIENUM_STOP)
> -
>        return DI_OK;
> -
>        if (lpCallback(&didevi, lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP)
> -
>        {
> -
>            HeapFree(GetProcessHeap(), 0, username_w);
> -
>            return DI_OK;
> -
>        }
> -
>    }
>
> }
> -
>
> HeapFree(GetProcessHeap(), 0, username_w);
> return DI_OK;
>
> }
>
> @@ -1049,7 +1119,8 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics(
>          /* Default behavior is to enumerate attached game controllers */
>          enumSuccess = dinput_devices[i]->enum_deviceW(DI8DEVCLASS_GAMECTRL, DIEDFL_ATTACHEDONLY | dwFlags, &didevi, This->dwVersion, j);
> -
>        if (enumSuccess == S_OK)
> -
>        if (enumSuccess == S_OK &&
> -
>            should_enumerate_device(ptszUserName, dwFlags, &This->device_players, &didevi.guidInstance))
>        {
>            if (device_count++)
>                didevis = HeapReAlloc(GetProcessHeap(), 0, didevis, sizeof(DIDEVICEINSTANCEW)*device_count);
>
> @@ -1061,8 +1132,15 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics(
>      }
>  remain = device_count;
> - /* Add keyboard and mouse to remaining device count */
> if (!(dwFlags & DIEDBSFL_FORCEFEEDBACK))
> -
>    remain += sizeof(guids)/sizeof(guids[0]);
> -
>
> {
> -
>    for (i = 0; i < sizeof(guids) / sizeof(guids[0]); i++)
> -
>    {
> -
>        if (should_enumerate_device(ptszUserName, dwFlags, &This->device_players, guids[i]))
> -
>            remain++;
> -
>    }
> -
>
> }
>
> for (i = 0; i < device_count; i++)
> {
>
> @@ -1083,13 +1161,16 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics(
>      /* Enumerate keyboard and mouse */
>      for(i=0; i < sizeof(guids)/sizeof(guids[0]); i++)
>      {
> -
>    callbackFlags = diactionformat_priorityW(lpdiActionFormat, actionMasks[i]);
> -
>    if (should_enumerate_device(ptszUserName, dwFlags, &This->device_players, guids[i]))
> -
>    {
> -
>        callbackFlags = diactionformat_priorityW(lpdiActionFormat, actionMasks[i]);
>
> -
>    IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL);
> -
>    IDirectInputDevice_GetDeviceInfo(lpdid, &didevi);
> -
>        IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL);
> -
>        IDirectInputDevice_GetDeviceInfo(lpdid, &didevi);
>
> -
>    if (lpCallback(&didevi, lpdid, callbackFlags, sizeof(guids)/sizeof(guids[0]) - (i+1), pvRef) == DIENUM_STOP)
> -
>        return DI_OK;
> -
>        if (lpCallback(&didevi, lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP)
> -
>            return DI_OK;
> -
>    }
>
> }
>
> return DI_OK;
>
> -- 
> 2.11.0