[PATCH v9 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. -- v9: 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/cfgmgr32/tests/cfgmgr32.c | 6 +++--- dlls/ntoskrnl.exe/pnp.c | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index b5af953d229..88d27328a14 100644 --- a/dlls/cfgmgr32/tests/cfgmgr32.c +++ b/dlls/cfgmgr32/tests/cfgmgr32.c @@ -3567,9 +3567,9 @@ static void test_CM_Get_DevNode_Property(void) type = 0xdeadbeef; len = sizeof(buffer); ret = CM_Get_DevNode_PropertyW( node, &DEVPKEY_Device_Parent, &type, (BYTE *)buffer, &len, 0 ); - todo_wine ok_x4( ret, ==, CR_SUCCESS ); - todo_wine ok_u4( type, ==, DEVPROP_TYPE_STRING ); - todo_wine ok_u4( len, >, 1 ); + ok_x4( ret, ==, CR_SUCCESS ); + ok_u4( type, ==, DEVPROP_TYPE_STRING ); + ok_u4( len, >, 1 ); len = sizeof(buffer); ret = CM_Get_DevNode_PropertyW( node, &DEVPKEY_Device_Children, &type, (BYTE *)buffer, &len, 0 ); ok_x4( ret, ==, CR_NO_SUCH_VALUE ); diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 56e378cf770..eef88264a56 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -379,12 +379,13 @@ 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}; SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; + WCHAR parent_id[MAX_DEVICE_ID_LEN]; DEVICE_CAPABILITIES caps; BOOL need_driver = TRUE; NTSTATUS status; @@ -434,6 +435,10 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) ExFreePool( id ); } + 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 +540,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 | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index eef88264a56..601e3919725 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -494,12 +494,16 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) { struct wine_device *wine_parent = CONTAINING_RECORD(parent, struct wine_device, device_obj); SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; + SP_DEVINFO_DATA parent_sp = {sizeof(parent_sp)}; + WCHAR parent_id[MAX_DEVICE_ID_LEN]; + WCHAR (*child_ids)[MAX_DEVICE_ID_LEN] = NULL; DEVICE_RELATIONS *relations; IO_STATUS_BLOCK irp_status; IO_STACK_LOCATION *irpsp; HDEVINFO set; KEVENT event; IRP *irp; + DWORD count = 0; ULONG i; TRACE( "(%p)\n", parent ); @@ -562,6 +566,35 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) ExFreePool( wine_parent->children ); wine_parent->children = relations; + count = relations->Count; + child_ids = malloc( count * sizeof(*child_ids) ); + + 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 ); + } + 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 | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 601e3919725..11a578a7545 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -595,6 +595,36 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) free( children_multi ); } + 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 | 43 ++++++++++++++++++++++++++++--------- dlls/cfgmgr32/cfgmgr32.spec | 2 +- dlls/setupapi/setupapi.spec | 2 +- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/dlls/cfgmgr32/cfgmgr32.c b/dlls/cfgmgr32/cfgmgr32.c index f69b39c8658..c53cf69bfac 100644 --- a/dlls/cfgmgr32/cfgmgr32.c +++ b/dlls/cfgmgr32/cfgmgr32.c @@ -1785,6 +1785,39 @@ 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 ) +{ + return CM_Get_Parent_Ex( parent, node, flags, NULL ); +} + +/*********************************************************************** + * CM_Get_Parent_Ex (cfgmgr32.@) + */ +CONFIGRET WINAPI CM_Get_Parent_Ex( DEVINST *parent, DEVINST node, ULONG flags, HMACHINE machine ) +{ + 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, machine %p\n", parent, node, flags, machine ); + if (machine) FIXME( "machine %p not implemented!\n", machine ); + + 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_Device_ID_Size_Ex (cfgmgr32.@) */ @@ -2053,16 +2086,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 | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 0c1361dca11..70c01543835 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -2236,6 +2236,17 @@ 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 == sizeof(L"ROOT\\WINETEST\\0"), "got size %lu\n", size); + ok(!wcscmp(buffer_w, L"ROOT\\WINETEST\\0"), "got parent ID %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
On Fri Apr 17 19:58:07 2026 +0000, Elizabeth Figura wrote:
One thing (I did mention this, but no worries): can we add tests for 1/7, 3/7, and 4/7? 2/7: ``` + { + 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 ); + } ``` We don't need a scope for this, just put parent_id[] at the beginning. Same for 3/7. 7/7: ``` + ok(buffer_w[0] != 0, "got empty parent ID\n"); + trace("DEVPKEY_Device_Parent: %s\n", debugstr_w(buffer_w)); ``` Can't we test the actual contents? I have addressed all three points:
2/7 and 3/7: removed the scope blocks, moved parent_id\[\] (and friends in 3/7: parent_sp, child_ids, count) to the functions declaration block. 7/7: replaced the non-empty check with ok(!wcscmp(buffer_w, L"ROOT\\WINETEST\\0"), ...) plus a matching size check. As a side effect, 4/7s scope block and duplicate child_ids/count are gone. The Siblings loop now reuses the function-level vars from 3/7, with a single free() at the end. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10604#note_136829
participants (2)
-
Jan Robert Gerigk (＠RgSg86) -
Robert Gerigk