[PATCH v5 0/8] 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. -- v5: ntoskrnl/tests: Add DEVPKEY_Device_Parent test. setupapi/tests: Add CM_Get_Parent tests. cfgmgr32: Implement CM_Get_Parent using DEVPKEY_Device_Parent property. cfgmgr32: Read device properties stored as registry subkeys. 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 11cdff54f14..3c9b057a70a 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -2060,7 +2060,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> SetupDiSetDevicePropertyW stores device properties as registry subkey hierarchies (Properties\{GUID}\XXXX with the value in the default entry). query_property attempted to read them as flat value names using RegQueryValueExW, which always failed with ERROR_FILE_NOT_FOUND. Open the property subkey and read its default value to match setupapi's storage format. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/cfgmgr32/cfgmgr32.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dlls/cfgmgr32/cfgmgr32.c b/dlls/cfgmgr32/cfgmgr32.c index 280f10dd4db..2aae2d9fa44 100644 --- a/dlls/cfgmgr32/cfgmgr32.c +++ b/dlls/cfgmgr32/cfgmgr32.c @@ -158,11 +158,23 @@ static LSTATUS init_registry_property( struct property *prop, const DEVPROPKEY * static LSTATUS query_property( HKEY root, const WCHAR *prefix, DEVPROPTYPE type, struct property *prop ) { WCHAR path[MAX_PATH]; - ULONG reg_type; + ULONG reg_type = 0; LSTATUS err; + HKEY subkey; + + /* Device properties are stored by setupapi as registry subkey hierarchies + * (Properties\{GUID}\XXXX with the value in the default entry), not as + * flat value names. Open the property subkey and read its default value. */ + if ((err = RegOpenKeyExW( root, propkey_string( &prop->key, prefix, path, ARRAY_SIZE(path) ), + 0, KEY_READ, &subkey ))) + { + if (prop->type) *prop->type = DEVPROP_TYPE_EMPTY; + return err == ERROR_FILE_NOT_FOUND ? ERROR_NOT_FOUND : err; + } + + err = RegQueryValueExW( subkey, NULL, NULL, ®_type, prop->buffer, prop->size ); + RegCloseKey( subkey ); - err = RegQueryValueExW( root, propkey_string( &prop->key, prefix, path, ARRAY_SIZE(path) ), - NULL, ®_type, prop->buffer, prop->size ); if (type == DEVPROP_TYPE_EMPTY) type = reg_type & 0xffff; if (!err && !prop->buffer) err = ERROR_MORE_DATA; -- 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 | 34 ++++++++++++++++++++++++++++++++++ dlls/cfgmgr32/cfgmgr32.spec | 4 ++-- dlls/setupapi/setupapi.spec | 4 ++-- dlls/setupapi/stubs.c | 11 ----------- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/dlls/cfgmgr32/cfgmgr32.c b/dlls/cfgmgr32/cfgmgr32.c index 2aae2d9fa44..2d5f0dfe896 100644 --- a/dlls/cfgmgr32/cfgmgr32.c +++ b/dlls/cfgmgr32/cfgmgr32.c @@ -1343,6 +1343,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 ); + 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.@) */ diff --git a/dlls/cfgmgr32/cfgmgr32.spec b/dlls/cfgmgr32/cfgmgr32.spec index 9f5ed24ebf0..7b5ec7d1bdf 100644 --- a/dlls/cfgmgr32/cfgmgr32.spec +++ b/dlls/cfgmgr32/cfgmgr32.spec @@ -142,8 +142,8 @@ @ stub CM_Get_Next_Log_Conf_Ex @ stub CM_Get_Next_Res_Des @ stub CM_Get_Next_Res_Des_Ex -@ stdcall CM_Get_Parent(ptr long long) setupapi.CM_Get_Parent -@ stub CM_Get_Parent_Ex +@ stdcall CM_Get_Parent(ptr long long) +@ 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 6deb5cd949b..fddb5c9eb55 100644 --- a/dlls/setupapi/setupapi.spec +++ b/dlls/setupapi/setupapi.spec @@ -119,8 +119,8 @@ @ stub CM_Get_Next_Log_Conf_Ex @ 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(ptr long long) cfgmgr32.CM_Get_Parent +@ 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 diff --git a/dlls/setupapi/stubs.c b/dlls/setupapi/stubs.c index eccb82295c8..63843e5823f 100644 --- a/dlls/setupapi/stubs.c +++ b/dlls/setupapi/stubs.c @@ -98,17 +98,6 @@ CONFIGRET WINAPI CM_Get_Child_Ex( return CR_SUCCESS; } -/*********************************************************************** - * CM_Get_Parent (SETUPAPI.@) - */ -DWORD WINAPI CM_Get_Parent(PDEVINST pdnDevInst, DEVINST dnDevInst, ULONG ulFlags) -{ - FIXME("%p 0x%08lx 0x%08lx stub\n", pdnDevInst, dnDevInst, ulFlags); - if(pdnDevInst) - *pdnDevInst = 0; - return CR_NO_SUCH_DEVNODE; -} - /*********************************************************************** * SetupInitializeFileLogW(SETUPAPI.@) */ -- 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
participants (2)
-
Jan Robert Gerigk (＠RgSg86) -
Robert Gerigk