[PATCH v3 1/2] winevulkan: Create JSON manifest and registry entry used by official Vulkan loader.
Jacek Caban
jacek at codeweavers.com
Mon Mar 23 18:50:59 CDT 2020
Hi Brendan,
On 23.03.2020 20:16, Brendan Shanks wrote:
> +static BOOL writestr(HANDLE handle, const char *str)
> +{
> + DWORD written;
> + return WriteFile(handle, str, strlen(str), &written, NULL);
> +}
It may be a matter of taste, but it doesn't seem worth a helper. You
could just do that with a single WriteFile() call.
> +
> +HRESULT WINAPI DllRegisterServer(void)
> +{
> + WCHAR path[MAX_PATH];
> + HANDLE file;
> + LRESULT result;
> + HKEY key;
> + DWORD zero = 0;
> +
> + /* Create the JSON manifest and registry key to register this ICD with the official Vulkan loader. */
> + TRACE("\n");
> + GetSystemDirectoryW(path, ARRAY_SIZE(path));
> + lstrcatW(path, winevulkan_jsonW);
> + file = CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (file == INVALID_HANDLE_VALUE)
> + { ERR("Unable to create JSON manifest.\n"); return E_UNEXPECTED; }
Please try to match style of the file you change.
> + writestr(file, "{\r\n");
> + writestr(file, " \"file_format_version\": \"1.0.0\",\r\n");
> + writestr(file, " \"ICD\": {\r\n");
> + writestr(file, " \"library_path\": \"winevulkan.dll\",\r\n");
It may not be too important in this case, but it's usually better to use
full path in such places, something like a result of
GetModuleFileNameW(). What do real IDSs use?
> + writestr(file, " \"api_version\": \"" WINE_VK_SPEC_VERSION "\"\r\n");
> + writestr(file, " }\r\n");
> + writestr(file, "}\r\n");
> + CloseHandle(file);
> +
> + result = RegCreateKeyExW(HKEY_LOCAL_MACHINE, vulkan_driversW,
> + 0, NULL, 0, KEY_SET_VALUE, NULL, &key, NULL);
> + if (result != ERROR_SUCCESS)
> + { ERR("Unable to create registry key.\n"); return E_UNEXPECTED; }
> +
> + result = RegSetValueExW(key, path, 0, REG_DWORD, (const BYTE *)&zero, sizeof(zero));
> + if (result != ERROR_SUCCESS)
> + { ERR("Unable to set registry value.\n"); return E_UNEXPECTED; }
Style aside, you leak a handle in second error path. Error handling
could also be simplified a bit.
> +HRESULT WINAPI DllUnregisterServer(void)
> +{
> + WCHAR path[MAX_PATH];
> + LRESULT result;
> + HKEY key;
> +
> + /* Remove the JSON manifest and registry key */
> + TRACE("\n");
> + GetSystemDirectoryW(path, ARRAY_SIZE(path));
> + lstrcatW(path, winevulkan_jsonW);
> + if (!DeleteFileW(path))
> + ERR("Unable to delete JSON file.\n");
> +
> + result = RegOpenKeyExW(HKEY_LOCAL_MACHINE, vulkan_driversW,
> + 0, KEY_SET_VALUE, &key);
> + if (result != ERROR_SUCCESS)
> + { ERR("Unable to open registry key.\n"); return E_UNEXPECTED; }
This is not really an error.
> +
> + result = RegDeleteValueW(key, path);
> + if (result != ERROR_SUCCESS)
> + ERR("Unable to delete registry key.\n");
> +
> + RegCloseKey(key);
> + return S_OK;
> +}
Those ERRs should we WARNs or removed. Eg. it's not really an error when
it's ran with no winevulkan registered or if you run regsvr32 -u twice
in a raw. We should report success and print no errors in such case.
Thanks,
Jacek
More information about the wine-devel
mailing list