[PATCH v6 0/7] MR10604: setupapi, ntoskrnl: Implement CM_Get_Parent with device tree properties
Wine Bug: https://bugs.winehq.org/show_bug.cgi?id=59604 Implement CM_Get_Parent by storing device tree relationship properties when ntoskrnl enumerates child devices via IRP_MN_QUERY_DEVICE_RELATIONS: - DEVPKEY_Device_Parent on each child device - DEVPKEY_Device_Children on the parent device - DEVPKEY_Device_Siblings on each child device CM_Get_Parent reads DEVPKEY_Device_Parent through SetupDiGetDevicePropertyW. Also fix SETUPDI_EnumerateMatchingInterfaces to remove devices without interfaces from DIGCF_DEVICEINTERFACE enumeration results. --\> no more phantom devices Based on discussion with Rémi Bernon (Bug 59604) Tested on a fresh Wine 11.6 setup with KNX ETS 6.4.1: A USB HID license dongle (VID 2A07, PID 0102) is now correctly identified through CM_Get_Parent device tree navigation. -- v6: ntoskrnl/tests: Add DEVPKEY_Device_Parent test. setupapi/tests: Add CM_Get_Parent tests. cfgmgr32: Implement CM_Get_Parent using DEVPKEY_Device_Parent property. ntoskrnl: Store DEVPKEY_Device_Siblings on each child after bus enumeration. ntoskrnl: Store DEVPKEY_Device_Children on parent after bus enumeration. ntoskrnl: Store DEVPKEY_Device_Parent when enumerating child devices. setupapi: Remove devices without interfaces from DIGCF_DEVICEINTERFACE enumeration. https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> When SetupDiGetClassDevs is called with DIGCF_DEVICEINTERFACE, SETUPDI_EnumerateMatchingInterfaces adds all devices from the registry to the device info set, even those that have no matching device interfaces. This causes SetupDiEnumDeviceInfo to return devices that SetupDiEnumDeviceInterfaces would not return, breaking applications that assume both enumerations are synchronized. Remove devices from the set if SETUPDI_AddDeviceInterfaces did not create any interfaces for them. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/setupapi/devinst.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index f6bfcc483b0..a3fd97f2cc5 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -2037,7 +2037,11 @@ static void SETUPDI_EnumerateMatchingInterfaces(HDEVINFO DeviceInfoSet, UuidFromStringW(&deviceClassStr[1], &deviceClass); if ((device = create_device(set, &deviceClass, deviceInst, FALSE))) + { SETUPDI_AddDeviceInterfaces(device, subKey, guid, flags); + if (list_empty(&device->interfaces)) + delete_device(device); + } } RegCloseKey(deviceKey); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> When handle_bus_relations enumerates child devices via IRP_MN_QUERY_DEVICE_RELATIONS, store the parent's device instance ID as DEVPKEY_Device_Parent on each child device. This enables CM_Get_Parent to look up the parent device node through the standard device property mechanism. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/pnp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 56e378cf770..4bc7aa33af3 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -379,7 +379,7 @@ static void start_device( DEVICE_OBJECT *device, HDEVINFO set, SP_DEVINFO_DATA * create_dyn_data_key( device ); } -static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) +static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set, DEVICE_OBJECT *parent_device ) { static const WCHAR infpathW[] = {'I','n','f','P','a','t','h',0}; @@ -434,6 +434,13 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) ExFreePool( id ); } + { + WCHAR parent_id[MAX_DEVICE_ID_LEN]; + if (!get_device_instance_id( parent_device, parent_id )) + SetupDiSetDevicePropertyW( set, &sp_device, &DEVPKEY_Device_Parent, DEVPROP_TYPE_STRING, + (BYTE *)parent_id, (wcslen( parent_id ) + 1) * sizeof(WCHAR), 0 ); + } + if (need_driver && !install_device_driver( device, set, &sp_device ) && !caps.RawDeviceOK) { ERR("Unable to install a function driver for device %s.\n", debugstr_w(device_instance_id)); @@ -535,7 +542,7 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) if (!wine_parent->children || !device_in_list( wine_parent->children, child )) { TRACE("Adding new device %p.\n", child); - enumerate_new_device( child, set ); + enumerate_new_device( child, set, parent ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> After enumerating child devices via IRP_MN_QUERY_DEVICE_RELATIONS, store the list of child device instance IDs as DEVPKEY_Device_Children on the parent device. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/pnp.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 4bc7aa33af3..25707ed9eb1 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -564,6 +564,40 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) ExFreePool( wine_parent->children ); wine_parent->children = relations; + { + WCHAR parent_id[MAX_DEVICE_ID_LEN]; + SP_DEVINFO_DATA parent_sp = {sizeof(parent_sp)}; + WCHAR (*child_ids)[MAX_DEVICE_ID_LEN] = malloc( relations->Count * sizeof(*child_ids) ); + DWORD count = relations->Count; + + for (i = 0; i < count; ++i) + get_device_instance_id( relations->Objects[i], child_ids[i] ); + + if (count && !get_device_instance_id( parent, parent_id ) + && SetupDiOpenDeviceInfoW( set, parent_id, NULL, 0, &parent_sp )) + { + DWORD multi_len = 1; + WCHAR *children_multi, *p; + + for (i = 0; i < count; ++i) + multi_len += wcslen( child_ids[i] ) + 1; + + children_multi = malloc( multi_len * sizeof(WCHAR) ); + p = children_multi; + for (i = 0; i < count; ++i) + { + wcscpy( p, child_ids[i] ); + p += wcslen( child_ids[i] ) + 1; + } + *p = 0; + SetupDiSetDevicePropertyW( set, &parent_sp, &DEVPKEY_Device_Children, + DEVPROP_TYPE_STRING_LIST, (BYTE *)children_multi, + multi_len * sizeof(WCHAR), 0 ); + free( children_multi ); + } + free( child_ids ); + } + SetupDiDestroyDeviceInfoList( set ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> After enumerating child devices via IRP_MN_QUERY_DEVICE_RELATIONS, store the list of sibling device instance IDs (all other children of the same parent) as DEVPKEY_Device_Siblings on each child device. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/pnp.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 25707ed9eb1..5096b646b58 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -598,6 +598,44 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) free( child_ids ); } + { + WCHAR (*child_ids)[MAX_DEVICE_ID_LEN] = malloc( relations->Count * sizeof(*child_ids) ); + DWORD count = relations->Count; + + for (i = 0; i < count; ++i) + get_device_instance_id( relations->Objects[i], child_ids[i] ); + + for (i = 0; i < count; ++i) + { + SP_DEVINFO_DATA child_sp = {sizeof(child_sp)}; + if (SetupDiOpenDeviceInfoW( set, child_ids[i], NULL, 0, &child_sp )) + { + DWORD sib_len = 1, j; + WCHAR *siblings_multi, *p; + + for (j = 0; j < count; ++j) + if (j != i) sib_len += wcslen( child_ids[j] ) + 1; + + siblings_multi = malloc( sib_len * sizeof(WCHAR) ); + p = siblings_multi; + for (j = 0; j < count; ++j) + { + if (j != i) + { + wcscpy( p, child_ids[j] ); + p += wcslen( child_ids[j] ) + 1; + } + } + *p = 0; + SetupDiSetDevicePropertyW( set, &child_sp, &DEVPKEY_Device_Siblings, + DEVPROP_TYPE_STRING_LIST, (BYTE *)siblings_multi, + sib_len * sizeof(WCHAR), 0 ); + free( siblings_multi ); + } + } + free( child_ids ); + } + SetupDiDestroyDeviceInfoList( set ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> Implement CM_Get_Parent by reading the DEVPKEY_Device_Parent property that ntoskrnl stores when enumerating child devices. Move the implementation from setupapi (which was a stub) to cfgmgr32, and have setupapi forward to it. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/cfgmgr32/cfgmgr32.c | 44 ++++++++++++++++++++++++++++--------- dlls/cfgmgr32/cfgmgr32.spec | 2 +- dlls/setupapi/setupapi.spec | 2 +- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/dlls/cfgmgr32/cfgmgr32.c b/dlls/cfgmgr32/cfgmgr32.c index f69b39c8658..51eb39f3f1f 100644 --- a/dlls/cfgmgr32/cfgmgr32.c +++ b/dlls/cfgmgr32/cfgmgr32.c @@ -1785,6 +1785,40 @@ CONFIGRET WINAPI CM_Locate_DevNodeA( DEVINST *node, DEVINSTID_A instance_id, ULO return CM_Locate_DevNode_ExA( node, instance_id, flags, NULL ); } +/*********************************************************************** + * CM_Get_Parent (cfgmgr32.@) + */ +CONFIGRET WINAPI CM_Get_Parent( DEVINST *parent, DEVINST node, ULONG flags ) +{ + WCHAR parent_id[MAX_DEVICE_ID_LEN]; + DEVPROPTYPE type; + struct device dev; + struct property prop; + DWORD size = sizeof(parent_id); + + TRACE( "parent %p, node %#lx, flags %#lx\n", parent, node, flags ); + + if (!parent) return CR_INVALID_POINTER; + *parent = 0; + + if (devnode_get_device( node, &dev )) return CR_INVALID_DEVNODE; + + init_property( &prop, &DEVPKEY_Device_Parent, &type, (BYTE *)parent_id, &size, TRUE ); + if (get_device_property( HKEY_LOCAL_MACHINE, &dev, &prop )) return CR_NO_SUCH_DEVNODE; + + return CM_Locate_DevNodeW( parent, parent_id, 0 ); +} + +/*********************************************************************** + * CM_Get_Parent_Ex (cfgmgr32.@) + */ +CONFIGRET WINAPI CM_Get_Parent_Ex( DEVINST *parent, DEVINST node, ULONG flags, HMACHINE machine ) +{ + TRACE( "parent %p, node %#lx, flags %#lx, machine %p\n", parent, node, flags, machine ); + if (machine) FIXME( "machine %p not implemented!\n", machine ); + return CM_Get_Parent( parent, node, flags ); +} + /*********************************************************************** * CM_Get_Device_ID_Size_Ex (cfgmgr32.@) */ @@ -2053,16 +2087,6 @@ CONFIGRET WINAPI CM_Get_Child( DEVINST *child, DEVINST node, ULONG flags ) return CM_Get_Child_Ex( child, node, flags, NULL ); } -/*********************************************************************** - * CM_Get_Parent (cfgmgr32.@) - */ -DWORD WINAPI CM_Get_Parent( DEVINST *parent, DEVINST child, ULONG flags ) -{ - FIXME( "parent %p, child %#lx, flags %#lx stub!\n", parent, child, flags ); - if (parent) *parent = 0; - return CR_NO_SUCH_DEVNODE; -} - /*********************************************************************** * CM_Create_DevNodeW (cfgmgr32.@) */ diff --git a/dlls/cfgmgr32/cfgmgr32.spec b/dlls/cfgmgr32/cfgmgr32.spec index df23120954c..7a0063f2366 100644 --- a/dlls/cfgmgr32/cfgmgr32.spec +++ b/dlls/cfgmgr32/cfgmgr32.spec @@ -143,7 +143,7 @@ @ stub CM_Get_Next_Res_Des @ stub CM_Get_Next_Res_Des_Ex @ stdcall CM_Get_Parent(ptr long long) -@ stub CM_Get_Parent_Ex +@ stdcall CM_Get_Parent_Ex(ptr long long ptr) @ stub CM_Get_Res_Des_Data @ stub CM_Get_Res_Des_Data_Ex @ stub CM_Get_Res_Des_Data_Size diff --git a/dlls/setupapi/setupapi.spec b/dlls/setupapi/setupapi.spec index 908019b6103..bb0bdb3497d 100644 --- a/dlls/setupapi/setupapi.spec +++ b/dlls/setupapi/setupapi.spec @@ -120,7 +120,7 @@ @ stub CM_Get_Next_Res_Des @ stub CM_Get_Next_Res_Des_Ex @ stdcall CM_Get_Parent(ptr long long) cfgmgr32.CM_Get_Parent -@ stub CM_Get_Parent_Ex +@ stdcall CM_Get_Parent_Ex(ptr long long ptr) cfgmgr32.CM_Get_Parent_Ex @ stub CM_Get_Res_Des_Data @ stub CM_Get_Res_Des_Data_Ex @ stub CM_Get_Res_Des_Data_Size -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> Test parameter validation (NULL pointer) and parent lookup for a registered device. Devices created manually via SetupDiCreateDeviceInfo may not have DEVPKEY_Device_Parent set, so CR_NO_SUCH_DEVNODE is accepted alongside CR_SUCCESS. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/setupapi/tests/devinst.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 605b0257667..b5d02d56a7b 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -2840,7 +2840,8 @@ static void test_devnode(void) { HDEVINFO set; SP_DEVINFO_DATA device = { sizeof(SP_DEVINFO_DATA) }; - char buffer[50]; + char buffer[200]; + DEVINST parent; DWORD ret; set = SetupDiGetClassDevsA(&guid, NULL, NULL, DIGCF_DEVICEINTERFACE); @@ -2853,6 +2854,29 @@ static void test_devnode(void) ok(!ret, "got %#lx\n", ret); ok(!strcmp(buffer, "ROOT\\LEGACY_BOGUS\\0000"), "got %s\n", buffer); + /* CM_Get_Parent invalid parameters. */ + ret = CM_Get_Parent(NULL, device.DevInst, 0); + ok(ret == CR_INVALID_POINTER, "got %#lx\n", ret); + + /* CM_Get_Parent on a registered device. The parent property is set by + * the PnP manager during bus enumeration; manually created devices may + * not have it, so CR_NO_SUCH_DEVNODE is acceptable. */ + ret = SetupDiRegisterDeviceInfo(set, &device, 0, NULL, NULL, NULL); + ok(ret, "SetupDiRegisterDeviceInfo failed: %#lx\n", GetLastError()); + + parent = 0; + ret = CM_Get_Parent(&parent, device.DevInst, 0); + ok(ret == CR_SUCCESS || ret == CR_NO_SUCH_DEVNODE, + "CM_Get_Parent: got %#lx\n", ret); + if (ret == CR_SUCCESS) + { + ok(parent != 0, "Expected valid parent devnode.\n"); + ret = CM_Get_Device_IDA(parent, buffer, sizeof(buffer), 0); + ok(ret == CR_SUCCESS, "CM_Get_Device_IDA on parent: got %#lx\n", ret); + } + + ret = SetupDiRemoveDevice(set, &device); + ok(ret, "SetupDiRemoveDevice failed: %#lx\n", GetLastError()); SetupDiDestroyDeviceInfoList(set); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
From: Robert Gerigk <Robert-Gerigk@online.de> Verify that DEVPKEY_Device_Parent is set on child devices after bus enumeration. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 0c1361dca11..7b5a843c465 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -2236,6 +2236,18 @@ static void test_pnp_devices(void) ok(!wcscmp(buffer_w, expect_device_location_w), "Got device location info %s.\n", debugstr_w(buffer_w)); } + /* DEVPKEY_Device_Parent — should be set by ntoskrnl during bus enumeration. */ + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + memset(buffer_w, 0, sizeof(buffer_w)); + ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_Parent, &prop_type, (BYTE *)buffer_w, + sizeof(buffer_w), &size, 0); + ok(ret, "Got error %#lx.\n", GetLastError()); + ok(prop_type == DEVPROP_TYPE_STRING, "got type %#lx\n", prop_type); + ok(size > 0, "got size %lu\n", size); + ok(buffer_w[0] != 0, "got empty parent ID\n"); + trace("DEVPKEY_Device_Parent: %s\n", debugstr_w(buffer_w)); + ret = SetupDiEnumDeviceInterfaces(set, NULL, &child_class, 0, &iface); ok(ret, "failed to get interface, error %#lx\n", GetLastError()); ok(IsEqualGUID(&iface.InterfaceClassGuid, &child_class), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10604
Rémi Bernon (@rbernon) commented about dlls/cfgmgr32/cfgmgr32.c:
+ + init_property( &prop, &DEVPKEY_Device_Parent, &type, (BYTE *)parent_id, &size, TRUE ); + if (get_device_property( HKEY_LOCAL_MACHINE, &dev, &prop )) return CR_NO_SUCH_DEVNODE; + + return CM_Locate_DevNodeW( parent, parent_id, 0 ); +} + +/*********************************************************************** + * CM_Get_Parent_Ex (cfgmgr32.@) + */ +CONFIGRET WINAPI CM_Get_Parent_Ex( DEVINST *parent, DEVINST node, ULONG flags, HMACHINE machine ) +{ + TRACE( "parent %p, node %#lx, flags %#lx, machine %p\n", parent, node, flags, machine ); + if (machine) FIXME( "machine %p not implemented!\n", machine ); + return CM_Get_Parent( parent, node, flags ); +} Please do it the other way around, CM_Get_Parent calling CM_Get_Parent_Ex with NULL HMACHINE. Also lets just omit the traces for CM_Get_Parent.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10604#note_136242
participants (3)
-
Jan Robert Gerigk (＠RgSg86) -
Robert Gerigk
-
Rémi Bernon (＠rbernon)