[PATCH 0/2] MR8015: ntoskrnl.exe: Do not send IRP to get device instance ID.
From: Alexander Morozov <amorozov(a)etersoft.ru> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57588 --- dlls/ntoskrnl.exe/pnp.c | 73 ++++++++++------------------ dlls/ntoskrnl.exe/tests/driver_pnp.c | 16 +++--- 2 files changed, 34 insertions(+), 55 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index e3825722ad6..f5954970b82 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -521,21 +521,18 @@ NTSTATUS WINAPI IoGetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY LCID lcid, ULONG flags, ULONG size, void *data, ULONG *required_size, DEVPROPTYPE *property_type ) { + struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; - WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; HDEVINFO set; - NTSTATUS status; TRACE( "device %p, property_key %s, lcid %#lx, flags %#lx, size %lu, data %p, required_size %p, property_type %p\n", device, debugstr_propkey( property_key ), lcid, flags, size, data, required_size, property_type ); + if (!wine_device->instance_id) return STATUS_INVALID_DEVICE_REQUEST; if (lcid == LOCALE_SYSTEM_DEFAULT || lcid == LOCALE_USER_DEFAULT) return STATUS_INVALID_PARAMETER; if (lcid != LOCALE_NEUTRAL) FIXME( "Only LOCALE_NEUTRAL is supported\n" ); - status = get_device_instance_id( device, device_instance_id ); - if (status != STATUS_SUCCESS) return status; - set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL ); if (set == INVALID_HANDLE_VALUE) { @@ -543,7 +540,7 @@ NTSTATUS WINAPI IoGetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY return GetLastError(); } - if (!SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) + if (!SetupDiOpenDeviceInfoW( set, wine_device->instance_id, NULL, 0, &sp_device )) { ERR( "Failed to open device, error %#lx.\n", GetLastError() ); SetupDiDestroyDeviceInfoList( set ); @@ -569,38 +566,31 @@ NTSTATUS WINAPI IoGetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY NTSTATUS WINAPI IoGetDeviceProperty( DEVICE_OBJECT *device, DEVICE_REGISTRY_PROPERTY property, ULONG length, void *buffer, ULONG *needed ) { + struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; - WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; DWORD sp_property = -1; - NTSTATUS status; + NTSTATUS status = STATUS_SUCCESS; HDEVINFO set; TRACE("device %p, property %u, length %lu, buffer %p, needed %p.\n", device, property, length, buffer, needed); + if (!wine_device->instance_id) return STATUS_INVALID_DEVICE_REQUEST; + switch (property) { case DevicePropertyEnumeratorName: { - WCHAR *id, *ptr; + const WCHAR *id = wine_device->instance_id; + const WCHAR *ptr = wcschr( id, '\\' ); + size_t len = ptr ? ptr - id : wcslen(id); - status = get_device_id( device, BusQueryDeviceID, &id ); - if (status != STATUS_SUCCESS) - { - ERR("Failed to get instance ID, status %#lx.\n", status); - break; - } - - ptr = wcschr( id, '\\' ); - if (ptr) *ptr = 0; - - *needed = sizeof(WCHAR) * (lstrlenW(id) + 1); + *needed = sizeof(WCHAR) * (len + 1); if (length >= *needed) - memcpy( buffer, id, *needed ); + wcsncpy_s( buffer, len + 1, id, len ); else status = STATUS_BUFFER_TOO_SMALL; - ExFreePool( id ); return status; } case DevicePropertyPhysicalDeviceObjectName: @@ -688,16 +678,13 @@ NTSTATUS WINAPI IoGetDeviceProperty( DEVICE_OBJECT *device, DEVICE_REGISTRY_PROP return STATUS_NOT_IMPLEMENTED; } - if ((status = get_device_instance_id( device, device_instance_id ))) - return status; - if ((set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL )) == INVALID_HANDLE_VALUE) { ERR("Failed to create device list, error %#lx.\n", GetLastError()); return GetLastError(); } - if (!SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device)) + if (!SetupDiOpenDeviceInfoW( set, wine_device->instance_id, NULL, 0, &sp_device )) { ERR("Failed to open device, error %#lx.\n", GetLastError()); SetupDiDestroyDeviceInfoList( set ); @@ -886,9 +873,8 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY *property_key, LCID lcid, ULONG flags, DEVPROPTYPE type, ULONG size, void *data ) { + struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; - WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; - NTSTATUS status; HDEVINFO set; TRACE( "device %p, property_key %s, lcid %#lx, flags %#lx, type %#lx, size %lu, data %p.\n", @@ -896,17 +882,16 @@ NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY /* flags is always treated as PLUGPLAY_PROPERTY_PERSISTENT starting with Win 8 / 2012 */ + if (!wine_device->instance_id) return STATUS_INVALID_DEVICE_REQUEST; if (lcid != LOCALE_NEUTRAL) FIXME( "only LOCALE_NEUTRAL is supported\n" ); - if ((status = get_device_instance_id( device, device_instance_id ))) return status; - if ((set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL )) == INVALID_HANDLE_VALUE) { ERR( "Failed to create device list, error %#lx.\n", GetLastError() ); return GetLastError(); } - if (!SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) + if (!SetupDiOpenDeviceInfoW( set, wine_device->instance_id, NULL, 0, &sp_device )) { ERR( "Failed to open device, error %#lx.\n", GetLastError() ); SetupDiDestroyDeviceInfoList( set ); @@ -931,9 +916,9 @@ NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY NTSTATUS WINAPI IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *class_guid, UNICODE_STRING *refstr, UNICODE_STRING *symbolic_link) { + struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); SP_DEVICE_INTERFACE_DATA sp_iface = {sizeof(sp_iface)}; SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; - WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; NTSTATUS status = STATUS_SUCCESS; UNICODE_STRING device_path; struct device_interface *iface; @@ -944,16 +929,15 @@ NTSTATUS WINAPI IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla TRACE("device %p, class_guid %s, refstr %s, symbolic_link %p.\n", device, debugstr_guid(class_guid), debugstr_us(refstr), symbolic_link); - if ((status = get_device_instance_id( device, device_instance_id ))) - return status; + if (!wine_device->instance_id) return STATUS_INVALID_DEVICE_REQUEST; set = SetupDiGetClassDevsW( class_guid, NULL, NULL, DIGCF_DEVICEINTERFACE ); if (set == INVALID_HANDLE_VALUE) return STATUS_UNSUCCESSFUL; - if (!SetupDiCreateDeviceInfoW( set, device_instance_id, class_guid, NULL, NULL, 0, &sp_device ) - && !SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) + if (!SetupDiCreateDeviceInfoW( set, wine_device->instance_id, class_guid, NULL, NULL, 0, &sp_device ) + && !SetupDiOpenDeviceInfoW( set, wine_device->instance_id, NULL, 0, &sp_device )) { - ERR("Failed to create device %s, error %#lx.\n", debugstr_w(device_instance_id), GetLastError()); + ERR("Failed to create device %s, error %#lx.\n", debugstr_w(wine_device->instance_id), GetLastError()); return GetLastError(); } @@ -962,7 +946,7 @@ NTSTATUS WINAPI IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla /* setupapi mangles the case; construct the interface path manually. */ - device_path.Length = (4 + wcslen( device_instance_id ) + 1 + 38) * sizeof(WCHAR); + device_path.Length = (4 + wcslen( wine_device->instance_id ) + 1 + 38) * sizeof(WCHAR); if (refstr) device_path.Length += sizeof(WCHAR) + refstr->Length; device_path.MaximumLength = device_path.Length + sizeof(WCHAR); @@ -970,7 +954,7 @@ NTSTATUS WINAPI IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla device_path.Buffer = RtlAllocateHeap( GetProcessHeap(), 0, device_path.MaximumLength ); swprintf( device_path.Buffer, device_path.MaximumLength / sizeof(WCHAR), L"\\??\\%s#{%08lx-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}", - device_instance_id, class_guid->Data1, class_guid->Data2, class_guid->Data3, + wine_device->instance_id, class_guid->Data1, class_guid->Data2, class_guid->Data3, class_guid->Data4[0], class_guid->Data4[1], class_guid->Data4[2], class_guid->Data4[3], class_guid->Data4[4], class_guid->Data4[5], class_guid->Data4[6], class_guid->Data4[7] ); for (p = device_path.Buffer + 4; *p; p++) @@ -1071,22 +1055,17 @@ NTSTATUS WINAPI IoReportTargetDeviceChangeAsynchronous( DEVICE_OBJECT *device, v */ NTSTATUS WINAPI IoOpenDeviceRegistryKey( DEVICE_OBJECT *device, ULONG type, ACCESS_MASK access, HANDLE *key ) { + struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; - WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; - NTSTATUS status; HDEVINFO set; TRACE("device %p, type %#lx, access %#lx, key %p.\n", device, type, access, key); - if ((status = get_device_instance_id( device, device_instance_id ))) - { - ERR("Failed to get device instance ID, error %#lx.\n", status); - return status; - } + if (!wine_device->instance_id) return STATUS_INVALID_PARAMETER; set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL ); - SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device ); + SetupDiOpenDeviceInfoW( set, wine_device->instance_id, NULL, 0, &sp_device ); *key = SetupDiOpenDevRegKey( set, &sp_device, DICS_FLAG_GLOBAL, 0, type, access ); SetupDiDestroyDeviceInfoList( set ); diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index a5fb145c387..e364fecc49a 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -292,7 +292,7 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) query_id_count = 0; status = IoRegisterDeviceInterface(device_obj, &child_class, NULL, &device->child_symlink); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(!status, "Failed to register interface, status %#lx.\n", status); ok(device->child_symlink.Length == sizeof(expect_symlink) - sizeof(WCHAR), "Got length %u.\n", device->child_symlink.Length); @@ -710,12 +710,12 @@ static void test_child_device_properties(DEVICE_OBJECT *device) query_id_count = 0; status = IoSetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, type, size, &value); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to set device property, status %#lx\n", status); query_id_count = 0; status = IoGetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, size, &stored_value, &req_size, &stored_type); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to get device property, status %#lx\n", status); ok(req_size == size, "expected required size %lu, got %lu\n", req_size, size); ok(stored_type == type, "expected DEVPROPTYPE value %#lx, got %#lx\n", type, stored_type); @@ -723,7 +723,7 @@ static void test_child_device_properties(DEVICE_OBJECT *device) query_id_count = 0; status = IoSetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, type, 0, NULL); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to delete device property, status %#lx\n", status); } @@ -735,7 +735,7 @@ static void test_enumerator_name(void) NTSTATUS status; status = IoGetDeviceProperty(bus_fdo, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); - todo_wine ok(status == STATUS_INVALID_DEVICE_REQUEST, "got unexpected status %#lx\n", status); + ok(status == STATUS_INVALID_DEVICE_REQUEST, "got unexpected status %#lx\n", status); req_size = 0; memset(buffer, 0, sizeof(buffer)); @@ -755,7 +755,7 @@ static void test_child_enumerator_name(DEVICE_OBJECT *device) query_id_count = 0; status = IoGetDeviceProperty(device, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "IoGetDeviceProperty failed: %#lx\n", status); ok(req_size == sizeof(wine), "unexpected size %lu\n", req_size); if (status == STATUS_SUCCESS) @@ -774,7 +774,7 @@ static void test_device_registry_key(void) DWORD size; status = IoOpenDeviceRegistryKey(bus_fdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "got unexpected status %#lx\n", status); + ok(status == STATUS_INVALID_PARAMETER, "got unexpected status %#lx\n", status); status = IoOpenDeviceRegistryKey(bus_pdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); ok(status == STATUS_SUCCESS, "IoOpenDeviceRegistryKey failed: %#lx\n", status); @@ -815,7 +815,7 @@ static void test_child_device_registry_key(DEVICE_OBJECT *device) query_id_count = 0; status = IoOpenDeviceRegistryKey(device, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "IoOpenDeviceRegistryKey failed: %#lx\n", status); if (status == STATUS_SUCCESS) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8015
From: Alexander Morozov <amorozov(a)etersoft.ru> --- dlls/ntoskrnl.exe/ntoskrnl.c | 1 + dlls/ntoskrnl.exe/ntoskrnl_private.h | 1 + dlls/ntoskrnl.exe/pnp.c | 58 +++++++++++++++++++++------- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index a15de6d1b95..976aa3f19dd 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -1726,6 +1726,7 @@ void WINAPI IoDeleteDevice( DEVICE_OBJECT *device ) ObDereferenceObject( children->Objects[i] ); ExFreePool( children ); } + HeapFree( GetProcessHeap(), 0, wine_device->instance_id ); ObDereferenceObject( device ); } } diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h index 472dc46fde3..e491f0ec638 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h @@ -120,6 +120,7 @@ static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', struct wine_device { DEVICE_OBJECT device_obj; + WCHAR *instance_id; DEVICE_RELATIONS *children; }; #endif diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 49af547d4fa..e3825722ad6 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -320,15 +320,26 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) 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 *device_instance_id, *id; DEVICE_CAPABILITIES caps; BOOL need_driver = TRUE; NTSTATUS status; HKEY key; - WCHAR *id; + + device_instance_id = HeapAlloc( GetProcessHeap(), 0, MAX_DEVICE_ID_LEN * sizeof(WCHAR) ); + if (!device_instance_id) + { + ERR("Failed to allocate memory for device instance ID\n"); + return; + } if (get_device_instance_id( device, device_instance_id )) + { + HeapFree( GetProcessHeap(), 0, device_instance_id ); return; + } + + CONTAINING_RECORD(device, struct wine_device, device_obj)->instance_id = device_instance_id; if (!SetupDiCreateDeviceInfoW( set, device_instance_id, &GUID_NULL, NULL, NULL, 0, &sp_device ) && !SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) @@ -1125,18 +1136,22 @@ static DRIVER_OBJECT *pnp_manager; struct root_pnp_device { - WCHAR id[MAX_DEVICE_ID_LEN]; struct list entry; DEVICE_OBJECT *device; }; +static WCHAR *get_root_pnp_device_id( const struct root_pnp_device *device ) +{ + return CONTAINING_RECORD(device->device, struct wine_device, device_obj)->instance_id; +} + static struct root_pnp_device *find_root_pnp_device( struct wine_driver *driver, const WCHAR *id ) { struct root_pnp_device *device; LIST_FOR_EACH_ENTRY( device, &driver->root_pnp_devices, struct root_pnp_device, entry ) { - if (!wcsicmp( id, device->id )) + if (!wcsicmp( id, get_root_pnp_device_id( device ) )) return device; } @@ -1171,6 +1186,7 @@ static NTSTATUS WINAPI pnp_manager_device_pnp( DEVICE_OBJECT *device, IRP *irp ) case IRP_MN_QUERY_ID: { BUS_QUERY_ID_TYPE type = stack->Parameters.QueryId.IdType; + WCHAR *root_device_id = get_root_pnp_device_id( root_device ); WCHAR *id, *p; TRACE("Received IRP_MN_QUERY_ID, type %#x.\n", type); @@ -1178,11 +1194,11 @@ static NTSTATUS WINAPI pnp_manager_device_pnp( DEVICE_OBJECT *device, IRP *irp ) switch (type) { case BusQueryDeviceID: - p = wcsrchr( root_device->id, '\\' ); - if ((id = ExAllocatePool( NonPagedPool, (p - root_device->id + 1) * sizeof(WCHAR) ))) + p = wcsrchr( root_device_id, '\\' ); + if ((id = ExAllocatePool( NonPagedPool, (p - root_device_id + 1) * sizeof(WCHAR) ))) { - memcpy( id, root_device->id, (p - root_device->id) * sizeof(WCHAR) ); - id[p - root_device->id] = 0; + memcpy( id, root_device_id, (p - root_device_id) * sizeof(WCHAR) ); + id[p - root_device_id] = 0; irp->IoStatus.Information = (ULONG_PTR)id; irp->IoStatus.Status = STATUS_SUCCESS; } @@ -1193,7 +1209,7 @@ static NTSTATUS WINAPI pnp_manager_device_pnp( DEVICE_OBJECT *device, IRP *irp ) } break; case BusQueryInstanceID: - p = wcsrchr( root_device->id, '\\' ); + p = wcsrchr( root_device_id, '\\' ); if ((id = ExAllocatePool( NonPagedPool, (wcslen( p + 1 ) + 1) * sizeof(WCHAR) ))) { wcscpy( id, p + 1 ); @@ -1294,7 +1310,8 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) { static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0}; static const WCHAR rootW[] = {'R','O','O','T',0}; - WCHAR buffer[MAX_SERVICE_NAME + ARRAY_SIZE(driverW)], id[MAX_DEVICE_ID_LEN]; + static const SIZE_T id_size = MAX_DEVICE_ID_LEN * sizeof(WCHAR); + WCHAR buffer[MAX_SERVICE_NAME + ARRAY_SIZE(driverW)]; SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; struct list new_list = LIST_INIT(new_list); struct root_pnp_device *pnp_device, *next; @@ -1303,6 +1320,7 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) NTSTATUS status; unsigned int i; HDEVINFO set; + WCHAR *id = NULL; TRACE("Searching for new root-enumerated devices for driver %s.\n", debugstr_w(driver_name)); @@ -1324,7 +1342,17 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) continue; } - SetupDiGetDeviceInstanceIdW( set, &sp_device, id, ARRAY_SIZE(id), NULL ); + if (!id) + { + id = HeapAlloc( GetProcessHeap(), 0, id_size ); + if (!id) + { + ERR("Failed to allocate memory for device instance ID\n"); + break; + } + } + + SetupDiGetDeviceInstanceIdW( set, &sp_device, id, id_size, NULL ); if ((pnp_device = find_root_pnp_device( driver, id ))) { @@ -1343,17 +1371,21 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) continue; } + CONTAINING_RECORD(device, struct wine_device, device_obj)->instance_id = id; + id = NULL; + pnp_device = device->DeviceExtension; - wcscpy( pnp_device->id, id ); pnp_device->device = device; list_add_tail( &new_list, &pnp_device->entry ); start_device( device, set, &sp_device ); } + HeapFree( GetProcessHeap(), 0, id ); + LIST_FOR_EACH_ENTRY_SAFE( pnp_device, next, &driver->root_pnp_devices, struct root_pnp_device, entry ) { - TRACE("Removing device %s.\n", debugstr_w(pnp_device->id)); + TRACE("Removing device %s.\n", debugstr_w( get_root_pnp_device_id( pnp_device ) )); remove_device( pnp_device->device ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8015
Vibhav Pant (@vibhavp) commented about dlls/ntoskrnl.exe/pnp.c:
- WCHAR *id; + + device_instance_id = HeapAlloc( GetProcessHeap(), 0, MAX_DEVICE_ID_LEN * sizeof(WCHAR) ); + if (!device_instance_id) + { + ERR("Failed to allocate memory for device instance ID\n"); + return; + }
if (get_device_instance_id( device, device_instance_id )) + { + HeapFree( GetProcessHeap(), 0, device_instance_id ); return; + } + + CONTAINING_RECORD(device, struct wine_device, device_obj)->instance_id = device_instance_id; <span dir="">This could also be accomplished by letting `device_instance_id` be a stack-allocated array, and using `wcsdup`</span>. That way we're not unnecessarily allocating 400 bytes for every device object.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8015#note_109757
On Mon Jul 14 12:33:49 2025 +0000, Vibhav Pant wrote:
<span dir="">This could also be accomplished by letting `device_instance_id` be a stack-allocated array, and using `wcsdup`</span>. That way we're not unnecessarily allocating 400 bytes for every device object. `HeapAlloc` is not called for every device object in this loop. Only for that in which we save id \+ maybe one excessive.
We could make `instance_id` in `wine_device` an array instead of a pointer. But then `wine_device` struct would have more size for every device, but only PDOs really have id. I do not know what is preferred. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8015#note_109977
On Tue Jul 15 19:47:21 2025 +0000, Alexander Morozov wrote:
`HeapAlloc` is not called for every device object in this loop. Only for that in which we save id \+ maybe one excessive. We could make `instance_id` in `wine_device` an array instead of a pointer. But then `wine_device` struct would have more size for every device, but only PDOs really have id. I do not know what is preferred. Sorry, I thought your comment was about the change in `wine_enumerate_root_devices`. In `enumerate_new_device` I make allocation because `wine_device` contains a pointer and so we need to do the allocation anyway. The only case when this allocaction is excessive is when `get_device_instance_id` fails but this is an error case and I think it should be rarer than successful execution path.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8015#note_110102
Sorry I missed this was assigned to me. I don't think we want to be removing id from root_pnp_device, since that's basically a regular PnP device. It may work this way but it's surely broken. ``` + if (!wine_device->instance_id) return STATUS_INVALID_DEVICE_REQUEST; ``` If the device didn't give us an instance ID when we asked for it we should be failing to even enumerate it, so I think this is unnecessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8015#note_124848
On Mon Dec 8 21:38:51 2025 +0000, Alexander Morozov wrote:
Thank you for your reply. Device ID is saved in wine_device for all PDOs instead of saving it in root_pnp_device only for root devices. Saving IDs for all devices is needed to avoid sending IRP when a driver does not expect to receive it. `if (!wine_device->instance_id)` is used to check if a device is PDO. It is a behavior documented in MSDN for some Io* functions (IoGetDeviceProperty, IoRegisterDeviceInterface, IoOpenDeviceRegistryKey) to return an error when they are not called with PDO. Sorry for the lateness again, I set this aside for regression season and then forgot about it...
Device ID is saved in wine_device for all PDOs instead of saving it in root_pnp_device only for root devices. Saving IDs for all devices is needed to avoid sending IRP when a driver does not expect to receive it.
I really don't think it makes sense to shortcut the process like this; it looks very fragile and it's hard to follow. I think it'd be better to leave root devices implemented like all other devices, without knowledge of wine_device internals.
`if (!wine_device->instance_id)` is used to check if a device is PDO. It is a behavior documented in MSDN for some Io\* functions (IoGetDeviceProperty, IoRegisterDeviceInterface, IoOpenDeviceRegistryKey) to return an error when they are not called with PDO.
I see. That should really be a separate change though, since it's a functional change not related, and given that it's not exactly obvious I'd probably appreciate a comment on the relevant functions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8015#note_129841
participants (5)
-
Alexander Morozov -
Alexander Morozov (＠amorozov)
-
Elizabeth Figura (＠zfigura) -
Elizabeth Figura (＠zfigura)
-
Vibhav Pant (＠vibhavp)