From: Arkadiusz Hiler <ahiler@codeweavers.com>
Subject: Re: [PATCH 1/3] RawInput: Uppercase the device names up to the GUID.
Message-Id: <20210224085006.pvv7s5nuuhwpt42l@failcraft.local>
Date: Wed, 24 Feb 2021 10:50:06 +0200
In-Reply-To: <e4d765e6-8622-ab4a-465f-c04eb34b34c0@codeweavers.com>
References: <20210222115038.359258-1-ahiler@codeweavers.com> <e4d765e6-8622-ab4a-465f-c04eb34b34c0@codeweavers.com>

On Wed, Feb 24, 2021 at 08:55:07AM +0100, Rémi Bernon wrote:
> On 2/22/21 12:50 PM, Arkadiusz Hiler wrote:
> > This is a preparation for a patch that changes setupapi to return lowercased
> > device paths, which is the source of RawInput device names. On Windows
> > RawInput returns mostly uppercase names (except the GUID part) and we want to
> > keep Wine's old behavior in this regards as SDL 2.0.14+ looks for uppercase IG_
> > to match xinput devices to their RawInput counterparts.
> > 
> > Signed-off-by: Arkadiusz Hiler <ahiler@codeweavers.com>
> > ---
> >   dlls/user32/rawinput.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c
> > index ba11a121bc5..13aac0278d5 100644
> > --- a/dlls/user32/rawinput.c
> > +++ b/dlls/user32/rawinput.c
> > @@ -94,7 +94,7 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface)
> >       SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail;
> >       struct device *device;
> >       HANDLE file;
> > -    WCHAR *path;
> > +    WCHAR *path, *pos;
> >       DWORD size;
> >       SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, NULL);
> > @@ -121,6 +121,9 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface)
> >       }
> >       heap_free(detail);
> > +    /* upper case everything but the GUID */
> > +    for (pos = path; *pos && *pos != '{'; pos++) *pos = towupper(*pos);
> > +
> >       file = CreateFileW(path, GENERIC_READ | GENERIC_WRITE,
> >               FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0);
> >       if (file == INVALID_HANDLE_VALUE)
> > 
> 
> Hi Arek!
> 
> I'd think a test to validate this could be nice. I know we don't have any
> gamepad device available on the test bot, but maybe it's still possible to
> validate at least with with mouse / keyboard devices? It would also let us
> check the test manually on a real Windows with a gamepad plugged in.

Hey,

Thanks for the feedback.

Looking at this a bit closer there seems to be more going on with the
paths on Windows:

rawinput:    \\?\HID#VID_054C&PID_0CE6&MI_03#7&207751a1&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
setupapi:    \\?\hid#vid_054c&pid_0ce6&mi_03#7&207751a1&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
instance_id:     HID\VID_054C&PID_0CE6&MI_03\7&207751A1&0&0000

or

rawinput:    \\?\HID#VID_046D&PID_C52B&MI_02&Col01#7&25fe7032&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
setupapi:    \\?\hid#vid_046d&pid_c52b&mi_01&col01#7&392e8c70&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
instance_id:     HID\VID_046D&PID_C52B&MI_01&COL01\7&392E8C70&0&0000

It's not even uppercasing up to second '#'. It looks like there's
another source of path/instance_id that is case sensitive (Col01 vs
COL01). So having a test that checks if everything is uppercase would
fail on Windows.

I would treat the patch in current form more as "keeping the old
behavior that we know some software depends on[0]" instead of "fixing
this once and for all".

[0]: https://github.com/libsdl-org/SDL/blob/fadfa5/src/joystick/windows/SDL_rawinputjoystick.c#L696

I'll do a bit more digging in the registry.

> Also, the commit title should probably be "user32:", not "RawInput:".

Not sure why I did that... I'll fix it in the next revision.

-- 
Cheers,
Arek