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

Array lifetime and type #1

Closed
bluss opened this issue Aug 25, 2015 · 4 comments
Closed

Array lifetime and type #1

bluss opened this issue Aug 25, 2015 · 4 comments

Comments

@bluss
Copy link

bluss commented Aug 25, 2015

This is some drive by code review as requested 😄

Issue 1) It looks like the code is not correctly connecting the lifetime of the borrow of the slice with the lifetime that belongs to the reference that comes from the transmute. Here's a testcase that shows an aliasing violation.. we can mutate the data while we have a reference to it!

By the way, for pointers, prefer just using casts instead of transmuting. The same caveat with lifetimes still applies though.

The best way to make sure the lifetimes match up is using a function. For example like copy_lifetime, a function you can copy.

Issue 2) Unrestricted transmute. The user's type parameter is trusted, and the user can get bogus results by giving the wrong type.

@bluss
Copy link
Author

bluss commented Aug 25, 2015

If you don't mind, I think I would write it like this. I like the idea behind this code, bounds check and then cast when it's ok. See also this Q & A on SO.

@droundy
Copy link
Owner

droundy commented Aug 26, 2015

Thanks so much! I had worried about aliasing, but wasn't clear as to how to do this right. I've made those changes, and in addition have made the as_array function generic, which seems to allow type inference to decide the type of the array for us. :)

If you'd care to go for another round of review, I'd be extremely grateful!

@bluss
Copy link
Author

bluss commented Aug 26, 2015

Looks good to me. I lifted out $offset, $len outside the unsafe block, but I forgot $arr. I guess just having let slice = &$arr[$offset .. $offset + $len] outside the unsafe block is the best. That way the user can't slip in unsafe code in the $arr expression by mistake.

@droundy
Copy link
Owner

droundy commented Aug 27, 2015

Thanks for the further suggestion, @bluss ! I hadn't understood why you'd defined a and l before, but now it makes sense. Even though rust macros are hygienic, that doesn't prevent expressions from containing unsafe code, and putting the "let slice =" outside the unsafe block protects from that.

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

No branches or pull requests

2 participants