Skip to content
Snippets Groups Projects

vkd3d: Always free the root signature descriptor range info (valgrind).

Merged Conor McCarthy requested to merge cmccarthy/vkd3d:range_info_leak into master

Fixes memory leaks in test_create_root_signature().

Merge request reports

Merge request pipeline #30799 skipped

Merge request pipeline skipped for 80e607e3

Merged by Henri VerbeetHenri Verbeet 6 months ago (Aug 26, 2024 9:30am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • -    if (FAILED(hr = d3d12_root_signature_info_from_desc(&info, desc, device->vk_info.EXT_descriptor_indexing)))
    +    hr = d3d12_root_signature_info_from_desc(&info, desc, device->vk_info.EXT_descriptor_indexing);
    +    d3d12_root_signature_info_cleanup(&info);
    +    if (FAILED(hr))
             return hr;
         if (info.cost > D3D12_MAX_ROOT_COST)

    I don't think this makes sense. In particular:

    • If d3d12_root_signature_info_cleanup() is supposed to be the generic cleanup function for the d3d12_root_signature_info structure, we shouldn't use "info" after calling d3d12_root_signature_info_cleanup(); its contents should be considered undefined. On the other hand, if it's explicitly only supposed to touch the range information, it shouldn't be called d3d12_root_signature_info_cleanup().
    • The range information doesn't seem to be used anywhere outside of d3d12_root_signature_info_from_desc(); the existing code certainly seems to suggest it's intended for internal use only. Arguably that means it shouldn't be part of the d3d12_root_signature_info structure in the first place, but I don't think making callers of d3d12_root_signature_info_from_desc() responsible for cleaning it up is an improvement.
  • added 1 commit

    • 5c9bce00 - vkd3d: Always free the root signature descriptor range info (valgrind).

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 8 commits

    added 8 commits

    Compare with previous version

Please register or sign in to reply
Loading