Skip to content

Commit

Permalink
PathBuf::as_mut_vec removed and verified for UEFI and Windows platf…
Browse files Browse the repository at this point in the history
…orms #126333
  • Loading branch information
Borgerr committed Jun 25, 2024
1 parent 7e187e8 commit aa46a33
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 27 deletions.
15 changes: 10 additions & 5 deletions library/std/src/ffi/os_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,15 +552,20 @@ impl OsString {
OsStr::from_inner_mut(self.inner.leak())
}

/// Part of a hack to make PathBuf::push/pop more efficient.
/// Provides plumbing to core `Vec::truncate`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec<u8> {
self.inner.as_mut_vec_for_path_buf()
pub(crate) fn truncate(&mut self, len: usize) {
self.inner.truncate(len);
}

/// Provides plumbing to core `Vec::extend_from_slice`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn truncate(&mut self, len: usize) {
self.inner.truncate(len);
pub(crate) fn extend_from_slice(&mut self, other: &[u8]) {
self.inner.extend_from_slice(other);
}
}

Expand Down
11 changes: 3 additions & 8 deletions library/std/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1163,11 +1163,6 @@ pub struct PathBuf {
}

impl PathBuf {
#[inline]
fn as_mut_vec(&mut self) -> &mut Vec<u8> {
self.inner.as_mut_vec_for_path_buf()
}

/// Allocates an empty `PathBuf`.
///
/// # Examples
Expand Down Expand Up @@ -2645,18 +2640,18 @@ impl Path {
None => {
// Enough capacity for the extension and the dot
let capacity = self_len + extension.len() + 1;
let whole_path = self_bytes.iter();
let whole_path = self_bytes;
(capacity, whole_path)
}
Some(previous_extension) => {
let capacity = self_len + extension.len() - previous_extension.len();
let path_till_dot = self_bytes[..self_len - previous_extension.len()].iter();
let path_till_dot = &self_bytes[..self_len - previous_extension.len()];
(capacity, path_till_dot)
}
};

let mut new_path = PathBuf::with_capacity(new_capacity);
new_path.as_mut_vec().extend(slice_to_copy);
new_path.inner.extend_from_slice(slice_to_copy);
new_path.set_extension(extension);
new_path
}
Expand Down
15 changes: 10 additions & 5 deletions library/std/src/sys/os_str/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,20 @@ impl Buf {
self.as_slice().into_rc()
}

/// Part of a hack to make PathBuf::push/pop more efficient.
/// Provides plumbing to core `Vec::truncate`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec<u8> {
&mut self.inner
pub(crate) fn truncate(&mut self, len: usize) {
self.inner.truncate(len);
}

/// Provides plumbing to core `Vec::extend_from_slice`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn truncate(&mut self, len: usize) {
self.inner.truncate(len);
pub(crate) fn extend_from_slice(&mut self, other: &[u8]) {
self.inner.extend_from_slice(other);
}
}

Expand Down
16 changes: 13 additions & 3 deletions library/std/src/sys/os_str/wtf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,20 @@ impl Buf {
self.as_slice().into_rc()
}

/// Part of a hack to make PathBuf::push/pop more efficient.
/// Provides plumbing to core `Vec::truncate`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec<u8> {
self.inner.as_mut_vec_for_path_buf()
pub(crate) fn truncate(&mut self, len: usize) {
self.inner.truncate(len);
}

/// Provides plumbing to core `Vec::extend_from_slice`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn extend_from_slice(&mut self, other: &[u8]) {
self.inner.extend_from_slice(other);
}
}

Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys_common/wtf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,13 +474,13 @@ impl Wtf8Buf {
Wtf8Buf { bytes: bytes.into_vec(), is_known_utf8: false }
}

/// Part of a hack to make PathBuf::push/pop more efficient.
/// Provides plumbing to core `Vec::extend_from_slice`.
/// More well behaving alternative to allowing outer types
/// full mutable access to the core `Vec`.
#[inline]
pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec<u8> {
// FIXME: this function should not even exist, as it implies violating Wtf8Buf invariants
// For now, simply assume that is about to happen.
self.is_known_utf8 = false;
&mut self.bytes
pub(crate) fn extend_from_slice(&mut self, other: &[u8]) {
self.bytes.extend_from_slice(other);
self.is_known_utf8 = self.is_known_utf8 || self.next_surrogate(0).is_none();

This comment has been minimized.

Copy link
@yhx-12243

yhx-12243 Jun 26, 2024

Maybe this is not sound because we use the old value of is_known_utf8 after calling extend_from_slice.

This comment has been minimized.

Copy link
@Borgerr

Borgerr Jun 26, 2024

Author Contributor

@workingjubilee this may be a good point; an OR gate here may be misplaced? Assume, for example, that self.next_surrogate(0).is_none() returns false as other is not valid UTF-8, but self.is_known_utf8 is still set to true.

Hate to have to put this through code review again for one simple line change, but there was admittedly a fault here on my part 😅 let's decide on what to do, or if we really are needing to do anything.

This comment has been minimized.

Copy link
@workingjubilee

workingjubilee Jun 26, 2024

Member

...Oh, I misread something at some point, because I asked for other to be checked for UTF-8ness.

This comment has been minimized.

Copy link
@Borgerr

Borgerr Jun 26, 2024

Author Contributor

Yep, entirely my fault on that. I had assumed that a similar check in into_string would be well-placed here. I'll make a quick PR for marking self.is_known_utf8 as false, unless we are okay with a second iteration through other following the extension.

This comment has been minimized.

Copy link
@workingjubilee

workingjubilee Jun 26, 2024

Member

Either way is unimportant without the regression test.

This comment has been minimized.

Copy link
@Borgerr

Borgerr Jun 26, 2024

Author Contributor

Created #126980 just in case 🤞 Jubilee, I'll be placing you as the reviewer again, in case you're thinking it's entirely unimportant.

}
}

Expand Down

0 comments on commit aa46a33

Please sign in to comment.