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

Added From<Vec<NonZeroU8>> for CString #64069

@@ -6,6 +6,7 @@ use crate::fmt::{self, Write};
use crate::io;
use crate::mem;
use crate::memchr;
use crate::num::NonZeroU8;
use crate::ops;
use crate::os::raw::c_char;
use crate::ptr;
@@ -718,6 +719,81 @@ impl From<Box<CStr>> for CString {
}
}

#[stable(feature = "cstring_from_vec_of_nonzerou8", since = "1.39.0")]
impl From<Vec<NonZeroU8>> for CString {
/// Converts a [`Vec`]`<`[`NonZeroU8`]`>` into a [`CString`] without
/// copying nor checking for inner null bytes.
///
/// [`CString`]: ../ffi/struct.CString.html
/// [`NonZeroU8`]: ../num/struct.NonZeroU8.html
/// [`Vec`]: ../vec/struct.Vec.html
#[inline]
fn from(v: Vec<NonZeroU8>) -> CString {
/// # Safety
///
/// - for any `length: usize`, it must be sound to transmute between
/// `[T; length]` and `[U; length]`.
///
/// - for any `cap: usize`, `Layout<[T; cap]>` needs to be equal to
/// `Layout<[U; cap]>` (for the allocator)

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
/// `Layout<[U; cap]>` (for the allocator)
/// `Layout<[U; cap]>` (for the allocator).
#[inline]
unsafe

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member

what's with the line break here?

This comment has been minimized.

Copy link
@danielhenrymantilla

danielhenrymantilla Sep 14, 2019

Author Contributor

Woops, my own Rust style leaked here (Out of topic: I see pub and unsafe, among others, as markers, hence the logic of them being on an extra line rather than on the same line, in the same vein as other #[meta] attributes)

fn transmute_vec<T, U>(v: Vec<T>) -> Vec<U> {
// necessary conditions for `Layout<[T; N]> == Layout<[U; N]>`

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member

Why not check Layout::array::<T / U>().unwrap() against each other?

This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
// necessary conditions for `Layout<[T; N]> == Layout<[U; N]>`
// Necessary conditions for `Layout<[T; N]> == Layout<[U; N]>`:
{
assert_eq!(
mem::size_of::<T>(),
mem::size_of::<U>(),
);
assert_eq!(
mem::align_of::<T>(),
mem::align_of::<U>(),
);
}
// # Safety
//
// - this is the pattern blessed by transmute's documentation
// (and replacing `mem::forget` by `ManuallyDrop`, _c.f._,
// issue #62553)
This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
// issue #62553)
// issue #62553).
//
// - `transmute_vec` contract guarantees sound transmuting
// between `[T; length]` and `[U; length]`
This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
// between `[T; length]` and `[U; length]`
// between `[T; length]` and `[U; length]`.
let length: usize = v.len();
let capacity: usize = v.capacity();
let ptr: *mut T = {
// end-of-scope `mem::forget` but without aliasing problems
This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
// end-of-scope `mem::forget` but without aliasing problems
// End-of-scope `mem::forget` but without aliasing problems.
let mut v = mem::ManuallyDrop::<Vec<T>>::new(v);
let v: &mut Vec<T> = &mut *v;
v.as_mut_ptr() // cannot be aliased
This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
v.as_mut_ptr() // cannot be aliased
v.as_mut_ptr() // Cannot be aliased.
};
Vec::<U>::from_raw_parts(ptr as *mut U, length, capacity)
}

let v = unsafe {

This comment has been minimized.

Copy link
@rkruppe

rkruppe Sep 14, 2019

Member

A type annotation won't hurt here IMO.

This comment has been minimized.

Copy link
@danielhenrymantilla

danielhenrymantilla Sep 14, 2019

Author Contributor

Agreed

// # Safety
//
// For all `N: usize`, transmuting between `[NonZeroU8; N]` and
// `[u8; N]` is sound since:
//
// - `[NonZeroU8; N]` and `[u8; N]` have the same layout in
// memory;
//
// - any valid representation for a `NonZeroU8` is a valid
// representation for a `u8` (loosened representation).
transmute_vec::<NonZeroU8, u8>(v)
};
unsafe {
// # Safety
//
// - `v` cannot contain null bytes, given the type-level
// invariant of `NonZeroU8` (this would still apply even if
// this invariant was a safety invariant and not a validity

This comment has been minimized.

Copy link
@rkruppe

rkruppe Sep 14, 2019

Member

Nit: strike the hypothetical. I am reasonably sure you do need the safety invariant here. Most operations on vectors don't assert validity of any elements except perhaps the ones they move into or out of the vec. Of course, a Vec<T> is only safe if elements 0..len are safe at T, so the soundness argument can rest on that plus the safety/validity (same thing) of the NonZeroU8.

(Insert standard disclaimer about all of these details being not yet ratified here.)

This comment has been minimized.

Copy link
@danielhenrymantilla

danielhenrymantilla Sep 14, 2019

Author Contributor

You're right, I will remove the part between brackets then (I'm still glad I put it, it is good to clearly think about these things before merging).

// invariant)
This conversation was marked as resolved by danielhenrymantilla

This comment has been minimized.

Copy link
@Centril

Centril Sep 14, 2019

Member
Suggested change
// invariant)
// invariant).

This comment has been minimized.

Copy link
@danielhenrymantilla

danielhenrymantilla Sep 14, 2019

Author Contributor

Always on point ;)

CString::from_vec_unchecked(v)
}
}
}

#[stable(feature = "more_box_slice_clone", since = "1.29.0")]
impl Clone for Box<CStr> {
#[inline]
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.