From 81de5d9519270e1c9dc3c1ee97067ac7058edefe Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 6 Dec 2018 11:26:11 -0700 Subject: [PATCH 1/7] Remove dependency on shell32.dll #56510 --- src/libstd/build.rs | 1 - src/libstd/sys/windows/args.rs | 244 +++++++++++++++++++++++----- src/libstd/sys/windows/c.rs | 3 - src/test/run-make-fulldeps/tools.mk | 2 +- 4 files changed, 207 insertions(+), 43 deletions(-) diff --git a/src/libstd/build.rs b/src/libstd/build.rs index 9d6e8c4cafdc..8641f28b23fa 100644 --- a/src/libstd/build.rs +++ b/src/libstd/build.rs @@ -68,7 +68,6 @@ fn main() { println!("cargo:rustc-link-lib=advapi32"); println!("cargo:rustc-link-lib=ws2_32"); println!("cargo:rustc-link-lib=userenv"); - println!("cargo:rustc-link-lib=shell32"); } else if target.contains("fuchsia") { println!("cargo:rustc-link-lib=zircon"); println!("cargo:rustc-link-lib=fdio"); diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index 4784633edc14..3eea9b19b6f9 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -11,12 +11,14 @@ #![allow(dead_code)] // runtime init functions not used during testing use os::windows::prelude::*; +use sys::windows::os::current_exe; use sys::c; -use slice; -use ops::Range; use ffi::OsString; -use libc::{c_int, c_void}; use fmt; +use collections::VecDeque; +use core::iter; +use slice; +use path::PathBuf; pub unsafe fn init(_argc: isize, _argv: *const *const u8) { } @@ -24,20 +26,146 @@ pub unsafe fn cleanup() { } pub fn args() -> Args { unsafe { - let mut nArgs: c_int = 0; - let lpCmdLine = c::GetCommandLineW(); - let szArgList = c::CommandLineToArgvW(lpCmdLine, &mut nArgs); - - // szArcList can be NULL if CommandLinToArgvW failed, - // but in that case nArgs is 0 so we won't actually - // try to read a null pointer - Args { cur: szArgList, range: 0..(nArgs as isize) } + let lp_cmd_line = c::GetCommandLineW(); + let parsed_args_list = parse_lp_cmd_line( + lp_cmd_line as *const u16, + || current_exe().map(PathBuf::into_os_string).unwrap_or_else(|_| OsString::new())); + + Args { parsed_args_list: parsed_args_list } } } +/// Implements the Windows command-line argument parsing algorithm, described at +/// . +/// +/// Windows includes a function to do this in shell32.dll, +/// but linking with that DLL causes the process to be registered as a GUI application. +/// GUI applications add a bunch of overhead, even if no windows are drawn. See +/// . +unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_name: F) + -> VecDeque { + const BACKSLASH: u16 = '\\' as u16; + const QUOTE: u16 = '"' as u16; + const TAB: u16 = '\t' as u16; + const SPACE: u16 = ' ' as u16; + let mut in_quotes = false; + let mut was_in_quotes = false; + let mut backslash_count: usize = 0; + let mut ret_val = VecDeque::new(); + let mut cur = Vec::new(); + if lp_cmd_line.is_null() || *lp_cmd_line == 0 { + ret_val.push_back(exe_name()); + return ret_val; + } + let mut i = 0; + // The executable name at the beginning is special. + match *lp_cmd_line { + // The executable name ends at the next quote mark, + // no matter what. + QUOTE => { + loop { + i += 1; + if *lp_cmd_line.offset(i) == 0 { + ret_val.push_back(OsString::from_wide( + slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) + )); + return ret_val; + } + if *lp_cmd_line.offset(i) == QUOTE { + break; + } + } + ret_val.push_back(OsString::from_wide( + slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) + )); + i += 1; + } + // Implement quirk: when they say whitespace here, + // they include the entire ASCII control plane: + // "However, if lpCmdLine starts with any amount of whitespace, CommandLineToArgvW + // will consider the first argument to be an empty string. Excess whitespace at the + // end of lpCmdLine is ignored." + 0...SPACE => { + ret_val.push_back(OsString::new()); + i += 1; + }, + // The executable name ends at the next quote mark, + // no matter what. + _ => { + loop { + i += 1; + if *lp_cmd_line.offset(i) == 0 { + ret_val.push_back(OsString::from_wide( + slice::from_raw_parts(lp_cmd_line, i as usize) + )); + return ret_val; + } + if let 0...SPACE = *lp_cmd_line.offset(i) { + break; + } + } + ret_val.push_back(OsString::from_wide( + slice::from_raw_parts(lp_cmd_line, i as usize) + )); + i += 1; + } + } + loop { + let c = *lp_cmd_line.offset(i); + match c { + // backslash + BACKSLASH => { + backslash_count += 1; + was_in_quotes = false; + }, + QUOTE if backslash_count % 2 == 0 => { + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count / 2)); + backslash_count = 0; + if was_in_quotes { + cur.push('"' as u16); + was_in_quotes = false; + } else { + was_in_quotes = in_quotes; + in_quotes = !in_quotes; + } + } + QUOTE if backslash_count % 2 != 0 => { + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count / 2)); + backslash_count = 0; + was_in_quotes = false; + cur.push(b'"' as u16); + } + SPACE | TAB if !in_quotes => { + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); + if !cur.is_empty() || was_in_quotes { + ret_val.push_back(OsString::from_wide(&cur[..])); + cur.truncate(0); + } + backslash_count = 0; + was_in_quotes = false; + } + 0x00 => { + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); + // include empty quoted strings at the end of the arguments list + if !cur.is_empty() || was_in_quotes || in_quotes { + ret_val.push_back(OsString::from_wide(&cur[..])); + } + break; + } + _ => { + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); + backslash_count = 0; + was_in_quotes = false; + cur.push(c); + } + } + i += 1; + } + ret_val +} + pub struct Args { - range: Range, - cur: *mut *mut u16, + parsed_args_list: VecDeque, } pub struct ArgsInnerDebug<'a> { @@ -48,14 +176,13 @@ impl<'a> fmt::Debug for ArgsInnerDebug<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("[")?; let mut first = true; - for i in self.args.range.clone() { + for i in &self.args.parsed_args_list { if !first { f.write_str(", ")?; } first = false; - // Here we do allocation which could be avoided. - fmt::Debug::fmt(&unsafe { os_string_from_ptr(*self.args.cur.offset(i)) }, f)?; + fmt::Debug::fmt(i, f)?; } f.write_str("]")?; Ok(()) @@ -70,38 +197,79 @@ impl Args { } } -unsafe fn os_string_from_ptr(ptr: *mut u16) -> OsString { - let mut len = 0; - while *ptr.offset(len) != 0 { len += 1; } - - // Push it onto the list. - let ptr = ptr as *const u16; - let buf = slice::from_raw_parts(ptr, len as usize); - OsStringExt::from_wide(buf) -} - impl Iterator for Args { type Item = OsString; - fn next(&mut self) -> Option { - self.range.next().map(|i| unsafe { os_string_from_ptr(*self.cur.offset(i)) } ) + fn next(&mut self) -> Option { self.parsed_args_list.pop_front() } + fn size_hint(&self) -> (usize, Option) { + (self.parsed_args_list.len(), Some(self.parsed_args_list.len())) } - fn size_hint(&self) -> (usize, Option) { self.range.size_hint() } } impl DoubleEndedIterator for Args { - fn next_back(&mut self) -> Option { - self.range.next_back().map(|i| unsafe { os_string_from_ptr(*self.cur.offset(i)) } ) - } + fn next_back(&mut self) -> Option { self.parsed_args_list.pop_back() } } impl ExactSizeIterator for Args { - fn len(&self) -> usize { self.range.len() } + fn len(&self) -> usize { self.parsed_args_list.len() } } -impl Drop for Args { - fn drop(&mut self) { - // self.cur can be null if CommandLineToArgvW previously failed, - // but LocalFree ignores NULL pointers - unsafe { c::LocalFree(self.cur as *mut c_void); } +#[cfg(test)] +mod tests { + use sys::windows::args::*; + use ffi::OsString; + + fn chk(string: &str, parts: &[&str]) { + let mut wide: Vec = OsString::from(string).encode_wide().collect(); + wide.push(0); + let parsed = unsafe { + parse_lp_cmd_line(wide.as_ptr() as *const u16, || OsString::from("TEST.EXE")) + }; + let expected: Vec = parts.iter().map(|k| OsString::from(k)).collect(); + assert_eq!(parsed, expected); + } + + #[test] + fn empty() { + chk("", &["TEST.EXE"]); + chk("\0", &["TEST.EXE"]); + } + + #[test] + fn single_words() { + chk("EXE one_word", &["EXE", "one_word"]); + chk("EXE a", &["EXE", "a"]); + chk("EXE 😅", &["EXE", "😅"]); + chk("EXE 😅🤦", &["EXE", "😅🤦"]); + } + + #[test] + fn official_examples() { + chk(r#"EXE "abc" d e"#, &["EXE", "abc", "d", "e"]); + chk(r#"EXE a\\\b d"e f"g h"#, &["EXE", r#"a\\\b"#, "de fg", "h"]); + chk(r#"EXE a\\\"b c d"#, &["EXE", r#"a\"b"#, "c", "d"]); + chk(r#"EXE a\\\\"b c" d e"#, &["EXE", r#"a\\b c"#, "d", "e"]); + } + + #[test] + fn whitespace_behavior() { + chk(r#" test"#, &["", "test"]); + chk(r#" test"#, &["", "test"]); + chk(r#" test test2"#, &["", "test", "test2"]); + chk(r#" test test2"#, &["", "test", "test2"]); + chk(r#"test test2 "#, &["test", "test2"]); + chk(r#"test test2 "#, &["test", "test2"]); + chk(r#"test "#, &["test"]); + } + + #[test] + fn genius_quotes() { + chk(r#"EXE "" """#, &["EXE", "", ""]); + chk(r#"EXE "" """"#, &["EXE", "", "\""]); + chk( + r#"EXE "this is """all""" in the same argument""#, + &["EXE", "this is \"all\" in the same argument"] + ); + chk(r#"EXE "\u{1}"""#, &["EXE", "\u{1}\""]); + chk(r#"EXE "a"" a"#, &["EXE", "a\"", "a"]); } } diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index c84874a3e880..fa21f459a8a8 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -1035,9 +1035,6 @@ extern "system" { pub fn SetLastError(dwErrCode: DWORD); pub fn GetCommandLineW() -> *mut LPCWSTR; - pub fn LocalFree(ptr: *mut c_void); - pub fn CommandLineToArgvW(lpCmdLine: *mut LPCWSTR, - pNumArgs: *mut c_int) -> *mut *mut u16; pub fn GetTempPathW(nBufferLength: DWORD, lpBuffer: LPCWSTR) -> DWORD; pub fn OpenProcessToken(ProcessHandle: HANDLE, diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk index 3de358fa5000..79399281804f 100644 --- a/src/test/run-make-fulldeps/tools.mk +++ b/src/test/run-make-fulldeps/tools.mk @@ -76,7 +76,7 @@ endif # Extra flags needed to compile a working executable with the standard library ifdef IS_WINDOWS ifdef IS_MSVC - EXTRACFLAGS := ws2_32.lib userenv.lib shell32.lib advapi32.lib + EXTRACFLAGS := ws2_32.lib userenv.lib advapi32.lib else EXTRACFLAGS := -lws2_32 -luserenv endif From 08fbbbd89c2dcdbec353fb0b02ad17015284bd2a Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Dec 2018 13:12:47 -0700 Subject: [PATCH 2/7] Fix nitpicks Switch to vec::IntoIter as our backing double-ended iterator. Fix incorrect comment. --- src/libstd/sys/windows/args.rs | 44 ++++++++++++++++------------------ 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index 3eea9b19b6f9..a0369fffa0c6 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -15,7 +15,7 @@ use sys::windows::os::current_exe; use sys::c; use ffi::OsString; use fmt; -use collections::VecDeque; +use vec; use core::iter; use slice; use path::PathBuf; @@ -43,7 +43,7 @@ pub fn args() -> Args { /// GUI applications add a bunch of overhead, even if no windows are drawn. See /// . unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_name: F) - -> VecDeque { + -> vec::IntoIter { const BACKSLASH: u16 = '\\' as u16; const QUOTE: u16 = '"' as u16; const TAB: u16 = '\t' as u16; @@ -51,11 +51,11 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na let mut in_quotes = false; let mut was_in_quotes = false; let mut backslash_count: usize = 0; - let mut ret_val = VecDeque::new(); + let mut ret_val = Vec::new(); let mut cur = Vec::new(); if lp_cmd_line.is_null() || *lp_cmd_line == 0 { - ret_val.push_back(exe_name()); - return ret_val; + ret_val.push(exe_name()); + return ret_val.into_iter(); } let mut i = 0; // The executable name at the beginning is special. @@ -66,16 +66,16 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na loop { i += 1; if *lp_cmd_line.offset(i) == 0 { - ret_val.push_back(OsString::from_wide( + ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) )); - return ret_val; + return ret_val.into_iter(); } if *lp_cmd_line.offset(i) == QUOTE { break; } } - ret_val.push_back(OsString::from_wide( + ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) )); i += 1; @@ -86,25 +86,25 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na // will consider the first argument to be an empty string. Excess whitespace at the // end of lpCmdLine is ignored." 0...SPACE => { - ret_val.push_back(OsString::new()); + ret_val.push(OsString::new()); i += 1; }, - // The executable name ends at the next quote mark, + // The executable name ends at the next whitespace, // no matter what. _ => { loop { i += 1; if *lp_cmd_line.offset(i) == 0 { - ret_val.push_back(OsString::from_wide( + ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line, i as usize) )); - return ret_val; + return ret_val.into_iter(); } if let 0...SPACE = *lp_cmd_line.offset(i) { break; } } - ret_val.push_back(OsString::from_wide( + ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line, i as usize) )); i += 1; @@ -138,7 +138,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na SPACE | TAB if !in_quotes => { cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); if !cur.is_empty() || was_in_quotes { - ret_val.push_back(OsString::from_wide(&cur[..])); + ret_val.push(OsString::from_wide(&cur[..])); cur.truncate(0); } backslash_count = 0; @@ -148,7 +148,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); // include empty quoted strings at the end of the arguments list if !cur.is_empty() || was_in_quotes || in_quotes { - ret_val.push_back(OsString::from_wide(&cur[..])); + ret_val.push(OsString::from_wide(&cur[..])); } break; } @@ -161,11 +161,11 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na } i += 1; } - ret_val + ret_val.into_iter() } pub struct Args { - parsed_args_list: VecDeque, + parsed_args_list: vec::IntoIter, } pub struct ArgsInnerDebug<'a> { @@ -176,7 +176,7 @@ impl<'a> fmt::Debug for ArgsInnerDebug<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("[")?; let mut first = true; - for i in &self.args.parsed_args_list { + for i in self.args.parsed_args_list.clone() { if !first { f.write_str(", ")?; } @@ -199,14 +199,12 @@ impl Args { impl Iterator for Args { type Item = OsString; - fn next(&mut self) -> Option { self.parsed_args_list.pop_front() } - fn size_hint(&self) -> (usize, Option) { - (self.parsed_args_list.len(), Some(self.parsed_args_list.len())) - } + fn next(&mut self) -> Option { self.parsed_args_list.next() } + fn size_hint(&self) -> (usize, Option) { self.parsed_args_list.size_hint() } } impl DoubleEndedIterator for Args { - fn next_back(&mut self) -> Option { self.parsed_args_list.pop_back() } + fn next_back(&mut self) -> Option { self.parsed_args_list.next_back() } } impl ExactSizeIterator for Args { From 5438465b68fd026f68308793c82a69b241788105 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Dec 2018 14:33:03 -0700 Subject: [PATCH 3/7] Fix poorly-transcribed test case --- src/libstd/sys/windows/args.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index a0369fffa0c6..62ef7a7f0f66 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -182,7 +182,7 @@ impl<'a> fmt::Debug for ArgsInnerDebug<'a> { } first = false; - fmt::Debug::fmt(i, f)?; + fmt::Debug::fmt(&i, f)?; } f.write_str("]")?; Ok(()) @@ -223,7 +223,7 @@ mod tests { parse_lp_cmd_line(wide.as_ptr() as *const u16, || OsString::from("TEST.EXE")) }; let expected: Vec = parts.iter().map(|k| OsString::from(k)).collect(); - assert_eq!(parsed, expected); + assert_eq!(parsed.as_slice(), expected.as_slice()); } #[test] @@ -267,7 +267,7 @@ mod tests { r#"EXE "this is """all""" in the same argument""#, &["EXE", "this is \"all\" in the same argument"] ); - chk(r#"EXE "\u{1}"""#, &["EXE", "\u{1}\""]); + chk(r#"EXE "a"""#, &["EXE", "a\""]); chk(r#"EXE "a"" a"#, &["EXE", "a\"", "a"]); } } From 05a22a72e4574b2105c0a190d7a4bd4a84da16ee Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Dec 2018 15:37:50 -0700 Subject: [PATCH 4/7] Fix nits Add comments explaining how we test this, and use a slice for debugging instead of a clone of the iterator. --- src/libstd/sys/windows/args.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index 62ef7a7f0f66..fd93d389e432 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -35,13 +35,20 @@ pub fn args() -> Args { } } -/// Implements the Windows command-line argument parsing algorithm, described at +/// Implements the Windows command-line argument parsing algorithm. +/// +/// Microsoft's documentation for the Windows CLI argument format can be found at /// . /// /// Windows includes a function to do this in shell32.dll, /// but linking with that DLL causes the process to be registered as a GUI application. /// GUI applications add a bunch of overhead, even if no windows are drawn. See /// . +/// +/// This function was tested for equivalence to the shell32.dll implementation in +/// Windows 10 Pro v1803, using an exhaustive test suite available at +/// or +/// . unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_name: F) -> vec::IntoIter { const BACKSLASH: u16 = '\\' as u16; @@ -176,13 +183,13 @@ impl<'a> fmt::Debug for ArgsInnerDebug<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("[")?; let mut first = true; - for i in self.args.parsed_args_list.clone() { + for i in self.args.parsed_args_list.as_slice() { if !first { f.write_str(", ")?; } first = false; - fmt::Debug::fmt(&i, f)?; + fmt::Debug::fmt(i, f)?; } f.write_str("]")?; Ok(()) From 083585859b2e499998c5b10ff5241f1f81ce6032 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Dec 2018 15:48:32 -0700 Subject: [PATCH 5/7] Fix nit Rewrite it to not use `if let`. --- src/libstd/sys/windows/args.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index fd93d389e432..6dee75214fe2 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -72,13 +72,14 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na QUOTE => { loop { i += 1; - if *lp_cmd_line.offset(i) == 0 { + let c = *lp_cmd_line.offset(i); + if c == 0 { ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) )); return ret_val.into_iter(); } - if *lp_cmd_line.offset(i) == QUOTE { + if c == QUOTE { break; } } @@ -101,13 +102,14 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na _ => { loop { i += 1; - if *lp_cmd_line.offset(i) == 0 { + let c = *lp_cmd_line.offset(i); + if c == 0 { ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line, i as usize) )); return ret_val.into_iter(); } - if let 0...SPACE = *lp_cmd_line.offset(i) { + if c > 0 && c <= SPACE { break; } } From 55420f0f42460e5dc1d724be68496f9ef8557e72 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Dec 2018 18:31:53 -0700 Subject: [PATCH 6/7] Fix iterator nits --- src/libstd/sys/windows/args.rs | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index 6dee75214fe2..935466d927c9 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -31,7 +31,7 @@ pub fn args() -> Args { lp_cmd_line as *const u16, || current_exe().map(PathBuf::into_os_string).unwrap_or_else(|_| OsString::new())); - Args { parsed_args_list: parsed_args_list } + Args { parsed_args_list: parsed_args_list.into_iter() } } } @@ -50,7 +50,7 @@ pub fn args() -> Args { /// or /// . unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_name: F) - -> vec::IntoIter { + -> Vec { const BACKSLASH: u16 = '\\' as u16; const QUOTE: u16 = '"' as u16; const TAB: u16 = '\t' as u16; @@ -62,7 +62,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na let mut cur = Vec::new(); if lp_cmd_line.is_null() || *lp_cmd_line == 0 { ret_val.push(exe_name()); - return ret_val.into_iter(); + return ret_val; } let mut i = 0; // The executable name at the beginning is special. @@ -77,7 +77,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) )); - return ret_val.into_iter(); + return ret_val; } if c == QUOTE { break; @@ -107,7 +107,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na ret_val.push(OsString::from_wide( slice::from_raw_parts(lp_cmd_line, i as usize) )); - return ret_val.into_iter(); + return ret_val; } if c > 0 && c <= SPACE { break; @@ -170,7 +170,7 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na } i += 1; } - ret_val.into_iter() + ret_val } pub struct Args { @@ -183,18 +183,7 @@ pub struct ArgsInnerDebug<'a> { impl<'a> fmt::Debug for ArgsInnerDebug<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("[")?; - let mut first = true; - for i in self.args.parsed_args_list.as_slice() { - if !first { - f.write_str(", ")?; - } - first = false; - - fmt::Debug::fmt(i, f)?; - } - f.write_str("]")?; - Ok(()) + self.args.parsed_args_list.as_slice().fmt(f) } } From 83fe6e4392b2b89005f3056cf56a382887c939d5 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 11 Dec 2018 11:55:02 -0700 Subject: [PATCH 7/7] Use iterators instead of raw offsets in Windows argument parser --- src/libstd/sys/windows/args.rs | 94 ++++++++++++++++------------------ 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/src/libstd/sys/windows/args.rs b/src/libstd/sys/windows/args.rs index 935466d927c9..9e9198e05ee0 100644 --- a/src/libstd/sys/windows/args.rs +++ b/src/libstd/sys/windows/args.rs @@ -55,38 +55,35 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na const QUOTE: u16 = '"' as u16; const TAB: u16 = '\t' as u16; const SPACE: u16 = ' ' as u16; - let mut in_quotes = false; - let mut was_in_quotes = false; - let mut backslash_count: usize = 0; let mut ret_val = Vec::new(); - let mut cur = Vec::new(); if lp_cmd_line.is_null() || *lp_cmd_line == 0 { ret_val.push(exe_name()); return ret_val; } - let mut i = 0; + let mut cmd_line = { + let mut end = 0; + while *lp_cmd_line.offset(end) != 0 { + end += 1; + } + slice::from_raw_parts(lp_cmd_line, end as usize) + }; // The executable name at the beginning is special. - match *lp_cmd_line { + cmd_line = match cmd_line[0] { // The executable name ends at the next quote mark, // no matter what. QUOTE => { - loop { - i += 1; - let c = *lp_cmd_line.offset(i); - if c == 0 { - ret_val.push(OsString::from_wide( - slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) - )); - return ret_val; - } - if c == QUOTE { - break; + let args = { + let mut cut = cmd_line[1..].splitn(2, |&c| c == QUOTE); + if let Some(exe) = cut.next() { + ret_val.push(OsString::from_wide(exe)); } + cut.next() + }; + if let Some(args) = args { + args + } else { + return ret_val; } - ret_val.push(OsString::from_wide( - slice::from_raw_parts(lp_cmd_line.offset(1), i as usize - 1) - )); - i += 1; } // Implement quirk: when they say whitespace here, // they include the entire ASCII control plane: @@ -95,32 +92,30 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na // end of lpCmdLine is ignored." 0...SPACE => { ret_val.push(OsString::new()); - i += 1; + &cmd_line[1..] }, // The executable name ends at the next whitespace, // no matter what. _ => { - loop { - i += 1; - let c = *lp_cmd_line.offset(i); - if c == 0 { - ret_val.push(OsString::from_wide( - slice::from_raw_parts(lp_cmd_line, i as usize) - )); - return ret_val; - } - if c > 0 && c <= SPACE { - break; + let args = { + let mut cut = cmd_line.splitn(2, |&c| c > 0 && c <= SPACE); + if let Some(exe) = cut.next() { + ret_val.push(OsString::from_wide(exe)); } + cut.next() + }; + if let Some(args) = args { + args + } else { + return ret_val; } - ret_val.push(OsString::from_wide( - slice::from_raw_parts(lp_cmd_line, i as usize) - )); - i += 1; } - } - loop { - let c = *lp_cmd_line.offset(i); + }; + let mut cur = Vec::new(); + let mut in_quotes = false; + let mut was_in_quotes = false; + let mut backslash_count: usize = 0; + for &c in cmd_line { match c { // backslash BACKSLASH => { @@ -153,14 +148,6 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na backslash_count = 0; was_in_quotes = false; } - 0x00 => { - cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); - // include empty quoted strings at the end of the arguments list - if !cur.is_empty() || was_in_quotes || in_quotes { - ret_val.push(OsString::from_wide(&cur[..])); - } - break; - } _ => { cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); backslash_count = 0; @@ -168,7 +155,11 @@ unsafe fn parse_lp_cmd_line OsString>(lp_cmd_line: *const u16, exe_na cur.push(c); } } - i += 1; + } + cur.extend(iter::repeat(b'\\' as u16).take(backslash_count)); + // include empty quoted strings at the end of the arguments list + if !cur.is_empty() || was_in_quotes || in_quotes { + ret_val.push(OsString::from_wide(&cur[..])); } ret_val } @@ -267,5 +258,10 @@ mod tests { ); chk(r#"EXE "a"""#, &["EXE", "a\""]); chk(r#"EXE "a"" a"#, &["EXE", "a\"", "a"]); + // quotes cannot be escaped in command names + chk(r#""EXE" check"#, &["EXE", "check"]); + chk(r#""EXE check""#, &["EXE check"]); + chk(r#""EXE """for""" check"#, &["EXE ", r#"for""#, "check"]); + chk(r#""EXE \"for\" check"#, &[r#"EXE \"#, r#"for""#, "check"]); } }