On 4/26/21 6:28 PM, Zebediah Figura (she/her) wrote:
On 4/26/21 5:37 AM, Rémi Bernon wrote:
From: Arkadiusz Hiler ahiler@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead.
It's maybe not exactly ideal, but it's also not clear how to map setupapi errors to NTSTATUS, and moreover it probably doesn't matter anyway (we don't expect those functions to fail).
I don't think we can implement setupapi functions on top of ntoskrnl ones; that crosses the kernel boundary (and there's no clear syscall interface for it). Probably on Windows the code is just duplicated.
FWIW, we have a PnP test driver now, so it's actually possible to write tests for this function.
In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary.
I think it's correct as-is to move it—as soon as we call IoInvalidateDeviceRelations() the device can be exposed.
Actually, to be completely correct (i.e. if we were running this driver in Windows), I think we should treat it as possible to receive IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be doing is to initialize everything necessary *before* the statement "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at IRP_MN_START_DEVICE (on the child PDO).
It's actually causing IoSetDevicePropertyData to fail the first time a device is detected (so on prefix creation for both mouse and keyboard devices), as setupapi doesn't know about it until we call IoInvalidateDeviceRelations.
Note however, although it fails to set the property, as SetupDiDestroyDeviceInfoList clears the last error, it still returns success, and the device initialization continues.
I think it may be better to move it after, as it's less likely that anything happens in between, while missing the property is going to make rawinput device enumeration skip the device. But it's just a workaround.