Changes to additive env var based on code review

diff --git a/docs/LoaderDriverInterface.md b/docs/LoaderDriverInterface.md
index 4999a06f1f68f2df4650403faa0175a0f8c0db57..d27efeb2078b017fbef9854fd2c4ba133c655ebf 100644 (file)
--- a/docs/LoaderDriverInterface.md
+++ b/docs/LoaderDriverInterface.md
@@ -103,6 +103,9 @@ drivers with either the `VK_DRIVER_FILES` or the older `VK_ICD_FILENAMES`
 environment variable.
 Both these environment variables behave the same, but `VK_ICD_FILENAMES`
 should be considered deprecated.
+If both `VK_DRIVER_FILES` and `VK_ICD_FILENAMES` environment variables are
+present, then the newer `VK_DRIVER_FILES` will be used, and the values in
+`VK_ICD_FILENAMES` will be ignored.
 
 The `VK_DRIVER_FILES` environment variable is a list of Driver Manifest
 files, containing the full path to the driver JSON Manifest file.

diff --git a/loader/loader.c b/loader/loader.c
index 4452710ec2cf76f0420c4e1d79265c79a04a17b0..49a211564004f78eb19e884e8954cd392ead08cc 100644 (file)
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -2896,11 +2896,10 @@ out:
 
 // Look for data files in the provided paths, but first check the environment override to determine if we should use that
 // instead.
-static VkResult read_data_files_in_search_paths(const struct loader_instance *inst, enum loader_json_type json_type,
+static VkResult read_data_files_in_search_paths(const struct loader_instance *inst, enum loader_data_files_type manifest_type,
                                                 const char *path_override, bool *override_active,
                                                 struct loader_data_files *out_files) {
     VkResult vk_result = VK_SUCCESS;
-    bool is_icd = false;
     char *override_env = NULL;
     const char *override_path = NULL;
     char *relative_location = NULL;
@@ -2995,25 +2994,28 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
     }
 #endif  // !_WIN32
 
-    switch (json_type) {
-        case VULKAN_LOADER_JSON_DRIVER:
-            is_icd = true;
+    switch (manifest_type) {
+        case LOADER_DATA_FILE_MANIFEST_DRIVER:
             override_env = loader_secure_getenv(VK_DRIVER_FILES_ENV_VAR, inst);
             if (NULL == override_env) {
                 // Not there, so fall back to the old name
                 override_env = loader_secure_getenv(VK_ICD_FILENAMES_ENV_VAR, inst);
             }
             additional_env = loader_secure_getenv(VK_ADDITIONAL_DRIVER_FILES_ENV_VAR, inst);
+            additional_env = loader_secure_getenv(VK_ADDITIONAL_DRIVER_FILES_ENV_VAR, inst);
             relative_location = VK_DRIVERS_INFO_RELATIVE_DIR;
             break;
-        case VULKAN_LOADER_JSON_IMPLICIT_LAYER:
+        case LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER:
             relative_location = VK_ILAYERS_INFO_RELATIVE_DIR;
             break;
-        case VULKAN_LOADER_JSON_EXPLICIT_LAYER:
+        case LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER:
             override_env = loader_secure_getenv(VK_LAYER_PATH_ENV_VAR, inst);
             additional_env = loader_secure_getenv(VK_ADDITIONAL_LAYER_PATH_ENV_VAR, inst);
             relative_location = VK_ELAYERS_INFO_RELATIVE_DIR;
             break;
+        default:
+            assert(false && "Shouldn't get here!");
+            break;
     }
 
     if (path_override != NULL) {
@@ -3095,8 +3097,11 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
             copy_data_file_info(additional_env, NULL, 0, &cur_path_ptr);
 
             // Remove the last path separator
-            --cur_path_ptr;
             *cur_path_ptr = '\0';
+            if (search_path[strlen(search_path) - 1] == ':') {
+                --cur_path_ptr;
+                *cur_path_ptr = '\0';
+            }
         }
     } else {
         // Add any additional search paths defined in the additive environment variable
@@ -3120,7 +3125,7 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
                         cur_path_ptr += rel_size;
                         *cur_path_ptr++ = PATH_SEPARATOR;
                         // only for ICD manifests
-                        if (override_env != NULL && is_icd) {
+                        if (override_env != NULL && manifest_type == LOADER_DATA_FILE_MANIFEST_DRIVER) {
                             use_first_found_manifest = true;
                         }
                     }
@@ -3194,7 +3199,7 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
         if (NULL != tmp_search_path) {
             strncpy(tmp_search_path, search_path, search_path_size);
             tmp_search_path[search_path_size] = '\0';
-            if (is_icd) {
+            if (manifest_type == LOADER_DATA_FILE_MANIFEST_DRIVER) {
                 log_flags = VULKAN_LOADER_DRIVER_BIT;
                 loader_log(inst, VULKAN_LOADER_DRIVER_BIT, 0, "Searching for driver manifest files");
             } else {
@@ -3276,7 +3281,7 @@ out:
 // Find the Vulkan library manifest files.
 //
 // This function scans the appropriate locations for a list of JSON manifest files based on the
-// "json_type".  The location is interpreted as Registry path on Windows and a directory path(s)
+// "manifest_type".  The location is interpreted as Registry path on Windows and a directory path(s)
 // on Linux.
 // "home_location" is an additional directory in the users home directory to look at. It is
 // expanded into the dir path $XDG_DATA_HOME/home_location or $HOME/.local/share/home_location
@@ -3296,8 +3301,8 @@ out:
 // Linux ICD  | dirs     | files
 // Linux Layer| dirs     | dirs
 
-VkResult loader_get_data_files(const struct loader_instance *inst, enum loader_json_type json_type, const char *path_override,
-                               struct loader_data_files *out_files) {
+VkResult loader_get_data_files(const struct loader_instance *inst, enum loader_data_files_type manifest_type,
+                               const char *path_override, struct loader_data_files *out_files) {
     VkResult res = VK_SUCCESS;
     bool override_active = false;
 
@@ -3315,7 +3320,7 @@ VkResult loader_get_data_files(const struct loader_instance *inst, enum loader_j
     out_files->alloc_count = 0;
     out_files->filename_list = NULL;
 
-    res = read_data_files_in_search_paths(inst, json_type, path_override, &override_active, out_files);
+    res = read_data_files_in_search_paths(inst, manifest_type, path_override, &override_active, out_files);
     if (VK_SUCCESS != res) {
         goto out;
     }
@@ -3325,22 +3330,19 @@ VkResult loader_get_data_files(const struct loader_instance *inst, enum loader_j
     if (!override_active) {
         bool warn_if_not_present = false;
         char *registry_location = NULL;
-        enum loader_data_files_type manifest_type = LOADER_DATA_FILE_MANIFEST_DRIVER;
 
-        switch (json_type) {
+        switch (manifest_type) {
             default:
                 goto out;
-            case VULKAN_LOADER_JSON_DRIVER:
+            case LOADER_DATA_FILE_MANIFEST_DRIVER:
                 warn_if_not_present = true;
                 registry_location = VK_DRIVERS_INFO_REGISTRY_LOC;
                 break;
-            case VULKAN_LOADER_JSON_IMPLICIT_LAYER:
-                manifest_type = LOADER_DATA_FILE_MANIFEST_LAYER;
+            case LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER:
                 registry_location = VK_ILAYERS_INFO_REGISTRY_LOC;
                 break;
-            case VULKAN_LOADER_JSON_EXPLICIT_LAYER:
+            case LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER:
                 warn_if_not_present = true;
-                manifest_type = LOADER_DATA_FILE_MANIFEST_LAYER;
                 registry_location = VK_ELAYERS_INFO_REGISTRY_LOC;
                 break;
         }
@@ -3399,7 +3401,7 @@ VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_t
         goto out;
     }
     // Get a list of manifest files for ICDs
-    res = loader_get_data_files(inst, VULKAN_LOADER_JSON_DRIVER, NULL, &manifest_files);
+    res = loader_get_data_files(inst, LOADER_DATA_FILE_MANIFEST_DRIVER, NULL, &manifest_files);
     if (VK_SUCCESS != res || manifest_files.count == 0) {
         goto out;
     }
@@ -3457,7 +3459,7 @@ VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_t
             json = NULL;
             continue;
         }
-        loader_log(inst, VULKAN_LOADER_INFO_BIT, 0, "Found ICD manifest file %s, version %s", file_str, file_vers);
+        loader_log(inst, VULKAN_LOADER_DRIVER_BIT, 0, "Found ICD manifest file %s, version %s", file_str, file_vers);
 
         // Get the version of the driver manifest
         json_file_version = loader_make_api_version(file_vers);
@@ -3578,7 +3580,7 @@ VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_t
                                        fullpath);
                             break;
                         case LOADER_LAYER_LIB_ERROR_WRONG_BIT_TYPE: {
-                            loader_log(inst, VULKAN_LOADER_INFO_BIT | VULKAN_LOADER_DRIVER_BIT, 0,
+                            loader_log(inst, VULKAN_LOADER_DRIVER_BIT, 0,
                                        "Requested layer %s was wrong bit-type. Ignoring this JSON", fullpath);
                             break;
                         }
@@ -3646,7 +3648,7 @@ void loader_scan_for_layers(struct loader_instance *inst, struct loader_layer_li
     loader_platform_thread_lock_mutex(&loader_json_lock);
 
     // Get a list of manifest files for any implicit layers
-    if (VK_SUCCESS != loader_get_data_files(inst, VULKAN_LOADER_JSON_IMPLICIT_LAYER, NULL, &manifest_files)) {
+    if (VK_SUCCESS != loader_get_data_files(inst, LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER, NULL, &manifest_files)) {
         goto out;
     }
 
@@ -3706,7 +3708,7 @@ void loader_scan_for_layers(struct loader_instance *inst, struct loader_layer_li
     }
 
     // Get a list of manifest files for explicit layers
-    if (VK_SUCCESS != loader_get_data_files(inst, VULKAN_LOADER_JSON_EXPLICIT_LAYER, override_paths, &manifest_files)) {
+    if (VK_SUCCESS != loader_get_data_files(inst, LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER, override_paths, &manifest_files)) {
         goto out;
     }
 
@@ -3778,7 +3780,7 @@ void loader_scan_for_implicit_layers(struct loader_instance *inst, struct loader
     // a failure occurs before allocating the manifest filename_list.
     memset(&manifest_files, 0, sizeof(struct loader_data_files));
 
-    VkResult res = loader_get_data_files(inst, VULKAN_LOADER_JSON_IMPLICIT_LAYER, NULL, &manifest_files);
+    VkResult res = loader_get_data_files(inst, LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER, NULL, &manifest_files);
     if (VK_SUCCESS != res || manifest_files.count == 0) {
         goto out;
     }
@@ -3853,7 +3855,7 @@ void loader_scan_for_implicit_layers(struct loader_instance *inst, struct loader
     // explicit layer info as well.  Not to worry, though, all explicit layers not included
     // in the override layer will be removed below in loader_remove_layers_in_blacklist().
     if (override_layer_valid || implicit_metalayer_present) {
-        if (VK_SUCCESS != loader_get_data_files(inst, VULKAN_LOADER_JSON_EXPLICIT_LAYER, override_paths, &manifest_files)) {
+        if (VK_SUCCESS != loader_get_data_files(inst, LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER, override_paths, &manifest_files)) {
             goto out;
         }
 

diff --git a/loader/loader_common.h b/loader/loader_common.h
index e00393885e47b2ea84b4fde3f6fae9bd337a4539..92b770cb2046c8a51ff8869d077ad9510e6c4082 100644 (file)
--- a/loader/loader_common.h
+++ b/loader/loader_common.h
@@ -31,18 +31,6 @@
 
 #include "vk_loader_platform.h"
 
-enum loader_json_type {
-    VULKAN_LOADER_JSON_DRIVER = 0,
-    VULKAN_LOADER_JSON_IMPLICIT_LAYER,
-    VULKAN_LOADER_JSON_EXPLICIT_LAYER,
-};
-
-enum layer_type_flags {
-    VK_LAYER_TYPE_FLAG_INSTANCE_LAYER = 0x1,  // If not set, indicates Device layer
-    VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER = 0x2,  // If not set, indicates Implicit layer
-    VK_LAYER_TYPE_FLAG_META_LAYER = 0x4,      // If not set, indicates standard layer
-};
-
 typedef enum VkStringErrorFlagBits {
     VK_STRING_ERROR_NONE = 0x00000000,
     VK_STRING_ERROR_LENGTH = 0x00000001,
@@ -127,6 +115,12 @@ enum loader_layer_library_status {
     LOADER_LAYER_LIB_ERROR_FAILED_TO_LOAD = 21,
 };
 
+enum layer_type_flags {
+    VK_LAYER_TYPE_FLAG_INSTANCE_LAYER = 0x1,  // If not set, indicates Device layer
+    VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER = 0x2,  // If not set, indicates Implicit layer
+    VK_LAYER_TYPE_FLAG_META_LAYER = 0x4,      // If not set, indicates standard layer
+};
+
 struct loader_layer_properties {
     VkLayerProperties info;
     enum layer_type_flags type_flags;
@@ -452,7 +446,8 @@ struct loader_scanned_icd {
 
 enum loader_data_files_type {
     LOADER_DATA_FILE_MANIFEST_DRIVER = 0,
-    LOADER_DATA_FILE_MANIFEST_LAYER,
+    LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER,
+    LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER,
     LOADER_DATA_FILE_NUM_TYPES  // Not a real field, used for possible loop terminator
 };
 

diff --git a/loader/loader_windows.c b/loader/loader_windows.c
index 7aa730e460b9fa19b029f3ce6132b8501666c388..6c3cdfa4adbe83e4bbffd7e6f5e319125626b383 100644 (file)
--- a/loader/loader_windows.c
+++ b/loader/loader_windows.c
@@ -738,8 +738,8 @@ VkResult windows_read_data_files_in_registry(const struct loader_instance *inst,
         goto out;
     }
 
-    // This call looks into the Khronos non-device specific section of the registry.
-    bool use_secondary_hive = (data_file_type == LOADER_DATA_FILE_MANIFEST_LAYER) && (!is_high_integrity());
+    // This call looks into the Khronos non-device specific section of the registry for layer files.
+    bool use_secondary_hive = (data_file_type != LOADER_DATA_FILE_MANIFEST_DRIVER) && (!is_high_integrity());
     VkResult reg_result = windows_get_registry_files(inst, registry_location, use_secondary_hive, &search_path, &reg_size);
     if (reg_result == VK_ERROR_OUT_OF_HOST_MEMORY) {
         vk_result = VK_ERROR_OUT_OF_HOST_MEMORY;
@@ -754,7 +754,8 @@ VkResult windows_read_data_files_in_registry(const struct loader_instance *inst,
             vk_result = VK_ERROR_INCOMPATIBLE_DRIVER;
         } else {
             if (warn_if_not_present) {
-                if (data_file_type == LOADER_DATA_FILE_MANIFEST_LAYER) {
+                if (data_file_type == LOADER_DATA_FILE_MANIFEST_IMPLICIT_LAYER ||
+                    data_file_type == LOADER_DATA_FILE_MANIFEST_EXPLICIT_LAYER) {
                     // This is only a warning for layers
                     loader_log(inst, VULKAN_LOADER_WARN_BIT | log_target_flag, 0,
                                "windows_read_data_files_in_registry: Registry lookup failed to get layer manifest files.");

diff --git a/tests/framework/test_environment.cpp b/tests/framework/test_environment.cpp
index e2b80fa76b8d9e042acb08d38176e1ff243a3a31..8c2b500ac0a447ae1f818df4dc946cd9a4469fa4 100644 (file)
--- a/tests/framework/test_environment.cpp
+++ b/tests/framework/test_environment.cpp
@@ -182,7 +182,7 @@ TestLayer& TestLayerHandle::reset_layer() noexcept {
 }
 fs::path TestLayerHandle::get_layer_full_path() noexcept { return layer_library.lib_path; }
 
-FrameworkEnvironment::FrameworkEnvironment(DebugMode debug_mode, bool override_icds, bool override_layers) noexcept
+FrameworkEnvironment::FrameworkEnvironment(DebugMode debug_mode, bool override_icds, bool override_explicit_layers) noexcept
     : platform_shim(debug_mode),
       null_folder(FRAMEWORK_BUILD_DIRECTORY, "null_dir", debug_mode),
       icd_folder(FRAMEWORK_BUILD_DIRECTORY, "icd_manifests", debug_mode),
@@ -196,7 +196,7 @@ FrameworkEnvironment::FrameworkEnvironment(DebugMode debug_mode, bool override_i
     } else {
         platform_shim->set_path(ManifestCategory::icd, icd_folder.location());
     }
-    if (override_layers) {
+    if (override_explicit_layers) {
         platform_shim->set_path(ManifestCategory::explicit_layer, null_folder.location());
     } else {
         platform_shim->set_path(ManifestCategory::explicit_layer, explicit_layer_folder.location());
@@ -230,7 +230,7 @@ void FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcept {
         }
         env_var_vk_icd_filenames += (icd_folder.location() / full_json_name).str();
         set_env_var("VK_DRIVER_FILES", env_var_vk_icd_filenames);
-    } else  if (icd_details.use_add_env_var_icd_filenames) {
+    } else if (icd_details.use_add_env_var_icd_filenames) {
         if (!add_env_var_vk_icd_filenames.empty()) {
             add_env_var_vk_icd_filenames += OS_ENV_VAR_LIST_SEPARATOR;
         }

diff --git a/tests/framework/test_environment.h b/tests/framework/test_environment.h
index 2cf228fb62dbe8ddd8c33a90be7218782e0a21de..5f9355c05acfa806eef3464652f024de1492b7f8 100644 (file)
--- a/tests/framework/test_environment.h
+++ b/tests/framework/test_environment.h
@@ -136,9 +136,7 @@ struct InstWrapper {
     operator VkInstance() { return inst; }
     VulkanFunctions* operator->() { return functions; }
 
-    FromVoidStarFunc load(const char* func_name) {
-        return FromVoidStarFunc(functions->vkGetInstanceProcAddr(inst, func_name));
-    }
+    FromVoidStarFunc load(const char* func_name) { return FromVoidStarFunc(functions->vkGetInstanceProcAddr(inst, func_name)); }
 
     // Enumerate physical devices using googletest to assert if it succeeded
     std::vector<VkPhysicalDevice> GetPhysDevs(VkResult result_to_check = VK_SUCCESS);  // query all physical devices
@@ -173,9 +171,7 @@ struct DeviceWrapper {
     operator VkDevice() { return dev; }
     VulkanFunctions* operator->() { return functions; }
 
-    FromVoidStarFunc load(const char* func_name) {
-        return FromVoidStarFunc(functions->vkGetDeviceProcAddr(dev, func_name));
-    }
+    FromVoidStarFunc load(const char* func_name) { return FromVoidStarFunc(functions->vkGetDeviceProcAddr(dev, func_name)); }
 
     VulkanFunctions* functions = nullptr;
     VkDevice dev = VK_NULL_HANDLE;
@@ -323,7 +319,8 @@ struct TestLayerDetails {
 };
 
 struct FrameworkEnvironment {
-    FrameworkEnvironment(DebugMode debug_mode = DebugMode::none, bool override_icds = false, bool override_layers = false) noexcept;
+    FrameworkEnvironment(DebugMode debug_mode = DebugMode::none, bool override_icds = false,
+                         bool override_explicit_layers = false) noexcept;
 
     void add_icd(TestICDDetails icd_details) noexcept;
     void add_implicit_layer(ManifestLayer layer_manifest, const std::string& json_name) noexcept;