[PATCH v9 0/3] MR8515: setupapi: Support in-built properties for device instances in SetupDiGetDevicePropertyW.
-- v9: setupapi: Support in-built properties in SetupDiGetDevicePropertyW and CM_Get_DevNode_Property_ExW.
From: Vibhav Pant vibhavp@gmail.com
--- dlls/setupapi/tests/devinst.c | 257 ++++++++++++++++++++++++++++++++-- 1 file changed, 249 insertions(+), 8 deletions(-)
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index ed9d197e3bd..093e8acf37a 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -49,6 +49,8 @@ static GUID guid2 = {0x6a55b5a5, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c, static GUID iface_guid = {0xdeadbeef, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}}; static GUID iface_guid2 = {0xdeadf00d, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}};
+static WCHAR guid_strw[] = L"{6a55b5a4-3f65-11db-b704-0011955c2bdb}"; + static HRESULT (WINAPI *pDriverStoreAddDriverPackageA)(const char *inf_path, void *unk1, void *unk2, WORD architecture, char *ret_path, DWORD *ret_len); static HRESULT (WINAPI *pDriverStoreDeleteDriverPackageA)(const char *path, void *unk1, void *unk2); @@ -872,17 +874,74 @@ static void test_device_info(void) SetupDiDestroyDeviceInfoList(set); }
+struct reg_property +{ + DWORD reg_prop; + DWORD reg_type; + BYTE *reg_value; + DWORD reg_size; + DEVPROPTYPE devprop_type; + BYTE *devprop_value; + DWORD devprop_size; + + /* Error returned by SetupDiSetDevicePropertyW. */ + DWORD error; +}; + +static const LONG dword = 0xdeadbeef; +static const WCHAR strw[] = L"dummy"; +static const WCHAR multi_strw[] = L"dummy1\0dummy2\0"; + +/* SPDRP_* properties that can be set and have an associated DEVPROPKEY. + * The property key is {{a45c254e-df1c-4efd-8020-67d146a850e0}, reg_prop + 2} */ +static const struct reg_property reg_props[] = { + { SPDRP_DEVICEDESC, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw) }, + { SPDRP_SERVICE, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw) }, + { SPDRP_DRIVER, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw) }, + { SPDRP_MFG, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw) }, + { SPDRP_LOCATION_INFORMATION, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw) }, + { SPDRP_CONFIGFLAGS, REG_DWORD, (BYTE *)&dword, sizeof(dword), DEVPROP_TYPE_UINT32, (BYTE *)&dword, sizeof(dword) }, + { SPDRP_UPPERFILTERS, REG_MULTI_SZ, (BYTE *)multi_strw, sizeof(multi_strw), DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw) }, + { SPDRP_HARDWAREID, REG_MULTI_SZ, (BYTE *)multi_strw, sizeof(multi_strw), DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw) }, + { SPDRP_COMPATIBLEIDS, REG_MULTI_SZ, (BYTE *)multi_strw, sizeof(multi_strw), DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw) }, + { SPDRP_CLASSGUID, REG_SZ, (BYTE *)guid_strw, sizeof(guid_strw), DEVPROP_TYPE_GUID, (BYTE *)&guid, sizeof(guid) }, +}; + +/* These properties cannot be set through SetupDiSetDevice(Registry)Property. */ +static const struct reg_property invalid_reg_props[] = { + { SPDRP_CAPABILITIES, REG_DWORD, (BYTE *)&dword, sizeof(dword), DEVPROP_TYPE_INT32, (BYTE *)&dword, sizeof(dword), ERROR_ACCESS_DENIED }, + { SPDRP_UI_NUMBER, REG_DWORD, (BYTE *)&dword, sizeof(dword), DEVPROP_TYPE_INT32, (BYTE *)&dword, sizeof(dword), ERROR_ACCESS_DENIED }, + { SPDRP_PHYSICAL_DEVICE_OBJECT_NAME, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw), ERROR_ACCESS_DENIED }, + { SPDRP_BASE_CONTAINERID, REG_SZ, (BYTE *)guid_strw, sizeof(guid_strw), DEVPROP_TYPE_GUID, (BYTE *)&guid, sizeof(guid), ERROR_INVALID_REG_PROPERTY }, +}; + static void test_device_property(void) { + static struct property { + DEVPROPKEY key; + DEVPROPTYPE type; + BYTE *value; + DWORD size; + BOOL exists; + } inbuilt_props[] = { + { DEVPKEY_Device_InstanceId, DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw), TRUE }, + { DEVPKEY_Device_Parent, DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw), TRUE }, + { DEVPKEY_Device_Siblings, DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw), TRUE }, + { DEVPKEY_Device_BusReportedDeviceDesc, DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw), FALSE }, + { DEVPKEY_Device_Children, DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw), FALSE }, + { DEVPKEY_Device_ContainerId, DEVPROP_TYPE_GUID, (BYTE *)&guid, sizeof(guid), FALSE }, + }; 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; DEVPROPTYPE type; - DWORD size; + DWORD size, i; BYTE buffer[256]; DWORD err; BOOL ret; + GUID guid_val = {0};
hmod = LoadLibraryA("setupapi.dll"); pSetupDiSetDevicePropertyW = (void *)GetProcAddress(hmod, "SetupDiSetDevicePropertyW"); @@ -899,7 +958,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 +1252,84 @@ 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; + memset(buffer, 0, sizeof(buffer)); + SetLastError(0xdeadbeef); + 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)); + + for (i = 0; i < ARRAY_SIZE(inbuilt_props); i++) + { + const DEVPROPKEY *key = &inbuilt_props[i].key; + DEVPROPTYPE type = inbuilt_props[i].type; + const BYTE *val = inbuilt_props[i].value; + DWORD size = inbuilt_props[i].size; + BYTE *buf; + + winetest_push_context("inbuilt_props[%lu]", i); + SetLastError(0xdeadbeef); + ret = pSetupDiSetDevicePropertyW(set, &device_data, key, type, val, size, 0); + err = GetLastError(); + todo_wine ok(!ret, "Expected failure.\n"); + todo_wine ok(err == ERROR_ACCESS_DENIED, "Expect last error %#x, got %#lx.\n", ERROR_INVALID_ACCESS, err); + if (ret) + { + winetest_pop_context(); + continue; + } + /* However, setting this property to its existing value (if it has one) succeeds. */ + size = 0; + SetLastError(0xdeadbeef); + ret = pSetupDiGetDevicePropertyW(set, &device_data, key, &type, NULL, 0, &size, 0); + err = GetLastError(); + ok(!ret, "Expect failure.\n"); + if (inbuilt_props[i].exists) + todo_wine ok(err == ERROR_INSUFFICIENT_BUFFER, "Expect last error %#x, got %#lx\n", ERROR_INSUFFICIENT_BUFFER, err); + else + { + todo_wine_if(inbuilt_props[i].exists) ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#lx\n", + ERROR_NOT_FOUND, err); + winetest_pop_context(); + continue; + } + + buf = calloc(1, size); + SetLastError(0xdeadbeef); + ret = pSetupDiGetDevicePropertyW(set, &device_data, key, &type, buf, size, &size, 0); + err = GetLastError(); + ok(ret, "Expect success.\n"); + ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + + SetLastError(0xdeadbeef); + ret = pSetupDiSetDevicePropertyW(set, &device_data, key, type, buf, size, 0); + err = GetLastError(); + ok(ret, "Expect success.\n"); + ok(err == NO_ERROR, "Expect last error %#x, got %#lx\n", NO_ERROR, err); + free(buf); + winetest_pop_context(); + } + if (pSetupDiGetDevicePropertyKeys) { - DWORD keys_len = 0, n, required_len, expected_keys = 1; + DEVPROPKEY expected_keys[] = { DEVPKEY_Device_ClassGuid, DEVPKEY_Device_InstanceId, DEVPKEY_Device_FriendlyName }; + DWORD keys_len = 0, n, required_len; DEVPROPKEY *keys;
ret = pSetupDiGetDevicePropertyKeys(NULL, NULL, NULL, 0, NULL, 0); @@ -1255,19 +1389,27 @@ 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); + ok(keys_len >= ARRAY_SIZE(expected_keys), "Expected %lu >= %lu\n", keys_len, (DWORD)ARRAY_SIZE(expected_keys));
keys_len = 0; if (keys) { for (n = 0; n < required_len; n++) { - if (!memcmp(&keys[n], &DEVPKEY_Device_FriendlyName, sizeof(keys[n]))) - keys_len++; + DWORD i; + + for (i = 0; i < ARRAY_SIZE(expected_keys); i++) + { + if (!memcmp(&keys[n], &expected_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 == ARRAY_SIZE(expected_keys), "%lu != %lu\n", keys_len, (DWORD)ARRAY_SIZE(expected_keys)); free(keys); } else @@ -1277,6 +1419,57 @@ static void test_device_property(void) ok(ret, "Got unexpected error %#lx.\n", GetLastError());
SetupDiDestroyDeviceInfoList(set); + + set = SetupDiCreateDeviceInfoList(NULL, NULL); + ok(set != INVALID_HANDLE_VALUE, "Failed to create device list, error %#lx.\n", GetLastError()); + + ret = SetupDiCreateDeviceInfoW(set, L"display", &GUID_DEVCLASS_DISPLAY, NULL, NULL, DICD_GENERATE_ID, &device_data); + ok(ret, "Failed to create device, error %#lx.\n", GetLastError()); + + for (i = 0; i < ARRAY_SIZE(reg_props); i++) + { + const struct reg_property *prop = ®_props[i]; + BOOL todo = prop->reg_prop != SPDRP_CLASSGUID; + DWORD size = 0, type = 0; + DEVPROPKEY key = { DEVPKEY_Device_DeviceDesc.fmtid, prop->reg_prop + 2 }; + BYTE buf[80] = { 0 }; + + winetest_push_context("reg_props[%lu] (%#lx)", i, prop->reg_prop); + SetLastError(0xdeadbeef); + ret = pSetupDiSetDevicePropertyW(set, &device_data, &key, prop->devprop_type, prop->devprop_value, + prop->devprop_size, 0); + err = GetLastError(); + ok(ret, "Failed to set property, error %#lx.\n", err); + ok(!err, "Expect last error %#x, got %#lx\n", ERROR_SUCCESS, err); + + ret = SetupDiGetDeviceRegistryPropertyW(set, &device_data, prop->reg_prop, &type, buf, sizeof(buf), &size); + todo_wine_if(todo) ok(ret, "Failed to get property, error %#lx.\n", GetLastError()); + todo_wine_if(todo) ok(type == prop->reg_type, "Got unexpected type %#lx.\n", type); + todo_wine_if(todo) ok(size == prop->reg_size, "Got unexpected size %lu.\n", size); + if (size == prop->reg_size) + todo_wine ok(!memcmp(buf, prop->reg_value, size), "Got unexpected property value.\n"); + winetest_pop_context(); + } + + for (i = 0; i < ARRAY_SIZE(invalid_reg_props); i++) + { + const struct reg_property *prop = &invalid_reg_props[i]; + DEVPROPKEY key = { DEVPKEY_Device_DeviceDesc.fmtid, prop->reg_prop + 2 }; + + winetest_push_context("invalid_reg_props[%lu] (%#lx)", i, prop->reg_prop); + SetLastError(0xdeadbeef); + ret = pSetupDiSetDevicePropertyW(set, &device_data, &key, prop->devprop_type, prop->devprop_value, + prop->devprop_size, 0); + todo_wine ok(!ret, "Expect failure.\n"); + todo_wine ok(GetLastError() == prop->error, "Got unexpected error %#lx.\n", GetLastError()); + winetest_pop_context(); + } + + ret = SetupDiRemoveDevice(set, &device_data); + ok(ret, "Got unexpected error %#lx.\n", GetLastError()); + + SetupDiDestroyDeviceInfoList(set); + FreeLibrary(hmod); }
@@ -2268,7 +2461,7 @@ static void test_registry_property_w(void) WCHAR friendly_name[] = {'B','o','g','u','s',0}; SP_DEVINFO_DATA device = {sizeof(device)}; WCHAR buf[64] = {0}; - DWORD size, type; + DWORD size, type, i; HDEVINFO set; BOOL ret; LONG res; @@ -2421,6 +2614,54 @@ todo_wine { ok(!lstrcmpW(buf, L"Display"), "Got unexpected value %s.\n", wine_dbgstr_w(buf));
SetupDiDestroyDeviceInfoList(set); + + set = SetupDiCreateDeviceInfoList(NULL, NULL); + ok(set != INVALID_HANDLE_VALUE, "Failed to create device list, error %#lx.\n", GetLastError()); + + ret = SetupDiCreateDeviceInfoW(set, L"display", &GUID_DEVCLASS_DISPLAY, NULL, NULL, DICD_GENERATE_ID, &device); + ok(ret, "Failed to create device, error %#lx.\n", GetLastError()); + + for (i = 0; i < ARRAY_SIZE(reg_props); i++) + { + const struct reg_property *prop = ®_props[i]; + DEVPROPKEY key = { DEVPKEY_Device_DeviceDesc.fmtid, prop->reg_prop + 2 }; + DEVPROPTYPE type = DEVPROP_TYPE_EMPTY; + BYTE buf[80] = {0}; + DWORD size = 0; + + winetest_push_context("reg_props[%lu] (%#lx)", i, prop->reg_prop); + SetLastError(0xdeadbeef); + ret = SetupDiSetDeviceRegistryPropertyW(set, &device, prop->reg_prop, prop->reg_value, prop->reg_size); + ok(ret, "Failed to set property, error %#lx.\n", GetLastError()); + ret = SetupDiGetDevicePropertyW(set, &device, &key, &type, buf, sizeof(buf), &size, 0); + todo_wine ok(ret, "Failed to get property, error %#lx.\n", GetLastError()); + todo_wine ok(type == prop->devprop_type, "Got unexpected type %#lx.\n", type); + todo_wine ok(size == prop->devprop_size, "Got unexpected size %lu.\n", size); + if (size == prop->devprop_size) + ok(!memcmp(buf, prop->devprop_value, size), "Got unexpected property value.\n"); + winetest_pop_context(); + } + SetupDiDestroyDeviceInfoList(set); + + set = SetupDiCreateDeviceInfoList(NULL, NULL); + ok(set != INVALID_HANDLE_VALUE, "Failed to create device list, error %#lx.\n", GetLastError()); + + ret = SetupDiCreateDeviceInfoW(set, L"display", &GUID_DEVCLASS_DISPLAY, NULL, NULL, DICD_GENERATE_ID, &device); + ok(ret, "Failed to create device, error %#lx.\n", GetLastError()); + + for (i = 0; i < ARRAY_SIZE(invalid_reg_props); i++) + { + const struct reg_property *prop = &invalid_reg_props[i]; + + /* SetupDiSetDeviceRegistryPropertyW seems to only return ERROR_INVALID_REG_PROPERTY. */ + winetest_push_context("invalid_reg_props[%lu] (%#lx)", i, prop->reg_prop); + SetLastError(0xdeadbeef); + ret = SetupDiSetDeviceRegistryPropertyW(set, &device, prop->reg_prop, prop->reg_value, prop->reg_size); + todo_wine_if(prop->reg_prop != SPDRP_PHYSICAL_DEVICE_OBJECT_NAME) ok(!ret, "Expected failure.\n"); + todo_wine ok(GetLastError() == ERROR_INVALID_REG_PROPERTY, "Got unexpected error %#lx.\n", GetLastError()); + winetest_pop_context(); + } + SetupDiDestroyDeviceInfoList(set); }
static void test_get_inf_class(void)
From: Vibhav Pant vibhavp@gmail.com
--- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 34 ++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 819d1497ff9..60cc530240e 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,36 @@ 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; + memset(buffer_w, 0, sizeof(buffer_w)); + 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; + memset(buffer_w, 0, sizeof(buffer_w)); + 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),
From: Vibhav Pant vibhavp@gmail.com
--- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 26 +++--- dlls/setupapi/devinst.c | 136 +++++++++++++++++++++++------ dlls/setupapi/tests/devinst.c | 26 +++--- 3 files changed, 136 insertions(+), 52 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 60cc530240e..1822923e552 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1696,30 +1696,30 @@ static void test_pnp_devices(void) memset(buffer_w, 0, sizeof(buffer_w)); 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; memset(buffer_w, 0, sizeof(buffer_w)); 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 066bd6f09c2..0b95128b266 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -629,30 +629,31 @@ struct PropertyMapEntry DWORD regType; LPCSTR nameA; LPCWSTR nameW; + DEVPROPTYPE devPropType; };
-#define PROPERTY_MAP_ENTRY(type, name) { type, name, L##name } +#define PROPERTY_MAP_ENTRY(type, name, devproptype) { type, name, L##name, devproptype } static const struct PropertyMapEntry PropertyMap[] = { - PROPERTY_MAP_ENTRY(REG_SZ, "DeviceDesc"), - PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "HardwareId"), - PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "CompatibleIDs"), + PROPERTY_MAP_ENTRY(REG_SZ, "DeviceDesc", DEVPROP_TYPE_STRING), + PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "HardwareId", DEVPROP_TYPE_STRING_LIST), + PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "CompatibleIDs", DEVPROP_TYPE_STRING_LIST), { 0, NULL, NULL }, /* SPDRP_UNUSED0 */ - PROPERTY_MAP_ENTRY(REG_SZ, "Service"), + PROPERTY_MAP_ENTRY(REG_SZ, "Service", DEVPROP_TYPE_STRING), { 0, NULL, NULL }, /* SPDRP_UNUSED1 */ { 0, NULL, NULL }, /* SPDRP_UNUSED2 */ - PROPERTY_MAP_ENTRY(REG_SZ, "Class"), - PROPERTY_MAP_ENTRY(REG_SZ, "ClassGUID"), - PROPERTY_MAP_ENTRY(REG_SZ, "Driver"), - PROPERTY_MAP_ENTRY(REG_DWORD, "ConfigFlags"), - PROPERTY_MAP_ENTRY(REG_SZ, "Mfg"), - PROPERTY_MAP_ENTRY(REG_SZ, "FriendlyName"), - PROPERTY_MAP_ENTRY(REG_SZ, "LocationInformation"), - { 0, NULL, NULL }, /* SPDRP_PHYSICAL_DEVICE_OBJECT_NAME */ - PROPERTY_MAP_ENTRY(REG_DWORD, "Capabilities"), - PROPERTY_MAP_ENTRY(REG_DWORD, "UINumber"), - PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "UpperFilters"), - PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "LowerFilters"), - [SPDRP_BASE_CONTAINERID] = PROPERTY_MAP_ENTRY(REG_SZ, "ContainerId"), + PROPERTY_MAP_ENTRY(REG_SZ, "Class", DEVPROP_TYPE_STRING), + PROPERTY_MAP_ENTRY(REG_SZ, "ClassGUID", DEVPROP_TYPE_GUID), + PROPERTY_MAP_ENTRY(REG_SZ, "Driver", DEVPROP_TYPE_STRING), + PROPERTY_MAP_ENTRY(REG_DWORD, "ConfigFlags", DEVPROP_TYPE_UINT32), + PROPERTY_MAP_ENTRY(REG_SZ, "Mfg", DEVPROP_TYPE_STRING), + PROPERTY_MAP_ENTRY(REG_SZ, "FriendlyName", 0), + PROPERTY_MAP_ENTRY(REG_SZ, "LocationInformation", DEVPROP_TYPE_STRING), + { 0, NULL, NULL, DEVPROP_TYPE_STRING }, /* SPDRP_PHYSICAL_DEVICE_OBJECT_NAME */ + PROPERTY_MAP_ENTRY(REG_DWORD, "Capabilities", DEVPROP_TYPE_INT32), + PROPERTY_MAP_ENTRY(REG_DWORD, "UINumber", DEVPROP_TYPE_INT32), + PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "UpperFilters", DEVPROP_TYPE_STRING_LIST), + PROPERTY_MAP_ENTRY(REG_MULTI_SZ, "LowerFilters", DEVPROP_TYPE_STRING_LIST), + [SPDRP_BASE_CONTAINERID] = PROPERTY_MAP_ENTRY(REG_SZ, "ContainerId", DEVPROP_TYPE_GUID), }; #undef PROPERTY_MAP_ENTRY
@@ -895,10 +896,9 @@ 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) }; - + data->cbSize = sizeof(*data); *set = NULL; if (devnode >= devinst_table_size || !devinst_table[devnode]) { @@ -908,13 +908,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,6 +5275,87 @@ BOOL WINAPI SetupDiGetDevicePropertyKeys( HDEVINFO devinfo, PSP_DEVINFO_DATA dev return !ret; }
+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; + + 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 (IsEqualGUID( &prop_key->fmtid, &DEVPKEY_Device_DeviceDesc.fmtid )) + { + DWORD reg_prop = prop_key->pid - 2, size, type; + + if (prop_key->pid < 2 || reg_prop >= ARRAY_SIZE(PropertyMap) || !PropertyMap[reg_prop].devPropType) + return get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags ); + ret = ERROR_SUCCESS; + type = PropertyMap[reg_prop].devPropType; + switch (type) + { + case DEVPROP_TYPE_GUID: + { + WCHAR guid_str[39]; + + size = sizeof( GUID ); + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, reg_prop, NULL, (BYTE *)guid_str, + sizeof( guid_str ), NULL )) + { + ret = GetLastError(); + break; + } + if (buf_size >= sizeof( GUID )) + { + if (guid_str[0] == '{' && guid_str[37] == '}') + { + guid_str[37] = 0; + if (!UuidFromStringW( &guid_str[1], (GUID *)buf )) + break; + } + ERR( "Invalid GUID string: %s\n", debugstr_w( guid_str ) ); + } + else + ret = ERROR_INSUFFICIENT_BUFFER; + break; + } + default: + if (!SetupDiGetDeviceRegistryPropertyW( devinfo, device_data, reg_prop, NULL, buf, buf_size, &size )) + ret = GetLastError(); + break; + } + if (ret != ERROR_INVALID_DATA) + { + *prop_type = type; + if (req_size) + *req_size = size; + } + else + ret = ERROR_NOT_FOUND; + } + else 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_ContainerId )) + return get_device_property( device, devinfo, device_data, &DEVPKEY_Device_BaseContainerId, prop_type, buf, + buf_size, req_size, flags ); + else + ret = get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags ); + return ret; +} + /*********************************************************************** * SetupDiGetDevicePropertyW (SETUPAPI.@) */ @@ -5291,7 +5372,8 @@ 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, devinfo, device_data, prop_key, prop_type, prop_buff, prop_buff_size, + required_size, flags);
SetLastError(ls); return !ls; @@ -5304,6 +5386,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;
@@ -5316,10 +5399,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_reg_property(device->key, 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) { diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 093e8acf37a..442267fbf2d 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1259,21 +1259,21 @@ static void test_device_property(void) SetLastError(0xdeadbeef); 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));
for (i = 0; i < ARRAY_SIZE(inbuilt_props); i++) { @@ -2634,9 +2634,9 @@ todo_wine { ret = SetupDiSetDeviceRegistryPropertyW(set, &device, prop->reg_prop, prop->reg_value, prop->reg_size); ok(ret, "Failed to set property, error %#lx.\n", GetLastError()); ret = SetupDiGetDevicePropertyW(set, &device, &key, &type, buf, sizeof(buf), &size, 0); - todo_wine ok(ret, "Failed to get property, error %#lx.\n", GetLastError()); - todo_wine ok(type == prop->devprop_type, "Got unexpected type %#lx.\n", type); - todo_wine ok(size == prop->devprop_size, "Got unexpected size %lu.\n", size); + ok(ret, "Failed to get property, error %#lx.\n", GetLastError()); + ok(type == prop->devprop_type, "Got unexpected type %#lx.\n", type); + ok(size == prop->devprop_size, "Got unexpected size %lu.\n", size); if (size == prop->devprop_size) ok(!memcmp(buf, prop->devprop_value, size), "Got unexpected property value.\n"); winetest_pop_context();
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
static GUID iface_guid = {0xdeadbeef, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}}; static GUID iface_guid2 = {0xdeadf00d, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}};
+static WCHAR guid_strw[] = L"{6a55b5a4-3f65-11db-b704-0011955c2bdb}";
```suggestion:-0+0 static const WCHAR guid_strw[] = L"{6a55b5a4-3f65-11db-b704-0011955c2bdb}"; ``` Forgot a const :)
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
- { SPDRP_HARDWAREID, REG_MULTI_SZ, (BYTE *)multi_strw, sizeof(multi_strw), DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw) },
- { SPDRP_COMPATIBLEIDS, REG_MULTI_SZ, (BYTE *)multi_strw, sizeof(multi_strw), DEVPROP_TYPE_STRING_LIST, (BYTE *)multi_strw, sizeof(multi_strw) },
- { SPDRP_CLASSGUID, REG_SZ, (BYTE *)guid_strw, sizeof(guid_strw), DEVPROP_TYPE_GUID, (BYTE *)&guid, sizeof(guid) },
+};
+/* These properties cannot be set through SetupDiSetDevice(Registry)Property. */ +static const struct reg_property invalid_reg_props[] = {
- { SPDRP_CAPABILITIES, REG_DWORD, (BYTE *)&dword, sizeof(dword), DEVPROP_TYPE_INT32, (BYTE *)&dword, sizeof(dword), ERROR_ACCESS_DENIED },
- { SPDRP_UI_NUMBER, REG_DWORD, (BYTE *)&dword, sizeof(dword), DEVPROP_TYPE_INT32, (BYTE *)&dword, sizeof(dword), ERROR_ACCESS_DENIED },
- { SPDRP_PHYSICAL_DEVICE_OBJECT_NAME, REG_SZ, (BYTE *)strw, sizeof(strw), DEVPROP_TYPE_STRING, (BYTE *)strw, sizeof(strw), ERROR_ACCESS_DENIED },
- { SPDRP_BASE_CONTAINERID, REG_SZ, (BYTE *)guid_strw, sizeof(guid_strw), DEVPROP_TYPE_GUID, (BYTE *)&guid, sizeof(guid), ERROR_INVALID_REG_PROPERTY },
+};
static void test_device_property(void) {
- static struct property {
```suggestion:-0+0 static const struct property { ```
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
- todo_wine ok(IsEqualGUID(&guid_val, &guid), "Expect buffer %s, got %s\n", debugstr_guid(&guid), debugstr_guid(&guid_val));
- for (i = 0; i < ARRAY_SIZE(inbuilt_props); i++)
- {
const DEVPROPKEY *key = &inbuilt_props[i].key;DEVPROPTYPE type = inbuilt_props[i].type;const BYTE *val = inbuilt_props[i].value;DWORD size = inbuilt_props[i].size;BYTE *buf;winetest_push_context("inbuilt_props[%lu]", i);SetLastError(0xdeadbeef);ret = pSetupDiSetDevicePropertyW(set, &device_data, key, type, val, size, 0);err = GetLastError();todo_wine ok(!ret, "Expected failure.\n");todo_wine ok(err == ERROR_ACCESS_DENIED, "Expect last error %#x, got %#lx.\n", ERROR_INVALID_ACCESS, err);
```suggestion:-0+0 todo_wine ok(err == ERROR_ACCESS_DENIED, "Expect last error %#x, got %#lx.\n", ERROR_ACCESS_DENIED, err); ``` You're checking for `err == ERROR_ACCESS_DENIED` here, but printing an expected value of `ERROR_INVALID_ACCESS`.
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);
ok(keys_len >= ARRAY_SIZE(expected_keys), "Expected %lu >= %lu\n", keys_len, (DWORD)ARRAY_SIZE(expected_keys));
```suggestion:-0+0 ok(keys_len >= ARRAY_SIZE(expected_keys), "Expected %lu >= %u\n", keys_len, ARRAY_SIZE(expected_keys)); ``` Casting this feels a bit funky, I'd just do the above instead.
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
keys_len++;
DWORD i;for (i = 0; i < ARRAY_SIZE(expected_keys); i++){if (!memcmp(&keys[n], &expected_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 == ARRAY_SIZE(expected_keys), "%lu != %lu\n", keys_len, (DWORD)ARRAY_SIZE(expected_keys));
```suggestion:-0+0 todo_wine ok(keys_len == ARRAY_SIZE(expected_keys), "%lu != %u\n", keys_len, ARRAY_SIZE(expected_keys)); ```
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
- ok(ret, "Failed to create device, error %#lx.\n", GetLastError());
- for (i = 0; i < ARRAY_SIZE(reg_props); i++)
- {
const struct reg_property *prop = ®_props[i];BOOL todo = prop->reg_prop != SPDRP_CLASSGUID;DWORD size = 0, type = 0;DEVPROPKEY key = { DEVPKEY_Device_DeviceDesc.fmtid, prop->reg_prop + 2 };BYTE buf[80] = { 0 };winetest_push_context("reg_props[%lu] (%#lx)", i, prop->reg_prop);SetLastError(0xdeadbeef);ret = pSetupDiSetDevicePropertyW(set, &device_data, &key, prop->devprop_type, prop->devprop_value,prop->devprop_size, 0);err = GetLastError();ok(ret, "Failed to set property, error %#lx.\n", err);
```suggestion:-0+0 ok(ret, "Expect success.\n"); ``` Not a _big_ deal, but since you're already printing the error on the next line, no need to double print it here as well.
Connor McAdams (@cmcadams) commented about dlls/setupapi/tests/devinst.c:
- for (i = 0; i < ARRAY_SIZE(reg_props); i++)
- {
const struct reg_property *prop = ®_props[i];DEVPROPKEY key = { DEVPKEY_Device_DeviceDesc.fmtid, prop->reg_prop + 2 };DEVPROPTYPE type = DEVPROP_TYPE_EMPTY;BYTE buf[80] = {0};DWORD size = 0;winetest_push_context("reg_props[%lu] (%#lx)", i, prop->reg_prop);SetLastError(0xdeadbeef);ret = SetupDiSetDeviceRegistryPropertyW(set, &device, prop->reg_prop, prop->reg_value, prop->reg_size);ok(ret, "Failed to set property, error %#lx.\n", GetLastError());ret = SetupDiGetDevicePropertyW(set, &device, &key, &type, buf, sizeof(buf), &size, 0);todo_wine ok(ret, "Failed to get property, error %#lx.\n", GetLastError());todo_wine ok(type == prop->devprop_type, "Got unexpected type %#lx.\n", type);todo_wine ok(size == prop->devprop_size, "Got unexpected size %lu.\n", size);
Since we're setting last error here, it'd be useful to add an extra test to make sure we're clearing it (assuming it's cleared on native).
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
+{
- 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 (IsEqualGUID( &prop_key->fmtid, &DEVPKEY_Device_DeviceDesc.fmtid ))
- {
DWORD reg_prop = prop_key->pid - 2, size, type;if (prop_key->pid < 2 || reg_prop >= ARRAY_SIZE(PropertyMap) || !PropertyMap[reg_prop].devPropType)return get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags );
```suggestion:-5+0 if (IsEqualGUID( &prop_key->fmtid, &DEVPKEY_Device_DeviceDesc.fmtid ) && prop_key->pid >= 2) { DWORD reg_prop = prop_key->pid - 2, size, type;
if (reg_prop >= ARRAY_SIZE(PropertyMap) || !PropertyMap[reg_prop].devPropType) return get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags ); ``` Moving the `prop_key->pid` check above makes this cleaner IMO.
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
- 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 (IsEqualGUID( &prop_key->fmtid, &DEVPKEY_Device_DeviceDesc.fmtid ))
- {
DWORD reg_prop = prop_key->pid - 2, size, type;if (prop_key->pid < 2 || reg_prop >= ARRAY_SIZE(PropertyMap) || !PropertyMap[reg_prop].devPropType)return get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags );ret = ERROR_SUCCESS;
This variable was already initialized at the start of the function, I'd remove this line.
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
- 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 (IsEqualGUID( &prop_key->fmtid, &DEVPKEY_Device_DeviceDesc.fmtid ))
- {
DWORD reg_prop = prop_key->pid - 2, size, type;if (prop_key->pid < 2 || reg_prop >= ARRAY_SIZE(PropertyMap) || !PropertyMap[reg_prop].devPropType)return get_device_reg_property( device->key, prop_key, prop_type, buf, buf_size, req_size, flags );ret = ERROR_SUCCESS;type = PropertyMap[reg_prop].devPropType;switch (type)
Now that we only have to handle one `DEVPROPTYPE` in a special way, this can probably just be an: ``` if (type == DEVPROP_TYPE_GUID) { ... } else { ... }
Connor McAdams (@cmcadams) commented about dlls/setupapi/devinst.c:
ret = GetLastError();break;}if (ret != ERROR_INVALID_DATA){*prop_type = type;if (req_size)*req_size = size;}elseret = ERROR_NOT_FOUND;- }
- else if (IsEqualDevPropKey( *prop_key, DEVPKEY_Device_InstanceId ))
- {
DWORD size = (wcslen( device->instanceId ) + 1) * sizeof( WCHAR );*prop_type = DEVPROP_TYPE_STRING;
```suggestion:-1+0 DWORD size = (wcslen( device->instanceId ) + 1) * sizeof( WCHAR );
*prop_type = DEVPROP_TYPE_STRING; ``` Split variable declaration and code here.
On Fri Jul 11 18:10:29 2025 +0000, Connor McAdams wrote:
ok(keys_len >= ARRAY_SIZE(expected_keys), "Expected %lu >= %u\n", keys_len, ARRAY_SIZE(expected_keys));Casting this feels a bit funky, I'd just do the above instead.
Unfortunately, anything with sizeof needs to be cast to a known-size type, or it results in a `-Wformat` warning (at least on Clang) on either i686 or x86_64.
On Fri Jul 11 18:10:30 2025 +0000, Connor McAdams wrote:
Now that we only have to handle one `DEVPROPTYPE` in a special way, this can probably just be an:
if (type == DEVPROP_TYPE_GUID) { ... } else { ... }
Yeah, that's cleaner. Thanks.
-
Connor McAdams (＠cmcadams) -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)