Fix problems detected by CodeQL static analysis

3 types of issues were found:
* Incorrect format specifiers
* Not providing required format specifies
* Using alloca in a loop

There are multiple instances of alloca in loops, but to fix them would
require significant refactoring. This commit includes 1 move of alloca inside
a for loop to the outside, but this is because the logic was redoing work
in a for loop that could have been done once at the start of the function.

diff --git a/loader/cJSON.c b/loader/cJSON.c
index 7f8c56f03fc161e9ab551137b878f43c8725bcce..04349fd9cd354903c91f19c9f6a2da32e6623c40 100644 (file)
--- a/loader/cJSON.c
+++ b/loader/cJSON.c
@@ -976,7 +976,7 @@ VkResult loader_get_json(const struct loader_instance *inst, const char *filenam
     json_buf = (char *)loader_instance_heap_calloc(inst, len + 1, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
     if (json_buf == NULL) {
         loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
-                   "loader_get_json: Failed to allocate space for JSON file %s buffer of length %d", filename, len);
+                   "loader_get_json: Failed to allocate space for JSON file %s buffer of length %lu", filename, len);
         res = VK_ERROR_OUT_OF_HOST_MEMORY;
         goto out;
     }

diff --git a/loader/loader.c b/loader/loader.c
index 048fa9fe5ea86889cc67b7202b00f2b0d5848082..471a7d8c173a90a864e057c5498baab195f0d81e 100644 (file)
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -1495,7 +1495,8 @@ VkResult loader_add_direct_driver(const struct loader_instance *inst, uint32_t i
         void *new_ptr = loader_instance_heap_realloc(inst, icd_tramp_list->scanned_list, icd_tramp_list->capacity,
                                                      icd_tramp_list->capacity * 2, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
         if (NULL == new_ptr) {
-            loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "loader_add_direct_driver: Realloc failed on icd library list for ICD %s");
+            loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
+                       "loader_add_direct_driver: Realloc failed on icd library list for ICD index %u", index);
             return VK_ERROR_OUT_OF_HOST_MEMORY;
         }
         icd_tramp_list->scanned_list = new_ptr;
@@ -5210,7 +5211,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateInstance(const VkInstanceCreateI
                    "#LLP_LAYER_21)");
     } else if (LOADER_MAGIC_NUMBER != ptr_instance->magic) {
         loader_log(ptr_instance, VULKAN_LOADER_WARN_BIT, 0,
-                   "terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08x. Instance value possibly "
+                   "terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08lx. Instance value possibly "
                    "corrupted by active layer (Policy #LLP_LAYER_21).  ",
                    ptr_instance, ptr_instance->magic);
     }

diff --git a/loader/settings.c b/loader/settings.c
index 3d83ec01b2e0009c9bb2653682fe346a2d9a50dc..c12ebac7ac54a1a5bae1db6d5c2d13b805c24c88 100644 (file)
--- a/loader/settings.c
+++ b/loader/settings.c
@@ -711,15 +711,20 @@ out:
 }
 
 VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters,
-                                             uint32_t name_count, const char* const* names,
+                                             uint32_t app_enabled_name_count, const char* const* app_enabled_names,
                                              const struct loader_layer_list* instance_layers,
                                              struct loader_pointer_layer_list* target_layer_list,
                                              struct loader_pointer_layer_list* activated_layer_list) {
     VkResult res = VK_SUCCESS;
     char* vk_instance_layers_env = loader_getenv(ENABLED_LAYERS_ENV, inst);
+    size_t vk_instance_layers_env_len = 0;
+    char* vk_instance_layers_env_copy = NULL;
     if (vk_instance_layers_env != NULL) {
-        loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers \"%s\"",
-                   ENABLED_LAYERS_ENV, names);
+        vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1;
+        vk_instance_layers_env_copy = loader_stack_alloc(vk_instance_layers_env_len);
+
+        loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers: %s",
+                   ENABLED_LAYERS_ENV, vk_instance_layers_env);
     }
     for (uint32_t i = 0; i < instance_layers->count; i++) {
         bool enable_layer = false;
@@ -745,30 +750,27 @@ VkResult enable_correct_layers_from_settings(const struct loader_instance* inst,
             enable_layer = true;
         }
 
-        if (!enable_layer && vk_instance_layers_env) {
-            size_t vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1;
-            char* name = loader_stack_alloc(vk_instance_layers_env_len);
-            if (name != NULL) {
-                loader_strncpy(name, vk_instance_layers_env_len, vk_instance_layers_env, vk_instance_layers_env_len);
-                // First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS
-                while (name && *name) {
-                    char* next = loader_get_next_path(name);
-
-                    if (strlen(name) > 0) {
-                        if (0 == strcmp(name, props->info.layerName)) {
-                            enable_layer = true;
-                            break;
-                        }
-                        name = next;
-                    }
+        // First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS
+        if (!enable_layer && vk_instance_layers_env && vk_instance_layers_env_copy && vk_instance_layers_env_len > 0) {
+            // Copy the env-var on each iteration, so that loader_get_next_path can correctly find the separators
+            // This solution only needs one stack allocation ahead of time rather than an allocation per layer in the env-var
+            loader_strncpy(vk_instance_layers_env_copy, vk_instance_layers_env_len, vk_instance_layers_env,
+                           vk_instance_layers_env_len);
+
+            while (vk_instance_layers_env_copy && *vk_instance_layers_env_copy) {
+                char* next = loader_get_next_path(vk_instance_layers_env_copy);
+                if (0 == strcmp(vk_instance_layers_env_copy, props->info.layerName)) {
+                    enable_layer = true;
+                    break;
                 }
+                vk_instance_layers_env_copy = next;
             }
         }
 
         // Check if it should be enabled by the application
         if (!enable_layer) {
-            for (uint32_t j = 0; j < name_count; j++) {
-                if (strcmp(props->info.layerName, names[j]) == 0) {
+            for (uint32_t j = 0; j < app_enabled_name_count; j++) {
+                if (strcmp(props->info.layerName, app_enabled_names[j]) == 0) {
                     enable_layer = true;
                     break;
                 }

diff --git a/loader/settings.h b/loader/settings.h
index 1e9dbf0a9acc0350f788ed0907063bf7df3d6be4..d3cd8b7cc9bec25fb8aeb0c8e04ad3d098c2b85a 100644 (file)
--- a/loader/settings.h
+++ b/loader/settings.h
@@ -108,7 +108,7 @@ VkResult combine_settings_layers_with_regular_layers(const struct loader_instanc
 // Fill out activated_layer_list with the layers that should be activated, based on environment variables, VkInstanceCreateInfo, and
 // the settings
 VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters,
-                                             uint32_t name_count, const char* const* names,
+                                             uint32_t app_enabled_name_count, const char* const* app_enabled_names,
                                              const struct loader_layer_list* instance_layers,
                                              struct loader_pointer_layer_list* target_layer_list,
                                              struct loader_pointer_layer_list* activated_layer_list);