[RFC PATCH 3/9] setupapi: Implement SetupDiGetDevicePropertyW.

Zhiyi Zhang zzhang at codeweavers.com
Wed Jan 23 02:01:15 CST 2019 

On 23/01/2019 10:31, Zebediah Figura wrote:
> On 12/18/18 10:20 AM, Zhiyi Zhang wrote:
>> Signed-off-by: Zhiyi Zhang <zzhang at codeweavers.com>
>> ---
>>   dlls/setupapi/devinst.c       |  69 ++++++++++++-
>>   dlls/setupapi/tests/devinst.c | 181 +++++++++++++++++++++++++++++++++-
>>   2 files changed, 244 insertions(+), 6 deletions(-)
>>
>> diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c
>> index 556bba4684..2a71b59518 100644
>> --- a/dlls/setupapi/devinst.c
>> +++ b/dlls/setupapi/devinst.c
>> @@ -3848,9 +3848,70 @@ BOOL WINAPI SetupDiGetDevicePropertyW(HDEVINFO 
>> info_set, PSP_DEVINFO_DATA info_d
>>                   const DEVPROPKEY *prop_key, DEVPROPTYPE *prop_type, 
>> BYTE *prop_buff,
>>                   DWORD prop_buff_size, DWORD *required_size, DWORD 
>> flags)
>>   {
>> -    FIXME("%p, %p, %p, %p, %p, %d, %p, 0x%08x stub\n", info_set, 
>> info_data, prop_key,
>> -               prop_type, prop_buff, prop_buff_size, required_size, 
>> flags);
>> +    static const WCHAR formatW[] = {'\\', '%', '0', '4', 'X', 0};
>> +    WCHAR property_hkey_path[55] = {'P', 'r', 'o', 'p', 'e', 'r', 
>> 't', 'i', 'e', 's', '\\'};
>> +    DWORD reg_key_type;
>> +    DWORD value_size;
>> +    LSTATUS ls;
>> +    struct device *device;
>>   -    SetLastError(ERROR_NOT_FOUND);
>> -    return FALSE;
>> +    TRACE("%p, %p, %p, %p, %p, %d, %p, %#x\n", info_set, info_data, 
>> prop_key, prop_type, prop_buff, prop_buff_size,
>> +          required_size, flags);
>> +
>> +    if (!(device = get_device(info_set, info_data)))
>> +        return FALSE;
>> +
>> +    if (!prop_key)
>> +    {
>> +        SetLastError(ERROR_INVALID_DATA);
>> +        return FALSE;
>> +    }
>> +
>> +    if (!prop_type || (!prop_buff && prop_buff_size))
>> +    {
>> +        SetLastError(ERROR_INVALID_USER_BUFFER);
>> +        return FALSE;
>> +    }
>> +
>> +    if (flags)
>> +    {
>> +        SetLastError(ERROR_INVALID_FLAGS);
>> +        return FALSE;
>> +    }
>
> Same comments as regards patch 2/9.
>
>> +
>> +    SETUPDI_GuidToString(&prop_key->fmtid, property_hkey_path + 11);
>> +    sprintfW(property_hkey_path + 49, formatW, prop_key->pid);
>> +
>> +    value_size = prop_buff_size;
>> +    ls = RegGetValueW(device->key, property_hkey_path, NULL, 
>> RRF_RT_ANY, &reg_key_type, prop_buff, &value_size);
>
> Do we really want to use RegGetValue() here and not RegQueryValueEx()? 
> This would need tests at least. (E.g. is a string value 
> null-terminated? Does REG_EXPAND_SZ auto-expand?)

RegQueryValueEx seems better indeed. Thanks.

>
>> +
>> +    switch (ls)
>> +    {
>> +    case ERROR_FILE_NOT_FOUND:
>> +        *prop_type = DEVPROP_TYPE_EMPTY;
>> +        if (required_size)
>> +            *required_size = 0;
>> +        SetLastError(ERROR_NOT_FOUND);
>> +        return FALSE;
>> +    default:
>
> Is there a reason why this is a switch and not a simple if-else? 
> Moreover, I'd expect we'd want to handle more errors than just 
> ERROR_FILE_NOT_FOUND (e.g. ERROR_ACCESS_DENIED). I'd guess the above 
> code path should be taken except for ERROR_SUCCESS and ERROR_MORE_DATA.

I was thinking maybe this would be extended to handle more failures, 
thus the switch.


>
>> +        *prop_type = 0xffff & reg_key_type;
>> +        if (required_size)
>> +            *required_size = value_size;
>> +
>> +        if (prop_buff_size < value_size)
>> +        {
>> +            SetLastError(ERROR_INSUFFICIENT_BUFFER);
>> +            return FALSE;
>> +        }
>> +        else if (ls == ERROR_UNSUPPORTED_TYPE)
>> +        {
>> +            SetLastError(NO_ERROR);
>> +            return TRUE;
>> +        }
>
> If you pass RRF_RT_ANY, this shouldn't even happen, should it?
Right. This happens with RegGetValueW and RRF_RT_ANY on Wine. Need some 
tests to verify its behavior. Using RegQueryValueEx probably won't need 
this anymore.
>
>> +        else
>> +        {
>> +            SetLastError(ls);
>> +            return FALSE;
>> +        }
>> +    }
>>   }
>> diff --git a/dlls/setupapi/tests/devinst.c 
>> b/dlls/setupapi/tests/devinst.c
>> index 274c28745c..6928415c76 100644
>> --- a/dlls/setupapi/tests/devinst.c
>> +++ b/dlls/setupapi/tests/devinst.c
>> @@ -39,6 +39,7 @@ static GUID guid = {0x6a55b5a4, 0x3f65, 0x11db, 
>> {0xb7,0x04,0x00,0x11,0x95,0x5c,0
>>   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);
>> +BOOL (WINAPI *pSetupDiGetDevicePropertyW)(HDEVINFO, 
>> PSP_DEVINFO_DATA, const DEVPROPKEY *, DEVPROPTYPE *, BYTE *, DWORD, 
>> DWORD *, DWORD);
>>     static LSTATUS devinst_RegDeleteTreeW(HKEY hKey, LPCWSTR lpszSubKey)
>>   {
>> @@ -418,15 +419,19 @@ static void test_device_property(void)
>>       HDEVINFO set;
>>       SP_DEVINFO_DATA device = {sizeof(device)};
>>       DEVPROPKEY key;
>> +    DEVPROPTYPE type;
>> +    DWORD size;
>> +    BYTE buffer[256];
>>       DWORD err;
>>       BOOL ret;
>>         hmod = LoadLibraryA("setupapi.dll");
>>       pSetupDiSetDevicePropertyW = (void *)GetProcAddress(hmod, 
>> "SetupDiSetDevicePropertyW");
>> +    pSetupDiGetDevicePropertyW = (void *)GetProcAddress(hmod, 
>> "SetupDiGetDevicePropertyW");
>>   -    if (!pSetupDiSetDevicePropertyW)
>> +    if (!pSetupDiSetDevicePropertyW || !pSetupDiGetDevicePropertyW)
>>       {
>> -        win_skip("SetupDiSetDevicePropertyW() are only available on 
>> vista+, skipping tests.\n");
>> +        win_skip("SetupDi{Set,Get}DevicePropertyW() are only 
>> available on vista+, skipping tests.\n");
>>           FreeLibrary(hmod);
>>           return;
>>       }
>> @@ -592,6 +597,178 @@ static void test_device_property(void)
>>       ok(!ret, "Expect failure\n");
>>       ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#x\n", 
>> ERROR_NOT_FOUND, err);
>>   +
>> +    /* SetupDiGetDevicePropertyW */
>> +    ret = pSetupDiSetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE 
>> *)valueW, sizeof(valueW), 0);
>> +    ok(ret, "Expect success\n");
>> +
>> +    /* #1 Null device info list */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(NULL, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INVALID_HANDLE, "Expect last error %#x, got 
>> %#x\n", ERROR_INVALID_HANDLE, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #2 Null device */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, NULL, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INVALID_PARAMETER, "Expect last error %#x, got 
>> %#x\n", ERROR_INVALID_PARAMETER, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #3 Null property key */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, NULL, &type, 
>> buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INVALID_DATA, "Expect last error %#x, got 
>> %#x\n", ERROR_INVALID_DATA, err);
>> +    /* Type unchanged on win8+ */
>> +    ok(type == DEVPROP_TYPE_STRING || type == DEVPROP_TYPE_EMPTY, 
>> "Expect type %#x, got %#x\n", DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #4 Null property type */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, NULL, buffer, sizeof(buffer), &size, 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);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #5 Null buffer */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, NULL, sizeof(buffer), &size, 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);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #6 Null buffer with zero size and wrong type */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_UINT64;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, NULL, 0, &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INSUFFICIENT_BUFFER, "Expect last error %#x, got 
>> %#x\n", ERROR_INSUFFICIENT_BUFFER, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == sizeof(valueW), "Expect size %d, got %d\n", 
>> sizeof(valueW), size);
>> +
>> +    /* #7 Zero buffer size */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, 0, &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INSUFFICIENT_BUFFER, "Expect last error %#x, got 
>> %#x\n", ERROR_INSUFFICIENT_BUFFER, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == sizeof(valueW), "Expect size %d, got %d\n", 
>> sizeof(valueW), size);
>> +
>> +    /* #8 Null required size */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), NULL, 0);
>> +    err = GetLastError();
>> +    ok(ret, "Expect success\n");
>> +    ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", 
>> NO_ERROR, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +
>> +    /* #9 Flags not zero */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 1);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INVALID_FLAGS, "Expect last error %#x, got 
>> %#x\n", ERROR_INVALID_FLAGS, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #10 Non-existent property key */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_HardwareIds, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#x\n", 
>> ERROR_NOT_FOUND, err);
>> +    /* Type unchanged on win8+ */
>> +    ok(type == DEVPROP_TYPE_EMPTY || broken(type == 
>> DEVPROP_TYPE_STRING), "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_EMPTY, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #11 Wrong property key type */
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_UINT64;
>> +    size = 0;
>> +    memset(buffer, 0, sizeof(buffer));
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(ret, "Expect success\n");
>> +    ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", 
>> NO_ERROR, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == sizeof(valueW), "Expect size %d, got %d\n", 
>> sizeof(valueW), size);
>> +    ok(!lstrcmpW((WCHAR *)buffer, valueW), "Expect buffer %s, got 
>> %s\n", wine_dbgstr_w(valueW), wine_dbgstr_w((WCHAR *)buffer));
>> +
>> +    /* #12 Get null property value */
>> +    ret = pSetupDiSetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE 
>> *)valueW, sizeof(valueW), 0);
>> +    ok(ret, "Expect success\n");
>> +    ret = pSetupDiSetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_NULL, NULL, 0, 0);
>> +    ok(ret, "Expect success\n");
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_NOT_FOUND, "Expect last error %#x, got %#x\n", 
>> ERROR_NOT_FOUND, err);
>> +    /* Type unchanged on win8+ */
>> +    ok(type == DEVPROP_TYPE_EMPTY || broken(type == 
>> DEVPROP_TYPE_STRING), "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_EMPTY, type);
>> +    ok(size == 0, "Expect size %d, got %d\n", 0, size);
>> +
>> +    /* #13 Insufficient buffer size */
>> +    ret = pSetupDiSetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE 
>> *)valueW, sizeof(valueW), 0);
>> +    ok(ret, "Expect success\n");
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(valueW) - 1, 
>> &size, 0);
>> +    err = GetLastError();
>> +    ok(!ret, "Expect failure\n");
>> +    ok(err == ERROR_INSUFFICIENT_BUFFER, "Expect last error %#x, got 
>> %#x\n", ERROR_INSUFFICIENT_BUFFER, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == sizeof(valueW), "Expect size %d, got %d\n", 
>> sizeof(valueW), size);
>> +
>> +    /* #14 Normal */
>> +    ret = pSetupDiSetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, DEVPROP_TYPE_STRING, (const BYTE 
>> *)valueW, sizeof(valueW), 0);
>> +    ok(ret, "Expect success\n");
>> +    SetLastError(0xdeadbeef);
>> +    type = DEVPROP_TYPE_STRING;
>> +    size = 0;
>> +    memset(buffer, 0, sizeof(buffer));
>> +    ret = pSetupDiGetDevicePropertyW(set, &device, 
>> &DEVPKEY_Device_FriendlyName, &type, buffer, sizeof(buffer), &size, 0);
>> +    err = GetLastError();
>> +    ok(ret, "Expect success\n");
>> +    ok(err == NO_ERROR, "Expect last error %#x, got %#x\n", 
>> NO_ERROR, err);
>> +    ok(type == DEVPROP_TYPE_STRING, "Expect type %#x, got %#x\n", 
>> DEVPROP_TYPE_STRING, type);
>> +    ok(size == sizeof(valueW), "Expect size %d, got %d\n", 
>> sizeof(valueW), size);
>> +    ok(!lstrcmpW((WCHAR *)buffer, valueW), "Expect buffer %s, got 
>> %s\n", wine_dbgstr_w(valueW), wine_dbgstr_w((WCHAR *)buffer));
>> +
>>       ret = SetupDiRemoveDevice(set, &device);
>>       ok(ret, "Got unexpected error %#x.\n", GetLastError());
>>
>
>
>

More information about the wine-devel mailing list