Remove actual unsafety in hdr #985
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Third part of the effort of removing `unsafe` from the library, the hdr decoder actually required an interface change. Several other instances had been removed in earlier commits. The method would use `Vec::set_len` even though it did not have full control over the type parameter, as it was a public interface. Now simply requires the memory into which to emplace the result to be provided and allocated externally. This makes the callers responsible for correct optimization. Removes the optimization for the library internal structures; it is expected that the compiler picks up on the pattern well enough for it to not be the main bottleneck. Leaves the instances that are deprecated methods, they should enjoy some deprecation period afterall and are not inherently bad as they are correctly marked with `unsafe` themselves. Co-Authored-By: Andreas Molzer <[email protected]>
fintelia
approved these changes
Jul 8, 2019
|
Do I understand correctly that this code could potentially drop uninitialized memory of arbitrary type? |
|
I fear so, but I didn't realize until a few days ago that it was actually that severe in the old interface, was always under the impression that our own type (non- @Shnatsel Thanks for the reminder, should probably get an advisory for this. |
|
Yeah, please open a RustSec advisory: https://github.com/RustSec/advisory-db |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Third part of the effort of removing
unsafefrom the library, the hdrdecoder actually required an interface change. Several other instances
had been removed in earlier commits. The method would use
Vec::set_leneven though it did not have full control over the type parameter, as it
was a public interface.
Now simply requires the memory into which to emplace the result to be
provided and allocated externally. This makes the callers responsible
for correct optimization.
Removes the optimization for the library internal structures; it is
expected that the compiler picks up on the pattern well enough for it to
not be the main bottleneck.
Leaves the instances that are deprecated methods, they should enjoy some
deprecation period afterall and are not inherently bad as they are
correctly marked with
unsafethemselves.Closes: #980