Fix a few errors and warnings found by clang-check (#2404)

Author: per-mathisen-arm

framework/decode/file_processor.cpp

@@ -868,7 +868,7 @@ bool FileProcessor::ProcessMetaData(const format::BlockHeader& block_header, for
         format::FillMemoryResourceValueCommandHeader header;
 
         success = ReadBytes(&header.thread_id, sizeof(header.thread_id));
-        success = ReadBytes(&header.resource_value_count, sizeof(header.resource_value_count));
+        success = success && ReadBytes(&header.resource_value_count, sizeof(header.resource_value_count));
 
         if (success)
         {

framework/decode/vulkan_offscreen_swapchain.cpp

@@ -300,12 +300,6 @@ VkResult VulkanOffscreenSwapchain::SignalSemaphoresFence(const VulkanQueueInfo*
                                                          const VkSemaphore*     signal_semaphores,
                                                          VkFence                fence)
 {
-    uint32_t queue_family_index = default_queue_family_index_;
-    if (queue_info)
-    {
-        queue_family_index = queue_info->family_index;
-    }
-
     VkPipelineStageFlags wait_stage  = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
     VkSubmitInfo         submit_info = { VK_STRUCTURE_TYPE_SUBMIT_INFO };
 

framework/decode/vulkan_replay_consumer_base.cpp

@@ -6860,8 +6860,6 @@ VkResult VulkanReplayConsumerBase::OverrideCreateShaderModule(
 
     // Replace shader in 'override_info'
     std::unique_ptr<char[]> file_code;
-    const uint32_t* const   orig_code = original_info->pCode;
-    const size_t            orig_size = original_info->codeSize;
     uint64_t                handle_id = *pShaderModule->GetPointer();
     std::string             file_name = "sh" + std::to_string(handle_id);
     std::string             file_path = util::filepath::Join(options_.replace_shader_dir, file_name);
@@ -8151,7 +8149,9 @@ VulkanReplayConsumerBase::OverrideQueuePresentKHR(PFN_vkQueuePresentKHR
         // If a swapchain was removed, pNext stucts that reference the swapchain need to be modified as well.
         if (!removed_swapchain_indices_.empty())
         {
-            const VkBaseInStructure* next = reinterpret_cast<const VkBaseInStructure*>(modified_present_info.pNext);
+            VkBaseInStructure* next =
+                reinterpret_cast<VkBaseInStructure*>(const_cast<void*>(modified_present_info.pNext));
+            VkBaseInStructure* prev = reinterpret_cast<VkBaseInStructure*>(&modified_present_info);
             while (next != nullptr)
             {
                 switch (next->sType)
@@ -8178,7 +8178,7 @@ VulkanReplayConsumerBase::OverrideQueuePresentKHR(PFN_vkQueuePresentKHR
                                 static_cast<uint32_t>(modified_device_masks_.size());
                             modified_device_group_present_info.pDeviceMasks = modified_device_masks_.data();
                             modified_device_group_present_info.mode         = pNext->mode;
-                            pNext                                           = &modified_device_group_present_info;
+                            prev->pNext = (const VkBaseInStructure*)&modified_device_group_present_info;
                         }
                         break;
                     }
@@ -8202,7 +8202,7 @@ VulkanReplayConsumerBase::OverrideQueuePresentKHR(PFN_vkQueuePresentKHR
                             modified_present_region_info.swapchainCount =
                                 static_cast<uint32_t>(modified_regions_.size());
                             modified_present_region_info.pRegions = modified_regions_.data();
-                            pNext                                 = &modified_present_region_info;
+                            prev->pNext = (const VkBaseInStructure*)&modified_present_region_info;
                         }
                         break;
                     }
@@ -8225,15 +8225,16 @@ VulkanReplayConsumerBase::OverrideQueuePresentKHR(PFN_vkQueuePresentKHR
                             modified_present_times_info.pNext          = pNext->pNext;
                             modified_present_times_info.swapchainCount = static_cast<uint32_t>(modified_times_.size());
                             modified_present_times_info.pTimes         = modified_times_.data();
-                            pNext                                      = &modified_present_times_info;
+                            prev->pNext = (const VkBaseInStructure*)&modified_present_times_info;
                         }
                         break;
                     }
                     default:
                         break;
                 }
 
-                next = reinterpret_cast<const VkBaseInStructure*>(next->pNext);
+                prev = next;
+                next = const_cast<VkBaseInStructure*>(next->pNext);
             }
         }
 
@@ -11191,6 +11192,7 @@ void VulkanReplayConsumerBase::OverrideUpdateDescriptorSets(
                         create_info->codeSize = file_size;
                         GFXRECON_LOG_INFO("Replacement shader found: %s", file_path.c_str());
                         replaced_file_code.emplace_back(std::move(file_code));
+                        util::platform::FileClose(fp);
                     }
                 }
                 pNext = const_cast<VkBaseInStructure*>(base->pNext);
@@ -11335,8 +11337,6 @@ VkResult VulkanReplayConsumerBase::OverrideCreateComputePipelines(
     for (size_t i = 0; i < create_info_count; i++)
     {
         auto*       create_info = &create_infos[i];
-        const void* orig_code   = create_info->pCode;
-        size_t      orig_size   = create_info->codeSize;
         uint64_t    handle_id   = shaders[i];
         std::string file_name   = "sh" + std::to_string(handle_id);
         std::string file_path   = util::filepath::Join(options_.replace_shader_dir, file_name);
@@ -11354,6 +11354,7 @@ VkResult VulkanReplayConsumerBase::OverrideCreateComputePipelines(
             create_info->codeSize = file_size;
             GFXRECON_LOG_INFO("Replacement shader found: %s", file_path.c_str());
             replaced_file_code.emplace_back(std::move(file_code));
+            util::platform::FileClose(fp);
         }
     }
 

framework/decode/vulkan_resource_tracking_consumer.cpp

@@ -813,14 +813,12 @@ void VulkanResourceTrackingConsumer::CalculateReplayBindingOffsetAndMemoryAlloca
             // during trace and update the replay binding offset  and then memory allocation size
             // accordingly.
 
-            VkDeviceSize replay_bind_offset = (*resources)[0]->GetTraceBindOffset();
-
             // loop through the bound resources and update replay resource binding offset
             // based on the memory alignment requirement and update memory allocation size
             for (size_t i = 0; i < (*resources).size(); i++)
             {
                 // assign replay bind offset to be the same as trace offset first
-                replay_bind_offset = (*resources)[i]->GetTraceBindOffset();
+                VkDeviceSize replay_bind_offset = (*resources)[i]->GetTraceBindOffset();
 
                 // make sure the assigned replay bind offset have the same alignment count as trace bind offset
                 // if trace alignment number is valid

framework/decode/vulkan_virtual_swapchain.cpp

@@ -52,15 +52,16 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainKHR(VkResult
                                                     HandlePointerDecoder<VkSwapchainKHR>* swapchain,
                                                     const graphics::VulkanDeviceTable*    device_table)
 {
-    VkDevice                 device = VK_NULL_HANDLE;
+    VkDevice                 device          = VK_NULL_HANDLE;
+    VkPhysicalDevice         physical_device = VK_NULL_HANDLE;
     VkSurfaceCapabilitiesKHR surfCapabilities{};
 
     if (device_info != nullptr)
     {
         device = device_info->handle;
+        physical_device = device_info->parent;
     }
-    device_table_                    = device_table;
-    VkPhysicalDevice physical_device = device_info->parent;
+    device_table_ = device_table;
 
     VkSwapchainCreateInfoKHR modified_create_info = *create_info;
     modified_create_info.imageUsage =

framework/graphics/vulkan_resources_util.cpp

@@ -422,11 +422,10 @@ bool GetTexelCoordinatesFromOffset(VkImageType                imageType,
                                    VkDeviceSize*              current_row_remaining_size_ptr)
 {
     bool         is_texel_block_size = false;
-    VkDeviceSize texel_size          = 0;
+    VkDeviceSize texel_size;
     uint16_t     block_width = 0, block_height = 0;
-    bool         result = GetImageTexelSize(format, &texel_size, &is_texel_block_size, &block_width, &block_height);
 
-    if (!result)
+    if (GetImageTexelSize(format, &texel_size, &is_texel_block_size, &block_width, &block_height))
     {
         // The image format is not supported
         return false;
@@ -464,9 +463,9 @@ bool GetTexelCoordinatesFromOffset(VkImageType                imageType,
             if (z >= extent.depth)
             {
                 // offset_to_subresource_data_start is beyond the range of subresource data. Because current
-                // Vulakn specification doesn't allow VK_IMAGE_TYPE_3D for array image, so no next array layer
+                // Vulkan specification doesn't allow VK_IMAGE_TYPE_3D for array image, so no next array layer
                 // exist;
-                result = false;
+                return false;
             }
             else
             {
@@ -586,7 +585,7 @@ bool GetTexelCoordinatesFromOffset(VkImageType                imageType,
         *current_row_remaining_size_ptr = current_row_remaining_size;
     }
 
-    return result;
+    return true;
 }
 
 // Get the offset which is relative to the start of subresource data for a location (pointed by texel

framework/util/logging.cpp

@@ -150,7 +150,7 @@ void Log::LogMessage(
     bool  opened_file      = false;
     bool  write_indent     = settings_.use_indent && (settings_.indent > 0);
     bool  output_to_stderr = false;
-    FILE* log_file_ptr;
+    FILE* log_file_ptr     = nullptr;
 
     // Log message prefix
     const char  process_tag[] = "gfxrecon";

framework/util/page_guard_manager.h

@@ -116,7 +116,7 @@ class PageGuardManager
     // shadow_memory is true.
     //
     // The shadow_memory_handle parameter is an option value that allows the lifetime of the shadow memory allocation to
-    // be managed externally.  Unless opy-on-map is disabled, copies from the mapped_range portion of mapped_memory to
+    // be managed externally.  Unless copy-on-map is disabled, copies from the mapped_range portion of mapped_memory to
     // the shadow memory are performed once, the first time that the shadow memory is added for tracking.  Copies will
     // not be performed if the mapped range is removed from tracking and then added again.
     //