Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed unwind safe within zip_elements #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Phosphorus15
Copy link

This should fixes #1 , which wraps a ManuallyDrop around the original matrix to prevent unintentional dropping of its elements.

@Phosphorus15 Phosphorus15 changed the title Fixed unwind safe within map_elements Fixed unwind safe within zip_elements Sep 13, 2019
@strake
Copy link
Owner

strake commented Sep 24, 2019

The PR title says "zip_elements" but it seems you are modifying map_elements. I assume zip_elements is also broken, yes?

src/lib.rs Outdated
@@ -172,13 +172,14 @@ impl<A, M: ArrayLength<A>, N: ArrayLength<GenericArray<A, M>>> Matrix<A, M, N> {
#[inline] fn map_elements<B, F: FnMut(A) -> B>(self, mut f: F) -> Matrix<B, M, N>
where M: ArrayLength<B>, N: ArrayLength<GenericArray<B, M>> {
let Matrix(a) = self;
let _wrapper = mem::ManuallyDrop::new(a);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the leading underscore? We use it below.

src/lib.rs Outdated
Matrix(c)
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrub this, please

@Phosphorus15
Copy link
Author

The PR title says "zip_elements" but it seems you are modifying map_elements. I assume zip_elements is also broken, yes?

Yeah, that's my fault, I'll fix this soon.

@Phosphorus15
Copy link
Author

The PR title says "zip_elements" but it seems you are modifying map_elements. I assume zip_elements is also broken, yes?

Yeah, that's my fault, I'll fix this soon.

By this I mean, they are both broken. 😂

@Phosphorus15
Copy link
Author

Phosphorus15 commented Sep 24, 2019

The code reformatter might have changed the codes format .. hope you don't mind 😭

@strake
Copy link
Owner

strake commented Sep 24, 2019

I'd prefer you not reformat the file...

@Phosphorus15
Copy link
Author

This should make it sound

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Method zip_elements suffers from double free vulnerability
2 participants