Remove mem::transmute used in Box<str> conversions #44890
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@bors: r+ Thanks! |
|
📌 Commit 36d663f has been approved by |
aidanhs
added
the
S-waiting-on-bors
|
I may add some more similar changes for |
|
Sure! Feel free to just ping me when that happens |
src/libstd/ffi/c_str.rs
Outdated
| @@ -312,7 +312,7 @@ impl CString { | |||
| pub unsafe fn from_raw(ptr: *mut c_char) -> CString { | |||
| let len = libc::strlen(ptr) + 1; // Including the NUL byte | |||
| let slice = slice::from_raw_parts(ptr, len as usize); | |||
| CString { inner: mem::transmute(slice) } | |||
| CString { inner: Box::from_raw(slice as *mut [c_char] as *mut [u8]) } | |||
There was a problem hiding this comment.
Use slice::from_raw_parts_mut to get a &mut [i8].
[00:02:36] error[E0606]: casting `&[i8]` as `*mut [i8]` is invalid
[00:02:36] --> /checkout/src/libstd/ffi/c_str.rs:315:40
[00:02:36] |
[00:02:36] 315 | CString { inner: Box::from_raw(slice as *mut [c_char] as *mut [u8]) }
[00:02:36] | ^^^^^^^^^^^^^^^^^^^^^^
|
@bors: r+ |
|
📌 Commit 627998e has been approved by |
|
@alexcrichton this should be good to go once cb2a656 passes. |
|
@bors: r+ Thanks! |
|
📌 Commit cb2a656 has been approved by |
|
⌛ Testing commit cb2a656 with merge 8b95ab453e0e8f3ccb3333a8f78007de889c8e39... |
|
💔 Test failed - status-appveyor |
The innermost type is not [u8] on all platforms but is assumed to have the same memory layout as [u8] since this conversion was done via mem::transmute before.
|
@bors: retry Edit: It was worth a shot 😅 |
|
@nvzqz: 🔑 Insufficient privileges: and not in try users |
1 similar comment
|
@nvzqz: 🔑 Insufficient privileges: and not in try users |
|
@nvzqz The bors error is legitimate. |
|
@kennytm I committed another change that should fix the error on appveyor, so I was trying to get bors to try again with the new changes in this PR |
|
@nvzqz retry only works on approved commits, if you create a new commit, someone will need to r+ it again 🙂 |
|
I didn't know that. Thanks! |
carols10cents
added
S-waiting-on-review
|
@bors: r+ |
|
📌 Commit f1798d3 has been approved by |
Remove mem::transmute used in Box<str> conversions Given that #44877 is failing, I decided to make a separate PR. This is done with the same motivation: to avoid `mem::transmute`-ing non `#[repr(C)]` types.
|
☀️ Test successful - status-appveyor, status-travis |
Neither PgConnection nor PgMetadataLookup are marked as `#[repr(C)]` so this is at least implemetation defined behaviour. (The implementation of `std::path::Path` changed in rust-lang/rust#44890)
Given that #44877 is failing, I decided to make a separate PR. This is done with the same motivation: to avoid
mem::transmute-ing non#[repr(C)]types.