From 9a8b516de046ac909f22f0280e1a8a0d87ba0d06 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 1/8] Sorting feature attributes in std --- src/libstd/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index bd585d39c242f..16a4b38a1ae33 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -242,8 +242,8 @@ #![feature(atomic_mut_ptr)] #![feature(box_syntax)] #![feature(c_variadic)] -#![feature(cfg_accessible)] #![feature(can_vector)] +#![feature(cfg_accessible)] #![feature(cfg_target_has_atomic)] #![feature(cfg_target_thread_local)] #![feature(char_error_internals)] @@ -276,8 +276,8 @@ #![feature(hashmap_internals)] #![feature(int_error_internals)] #![feature(int_error_matching)] -#![feature(into_future)] #![feature(integer_atomics)] +#![feature(into_future)] #![feature(lang_items)] #![feature(libc)] #![feature(link_args)] @@ -286,6 +286,7 @@ #![feature(log_syntax)] #![feature(maybe_uninit_ref)] #![feature(maybe_uninit_slice)] +#![feature(min_specialization)] #![feature(needs_panic_runtime)] #![feature(negative_impls)] #![feature(never_type)] @@ -305,7 +306,6 @@ #![feature(shrink_to)] #![feature(slice_concat_ext)] #![feature(slice_internals)] -#![feature(min_specialization)] #![feature(staged_api)] #![feature(std_internals)] #![feature(stdsimd)] From 1e05e09fe9df9919dc1c1c0f36255f4f1098fc24 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 2/8] Remove the useless indentation --- src/libstd/thread/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index d435ca6842518..d354a9b1842c2 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -641,9 +641,8 @@ where #[stable(feature = "rust1", since = "1.0.0")] pub fn current() -> Thread { thread_info::current_thread().expect( - "use of std::thread::current() is not \ - possible after the thread's local \ - data has been destroyed", + "use of std::thread::current() is not possible \ + after the thread's local data has been destroyed", ) } From 0ff820cb62f0b21d422440353b8def10c05ed735 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 12 Jul 2020 10:17:22 +0000 Subject: [PATCH 3/8] Move constants to top file --- src/libstd/sys/windows/path.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index 524f21f889bc2..e70ddce3aa579 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -2,6 +2,9 @@ use crate::ffi::OsStr; use crate::mem; use crate::path::Prefix; +pub const MAIN_SEP_STR: &str = "\\"; +pub const MAIN_SEP: char = '\\'; + fn os_str_as_u8_slice(s: &OsStr) -> &[u8] { unsafe { mem::transmute(s) } } @@ -90,5 +93,3 @@ pub fn parse_prefix(path: &OsStr) -> Option> { } } -pub const MAIN_SEP_STR: &str = "\\"; -pub const MAIN_SEP: char = '\\'; From 90a7d2470a7ae29c529ae2a3684c47839c57d763 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 4/8] Make is_valid_drive_letter function --- src/libstd/sys/windows/path.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index e70ddce3aa579..e2b48a22dcdb7 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -22,6 +22,12 @@ pub fn is_verbatim_sep(b: u8) -> bool { b == b'\\' } +// In most DOS systems, it is not possible to have more than 26 drive letters. +// See . +pub fn is_valid_drive_letter(disk: u8) -> bool { + disk.is_ascii_alphabetic() +} + pub fn parse_prefix(path: &OsStr) -> Option> { use crate::path::Prefix::*; unsafe { @@ -52,7 +58,7 @@ pub fn parse_prefix(path: &OsStr) -> Option> { let idx = path.iter().position(|&b| b == b'\\'); if idx == Some(2) && path[1] == b':' { let c = path[0]; - if c.is_ascii() && (c as char).is_alphabetic() { + if is_valid_drive_letter(c) { // \\?\C:\ path return Some(VerbatimDisk(c.to_ascii_uppercase())); } @@ -77,7 +83,7 @@ pub fn parse_prefix(path: &OsStr) -> Option> { } else if path.get(1) == Some(&b':') { // C: let c = path[0]; - if c.is_ascii() && (c as char).is_alphabetic() { + if is_valid_drive_letter(c) { return Some(Disk(c.to_ascii_uppercase())); } } From 27a966a149f55fe29a25a2fc07e6c8a011ae3dbf Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 5/8] Make use of slice::strip_prefix and slice pattern --- src/libstd/lib.rs | 1 + src/libstd/sys/windows/path.rs | 51 ++++++++++++++++------------------ 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 16a4b38a1ae33..5215db7cdb3ce 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -306,6 +306,7 @@ #![feature(shrink_to)] #![feature(slice_concat_ext)] #![feature(slice_internals)] +#![feature(slice_strip)] #![feature(staged_api)] #![feature(std_internals)] #![feature(stdsimd)] diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index e2b48a22dcdb7..25ecc79abf0f1 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -29,23 +29,20 @@ pub fn is_valid_drive_letter(disk: u8) -> bool { } pub fn parse_prefix(path: &OsStr) -> Option> { - use crate::path::Prefix::*; + use Prefix::{DeviceNS, Disk, Verbatim, VerbatimDisk, VerbatimUNC, UNC}; unsafe { // The unsafety here stems from converting between &OsStr and &[u8] // and back. This is safe to do because (1) we only look at ASCII // contents of the encoding and (2) new &OsStr values are produced // only from ASCII-bounded slices of existing &OsStr values. - let mut path = os_str_as_u8_slice(path); + let path = os_str_as_u8_slice(path); - if path.starts_with(br"\\") { - // \\ - path = &path[2..]; - if path.starts_with(br"?\") { - // \\?\ - path = &path[2..]; - if path.starts_with(br"UNC\") { - // \\?\UNC\server\share - path = &path[4..]; + // \\ + if let Some(path) = path.strip_prefix(br"\\") { + // \\?\ + if let Some(path) = path.strip_prefix(br"?\") { + // \\?\UNC\server\share + if let Some(path) = path.strip_prefix(br"UNC\") { let (server, share) = match parse_two_comps(path, is_verbatim_sep) { Some((server, share)) => { (u8_slice_as_os_str(server), u8_slice_as_os_str(share)) @@ -55,22 +52,23 @@ pub fn parse_prefix(path: &OsStr) -> Option> { return Some(VerbatimUNC(server, share)); } else { // \\?\path - let idx = path.iter().position(|&b| b == b'\\'); - if idx == Some(2) && path[1] == b':' { - let c = path[0]; - if is_valid_drive_letter(c) { - // \\?\C:\ path + match path { + // \\?\C:\path + [c, b':', b'\\', ..] if is_valid_drive_letter(*c) => { return Some(VerbatimDisk(c.to_ascii_uppercase())); } + // \\?\cat_pics + _ => { + let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); + let slice = &path[..idx]; + return Some(Verbatim(u8_slice_as_os_str(slice))); + } } - let slice = &path[..idx.unwrap_or(path.len())]; - return Some(Verbatim(u8_slice_as_os_str(slice))); } - } else if path.starts_with(b".\\") { - // \\.\path - path = &path[2..]; - let pos = path.iter().position(|&b| b == b'\\'); - let slice = &path[..pos.unwrap_or(path.len())]; + } else if let Some(path) = path.strip_prefix(b".\\") { + // \\.\COM42 + let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); + let slice = &path[..idx]; return Some(DeviceNS(u8_slice_as_os_str(slice))); } match parse_two_comps(path, is_sep_byte) { @@ -78,12 +76,11 @@ pub fn parse_prefix(path: &OsStr) -> Option> { // \\server\share return Some(UNC(u8_slice_as_os_str(server), u8_slice_as_os_str(share))); } - _ => (), + _ => {} } - } else if path.get(1) == Some(&b':') { + } else if let [c, b':', ..] = path { // C: - let c = path[0]; - if is_valid_drive_letter(c) { + if is_valid_drive_letter(*c) { return Some(Disk(c.to_ascii_uppercase())); } } From b1d6798899b7830e869d06e1ddb106a75cb61cc8 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 6/8] Rewrite parse_two_comps --- src/libstd/sys/windows/path.rs | 30 ++++++++++++++++++---------- src/libstd/sys/windows/path/tests.rs | 21 +++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 src/libstd/sys/windows/path/tests.rs diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index 25ecc79abf0f1..fa22d3bb7d6f4 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -2,6 +2,9 @@ use crate::ffi::OsStr; use crate::mem; use crate::path::Prefix; +#[cfg(test)] +mod tests; + pub const MAIN_SEP_STR: &str = "\\"; pub const MAIN_SEP: char = '\\'; @@ -43,7 +46,7 @@ pub fn parse_prefix(path: &OsStr) -> Option> { if let Some(path) = path.strip_prefix(br"?\") { // \\?\UNC\server\share if let Some(path) = path.strip_prefix(br"UNC\") { - let (server, share) = match parse_two_comps(path, is_verbatim_sep) { + let (server, share) = match get_first_two_components(path, is_verbatim_sep) { Some((server, share)) => { (u8_slice_as_os_str(server), u8_slice_as_os_str(share)) } @@ -71,7 +74,7 @@ pub fn parse_prefix(path: &OsStr) -> Option> { let slice = &path[..idx]; return Some(DeviceNS(u8_slice_as_os_str(slice))); } - match parse_two_comps(path, is_sep_byte) { + match get_first_two_components(path, is_sep_byte) { Some((server, share)) if !server.is_empty() && !share.is_empty() => { // \\server\share return Some(UNC(u8_slice_as_os_str(server), u8_slice_as_os_str(share))); @@ -86,13 +89,20 @@ pub fn parse_prefix(path: &OsStr) -> Option> { } return None; } - - fn parse_two_comps(mut path: &[u8], f: fn(u8) -> bool) -> Option<(&[u8], &[u8])> { - let first = &path[..path.iter().position(|x| f(*x))?]; - path = &path[(first.len() + 1)..]; - let idx = path.iter().position(|x| f(*x)); - let second = &path[..idx.unwrap_or(path.len())]; - Some((first, second)) - } } +/// Returns the first two path components with predicate `f`. +/// +/// The two components returned will be use by caller +/// to construct `VerbatimUNC` or `UNC` Windows path prefix. +/// +/// Returns [`None`] if there are no separators in path. +fn get_first_two_components(path: &[u8], f: fn(u8) -> bool) -> Option<(&[u8], &[u8])> { + let idx = path.iter().position(|&x| f(x))?; + // Panic safe + // The max `idx+1` is `path.len()` and `path[path.len()..]` is a valid index. + let (first, path) = (&path[..idx], &path[idx + 1..]); + let idx = path.iter().position(|&x| f(x)).unwrap_or(path.len()); + let second = &path[..idx]; + Some((first, second)) +} diff --git a/src/libstd/sys/windows/path/tests.rs b/src/libstd/sys/windows/path/tests.rs new file mode 100644 index 0000000000000..fbac1dc1ca17a --- /dev/null +++ b/src/libstd/sys/windows/path/tests.rs @@ -0,0 +1,21 @@ +use super::*; + +#[test] +fn test_get_first_two_components() { + assert_eq!( + get_first_two_components(br"server\share", is_verbatim_sep), + Some((&b"server"[..], &b"share"[..])), + ); + + assert_eq!( + get_first_two_components(br"server\", is_verbatim_sep), + Some((&b"server"[..], &b""[..])) + ); + + assert_eq!( + get_first_two_components(br"\server\", is_verbatim_sep), + Some((&b""[..], &b"server"[..])) + ); + + assert_eq!(get_first_two_components(br"there are no separators here", is_verbatim_sep), None,); +} From 0281a05f66aada7afc3e5b665b4c48a44abbc517 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 10 Jul 2020 15:59:25 +0000 Subject: [PATCH 7/8] Prefer empty OsStr over unsafe cast from [u8] --- src/libstd/sys/windows/path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index fa22d3bb7d6f4..899a3065038d7 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -50,7 +50,7 @@ pub fn parse_prefix(path: &OsStr) -> Option> { Some((server, share)) => { (u8_slice_as_os_str(server), u8_slice_as_os_str(share)) } - None => (u8_slice_as_os_str(path), u8_slice_as_os_str(&[])), + None => (u8_slice_as_os_str(path), OsStr::new("")), }; return Some(VerbatimUNC(server, share)); } else { From e31898b02409b8ce5db32761d45c78f9f74dbdc6 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 12 Jul 2020 14:46:00 +0000 Subject: [PATCH 8/8] Reduce unsafe scope --- src/libstd/sys/windows/path.rs | 97 +++++++++++++++++----------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/src/libstd/sys/windows/path.rs b/src/libstd/sys/windows/path.rs index 899a3065038d7..dda3ed68cfc95 100644 --- a/src/libstd/sys/windows/path.rs +++ b/src/libstd/sys/windows/path.rs @@ -8,6 +8,10 @@ mod tests; pub const MAIN_SEP_STR: &str = "\\"; pub const MAIN_SEP: char = '\\'; +// The unsafety here stems from converting between `&OsStr` and `&[u8]` +// and back. This is safe to do because (1) we only look at ASCII +// contents of the encoding and (2) new &OsStr values are produced +// only from ASCII-bounded slices of existing &OsStr values. fn os_str_as_u8_slice(s: &OsStr) -> &[u8] { unsafe { mem::transmute(s) } } @@ -33,62 +37,57 @@ pub fn is_valid_drive_letter(disk: u8) -> bool { pub fn parse_prefix(path: &OsStr) -> Option> { use Prefix::{DeviceNS, Disk, Verbatim, VerbatimDisk, VerbatimUNC, UNC}; - unsafe { - // The unsafety here stems from converting between &OsStr and &[u8] - // and back. This is safe to do because (1) we only look at ASCII - // contents of the encoding and (2) new &OsStr values are produced - // only from ASCII-bounded slices of existing &OsStr values. - let path = os_str_as_u8_slice(path); - // \\ - if let Some(path) = path.strip_prefix(br"\\") { - // \\?\ - if let Some(path) = path.strip_prefix(br"?\") { - // \\?\UNC\server\share - if let Some(path) = path.strip_prefix(br"UNC\") { - let (server, share) = match get_first_two_components(path, is_verbatim_sep) { - Some((server, share)) => { - (u8_slice_as_os_str(server), u8_slice_as_os_str(share)) - } - None => (u8_slice_as_os_str(path), OsStr::new("")), - }; - return Some(VerbatimUNC(server, share)); - } else { - // \\?\path - match path { - // \\?\C:\path - [c, b':', b'\\', ..] if is_valid_drive_letter(*c) => { - return Some(VerbatimDisk(c.to_ascii_uppercase())); - } - // \\?\cat_pics - _ => { - let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); - let slice = &path[..idx]; - return Some(Verbatim(u8_slice_as_os_str(slice))); - } + let path = os_str_as_u8_slice(path); + + // \\ + if let Some(path) = path.strip_prefix(br"\\") { + // \\?\ + if let Some(path) = path.strip_prefix(br"?\") { + // \\?\UNC\server\share + if let Some(path) = path.strip_prefix(br"UNC\") { + let (server, share) = match get_first_two_components(path, is_verbatim_sep) { + Some((server, share)) => unsafe { + (u8_slice_as_os_str(server), u8_slice_as_os_str(share)) + }, + None => (unsafe { u8_slice_as_os_str(path) }, OsStr::new("")), + }; + return Some(VerbatimUNC(server, share)); + } else { + // \\?\path + match path { + // \\?\C:\path + [c, b':', b'\\', ..] if is_valid_drive_letter(*c) => { + return Some(VerbatimDisk(c.to_ascii_uppercase())); + } + // \\?\cat_pics + _ => { + let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); + let slice = &path[..idx]; + return Some(Verbatim(unsafe { u8_slice_as_os_str(slice) })); } } - } else if let Some(path) = path.strip_prefix(b".\\") { - // \\.\COM42 - let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); - let slice = &path[..idx]; - return Some(DeviceNS(u8_slice_as_os_str(slice))); - } - match get_first_two_components(path, is_sep_byte) { - Some((server, share)) if !server.is_empty() && !share.is_empty() => { - // \\server\share - return Some(UNC(u8_slice_as_os_str(server), u8_slice_as_os_str(share))); - } - _ => {} } - } else if let [c, b':', ..] = path { - // C: - if is_valid_drive_letter(*c) { - return Some(Disk(c.to_ascii_uppercase())); + } else if let Some(path) = path.strip_prefix(b".\\") { + // \\.\COM42 + let idx = path.iter().position(|&b| b == b'\\').unwrap_or(path.len()); + let slice = &path[..idx]; + return Some(DeviceNS(unsafe { u8_slice_as_os_str(slice) })); + } + match get_first_two_components(path, is_sep_byte) { + Some((server, share)) if !server.is_empty() && !share.is_empty() => { + // \\server\share + return Some(unsafe { UNC(u8_slice_as_os_str(server), u8_slice_as_os_str(share)) }); } + _ => {} + } + } else if let [c, b':', ..] = path { + // C: + if is_valid_drive_letter(*c) { + return Some(Disk(c.to_ascii_uppercase())); } - return None; } + None } /// Returns the first two path components with predicate `f`.