From: Charles Giessen Date: Fri, 10 Dec 2021 00:03:42 +0000 (-0700) Subject: Fix Test folders and registry cleaning logic X-Git-Tag: upstream/v1.3.207~123 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0640d7acac773623c81d651bea751d8b7cc9be4f;p=platform%2Fupstream%2FVulkan-Loader.git Fix Test folders and registry cleaning logic The test framework was not clearing out old registry values after successive test runs. The new logic now correctly clears out the files and registry keys after each run. --- diff --git a/tests/framework/shim/shim_common.cpp b/tests/framework/shim/shim_common.cpp index 227bc2c1..807ccd88 100644 --- a/tests/framework/shim/shim_common.cpp +++ b/tests/framework/shim/shim_common.cpp @@ -94,11 +94,18 @@ HKEY create_key(HKEY key_root, const char* key_path) { return key; } -void close_key(HKEY& key) { +void close_key(HKEY key) { LSTATUS out = RegCloseKey(key); if (out != ERROR_SUCCESS) std::cerr << win_api_error_str(out) << " failed to close key " << key << "\n"; } +void delete_key(HKEY key, const char* key_path, bool report_failure = true) { + LSTATUS out = RegDeleteKeyA(key, key_path); + if (out != ERROR_SUCCESS) + if (report_failure) + std::cerr << win_api_error_str(out) << " failed to close key " << key << " with path " << key_path << "\n"; +} + void setup_override_key(HKEY root_key, uint32_t random_base_path) { DWORD dDisposition{}; LSTATUS out; @@ -157,26 +164,6 @@ void remove_key_value(HKEY const& key, fs::path const& manifest_path) { std::cerr << win_api_error_str(out) << " failed to delete key value for " << manifest_path.str() << "\n"; } -void clear_key_values(HKEY const& key) { - DWORD dwNumValues, dwValueNameLen; - - LSTATUS out = RegQueryInfoKey(key, 0, 0, 0, 0, 0, 0, &dwNumValues, &dwValueNameLen, 0, 0, 0); - if (out != ERROR_SUCCESS) { - std::cerr << win_api_error_str(out) << "couldn't query enum " << key << " values\n"; - return; - } - std::string tchValName(dwValueNameLen + 1, '\0'); - for (DWORD i = 0; i < dwNumValues; i++) { - DWORD length = static_cast(tchValName.size()); - LPSTR lpstr = &tchValName[0]; - out = RegEnumValue(key, i, lpstr, &length, 0, 0, 0, 0); - if (out != ERROR_SUCCESS) { - std::cerr << win_api_error_str(out) << "couldn't get enum value " << tchValName << "\n"; - return; - } - } -} - uint32_t setup_override(DebugMode debug_mode) { uint32_t random_base_path = 0; std::random_device rd; @@ -221,14 +208,16 @@ void clear_override(DebugMode debug_mode, uint32_t random_base_path) { } } void PlatformShim::reset(DebugMode debug_mode) { - KeyWrapper implicit_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers"}; - KeyWrapper explicit_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers"}; - KeyWrapper icd_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\Drivers"}; - if (debug_mode != DebugMode::no_delete) { - clear_key_values(implicit_key); - clear_key_values(explicit_key); - clear_key_values(icd_key); - } + delete_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers", false); + delete_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers", false); + + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers", false); + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers", false); + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\Drivers", false); + + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\Drivers", false); + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\ExplicitLayers", false); + delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\ImplicitLayers", false); } void PlatformShim::set_path(ManifestCategory category, fs::path const& path) {} @@ -270,11 +259,7 @@ void PlatformShim::add_CM_Device_ID(std::wstring const& id, fs::path const& icd_ // add_key_value_string(id_key, "VulkanLayerName", layer_path.c_str()); } -HKEY GetRegistryKey() { return HKEY{}; } - void PlatformShim::redirect_category(fs::path const& new_path, ManifestCategory search_category) { - // create_key(HKEY_LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Control\\Class\\{4d36e968-e325-11ce-bfc1-08002be10318}\\0001"); - // create_key(HKEY_LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Control\\Class\\{4d36e968-e325-11ce-bfc1-08002be10318}\\0002"); switch (search_category) { case (ManifestCategory::implicit_layer): create_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers"); diff --git a/tests/framework/test_util.cpp b/tests/framework/test_util.cpp index 923de83d..193dd587 100644 --- a/tests/framework/test_util.cpp +++ b/tests/framework/test_util.cpp @@ -363,11 +363,25 @@ int delete_folder(path const& folder) { // nothing to delete return 0; } - bool ret = RemoveDirectoryA(folder.c_str()); - if (ret == 0) { - print_error_message(ERROR_REMOVEDIRECTORY_FAILED, "RemoveDirectoryA"); + std::string search_path = folder.str() + "/*.*"; + std::string s_p = folder.str() + "/"; + WIN32_FIND_DATA fd; + HANDLE hFind = ::FindFirstFileA(search_path.c_str(), &fd); + if (hFind != INVALID_HANDLE_VALUE) { + do { + if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + if (!string_eq(fd.cFileName, ".") && !string_eq(fd.cFileName, "..")) { + delete_folder(s_p + fd.cFileName); + } + } else { + std::string child_name = s_p + fd.cFileName; + DeleteFile(child_name.c_str()); + } + } while (::FindNextFile(hFind, &fd)); + ::FindClose(hFind); + _rmdir(folder.c_str()); } - return ret; + return 0; #else DIR* dir = opendir(folder.c_str()); if (!dir) { @@ -403,10 +417,15 @@ FolderManager::FolderManager(path root_path, std::string name, DebugMode debug) create_folder(folder); } FolderManager::~FolderManager() { - for (auto& file : files) { + auto list_of_files_to_delete = files; + // remove(file) modifies the files variable, copy the list before deleting it + // Note: the allocation tests currently leak the loaded driver handles because in an OOM scenario the loader doesn't bother + // removing those. Since this is in an OOM situation, it is a low priority to fix. It does have the effect that Windows will + // be unable to delete the binaries that were leaked. + for (auto& file : list_of_files_to_delete) { if (debug >= DebugMode::log) std::cout << "Removing manifest " << file << " at " << (folder / file).str() << "\n"; if (debug != DebugMode::no_delete) { - std::remove((folder / file).c_str()); + remove(file); } } if (debug != DebugMode::no_delete) { @@ -455,13 +474,17 @@ void FolderManager::remove(std::string const& name) { path out_path = folder / name; auto found = std::find(files.begin(), files.end(), name); if (found != files.end()) { - if (debug >= DebugMode::log) std::cout << "Removing manifest " << name << " at " << out_path.str() << "\n"; + if (debug >= DebugMode::log) std::cout << "Removing file " << name << " at " << out_path.str() << "\n"; if (debug != DebugMode::no_delete) { - std::remove(out_path.c_str()); + int rc = std::remove(out_path.c_str()); + if (rc != 0 && debug >= DebugMode::log) { + std::cerr << "Failed to remove file " << name << " at " << out_path.str() << "\n"; + } + files.erase(found); } } else { - if (debug >= DebugMode::log) std::cout << "Couldn't remove manifest " << name << " at " << out_path.str() << ".\n"; + if (debug >= DebugMode::log) std::cout << "Couldn't remove file " << name << " at " << out_path.str() << ".\n"; } } @@ -471,6 +494,8 @@ path FolderManager::copy_file(path const& file, std::string const& new_name) { auto found = std::find(files.begin(), files.end(), new_name); if (found != files.end()) { if (debug >= DebugMode::log) std::cout << "File location already contains" << new_name << ". Is this a bug?\n"; + } else if (file.str() == new_filepath.str()) { + if (debug >= DebugMode::log) std::cout << "Trying to copy " << new_name << " into itself. Is this a bug?\n"; } else { if (debug >= DebugMode::log) std::cout << "Copying file" << file.str() << " to " << new_filepath.str() << "\n"; files.emplace_back(new_name); @@ -482,7 +507,7 @@ path FolderManager::copy_file(path const& file, std::string const& new_name) { } std::ofstream dst(new_filepath.str(), std::ios::binary); if (!dst) { - std::cerr << "Failed to create file " << file.str() << " for copying to\n"; + std::cerr << "Failed to create file " << new_filepath.str() << " for copying to\n"; return new_filepath; } dst << src.rdbuf(); diff --git a/tests/loader_testing_main.cpp b/tests/loader_testing_main.cpp index c9e652ac..a2aefd00 100644 --- a/tests/loader_testing_main.cpp +++ b/tests/loader_testing_main.cpp @@ -70,8 +70,8 @@ int main(int argc, char** argv) { // Use an env-var to signal whether the override has been set up. uint32_t random_base_path = 0; std::string env_var{"size_large_enough"}; - DWORD ret = GetEnvironmentVariable("VK_LOADER_TEST_REGISTRY_IS_SETUP", (LPSTR)env_var.c_str(), 256); - if (ret == 0) { + DWORD has_not_setup_tests = GetEnvironmentVariable("VK_LOADER_TEST_REGISTRY_IS_SETUP", (LPSTR)env_var.c_str(), 256); + if (has_not_setup_tests == 0) { random_base_path = setup_override(DebugMode::none); set_env_var("VK_LOADER_TEST_REGISTRY_IS_SETUP", "SETUP"); } @@ -80,7 +80,9 @@ int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); int result = RUN_ALL_TESTS(); #if defined(_WIN32) - clear_override(DebugMode::none, random_base_path); + if (has_not_setup_tests == 0) { + clear_override(DebugMode::none, random_base_path); + } #endif return result; }