Damjan Jovanovic wrote:
> Hi
>
> My work on the still image system for wine has
> highlighted a bug in setupapi's
> SetupDiOpenClassRegKeyExW(). In short, the registry
> keys for device classes have the form
>
> HKEY_LOCAL_MACHINE\System\CurrentControlSet\
> Class\{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}
>
> while wine incorrectly tries to open
> HKEY_LOCAL_MACHINE\System\CurrentControlSet\
> Class\xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> @@ -1453,21 +1454,30 @@
> if (UuidToStringW((UUID*)ClassGuid, &lpGuidString) != RPC_S_OK)
> {
> RegCloseKey(hClassesKey);
> - return FALSE;
> + return INVALID_HANDLE_VALUE;
> }
Please send this as a separate patch.
>
> + lpBracedGuidString = (LPWSTR) HeapAlloc(GetProcessHeap(), 0,
> + (1 + strlenW(lpGuidString) + 1 + 1) * sizeof(WCHAR));
> + lpBracedGuidString[0] = (WCHAR) '{';
> + memcpy(&lpBracedGuidString[1], lpGuidString,
> + strlenW(lpGuidString) * sizeof(WCHAR));
> + lpBracedGuidString[1 + strlenW(lpGuidString)] = (WCHAR) '}';
> + lpBracedGuidString[1 + strlenW(lpGuidString) + 1] = (WCHAR) 0;
> + RpcStringFreeW(&lpGuidString);
> +
All that looks messy and way too many calls to strlenW for no reason. Length of
GUID
string is known and hard-coded to 36.
lpBracedGuidString = HeapAlloc(GetProcessHeap(), 0, (36 +1+2) *
sizeof(WCHAR));
memcpy(lpBracedGuidString + 1, lpGuidString, 36 * sizeof(WCHAR));
lpBracedGuidString[ 0] = '{';
lpBracedGuidString[37] = '}';
lpBracedGuidString[38] = 0;
RpcStringFreeW(&lpGuidString);
> @@ -43,6 +45,7 @@
> {
> pSetupDiCreateDeviceInfoListExW = (void *)GetProcAddress(hSetupAPI,
> "SetupDiCreateDeviceInfoListExW");
> pSetupDiDestroyDeviceInfoList = (void *)GetProcAddress(hSetupAPI,
> "SetupDiDestroyDeviceInfoList");
> + pSetupDiOpenClassRegKeyExW = (void *)GetProcAddress(hSetupAPI,
> "SetupDiOpenClassRegKeyExW");
> }
Please try to test Ascii variants instead of Unicode. This way we will be
testing both.
>
> + rpcStatus = UuidCreate(&guid);
> + if (rpcStatus != RPC_S_OK)
> + {
> + ok(FALSE, "failed to generate test guid, error %ld", rpcStatus);
> + return;
> + }
This is not correct. You shouldn't use ok() this way. Also it will already
print "Test failed:"
so no need to repeat it again. Instead write something like:
rpcStatus = UuidCreate(&guid);
ok(rpcStatus == RPC_S_OK, "error %ld", rpcStatus);
if (rpcStatus != RPC_S_OK) return;
> + /* Check return value for non-existant key */
> + hkey = pSetupDiOpenClassRegKeyExW(&guid, KEY_ALL_ACCESS,
> + DIOCR_INSTALLER, NULL, NULL);
> + ok(hkey == INVALID_HANDLE_VALUE,
> + "invalid return value %p from SetupDiOpenClassRegKeyExW "
> + "for non-existant key, expected %p\n", hkey, INVALID_HANDLE_VALUE);
There is no need for such a long error message. Simple "got %p\n" would be
enough.
Also don't forget to test LastError as well.
> + ok(FALSE, "failed creating device key: error %ld\n", ret);
Please use trace() instead.
Vitaliy.
