[PATCH 0/1] MR8434: cfgmgr32: Implement CM_Get_Device_Interface_PropertyW for all property keys.

[Vibhav Pant (＠vibhavp)]

Use SetupDiGetDeviceInterfacePropertyW to retrieve interface property values.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8434

[PATCH 1/1] cfgmgr32: Implement CM_Get_Device_Interface_PropertyW for all property keys.

[Vibhav Pant]

From: Vibhav Pant <vibhavp(a)gmail.com>

---
 dlls/cfgmgr32/main.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c
index fbff2c2fbf9..658844483da 100644
--- a/dlls/cfgmgr32/main.c
+++ b/dlls/cfgmgr32/main.c
@@ -255,7 +255,6 @@ CONFIGRET WINAPI CM_Get_Device_Interface_PropertyW( LPCWSTR device_interface, co
                                                     ULONG *property_buffer_size, ULONG flags )
 {
     SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)};
-    SP_DEVINFO_DATA device = { sizeof(device) };
     HDEVINFO set;
     DWORD err;
     BOOL ret;
@@ -268,12 +267,6 @@ CONFIGRET WINAPI CM_Get_Device_Interface_PropertyW( LPCWSTR device_interface, co
     if (*property_buffer_size && !property_buffer) return CR_INVALID_POINTER;
     if (flags) return CR_INVALID_FLAG;
 
-    if (memcmp( property_key, &DEVPKEY_Device_InstanceId, sizeof(*property_key) ))
-    {
-        FIXME( "property %s\\%lx.\n", debugstr_guid( &property_key->fmtid ), property_key->pid );
-        return CR_NO_SUCH_VALUE;
-    }
-
     set = SetupDiCreateDeviceInfoListExW( NULL, NULL, NULL, NULL );
     if (set == INVALID_HANDLE_VALUE) return CR_OUT_OF_MEMORY;
     if (!SetupDiOpenDeviceInterfaceW( set, device_interface, 0, &iface ))
@@ -282,17 +275,11 @@ CONFIGRET WINAPI CM_Get_Device_Interface_PropertyW( LPCWSTR device_interface, co
         TRACE( "No interface %s, err %lu.\n", debugstr_w( device_interface ), GetLastError());
         return CR_NO_SUCH_DEVICE_INTERFACE;
     }
-    if (!SetupDiEnumDeviceInfo( set, 0, &device ))
-    {
-        SetupDiDestroyDeviceInfoList( set );
-        return CR_FAILURE;
-    }
-    ret = SetupDiGetDeviceInstanceIdW( set, &device, (WCHAR *)property_buffer, *property_buffer_size / sizeof(WCHAR),
-                                       property_buffer_size );
+
+    ret = SetupDiGetDeviceInterfacePropertyW( set, &iface, property_key, property_type, property_buffer,
+                                              *property_buffer_size, property_buffer_size, 0 );
     err = ret ? 0 : GetLastError();
     SetupDiDestroyDeviceInfoList( set );
-    *property_type = DEVPROP_TYPE_STRING;
-    *property_buffer_size *= sizeof(WCHAR);
     if (!err) return CR_SUCCESS;
     return err == ERROR_INSUFFICIENT_BUFFER ? CR_BUFFER_SMALL : CR_FAILURE;
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8434

[Elizabeth Figura (＠zfigura)]

This looks pretty trivial, but it would be nice to have a test for it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8434#note_108001

[Paul Gofman (＠gofman)]

I think for the supported part (DEVPKEY_Device_InstanceId) we have detailed tests already?

The only thing I'd change is error return here:

```
-    if (!SetupDiEnumDeviceInfo( set, 0, &device ))
-    {
-        SetupDiDestroyDeviceInfoList( set );
-        return CR_FAILURE;
-    }
-    ret = SetupDiGetDeviceInstanceIdW( set, &device, (WCHAR *)property_buffer, *property_buffer_size / sizeof(WCHAR),
-                                       property_buffer_size );
+
+    ret = SetupDiGetDeviceInterfacePropertyW( set, &iface, property_key, property_type, property_buffer,
+                                              *property_buffer_size, property_buffer_size, 0 );
     err = ret ? 0 : GetLastError();
     SetupDiDestroyDeviceInfoList( set );
```

GetLastError() from SetupDiGetDeviceInterfacePropertyW() looks wrong, cfgmgr has its own constants. The line itself is pre-existing but before this change failing in SetupDiGetDeviceInstanceIdW() in this place was somewhat moot point and not expected under normal condition. Now with SetupDiGetDeviceInterfacePropertyW() we will be getting all sort of different errors. I'd suggest to add a trivial test for SetupDiGetDeviceInterfacePropertyW() with nonexistent property checking error code and then fix 'err =' in a way it returns CR_NO_SUCH_VALUE (probably, depending on the test) and CR_FAILURE for all the other errors.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8434#note_108002

[Elizabeth Figura (＠zfigura)]

> I think for the supported part (DEVPKEY_Device_InstanceId) we have detailed tests already?

The point is now we're supporting other keys; we should test one of those. We already have tests for SetupDiGetDeviceInterfaceProperty() so this shouldn't be too difficult.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8434#note_108004