On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
dlls/setupapi/devinst.c | 139 ++++++++++++++++++++++++ dlls/setupapi/setupapi.spec | 2 +- dlls/setupapi/tests/devinst.c | 194 +++++++++++++++++++++++++++++++++- include/setupapi.h | 2 + 4 files changed, 335 insertions(+), 2 deletions(-)
diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 55746562c8..556bba4684 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -327,6 +327,28 @@ static WCHAR *get_refstr_key_path(struct device_iface *iface) return path; }
+static BOOL is_valid_property_type(DEVPROPTYPE prop_type) +{
- DWORD type = prop_type & DEVPROP_MASK_TYPE;
- DWORD typemod = prop_type & DEVPROP_MASK_TYPEMOD;
- if (type > MAX_DEVPROP_TYPE)
return FALSE;
- if (typemod > MAX_DEVPROP_TYPEMOD)
return FALSE;
- if (typemod == DEVPROP_TYPEMOD_ARRAY
&& (type == DEVPROP_TYPE_EMPTY || type == DEVPROP_TYPE_NULL || type == DEVPROP_TYPE_STRING
|| type == DEVPROP_TYPE_SECURITY_DESCRIPTOR_STRING))
return FALSE;
- if (typemod == DEVPROP_TYPEMOD_LIST
&& !(type == DEVPROP_TYPE_STRING || type == DEVPROP_TYPE_SECURITY_DESCRIPTOR_STRING))
return FALSE;
- return TRUE;
+}
This level of extensive error checking is probably not a bad idea, but—the current state of most of SetupDi notwithstanding—I'm not sure it's a good one either (and it's certainly a little troublesome to review). I won't stand opposed to it if pressed, but is it really necessary?
static LPWSTR SETUPDI_CreateSymbolicLinkPath(LPCWSTR instanceId, const GUID *InterfaceClassGuid, LPCWSTR ReferenceString) { @@ -3343,6 +3365,123 @@ BOOL WINAPI SetupDiSetDeviceInstallParamsW( return TRUE; }
+BOOL WINAPI SetupDiSetDevicePropertyW(HDEVINFO devinfo, PSP_DEVINFO_DATA devinfo_data, const DEVPROPKEY *key,
DEVPROPTYPE type, const BYTE *buffer, DWORD size, DWORD flags)
+{
- static const WCHAR propertiesW[] = {'P', 'r', 'o', 'p', 'e', 'r', 't', 'i', 'e', 's', 0};
- static const WCHAR formatW[] = {'\', '%', '0', '4', 'X', 0};
- struct device *device;
- HKEY properties_hkey, property_hkey;
- WCHAR property_hkey_path[44];
- LSTATUS ls;
- TRACE("%p %p %p %#x %p %d %#x\n", devinfo, devinfo_data, key, type, buffer, size, flags);
- if (!(device = get_device(devinfo, devinfo_data)))
return FALSE;
- if (!key || !is_valid_property_type(type)
|| (buffer && !size && !(type == DEVPROP_TYPE_EMPTY || type == DEVPROP_TYPE_NULL))
|| (buffer && size && (type == DEVPROP_TYPE_EMPTY || type == DEVPROP_TYPE_NULL)))
- {
SetLastError(ERROR_INVALID_DATA);
return FALSE;
- }
- if (size && !buffer)
- {
SetLastError(ERROR_INVALID_USER_BUFFER);
return FALSE;
- }
- if (flags)
- {
SetLastError(ERROR_INVALID_FLAGS);
return FALSE;
- }
- if (key->pid < 2)
- {
SetLastError(ERROR_INVALID_REG_PROPERTY);
return FALSE;
- }
As above.
- ls = RegCreateKeyExW(device->key, propertiesW, 0, NULL, 0, KEY_READ | KEY_WRITE, NULL, &properties_hkey, NULL);
- if (ls)
- {
SetLastError(ls);
return FALSE;
- }
As long as we're not using this key directly, couldn't we just put "Properties" into the format string, and get rid of properties_hkey?
- SETUPDI_GuidToString(&key->fmtid, property_hkey_path);
- sprintfW(property_hkey_path + 38, formatW, key->pid);
- if (type == DEVPROP_TYPE_EMPTY)
- {
if (RegDeleteKeyW(properties_hkey, property_hkey_path))
{
RegCloseKey(properties_hkey);
SetLastError(ERROR_NOT_FOUND);
return FALSE;
}
else
{
RegCloseKey(properties_hkey);
SetLastError(NO_ERROR);
return TRUE;
}
Some of the branches in this patch are a bit redundant. For example, the above could be:
ls = RegDeleteKeyW(properties_hkey, property_hkey_path); RegCloseKey(properties_hkey); SetLastError(ls); return !ls;
which feels simpler and more readable to me.
- }
- else if (type == DEVPROP_TYPE_NULL)
- {
if (RegOpenKeyW(properties_hkey, property_hkey_path, &property_hkey))
{
RegCloseKey(properties_hkey);
SetLastError(ERROR_NOT_FOUND);
return FALSE;
}
if (RegDeleteValueW(property_hkey, NULL))
{
RegCloseKey(property_hkey);
RegCloseKey(properties_hkey);
SetLastError(ERROR_NOT_FOUND);
return FALSE;
}
else
{
RegCloseKey(property_hkey);
RegCloseKey(properties_hkey);
SetLastError(NO_ERROR);
return TRUE;
}
- }
- else
- {
if ((ls = RegCreateKeyExW(properties_hkey, property_hkey_path, 0, NULL, 0, KEY_READ | KEY_WRITE, NULL,
&property_hkey, NULL)))
{
RegCloseKey(properties_hkey);
SetLastError(ls);
return FALSE;
}
if ((ls = RegSetValueExW(property_hkey, NULL, 0, 0xffff0000 | (0xffff & type), buffer, size)))
{
RegCloseKey(property_hkey);
RegCloseKey(properties_hkey);
SetLastError(ls);
return FALSE;
}
else
{
RegCloseKey(property_hkey);
RegCloseKey(properties_hkey);
SetLastError(NO_ERROR);
return TRUE;
}
- }
+}
- static HKEY SETUPDI_OpenDevKey(struct device *device, REGSAM samDesired) { HKEY enumKey, key = INVALID_HANDLE_VALUE;
diff --git a/dlls/setupapi/setupapi.spec b/dlls/setupapi/setupapi.spec index eb2dab204d..fb44298292 100644 --- a/dlls/setupapi/setupapi.spec +++ b/dlls/setupapi/setupapi.spec @@ -475,7 +475,7 @@ @ stdcall SetupDiSetDeviceInstallParamsW(ptr ptr ptr) @ stub SetupDiSetDeviceInterfaceDefault @ stub SetupDiSetDeviceInterfacePropertyW -@ stub SetupDiSetDevicePropertyW +@ stdcall SetupDiSetDevicePropertyW(ptr ptr ptr long ptr long long) @ stdcall SetupDiSetDeviceRegistryPropertyA(ptr ptr long ptr long) @ stdcall SetupDiSetDeviceRegistryPropertyW(ptr ptr long ptr long) @ stub SetupDiSetDriverInstallParamsA diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index bfffcb9c32..274c28745c 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -26,7 +26,8 @@ #include "wingdi.h" #include "winuser.h" #include "winreg.h" -#include "guiddef.h" +#include "initguid.h" +#include "devpkey.h" #include "setupapi.h" #include "cfgmgr32.h"
@@ -37,6 +38,8 @@ static GUID guid = {0x6a55b5a4, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}}; static GUID guid2 = {0x6a55b5a5, 0x3f65, 0x11db, {0xb7,0x04,0x00,0x11,0x95,0x5c,0x2b,0xdb}};
+BOOL (WINAPI *pSetupDiSetDevicePropertyW)(HDEVINFO, PSP_DEVINFO_DATA, const DEVPROPKEY *, DEVPROPTYPE, const BYTE *, DWORD, DWORD);
- static LSTATUS devinst_RegDeleteTreeW(HKEY hKey, LPCWSTR lpszSubKey) { LONG ret;
@@ -408,6 +411,194 @@ static void test_device_info(void) SetupDiDestroyDeviceInfoList(set); }
+static void test_device_property(void) +{
- static const WCHAR valueW[] = {'d', 'e', 'a', 'd', 'b', 'e', 'e', 'f', 0};
- HMODULE hmod;
- HDEVINFO set;
- SP_DEVINFO_DATA device = {sizeof(device)};
- DEVPROPKEY key;
- DWORD err;
- BOOL ret;
- hmod = LoadLibraryA("setupapi.dll");
- pSetupDiSetDevicePropertyW = (void *)GetProcAddress(hmod, "SetupDiSetDevicePropertyW");
- if (!pSetupDiSetDevicePropertyW)
- {
win_skip("SetupDiSetDevicePropertyW() are only available on vista+, skipping tests.\n");
FreeLibrary(hmod);
return;
- }
- set = SetupDiCreateDeviceInfoList(&guid, NULL);
- ok(set != INVALID_HANDLE_VALUE, "Failed to create device list, error %#x.\n", GetLastError());
- ret = SetupDiCreateDeviceInfoA(set, "Root\LEGACY_BOGUS\0000", &guid, NULL, NULL, 0, &device);
- ok(ret, "Failed to create device, error %#x.\n", GetLastError());
- /* SetupDiSetDevicePropertyW */
- /* #1 Null device info list */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(NULL, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_HANDLE, "Expect last error %#x, got %#x\n", ERROR_INVALID_HANDLE, err);
As above, exhaustive invalid-parameter tests seem unnecessary, especially when some fail (e.g. #8 below).
- /* #2 Null device */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, NULL, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_PARAMETER, "Expect last error %#x, got %#x\n", ERROR_INVALID_PARAMETER, err);
- /* #3 Null property key pointer */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, NULL, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_DATA, "Expect last error %#x, got %#x\n", ERROR_INVALID_DATA, err);
- /* #4 Invalid property key type */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, 0xffff, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_DATA, "Expect last error %#x, got %#x\n", ERROR_INVALID_DATA, err);
- /* #5 Null buffer pointer */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, NULL, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_USER_BUFFER, "Expect last error %#x, got %#x\n", ERROR_INVALID_USER_BUFFER, err);
- /* #6 Zero buffer size */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, 0, 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_DATA, "Expect last error %#x, got %#x\n", ERROR_INVALID_DATA, err);
- /* #7 Flags not zero */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 1);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_FLAGS, "Expect last error %#x, got %#x\n", ERROR_INVALID_FLAGS, err);
- /* #8 Invalid key pid */
- key.fmtid = DEVPKEY_Device_FriendlyName.fmtid;
- key.pid = 0;
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &key, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- /* Failure on win8+ */
- if(0) ok(ret, "Expect success\n");
- ok(err == ERROR_INVALID_REG_PROPERTY || broken(err == NO_ERROR), "Expect last error %#x, got %#x\n", ERROR_INVALID_REG_PROPERTY, err);
- /* #9 Normal */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- ok(ret, "Expect success\n");
- ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", NO_ERROR, err);
- /* #10 Delete property with buffer not null and size not zero */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_EMPTY, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- /* Failure on win8+ */
- if(0) ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_DATA || broken(err == NO_ERROR), "Expect last error %#x, got %#x\n", ERROR_INVALID_DATA, err);
- /* #11 Delete property with buffer not null */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_EMPTY, (const BYTE *)valueW, 0, 0);
- err = GetLastError();
- ok(ret, "Expect success\n");
- ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", NO_ERROR, err);
- /* #12 Delete property with size not zero */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_EMPTY, NULL, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_USER_BUFFER, "Expect last error %#x, got %#x\n", ERROR_INVALID_USER_BUFFER, err);
- /* #13 Delete property */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_EMPTY, NULL, 0, 0);
- err = GetLastError();
- ok(ret, "Expect success\n");
- ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", NO_ERROR, err);
- /* #14 Delete non-existent property */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_EMPTY, NULL, 0, 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#x\n", ERROR_NOT_FOUND, err);
- /* #15 Delete property value with buffer not null and size not zero */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, (const BYTE *)valueW, sizeof(valueW), 0);
- err = GetLastError();
- /* Failure on win8+ */
- if(0) ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_DATA || broken(err == NO_ERROR), "Expect last error %#x, got %#x\n", ERROR_INVALID_DATA, err);
- /* #16 Delete property value with buffer not null */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, (const BYTE *)valueW, 0, 0);
- err = GetLastError();
- ok(ret, "Expect success\n");
- ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", NO_ERROR, err);
- /* #17 Delete property value with size not zero */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, NULL, sizeof(valueW), 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_INVALID_USER_BUFFER, "Expect last error %#x, got %#x\n", ERROR_INVALID_USER_BUFFER, err);
- /* #18 Delete property value */
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE *)valueW, sizeof(valueW), 0);
- ok(ret, "Expect success\n");
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, NULL, 0, 0);
- err = GetLastError();
- ok(ret, "Expect success\n");
- ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", NO_ERROR, err);
- /* #19 Delete non-existent property value */
- SetLastError(0xdeadbeef);
- ret = pSetupDiSetDevicePropertyW(set, &device, &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, NULL, 0, 0);
- err = GetLastError();
- ok(!ret, "Expect failure\n");
- ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#x\n", ERROR_NOT_FOUND, err);
- ret = SetupDiRemoveDevice(set, &device);
- ok(ret, "Got unexpected error %#x.\n", GetLastError());
- SetupDiDestroyDeviceInfoList(set);
- FreeLibrary(hmod);
+}
- static void test_get_device_instance_id(void) { BOOL ret;
@@ -1366,6 +1557,7 @@ START_TEST(devinst) test_open_class_key(); test_install_class(); test_device_info();
- test_device_property(); test_get_device_instance_id(); test_register_device_info(); test_device_iface();
diff --git a/include/setupapi.h b/include/setupapi.h index 76e255cc44..4491617d8a 100644 --- a/include/setupapi.h +++ b/include/setupapi.h @@ -1634,6 +1634,8 @@ BOOL WINAPI SetupDiSetDeviceInterfaceDefault(HDEVINFO, PSP_DEVICE_INTERFACE_ BOOL WINAPI SetupDiSetDeviceInstallParamsA(HDEVINFO, PSP_DEVINFO_DATA, PSP_DEVINSTALL_PARAMS_A); BOOL WINAPI SetupDiSetDeviceInstallParamsW(HDEVINFO, PSP_DEVINFO_DATA, PSP_DEVINSTALL_PARAMS_W); #define SetupDiSetDeviceInstallParams WINELIB_NAME_AW(SetupDiSetDeviceInstallParams) +BOOL WINAPI SetupDiSetDevicePropertyW(HDEVINFO, PSP_DEVINFO_DATA, const DEVPROPKEY *, DEVPROPTYPE, const BYTE *, DWORD, DWORD); +#define SetupDiSetDeviceProperty WINELIB_NAME_AW(SetupDiSetDeviceProperty) /* note: A function doesn't exist */ BOOL WINAPI SetupDiSetDeviceRegistryPropertyA(HDEVINFO, PSP_DEVINFO_DATA, DWORD, const BYTE *, DWORD); BOOL WINAPI SetupDiSetDeviceRegistryPropertyW(HDEVINFO, PSP_DEVINFO_DATA, DWORD, const BYTE *, DWORD); #define SetupDiSetDeviceRegistryProperty WINELIB_NAME_AW(SetupDiSetDeviceRegistryProperty)