[PATCH v11 0/10] 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. -- v11: setupapi/tests: Add CM_Get_Parent tests. cfgmgr32: Implement CM_Get_Parent using DEVPKEY_Device_Parent property. ntoskrnl/tests: Add DEVPKEY_Device_Siblings test. ntoskrnl: Store DEVPKEY_Device_Siblings on each child after bus enumeration. ntoskrnl/tests: Add DEVPKEY_Device_Children test. ntoskrnl: Store DEVPKEY_Device_Children on parent after bus enumeration. ntoskrnl/tests: Add DEVPKEY_Device_Parent test. ntoskrnl: Store DEVPKEY_Device_Parent when enumerating child devices. setupapi/tests: Add test for DIGCF_DEVICEINTERFACE filter. 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> Verify that SetupDiGetClassDevs with DIGCF_DEVICEINTERFACE only enumerates devices that have at least one registered interface, which is the behaviour the previous "Remove devices without interfaces from DIGCF_DEVICEINTERFACE enumeration" change added. Creating a device without registering an interface must not leak into the enumeration; after adding an interface the same device must appear. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/setupapi/tests/devinst.c | 70 +++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 605b0257667..990b468c1ba 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -2856,6 +2856,75 @@ static void test_devnode(void) SetupDiDestroyDeviceInfoList(set); } +/* Regression test for the DIGCF_DEVICEINTERFACE filter introduced in + * "setupapi: Remove devices without interfaces from DIGCF_DEVICEINTERFACE + * enumeration": a registered device without any interface must not be + * returned, but the same device must reappear after an interface is + * registered. Uses instance ID 0005 to stay clear of device IDs exercised + * by the neighbouring test_devnode / test_register_device_iface tests. */ +static void test_devnode_interface_filter(void) +{ + static const char device_id[] = "Root\\LEGACY_BOGUS\\0005"; + static const char device_id_upper[] = "ROOT\\LEGACY_BOGUS\\0005"; + SP_DEVICE_INTERFACE_DATA iface = { sizeof(iface) }; + SP_DEVINFO_DATA device = { sizeof(device) }; + SP_DEVINFO_DATA found = { sizeof(found) }; + char buffer[200]; + HDEVINFO set, enum_set; + BOOL ret, is_present; + DWORD i; + + set = SetupDiGetClassDevsA(&guid, NULL, NULL, DIGCF_DEVICEINTERFACE); + ok(set != INVALID_HANDLE_VALUE, "SetupDiGetClassDevs failed: %#lx\n", GetLastError()); + + ret = SetupDiCreateDeviceInfoA(set, device_id, &guid, NULL, NULL, 0, &device); + ok(ret, "SetupDiCreateDeviceInfo failed: %#lx\n", GetLastError()); + ret = SetupDiRegisterDeviceInfo(set, &device, 0, NULL, NULL, NULL); + ok(ret, "SetupDiRegisterDeviceInfo failed: %#lx\n", GetLastError()); + + /* No interface registered — a fresh DIGCF_DEVICEINTERFACE enumeration + * must not return the device. */ + enum_set = SetupDiGetClassDevsA(&guid, NULL, NULL, DIGCF_DEVICEINTERFACE); + ok(enum_set != INVALID_HANDLE_VALUE, "got %#lx\n", GetLastError()); + is_present = FALSE; + for (i = 0; SetupDiEnumDeviceInfo(enum_set, i, &found); i++) + { + if (SetupDiGetDeviceInstanceIdA(enum_set, &found, buffer, sizeof(buffer), NULL) + && !strcmp(buffer, device_id_upper)) + { + is_present = TRUE; + break; + } + } + ok(!is_present, "device without interface should not appear in DIGCF_DEVICEINTERFACE enumeration\n"); + SetupDiDestroyDeviceInfoList(enum_set); + + /* After registering an interface, the same enumeration must include it. */ + ret = SetupDiCreateDeviceInterfaceA(set, &device, &guid, NULL, 0, &iface); + ok(ret, "SetupDiCreateDeviceInterface failed: %#lx\n", GetLastError()); + + enum_set = SetupDiGetClassDevsA(&guid, NULL, NULL, DIGCF_DEVICEINTERFACE); + ok(enum_set != INVALID_HANDLE_VALUE, "got %#lx\n", GetLastError()); + is_present = FALSE; + for (i = 0; SetupDiEnumDeviceInfo(enum_set, i, &found); i++) + { + if (SetupDiGetDeviceInstanceIdA(enum_set, &found, buffer, sizeof(buffer), NULL) + && !strcmp(buffer, device_id_upper)) + { + is_present = TRUE; + break; + } + } + ok(is_present, "device with interface should appear in DIGCF_DEVICEINTERFACE enumeration\n"); + SetupDiDestroyDeviceInfoList(enum_set); + + ret = SetupDiRemoveDeviceInterface(set, &iface); + ok(ret, "SetupDiRemoveDeviceInterface failed: %#lx\n", GetLastError()); + ret = SetupDiRemoveDevice(set, &device); + ok(ret, "SetupDiRemoveDevice failed: %#lx\n", GetLastError()); + SetupDiDestroyDeviceInfoList(set); +} + static void test_device_interface_key(void) { const char keypath[] = "System\\CurrentControlSet\\Control\\DeviceClasses\\" @@ -5354,6 +5423,7 @@ START_TEST(devinst) test_registry_property_w(); test_get_inf_class(); test_devnode(); + test_devnode_interface_filter(); test_device_interface_key(); test_open_device_interface_key(); test_device_interface_properties(); -- 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> 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
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> Verify that DEVPKEY_Device_Children is set on the bus PDO after child enumeration and contains the child's instance ID as a MULTI_SZ entry. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 70c01543835..9d9b8b37c62 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -2247,6 +2247,45 @@ static void test_pnp_devices(void) 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)); + /* DEVPKEY_Device_Children — must be set on the bus PDO after child + * enumeration. Use the same bus-class + DIGCF_DEVICEINTERFACE path the + * surrounding test already uses (line 2043 etc.) to fetch the bus + * device info, then query the property and verify the current child + * appears in the MULTI_SZ list. */ + { + SP_DEVINFO_DATA bus_dev = { sizeof(bus_dev) }; + HDEVINFO bus_set; + WCHAR *entry; + BOOL found_child; + + bus_set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, + DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); + ok(bus_set != INVALID_HANDLE_VALUE, "failed to get bus device list, error %#lx\n", + GetLastError()); + ret = SetupDiEnumDeviceInfo(bus_set, 0, &bus_dev); + ok(ret, "failed to get bus device info, error %#lx\n", GetLastError()); + + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + memset(buffer_w, 0, sizeof(buffer_w)); + ret = SetupDiGetDevicePropertyW(bus_set, &bus_dev, &DEVPKEY_Device_Children, + &prop_type, (BYTE *)buffer_w, sizeof(buffer_w), &size, 0); + ok(ret, "DEVPKEY_Device_Children missing, error %#lx\n", GetLastError()); + ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + + found_child = FALSE; + for (entry = buffer_w; *entry; entry += wcslen(entry) + 1) + { + if (!wcscmp(entry, L"WINE\\TEST\\1")) + { + found_child = TRUE; + break; + } + } + ok(found_child, "expected WINE\\TEST\\1 in DEVPKEY_Device_Children\n"); + SetupDiDestroyDeviceInfoList(bus_set); + } + 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
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> Temporarily expose a second child on the test bus, enumerate both present children and verify DEVPKEY_Device_Siblings on each: - the property is a DEVPROP_TYPE_STRING_LIST, - it contains the instance ID of every other child, - it does not contain the device's own instance ID. The child is removed and the arrival/removal counters restored so the surrounding test flow (which asserts exactly one child arrival and one child removal) is not affected. Signed-off-by: Jan Robert Gerigk <Robert-Gerigk@online.de> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 82 ++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 9d9b8b37c62..867816bda33 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -2341,6 +2341,88 @@ static void test_pnp_devices(void) ok(GetLastError() == ERROR_IO_PENDING, "got error %lu\n", GetLastError()); ok(size == 0, "got size %lu\n", size); + /* DEVPKEY_Device_Siblings — add a transient second child so each + * PDO has a non-empty siblings list, then verify the property + * contents. Counters AND the DevQuery arrival/removal semaphores + * are saved / drained so the surrounding test flow (which asserts + * exactly one child arrival and one removal for child 1) is not + * perturbed by the transient child 2 events. */ + { + unsigned int saved_child_arrival = got_child_arrival; + unsigned int saved_child_removal = got_child_removal; + ULONG saved_dev_added = devquery_data.child_dev_added; + ULONG saved_dev_removed = devquery_data.child_dev_removed; + SP_DEVINFO_DATA child_dev = { sizeof(child_dev) }; + HDEVINFO sibset; + int extra_id = 2; + int scan_i; + WCHAR *entry; + + ret = DeviceIoControl(bus, IOCTL_WINETEST_BUS_ADD_CHILD, &extra_id, sizeof(extra_id), + NULL, 0, &size, NULL); + ok(ret, "failed to add sibling child, error %lu\n", GetLastError()); + pump_messages(); + + sibset = SetupDiGetClassDevsA(&child_class, NULL, NULL, + DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); + ok(sibset != INVALID_HANDLE_VALUE, "got %#lx\n", GetLastError()); + + /* For each enumerated child, DEVPKEY_Device_Siblings must list + * every *other* child and must not list the child itself. */ + for (scan_i = 0; SetupDiEnumDeviceInfo(sibset, scan_i, &child_dev); scan_i++) + { + WCHAR self_id[MAX_PATH]; + BOOL found_other, found_self; + + ret = SetupDiGetDeviceInstanceIdW(sibset, &child_dev, self_id, + ARRAY_SIZE(self_id), NULL); + ok(ret, "failed to get instance ID, error %#lx\n", GetLastError()); + + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + memset(buffer_w, 0, sizeof(buffer_w)); + ret = SetupDiGetDevicePropertyW(sibset, &child_dev, &DEVPKEY_Device_Siblings, + &prop_type, (BYTE *)buffer_w, sizeof(buffer_w), + &size, 0); + ok(ret, "DEVPKEY_Device_Siblings missing for %s, error %#lx\n", + debugstr_w(self_id), GetLastError()); + ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + + found_other = FALSE; + found_self = FALSE; + for (entry = buffer_w; *entry; entry += wcslen(entry) + 1) + { + if (!wcscmp(entry, self_id)) + found_self = TRUE; + else + found_other = TRUE; + } + ok(!found_self, "%s appeared in its own sibling list\n", debugstr_w(self_id)); + ok(found_other, "no sibling entry for %s\n", debugstr_w(self_id)); + } + SetupDiDestroyDeviceInfoList(sibset); + + /* Tear down the transient child. */ + ret = DeviceIoControl(bus, IOCTL_WINETEST_BUS_REMOVE_CHILD, &extra_id, sizeof(extra_id), + NULL, 0, &size, NULL); + ok(ret, "failed to remove sibling child, error %lu\n", GetLastError()); + pump_messages(); + + /* Restore counters and drain the DevQuery semaphores that child 2 + * arrival / removal signalled — they are binary (max count 1) and + * stay signalled until consumed, which would break the later + * WAIT_TIMEOUT assertions on the same semaphores. */ + got_child_arrival = saved_child_arrival; + got_child_removal = saved_child_removal; + devquery_data.child_dev_added = saved_dev_added; + devquery_data.child_dev_removed = saved_dev_removed; + if (have_devquery) + { + WaitForSingleObject(devquery_data.device_added_sem, 0); + WaitForSingleObject(devquery_data.device_removed_sem, 0); + } + } + id = 1; ret = DeviceIoControl(bus, IOCTL_WINETEST_BUS_REMOVE_CHILD, &id, sizeof(id), NULL, 0, &size, NULL); ok(ret, "got error %lu\n", GetLastError()); -- 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 990b468c1ba..11fb225fbf8 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
On Mon Apr 20 16:16:54 2026 +0000, Elizabeth Figura wrote:
And tests for 1/7, 3/7, and 4/7? Rebased so each test sits next to its source commit. I also did a second review pass on the three new tests before pushing:
2/10 DIGCF_DEVICEINTERFACE filter: uses device ID 0005 to stay clear of other tests using 0000-0003, uppercase-strcmp for the instance ID comparison (matches existing convention in the file). 6/10 DEVPKEY_Device_Children: fetches the bus device via bus_class + DIGCF_DEVICEINTERFACE | DIGCF_PRESENT, matching the enumeration pattern used elsewhere in test_pnp_devices. 8/10 DEVPKEY_Device_Siblings: transient second child; counters and the DevQuery binary semaphores (device_added_sem / device_removed_sem) are drained on exit so the later WAIT_TIMEOUT assertions on those semaphores remain valid. Local setupapi_test devinst run: 4376 tests, 0 failures. ntoskrnl test compiles clean Sorry for the back-and-forth across multiple review rounds, hopefully this one is now fine. One question: I saw a merge attempt last Friday that did not go through because the branch was 49 (?) commits behind master. The series is now rebased onto wine 11.7. Does the merge pool automatically retry on the next merge window, or is there something I should do? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10604#note_137247
On Tue Apr 21 18:44:52 2026 +0000, Jan Robert Gerigk wrote:
Rebased so each test sits next to its source commit. I also did a second review pass on the three new tests before pushing: 2/10 DIGCF_DEVICEINTERFACE filter: uses device ID 0005 to stay clear of other tests using 0000-0003, uppercase-strcmp for the instance ID comparison (matches existing convention in the file). 6/10 DEVPKEY_Device_Children: fetches the bus device via bus_class + DIGCF_DEVICEINTERFACE | DIGCF_PRESENT, matching the enumeration pattern used elsewhere in test_pnp_devices. 8/10 DEVPKEY_Device_Siblings: transient second child; counters and the DevQuery binary semaphores (device_added_sem / device_removed_sem) are drained on exit so the later WAIT_TIMEOUT assertions on those semaphores remain valid. Local setupapi_test devinst run: 4376 tests, 0 failures. ntoskrnl test compiles clean Sorry for the back-and-forth across multiple review rounds, hopefully this one is now fine. One question: I saw a merge attempt last Friday that did not go through because the branch was 49 (?) commits behind master. The series is now rebased onto wine 11.7. Does the merge pool automatically retry on the next merge window, or is there something I should do? Pipeline showed 13 failures from my last push. Analysed locally:
6/10 (now 6/9): my "cleaner" SetupDi pattern from the previous review pass was wrong for Wine. SetupDiEnumDeviceInfo on a DIGCF_DEVICEINTERFACE set returned no items. Fixed by switching to SetupDiEnumDeviceInterfaces + SetupDiGetDeviceInterfaceDetail-with-NULL-detail to extract the bus device's SP_DEVINFO_DATA. Setupapi devinst test still 0 failures locally. 8/10 (the Siblings test): the transient second-child approach broke kernel-side counters in driver_pnp.c (hardcoded remove_device_count == 0) and the user-mode notification handlers (hardcoded "Wine#Test #1" strings). I dropped the test from this MR. The Siblings property storage commit (now 7/9) stays in. If that is OK with you, I would submit a dedicated Siblings test as a follow-up MR with its own bus-test fixture so the existing test_pnp_devices invariants stay intact? Series rebased onto current master (was 34 commits behind), 9 commits total now. Running some tests again locally, should by ready any time soon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10604#note_137253
participants (2)
-
Jan Robert Gerigk (＠RgSg86) -
Robert Gerigk