On 8/30/21 6:22 PM, Zebediah Figura (she/her) wrote:
On 8/27/21 3:55 PM, Rémi Bernon wrote:
On 8/27/21 10:31 PM, Zebediah Figura (she/her) wrote:
wcsrchr(device_id, '\') + 1); wcscpy(ext->instance_id, instance_id); + ext->interface_guid = !wcsncmp(device_id, L"XINPUT\", 7) ? &GUID_DEVINTERFACE_XINPUT + : &GUID_DEVINTERFACE_HID;
It's a matter of taste, I guess, but personally if I have to wrap a ternary operator, I take it as a sign I should be using if/else instead ;-)
Yeah, I spent a lot of time hesitating on the best looking way to write that.
It also occurs to me that since this is very internal it might be a good idea to use WINEXINPUT or something rather than just XINPUT. Unlikely to make a difference, of course, but it also sort of communicates that this is internal, and more clearly tells anyone looking for the code where to look for the driver that reports WINEXINPUT devices (viz. elsewhere in the code tree.)
Works for me, I'm still not convinced that winexinput.sys is much clearer though, but I can probably live with it.
I'm fine with xinput.sys, but I'd rather see the device ID named WINEXINPUT.
That's what I did in the new version, although the driver is also named winexinput.sys now. I think it'd be less confusing that way.
@@ -52,14 +54,24 @@ struct device_state { DEVICE_OBJECT *bus_pdo; DEVICE_OBJECT *gamepad_pdo; + DEVICE_OBJECT *xinput_pdo; WCHAR bus_id[MAX_DEVICE_ID_LEN]; WCHAR instance_id[MAX_DEVICE_ID_LEN];
+ /* everything below requires holding the cs */ + CRITICAL_SECTION cs; + IRP *pending_read; + BOOL pending_is_gamepad;
Honestly after thinking about it I don't dislike "internal" as much I dislike "xinput" now. Because you'd think that "xinput" refers to the device we access from xinput, but actually it's the other way around.
It's still the case there, but eventually (ie: in an eventual PATCH 7/6 where xinput is switched to the internal device interface class), this is how it will indeed be. So to me it's actually intuitive.
Well, right, I'm talking about the eventual situation.
And, I hate to say it, but "to me it's actually intuitive" isn't great :-(
In any case, I don't think the driver is supposed to become more complicated than these patches (it just misses the report translation and that's it), it should be quite straightforward to get the whole picture. With a few comments maybe?
I think a comment near the top is definitely going to be necessary regardless of what terminology we eventually use.
My logic with "real"/"fake" is that the "real" device reports the "real" data that we get from winebus; the "fake" device fixes it up and hides some of it. But yeah, it's clearly not unambiguous either. The best thing it has going for it is that they're clearly attributes of the child devices rather than namespace prefixes or referring to internal IOCTLs or anything else.
In the new version I kept the names "gamepad_device" -for the bogus HID gamepad- with the "IG_" (as in I-something G-amepad) suffix, and "xinput_device" which will have an "XI_" (as in X-I-nput) suffix to distinguish them. Both will have the WINEXINPUT\ bus namespace.
I think it's the best I can come up with that doesn't sound like something too generic (fake/real is completely generic IMHO and could mean anything depending on the reader current understanding).
Cheers,