search

ebkalderon / renderdoc-rs

RenderDoc application bindings for Rust
Apache License 2.0
45 stars 6 forks source link

Change set_capture_file_comments() to take &mut self #28

Closed ebkalderon closed 5 years ago

ebkalderon commented 5 years ago

At the time of writing, set_capture_file_comments() currently takes self by immutable reference, despite requesting the RenderDoc API to set a mutable value internally. This is technically unsound and calling this method from multiple threads could lead to unexpected and unpredictable behavior. As such, set_capture_file_comments() should be changed to take &mut self instead, rectifying the issue while providing better indication of what the method actually does and improving consistency with other existing set_.*() methods.

This is technically a breaking API change, but it is critical to fix this in order to prevent unsound behavior in future versions. It should be clearly documented in the CHANGELOG.md file so users looking to upgrade versions are aware of the change.

Shnatsel commented 5 years ago

FYI: when fixing UB it's a good idea to file a security advisory at https://github.com/RustSec/advisory-db so that users of your crate can check if they're running a potentially vulnerable version and upgrade.

ebkalderon commented 5 years ago

@Shnatsel Good idea. I think I'll file one in that case. Thank you for the suggestion!