Get rid of a lot of transmutes #700
Conversation
| unsafe { | ||
| quotactl(quota::QuotaCmd(quota::Q_GETQUOTA, which), Some(special), id, mem::transmute(dqblk)) | ||
| } | ||
| quotactl(quota::QuotaCmd(quota::Q_GETQUOTA, which), Some(special), id, dqblk as *mut _ as *mut c_char) |
There was a problem hiding this comment.
Because you can only cast a reference to a raw pointer to the exact same type
src/sys/socket/mod.rs
Outdated
| @@ -139,7 +139,7 @@ impl<'a> Iterator for CmsgIterator<'a> { | |||
| if buf.len() < sizeof_cmsghdr { | |||
| return None; | |||
| } | |||
| let cmsg: &cmsghdr = unsafe { mem::transmute(buf.as_ptr()) }; | |||
| let cmsg: &'a cmsghdr = unsafe { &*(buf.as_ptr() as *const cmsghdr) }; | |||
There was a problem hiding this comment.
To dereference the raw pointer, then get a reference to the value behind (the dereference doesn't copy the value out)
src/sys/socket/sockopt.rs
Outdated
| } | ||
|
|
||
| unsafe fn ffi_len(&mut self) -> *mut socklen_t { | ||
| mem::transmute(&mut self.len) | ||
| &mut self.len |
src/sys/socket/sockopt.rs
Outdated
| @@ -368,7 +368,7 @@ impl<'a> Set<'a, usize> for SetUsize { | |||
| } | |||
|
|
|||
| unsafe fn ffi_ptr(&self) -> *const c_void { | |||
There was a problem hiding this comment.
This shouldn't actually be unsafe, since they are creating raw pointers from safe references. Creating raw pointers is safe, dereferencing raw pointers is unsafe. I suggest we also change all of these functions that do that to not be unsafe.
There was a problem hiding this comment.
It would be more accurate in terms of safety to make the Get and Set traits unsafe instead of those functions.
There was a problem hiding this comment.
That would be required, yes. Could you change those in a separate commit after the current changes? I'm pretty certain the other function in Get are also safe, but I don't feel like getting familiar with it enough here to suggest we change those as well.
There was a problem hiding this comment.
Oh, sorry, I misread. You suggest the traits themselves be unsafe? The docs say:
If unsafe code cannot reasonably expect to defend against a bad implementation of the trait, then marking the trait unsafe is a reasonable choice.
Given this and other topics around unsafe, it does appear that the traits should be unsafe and all the functions should be safe.
There was a problem hiding this comment.
unwrap and blank should probably stay unsafe: The caller must uphold safety for those, not the callee or implementor.
|
This looks ready-to-go to me. There are no public API changes, so no CHANGELOG entry is needed. @asomers Any last comments? Didn't know if you got all your questions resolved yet? |
|
@jonas-schievink Can you rebase this? Then I think it's ready for merge. |
Most could be replaced by simple raw pointer casts (or even perfectly safe coercions!). cc #373
|
@asomers I think this is ready for merge, are you alright with it? |
|
Yep. bors r+ |
Timed out (retrying...) |
Most could be replaced by simple raw pointer casts (or even perfectly
safe coercions!).
cc #373