[PATCH v2 0/4] MR8515: setupapi: Support in-built properties for device instances in SetupDiGetDevicePropertyW.
-- v2: setupapi: Support additional built-in properties for device instances. ntoskrnl.exe/tests: Add tests for built-in properties for PnP device instances. setupapi: Support getting the instance id and class GUID as in-built properties in SetupDiGetDevicePropertyW and CM_Get_DevNode_Property_ExW. setupapi/tests: Add tests for getting "in-built" properties from SetupDiGetDevicePropertyW and SetupDiGetDevicePropertyKeys. https://gitlab.winehq.org/wine/wine/-/merge_requests/8515
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/setupapi/tests/devinst.c | 44 +++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index ed9d197e3bd..bc3eae57783 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -875,6 +875,7 @@ static void test_device_info(void) static void test_device_property(void) { static const WCHAR valueW[] = {'d', 'e', 'a', 'd', 'b', 'e', 'e', 'f', 0}; + static const WCHAR instance_id[] = L"Root\\LEGACY_BOGUS\\0000"; SP_DEVINFO_DATA device_data = {sizeof(device_data)}; HMODULE hmod; HDEVINFO set; @@ -883,6 +884,7 @@ static void test_device_property(void) BYTE buffer[256]; DWORD err; BOOL ret; + GUID guid_val = {0}; hmod = LoadLibraryA("setupapi.dll"); pSetupDiSetDevicePropertyW = (void *)GetProcAddress(hmod, "SetupDiSetDevicePropertyW"); @@ -899,7 +901,7 @@ static void test_device_property(void) set = SetupDiCreateDeviceInfoList(&guid, NULL); ok(set != INVALID_HANDLE_VALUE, "Failed to create device list, error %#lx.\n", GetLastError()); - ret = SetupDiCreateDeviceInfoA(set, "Root\\LEGACY_BOGUS\\0000", &guid, NULL, NULL, 0, &device_data); + ret = SetupDiCreateDeviceInfoW(set, instance_id, &guid, NULL, NULL, 0, &device_data); ok(ret, "Failed to create device, error %#lx.\n", GetLastError()); /* SetupDiSetDevicePropertyW */ @@ -1193,9 +1195,31 @@ static void test_device_property(void) ok(size == sizeof(valueW), "Got size %ld\n", size); ok(!lstrcmpW((WCHAR *)buffer, valueW), "Expect buffer %s, got %s\n", wine_dbgstr_w(valueW), wine_dbgstr_w((WCHAR *)buffer)); + /* #15 Innate properties */ + type = DEVPROP_TYPE_EMPTY; + size = 0; + buffer[0] = '\0'; + ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_InstanceId, &type, buffer, sizeof(buffer), &size, 0); + err = GetLastError(); + todo_wine ok(ret, "Expect success\n"); + todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + todo_wine ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_STRING, type); + todo_wine ok(size == sizeof(instance_id), "Got size %lu\n", size); + todo_wine ok(!wcsicmp(instance_id, (WCHAR *)buffer), "Expect buffer %s, got %s\n", debugstr_w(instance_id), debugstr_w((WCHAR*)buffer)); + type = DEVPROP_TYPE_EMPTY; + size = 0; + ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_ClassGuid, &type, (BYTE *)&guid_val, sizeof(guid_val), &size, 0); + err = GetLastError(); + todo_wine ok(ret, "Expect success\n"); + todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + todo_wine ok(type == DEVPROP_TYPE_GUID, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_GUID, type); + todo_wine ok(size == sizeof(guid_val), "Got size %lu\n", size); + todo_wine ok(IsEqualGUID(&guid_val, &guid), "Expect buffer %s, got %s\n", debugstr_guid(&guid), debugstr_guid(&guid_val)); + if (pSetupDiGetDevicePropertyKeys) { - DWORD keys_len = 0, n, required_len, expected_keys = 1; + DEVPROPKEY default_keys[] = { DEVPKEY_Device_ClassGuid, DEVPKEY_Device_InstanceId }; + DWORD keys_len = 0, n, required_len, expected_keys = 1 + ARRAY_SIZE(default_keys); DEVPROPKEY *keys; ret = pSetupDiGetDevicePropertyKeys(NULL, NULL, NULL, 0, NULL, 0); @@ -1255,19 +1279,31 @@ static void test_device_property(void) err = GetLastError(); ok(!err, "Expect last error %#x, got %#lx\n", ERROR_SUCCESS, err); ok(keys_len == required_len, "%lu != %lu\n", keys_len, required_len); - ok(keys_len >= expected_keys, "Expected %lu >= %lu\n", keys_len, expected_keys); + todo_wine ok(keys_len >= expected_keys, "Expected %lu >= %lu\n", keys_len, expected_keys); keys_len = 0; if (keys) { for (n = 0; n < required_len; n++) { + DWORD i; if (!memcmp(&keys[n], &DEVPKEY_Device_FriendlyName, sizeof(keys[n]))) + { keys_len++; + continue; + } + for (i = 0; i < ARRAY_SIZE(default_keys); i++) + { + if (!memcmp(&keys[n], &default_keys[i], sizeof(keys[n]))) + { + keys_len++; + break; + } + } } } - ok(keys_len == expected_keys, "%lu != %lu\n", keys_len, expected_keys); + todo_wine ok(keys_len == expected_keys, "%lu != %lu\n", keys_len, expected_keys); free(keys); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8515
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/setupapi/devinst.c | 43 +++++++++++++++++++++++++++++++++-- dlls/setupapi/tests/devinst.c | 20 ++++++++-------- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 066bd6f09c2..e065e6cbe98 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -5275,6 +5275,45 @@ BOOL WINAPI SetupDiGetDevicePropertyKeys( HDEVINFO devinfo, PSP_DEVINFO_DATA dev return !ret; } +static DWORD get_device_property( struct device *device, const DEVPROPKEY *prop_key, DEVPROPTYPE *prop_type, + BYTE *buf, DWORD buf_size, DWORD *req_size, DWORD flags ) +{ + DWORD ret = ERROR_SUCCESS; + + if (!prop_key) + return ERROR_INVALID_DATA; + if (!prop_type || (!buf && buf_size)) + return ERROR_INVALID_USER_BUFFER; + if (flags) + return ERROR_INVALID_FLAGS; + + if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_InstanceId )) + { + DWORD size = (wcslen( device->instanceId ) + 1) * sizeof( WCHAR ); + *prop_type = DEVPROP_TYPE_STRING; + if (buf_size >= size) + wcscpy( (WCHAR *)buf, device->instanceId ); + else + ret = ERROR_INSUFFICIENT_BUFFER; + if (req_size) + *req_size = size; + } + else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_ClassGuid )) + { + DWORD size = sizeof( device->class ); + *prop_type = DEVPROP_TYPE_GUID; + if (buf_size >= size) + memcpy( buf, &device->class, sizeof( device->class ) ); + else + ret = ERROR_INSUFFICIENT_BUFFER; + if (req_size) + *req_size = size; + } + else + ret = get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags ); + return ret; +} + /*********************************************************************** * SetupDiGetDevicePropertyW (SETUPAPI.@) */ @@ -5291,7 +5330,7 @@ BOOL WINAPI SetupDiGetDevicePropertyW(HDEVINFO devinfo, PSP_DEVINFO_DATA device_ if (!(device = get_device(devinfo, device_data))) return FALSE; - ls = get_device_reg_property(device->key, prop_key, prop_type, prop_buff, prop_buff_size, required_size, flags); + ls = get_device_property(device, prop_key, prop_type, prop_buff, prop_buff_size, required_size, flags); SetLastError(ls); return !ls; @@ -5319,7 +5358,7 @@ CONFIGRET WINAPI CM_Get_DevNode_Property_ExW(DEVINST devnode, const DEVPROPKEY * if (!(device = get_devnode_device(devnode, &set))) return CR_NO_SUCH_DEVINST; - ls = get_device_reg_property(device->key, prop_key, prop_type, prop_buff, *prop_buff_size, prop_buff_size, flags); + ls = get_device_property(device, prop_key, prop_type, prop_buff, *prop_buff_size, prop_buff_size, flags); SetupDiDestroyDeviceInfoList(set); switch (ls) { diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index bc3eae57783..e6adf81ed27 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1201,20 +1201,20 @@ static void test_device_property(void) buffer[0] = '\0'; ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_InstanceId, &type, buffer, sizeof(buffer), &size, 0); err = GetLastError(); - todo_wine ok(ret, "Expect success\n"); - todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); - todo_wine ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_STRING, type); - todo_wine ok(size == sizeof(instance_id), "Got size %lu\n", size); - todo_wine ok(!wcsicmp(instance_id, (WCHAR *)buffer), "Expect buffer %s, got %s\n", debugstr_w(instance_id), debugstr_w((WCHAR*)buffer)); + ok(ret, "Expect success\n"); + ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_STRING, type); + ok(size == sizeof(instance_id), "Got size %lu\n", size); + ok(!wcsicmp(instance_id, (WCHAR *)buffer), "Expect buffer %s, got %s\n", debugstr_w(instance_id), debugstr_w((WCHAR*)buffer)); type = DEVPROP_TYPE_EMPTY; size = 0; ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_ClassGuid, &type, (BYTE *)&guid_val, sizeof(guid_val), &size, 0); err = GetLastError(); - todo_wine ok(ret, "Expect success\n"); - todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); - todo_wine ok(type == DEVPROP_TYPE_GUID, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_GUID, type); - todo_wine ok(size == sizeof(guid_val), "Got size %lu\n", size); - todo_wine ok(IsEqualGUID(&guid_val, &guid), "Expect buffer %s, got %s\n", debugstr_guid(&guid), debugstr_guid(&guid_val)); + ok(ret, "Expect success\n"); + ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + ok(type == DEVPROP_TYPE_GUID, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_GUID, type); + ok(size == sizeof(guid_val), "Got size %lu\n", size); + ok(IsEqualGUID(&guid_val, &guid), "Expect buffer %s, got %s\n", debugstr_guid(&guid), debugstr_guid(&guid_val)); if (pSetupDiGetDevicePropertyKeys) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8515
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 819d1497ff9..7ed5f4e22ba 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1445,8 +1445,11 @@ static void pump_messages(void) static void test_pnp_devices(void) { + static const GUID expect_container_id_guid = {0x12345678, 0x1234, 0x1234, {0x12, 0x34, 0x12, 0x34, 0x56, 0x78, 0x91, 0x23}}; static const char expect_hardware_id[] = "winetest_hardware\0winetest_hardware_1\0"; + static const WCHAR expect_hardware_id_w[] = L"winetest_hardware\0winetest_hardware_1\0"; static const char expect_compat_id[] = "winetest_compat\0winetest_compat_1\0"; + static const WCHAR expect_compat_id_w[] = L"winetest_compat\0winetest_compat_1\0"; static const WCHAR expect_container_id_w[] = L"{12345678-1234-1234-1234-123456789123}"; static const char foobar[] = "foobar"; static const char foo[] = "foo"; @@ -1454,6 +1457,7 @@ static void test_pnp_devices(void) char buffer[200]; WCHAR buffer_w[200]; + GUID buffer_guid = {0}; SP_DEVICE_INTERFACE_DETAIL_DATA_A *iface_detail = (void *)buffer; SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; SP_DEVINFO_DATA device = {sizeof(device)}; @@ -1687,6 +1691,33 @@ static void test_pnp_devices(void) ok(!strcmp(buffer, "\\Device\\winetest_pnp_1"), "got PDO name %s\n", debugstr_a(buffer)); } + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_HardwareIds, &prop_type, (BYTE *)buffer_w, + sizeof(buffer_w), &size, 0); + todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + todo_wine ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + todo_wine ok(size == sizeof(expect_hardware_id_w), "got size %lu\n", size); + todo_wine ok(!memcmp(buffer_w, expect_hardware_id_w, size), "got hardware IDs %s\n", debugstr_wn(buffer_w, size)); + + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_CompatibleIds, &prop_type, (BYTE *)buffer_w, + sizeof(buffer_w), &size, 0); + todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + todo_wine ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + todo_wine ok(size == sizeof(expect_compat_id_w), "got size %lu\n", size); + todo_wine ok(!memcmp(buffer_w, expect_compat_id_w, size), "got compatible IDs %s\n", debugstr_wn(buffer_w, size)); + + prop_type = DEVPROP_TYPE_EMPTY; + size = 0; + ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_ContainerId, &prop_type, (BYTE *)&buffer_guid, + sizeof(buffer_guid), &size, 0); + todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + todo_wine ok(prop_type == DEVPROP_TYPE_GUID, "got type %#lx\n", prop_type); + todo_wine ok(size == sizeof(expect_container_id_guid), "got size %lu\n", size); + todo_wine ok(IsEqualGUID(&buffer_guid, &expect_container_id_guid), "got container ID %s != %s\n", debugstr_guid(&buffer_guid), debugstr_guid(&expect_container_id_guid)); + 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/8515
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 24 ++++++------- dlls/setupapi/devinst.c | 54 ++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 7ed5f4e22ba..673a38a05b1 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1695,28 +1695,28 @@ static void test_pnp_devices(void) size = 0; ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_HardwareIds, &prop_type, (BYTE *)buffer_w, sizeof(buffer_w), &size, 0); - todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); - todo_wine ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); - todo_wine ok(size == sizeof(expect_hardware_id_w), "got size %lu\n", size); - todo_wine ok(!memcmp(buffer_w, expect_hardware_id_w, size), "got hardware IDs %s\n", debugstr_wn(buffer_w, size)); + ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + ok(size == sizeof(expect_hardware_id_w), "got size %lu\n", size); + ok(!memcmp(buffer_w, expect_hardware_id_w, size), "got hardware IDs %s\n", debugstr_wn(buffer_w, size)); prop_type = DEVPROP_TYPE_EMPTY; size = 0; ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_CompatibleIds, &prop_type, (BYTE *)buffer_w, sizeof(buffer_w), &size, 0); - todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); - todo_wine ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); - todo_wine ok(size == sizeof(expect_compat_id_w), "got size %lu\n", size); - todo_wine ok(!memcmp(buffer_w, expect_compat_id_w, size), "got compatible IDs %s\n", debugstr_wn(buffer_w, size)); + ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + ok(prop_type == DEVPROP_TYPE_STRING_LIST, "got type %#lx\n", prop_type); + ok(size == sizeof(expect_compat_id_w), "got size %lu\n", size); + ok(!memcmp(buffer_w, expect_compat_id_w, size), "got compatible IDs %s\n", debugstr_wn(buffer_w, size)); prop_type = DEVPROP_TYPE_EMPTY; size = 0; ret = SetupDiGetDevicePropertyW(set, &device, &DEVPKEY_Device_ContainerId, &prop_type, (BYTE *)&buffer_guid, sizeof(buffer_guid), &size, 0); - todo_wine ok(ret, "failed to get device property, error %#lx\n", GetLastError()); - todo_wine ok(prop_type == DEVPROP_TYPE_GUID, "got type %#lx\n", prop_type); - todo_wine ok(size == sizeof(expect_container_id_guid), "got size %lu\n", size); - todo_wine ok(IsEqualGUID(&buffer_guid, &expect_container_id_guid), "got container ID %s != %s\n", debugstr_guid(&buffer_guid), debugstr_guid(&expect_container_id_guid)); + ok(ret, "failed to get device property, error %#lx\n", GetLastError()); + ok(prop_type == DEVPROP_TYPE_GUID, "got type %#lx\n", prop_type); + ok(size == sizeof(expect_container_id_guid), "got size %lu\n", size); + ok(IsEqualGUID(&buffer_guid, &expect_container_id_guid), "got container ID %s != %s\n", debugstr_guid(&buffer_guid), debugstr_guid(&expect_container_id_guid)); ret = SetupDiEnumDeviceInterfaces(set, NULL, &child_class, 0, &iface); ok(ret, "failed to get interface, error %#lx\n", GetLastError()); diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index e065e6cbe98..3c98244b8d1 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -895,10 +895,8 @@ static struct device *create_device(struct DeviceInfoSet *set, return device; } -static struct device *get_devnode_device(DEVINST devnode, HDEVINFO *set) +static struct device *get_devnode_device(DEVINST devnode, HDEVINFO *set, SP_DEVINFO_DATA *data) { - SP_DEVINFO_DATA data = { sizeof(data) }; - *set = NULL; if (devnode >= devinst_table_size || !devinst_table[devnode]) { @@ -908,13 +906,13 @@ static struct device *get_devnode_device(DEVINST devnode, HDEVINFO *set) *set = SetupDiCreateDeviceInfoListExW(NULL, NULL, NULL, NULL); if (*set == INVALID_HANDLE_VALUE) return NULL; - if (!SetupDiOpenDeviceInfoW(*set, devinst_table[devnode], NULL, 0, &data)) + if (!SetupDiOpenDeviceInfoW(*set, devinst_table[devnode], NULL, 0, data)) { SetupDiDestroyDeviceInfoList(*set); *set = NULL; return NULL; } - return get_device(*set, &data); + return get_device(*set, data); } /*********************************************************************** @@ -5275,8 +5273,9 @@ BOOL WINAPI SetupDiGetDevicePropertyKeys( HDEVINFO devinfo, PSP_DEVINFO_DATA dev return !ret; } -static DWORD get_device_property( struct device *device, const DEVPROPKEY *prop_key, DEVPROPTYPE *prop_type, - BYTE *buf, DWORD buf_size, DWORD *req_size, DWORD flags ) +static DWORD get_device_property( struct device *device, HDEVINFO devinfo, PSP_DEVINFO_DATA device_data, + const DEVPROPKEY *prop_key, DEVPROPTYPE *prop_type, BYTE *buf, DWORD buf_size, + DWORD *req_size, DWORD flags ) { DWORD ret = ERROR_SUCCESS; @@ -5309,6 +5308,38 @@ static DWORD get_device_property( struct device *device, const DEVPROPKEY *prop_ if (req_size) *req_size = size; } + else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_HardwareIds ) + || IsEqualDevPropKey( *prop_key, DEVPKEY_Device_CompatibleIds )) + { + DWORD prop = IsEqualDevPropKey( *prop_key, DEVPKEY_Device_HardwareIds ) ? SPDRP_HARDWAREID : SPDRP_COMPATIBLEIDS, size = 0; + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, prop, NULL, buf, buf_size, &size )) + ret = GetLastError() == ERROR_INVALID_DATA ? ERROR_NOT_FOUND : GetLastError(); + if (ret != ERROR_NOT_FOUND) + { + *prop_type = DEVPROP_TYPE_STRING_LIST; + if (req_size) + *req_size = size; + } + } + else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_ContainerId )) + { + WCHAR id_str[39]; + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, SPDRP_BASE_CONTAINERID, NULL, (BYTE *)id_str, + sizeof(id_str), NULL )) + return GetLastError() == ERROR_INVALID_DATA ? ERROR_NOT_FOUND : GetLastError(); + *prop_type = DEVPROP_TYPE_GUID; + if (buf_size >= sizeof( GUID )) + { + GUID guid = {0}; + id_str[37] = 0; + UuidFromStringW( &id_str[1], &guid ); + memcpy( buf, &guid, sizeof( guid ) ); + } + else + ret = ERROR_INSUFFICIENT_BUFFER; + if (req_size) + *req_size = sizeof( GUID ); + } else ret = get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags ); return ret; @@ -5330,7 +5361,8 @@ BOOL WINAPI SetupDiGetDevicePropertyW(HDEVINFO devinfo, PSP_DEVINFO_DATA device_ if (!(device = get_device(devinfo, device_data))) return FALSE; - ls = get_device_property(device, prop_key, prop_type, prop_buff, prop_buff_size, required_size, flags); + ls = get_device_property(device, devinfo, device_data, prop_key, prop_type, prop_buff, prop_buff_size, + required_size, flags); SetLastError(ls); return !ls; @@ -5343,6 +5375,7 @@ CONFIGRET WINAPI CM_Get_DevNode_Property_ExW(DEVINST devnode, const DEVPROPKEY * BYTE *prop_buff, ULONG *prop_buff_size, ULONG flags, HMACHINE machine) { HDEVINFO set; + SP_DEVINFO_DATA data = { sizeof(data) }; struct device *device; LSTATUS ls; @@ -5355,10 +5388,11 @@ CONFIGRET WINAPI CM_Get_DevNode_Property_ExW(DEVINST devnode, const DEVPROPKEY * if (!prop_buff_size) return CR_INVALID_POINTER; - if (!(device = get_devnode_device(devnode, &set))) + if (!(device = get_devnode_device(devnode, &set, &data))) return CR_NO_SUCH_DEVINST; - ls = get_device_property(device, prop_key, prop_type, prop_buff, *prop_buff_size, prop_buff_size, flags); + ls = get_device_property(device, set, &data, prop_key, prop_type, prop_buff, *prop_buff_size, + prop_buff_size, flags); SetupDiDestroyDeviceInfoList(set); switch (ls) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8515
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
ok(size == sizeof(valueW), "Got size %ld\n", size); ok(!lstrcmpW((WCHAR *)buffer, valueW), "Expect buffer %s, got %s\n", wine_dbgstr_w(valueW), wine_dbgstr_w((WCHAR *)buffer));
+ /* #15 Innate properties */ + type = DEVPROP_TYPE_EMPTY; + size = 0; + buffer[0] = '\0';
This is a `BYTE` type buffer being used to store a wide character string return value, so I'm pretty sure what you're doing here would only set half of the character value to NULL. I think the better way to do this is to just do: ```suggestion:-0+0 memset(buffer, 0, sizeof(buffer)); ``` Like the previous test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109122
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
+ ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_InstanceId, &type, buffer, sizeof(buffer), &size, 0); + err = GetLastError(); + todo_wine ok(ret, "Expect success\n"); + todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + todo_wine ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_STRING, type); + todo_wine ok(size == sizeof(instance_id), "Got size %lu\n", size); + todo_wine ok(!wcsicmp(instance_id, (WCHAR *)buffer), "Expect buffer %s, got %s\n", debugstr_w(instance_id), debugstr_w((WCHAR*)buffer)); + type = DEVPROP_TYPE_EMPTY; + size = 0; + ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_ClassGuid, &type, (BYTE *)&guid_val, sizeof(guid_val), &size, 0); + err = GetLastError(); + todo_wine ok(ret, "Expect success\n"); + todo_wine ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + todo_wine ok(type == DEVPROP_TYPE_GUID, "Expect type %#x, got %#lx\n", DEVPROP_TYPE_GUID, type); + todo_wine ok(size == sizeof(guid_val), "Got size %lu\n", size); + todo_wine ok(IsEqualGUID(&guid_val, &guid), "Expect buffer %s, got %s\n", debugstr_guid(&guid), debugstr_guid(&guid_val)); Adding a newline between these two tests would make this a bit more readable, e.g an empty line after line 1208.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109123
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
err = GetLastError(); ok(!err, "Expect last error %#x, got %#lx\n", ERROR_SUCCESS, err); ok(keys_len == required_len, "%lu != %lu\n", keys_len, required_len); - ok(keys_len >= expected_keys, "Expected %lu >= %lu\n", keys_len, expected_keys); + todo_wine ok(keys_len >= expected_keys, "Expected %lu >= %lu\n", keys_len, expected_keys);
keys_len = 0; if (keys) { for (n = 0; n < required_len; n++) { + DWORD i;
Newline after variable declaration please. :) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109124
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
{ + DWORD i; if (!memcmp(&keys[n], &DEVPKEY_Device_FriendlyName, sizeof(keys[n]))) + { keys_len++; + continue; + } + for (i = 0; i < ARRAY_SIZE(default_keys); i++) + { + if (!memcmp(&keys[n], &default_keys[i], sizeof(keys[n]))) + { + keys_len++; + break; + } + } }
This loop in general feels a little clunky, is it possible to recreate the device we're testing/delete the key we've set for the device in prior tests, and then do a separate for loop so that we're only checking the default built-in keys? I think that'd make it clearer what the built-in keys are, and simplify this loop a bit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109125
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
BYTE *prop_buff, ULONG *prop_buff_size, ULONG flags, HMACHINE machine) { HDEVINFO set; + SP_DEVINFO_DATA data = { sizeof(data) };
I don't have a super strong opinion on this, but if you initialize this in `get_devnode_device()` instead you'd avoid the possibility in the future of accidentally leaving this uninitialized, as an example if that function is ever called from another function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109128
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
+static DWORD get_device_property( struct device *device, const DEVPROPKEY *prop_key, DEVPROPTYPE *prop_type, + BYTE *buf, DWORD buf_size, DWORD *req_size, DWORD flags ) +{ + DWORD ret = ERROR_SUCCESS; + + if (!prop_key) + return ERROR_INVALID_DATA; + if (!prop_type || (!buf && buf_size)) + return ERROR_INVALID_USER_BUFFER; + if (flags) + return ERROR_INVALID_FLAGS; + + if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_InstanceId )) + { + DWORD size = (wcslen( device->instanceId ) + 1) * sizeof( WCHAR );
Newline after this to separate variable declaration/code. Same goes for the `else if` below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109126
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
ok(size == sizeof(valueW), "Got size %ld\n", size); ok(!lstrcmpW((WCHAR *)buffer, valueW), "Expect buffer %s, got %s\n", wine_dbgstr_w(valueW), wine_dbgstr_w((WCHAR *)buffer));
+ /* #15 Innate properties */ + type = DEVPROP_TYPE_EMPTY; + size = 0; + buffer[0] = '\0'; + ret = pSetupDiGetDevicePropertyW(set, &device_data, &DEVPKEY_Device_InstanceId, &type, buffer, sizeof(buffer), &size, 0);
Just out of curiosity, are these built-in properties settable? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109127
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
+ *prop_type = DEVPROP_TYPE_STRING_LIST; + if (req_size) + *req_size = size; + } + } + else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_ContainerId )) + { + WCHAR id_str[39]; + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, SPDRP_BASE_CONTAINERID, NULL, (BYTE *)id_str, + sizeof(id_str), NULL )) + return GetLastError() == ERROR_INVALID_DATA ? ERROR_NOT_FOUND : GetLastError(); + *prop_type = DEVPROP_TYPE_GUID; + if (buf_size >= sizeof( GUID )) + { + GUID guid = {0}; + id_str[37] = 0; Is it ever possible for this function to return an empty string, or a string not representing a GUID? Elsewhere in setupapi it seems there are checks to ensure the correct characters are there, i.e:
if (szKeyName[0] == '{' && szKeyName[37] == '}')
If it's something that is only ever handled by us I guess we could do an assert to make sure it never gets messed up. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109129
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
if (req_size) *req_size = size; } + else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_HardwareIds ) + || IsEqualDevPropKey( *prop_key, DEVPKEY_Device_CompatibleIds )) + { + DWORD prop = IsEqualDevPropKey( *prop_key, DEVPKEY_Device_HardwareIds ) ? SPDRP_HARDWAREID : SPDRP_COMPATIBLEIDS, size = 0; + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, prop, NULL, buf, buf_size, &size ))
Like the earlier commit, newline after variable declaration (same for the else if below) :) Also, splitting these commits up to only address one `DEVPKEY` at a time would make this easier to review and probably help regression testing. I.e, 3 separate commits, one for `DEVPKEY_Device_HardwareIds`, the next for `DEVPKEY_Device_CompatibleIds`, and then the final one for `DEVPKEY_Device_ContainerId`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109130
Also, something I forgot about until just now: If we know what the built-in device properties are on native and we don't implement them, it might be worth adding a check and emitting a FIXME, i.e `FIXME("Unimplemented builtin property.\n")`. Just a thought. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109135
On Tue Jul 8 16:24:38 2025 +0000, Connor McAdams wrote:
This is a `BYTE` type buffer being used to store a wide character string return value, so I'm pretty sure what you're doing here would only set half of the character value to NULL. I think the better way to do this is to just do: ```suggestion:-0+0 memset(buffer, 0, sizeof(buffer)); ``` Like the previous test. Oh right, these are wide strings. Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8515#note_109167
participants (3)
-
Connor McAdams (＠cmcadams) -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)