18 Dec
2018
18 Dec
'18
4:20 p.m.
Signed-off-by: Zhiyi Zhang <zzhang(a)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;
+}
+
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;
+ }
+
+ ls = RegCreateKeyExW(device->key, propertiesW, 0, NULL, 0, KEY_READ | KEY_WRITE,
NULL, &properties_hkey, NULL);
+ if (ls)
+ {
+ SetLastError(ls);
+ return FALSE;
+ }
+
+ 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;
+ }
+ }
+ 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);
+
+ /* #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)
--
2.19.2
23 Jan
23 Jan
2:17 a.m.
On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
Signed-off-by: Zhiyi Zhang
<zzhang(a)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)
7:39 a.m.
On 23/01/2019 10:17, Zebediah Figura wrote:
Sure.
Thanks.
On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
I would rather do the extensive error/parameter checking now than fixing
the crash caused by not checking later. I feel like there are already so
many bugs because of not enough checking.
As long as they are covered by tests. They shouldn't be too much an issue.
Signed-off-by: Zhiyi Zhang
<zzhang(a)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)
3:47 p.m.
On 1/23/19 1:39 AM, Zhiyi Zhang wrote:
On 23/01/2019 10:17, Zebediah Figura wrote:
I'm not sure I agree, honestly. I don't see many applications that break
on invalid parameters (after all, why would they?), and those that do
are generally rather easy to debug.
If you'd really prefer to leave these in, I'd at least advocate for
removing the tests (and perhaps corresponding checks) that don't fail
consistently across versions.
On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
I would rather do the extensive error/parameter checking now than fixing
the crash caused by not checking later. I feel like there are already so
many bugs because of not enough checking.
As long as they are covered by tests. They shouldn't be too much an issue.
Signed-off-by: Zhiyi Zhang
<zzhang(a)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?
27 Jan
27 Jan
7:58 a.m.
On 23/01/2019 23:47, Zebediah Figura wrote:
On 1/23/19 1:39 AM, Zhiyi Zhang wrote:
Well, there are many patches that fix crashes just by adding simple parameter checks
because somehow there are invalid parameters.
And yes, they are usually easy to debug, I prefer not having to debug these kind of things
at all.
On 23/01/2019 10:17, Zebediah Figura wrote:
I'm not sure I agree, honestly. I don't see many applications that break on
invalid parameters (after all, why would they?), and those that do are generally rather
easy to debug. On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
I would rather do the extensive error/parameter checking now than fixing
the crash caused by not checking later. I feel like there are already so
many bugs because of not enough checking.
As long as they are covered by tests. They shouldn't be too much an issue.
Signed-off-by: Zhiyi Zhang
<zzhang(a)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?
If you'd really prefer to leave these in, I'd at least advocate for removing the
tests (and perhaps corresponding checks) that don't fail consistently across
versions.
I'll remove some of the inconsistent tests.
1925
days inactive
1965
days old
4 comments
2 participants
participants (2)
-
Zebediah Figura -
Zhiyi Zhang