From d42cc130f9eaaa4b35944854c3ba34ae98d6361e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 2 Jun 2014 14:51:58 -0700 Subject: [PATCH] std: Remove the as_utf16_p functions These functions are all much better expressed via RAII using the to_utf16() method on strings. This refactoring also takes this opportunity to properly handle when filenames aren't valid unicode when passed through to the windows I/O layer by properly returning I/O errors. All previous users of the `as_utf16_p` or `as_utf16_mut_p` functions will need to convert their code to using `foo.to_utf16().append_one(0)` to get a null-terminated utf16 string. [breaking-change] --- src/libnative/io/c_win32.rs | 12 ++- src/libnative/io/file_win32.rs | 114 +++++++++++++------------ src/libnative/io/pipe_win32.rs | 132 ++++++++++++++--------------- src/libnative/io/process.rs | 45 +++++----- src/librustdoc/flock.rs | 8 +- src/libstd/os.rs | 46 ++++------ src/libstd/unstable/dynamic_lib.rs | 10 +-- 7 files changed, 178 insertions(+), 189 deletions(-) diff --git a/src/libnative/io/c_win32.rs b/src/libnative/io/c_win32.rs index 93b3ec7ccef..1ec25933eec 100644 --- a/src/libnative/io/c_win32.rs +++ b/src/libnative/io/c_win32.rs @@ -69,7 +69,6 @@ extern "system" { pub mod compat { use std::intrinsics::{atomic_store_relaxed, transmute}; use libc::types::os::arch::extra::{LPCWSTR, HMODULE, LPCSTR, LPVOID}; - use std::os::win32::as_utf16_p; extern "system" { fn GetModuleHandleW(lpModuleName: LPCWSTR) -> HMODULE; @@ -80,12 +79,11 @@ pub mod compat { // This way, calling a function in this compatibility layer (after it's loaded) shouldn't // be any slower than a regular DLL call. unsafe fn store_func(ptr: *mut T, module: &str, symbol: &str, fallback: T) { - as_utf16_p(module, |module| { - symbol.with_c_str(|symbol| { - let handle = GetModuleHandleW(module); - let func: Option = transmute(GetProcAddress(handle, symbol)); - atomic_store_relaxed(ptr, func.unwrap_or(fallback)) - }) + let module = module.to_utf16().append_one(0); + symbol.with_c_str(|symbol| { + let handle = GetModuleHandleW(module.as_ptr()); + let func: Option = transmute(GetProcAddress(handle, symbol)); + atomic_store_relaxed(ptr, func.unwrap_or(fallback)) }) } diff --git a/src/libnative/io/file_win32.rs b/src/libnative/io/file_win32.rs index 2a5b78e5506..0e6c5c744f4 100644 --- a/src/libnative/io/file_win32.rs +++ b/src/libnative/io/file_win32.rs @@ -15,7 +15,7 @@ use libc::{c_int, c_void}; use libc; use std::c_str::CString; use std::mem; -use std::os::win32::{as_utf16_p, fill_utf16_buf_and_decode}; +use std::os::win32::fill_utf16_buf_and_decode; use std::ptr; use std::rt::rtio; use std::rt::rtio::IoResult; @@ -253,6 +253,17 @@ impl Drop for Inner { } } +pub fn to_utf16(s: &CString) -> IoResult> { + match s.as_str() { + Some(s) => Ok(s.to_utf16().append_one(0)), + None => Err(IoError { + code: libc::ERROR_INVALID_NAME as uint, + extra: 0, + detail: Some("valid unicode input required".to_str()), + }) + } +} + pub fn open(path: &CString, fm: rtio::FileMode, fa: rtio::FileAccess) -> IoResult { // Flags passed to open_osfhandle @@ -299,15 +310,16 @@ pub fn open(path: &CString, fm: rtio::FileMode, fa: rtio::FileAccess) // Compat with unix, this allows opening directories (see libuv) dwFlagsAndAttributes |= libc::FILE_FLAG_BACKUP_SEMANTICS; - let handle = as_utf16_p(path.as_str().unwrap(), |buf| unsafe { - libc::CreateFileW(buf, + let path = try!(to_utf16(path)); + let handle = unsafe { + libc::CreateFileW(path.as_ptr(), dwDesiredAccess, dwShareMode, ptr::mut_null(), dwCreationDisposition, dwFlagsAndAttributes, ptr::mut_null()) - }); + }; if handle == libc::INVALID_HANDLE_VALUE as libc::HANDLE { Err(super::last_error()) } else { @@ -324,11 +336,10 @@ pub fn open(path: &CString, fm: rtio::FileMode, fa: rtio::FileAccess) } pub fn mkdir(p: &CString, _mode: uint) -> IoResult<()> { + let p = try!(to_utf16(p)); super::mkerr_winbool(unsafe { // FIXME: turn mode into something useful? #2623 - as_utf16_p(p.as_str().unwrap(), |buf| { - libc::CreateDirectoryW(buf, ptr::mut_null()) - }) + libc::CreateDirectoryW(p.as_ptr(), ptr::mut_null()) }) } @@ -351,9 +362,11 @@ pub fn readdir(p: &CString) -> IoResult> { let star = Path::new(unsafe { CString::new(p.with_ref(|p| p), false) }).join("*"); - as_utf16_p(star.as_str().unwrap(), |path_ptr| unsafe { + let path = try!(to_utf16(&star.to_c_str())); + + unsafe { let wfd_ptr = malloc_raw(rust_list_dir_wfd_size() as uint); - let find_handle = libc::FindFirstFileW(path_ptr, wfd_ptr as libc::HANDLE); + let find_handle = libc::FindFirstFileW(path.as_ptr(), wfd_ptr as libc::HANDLE); if find_handle as libc::c_int != libc::INVALID_HANDLE_VALUE { let mut paths = vec!(); let mut more_files = 1 as libc::c_int; @@ -377,37 +390,35 @@ pub fn readdir(p: &CString) -> IoResult> { } else { Err(super::last_error()) } - }) + } } pub fn unlink(p: &CString) -> IoResult<()> { + let p = try!(to_utf16(p)); super::mkerr_winbool(unsafe { - as_utf16_p(p.as_str().unwrap(), |buf| { - libc::DeleteFileW(buf) - }) + libc::DeleteFileW(p.as_ptr()) }) } pub fn rename(old: &CString, new: &CString) -> IoResult<()> { + let old = try!(to_utf16(old)); + let new = try!(to_utf16(new)); super::mkerr_winbool(unsafe { - as_utf16_p(old.as_str().unwrap(), |old| { - as_utf16_p(new.as_str().unwrap(), |new| { - libc::MoveFileExW(old, new, libc::MOVEFILE_REPLACE_EXISTING) - }) - }) + libc::MoveFileExW(old.as_ptr(), new.as_ptr(), + libc::MOVEFILE_REPLACE_EXISTING) }) } pub fn chmod(p: &CString, mode: uint) -> IoResult<()> { - super::mkerr_libc(as_utf16_p(p.as_str().unwrap(), |p| unsafe { - libc::wchmod(p, mode as libc::c_int) - })) + let p = try!(to_utf16(p)); + super::mkerr_libc(unsafe { + libc::wchmod(p.as_ptr(), mode.bits() as libc::c_int) + }) } pub fn rmdir(p: &CString) -> IoResult<()> { - super::mkerr_libc(as_utf16_p(p.as_str().unwrap(), |p| unsafe { - libc::wrmdir(p) - })) + let p = try!(to_utf16(p)); + super::mkerr_libc(unsafe { libc::wrmdir(p.as_ptr()) }) } pub fn chown(_p: &CString, _uid: int, _gid: int) -> IoResult<()> { @@ -418,16 +429,15 @@ pub fn chown(_p: &CString, _uid: int, _gid: int) -> IoResult<()> { pub fn readlink(p: &CString) -> IoResult { // FIXME: I have a feeling that this reads intermediate symlinks as well. use io::c::compat::kernel32::GetFinalPathNameByHandleW; + let p = try!(to_utf16(p)); let handle = unsafe { - as_utf16_p(p.as_str().unwrap(), |p| { - libc::CreateFileW(p, - libc::GENERIC_READ, - libc::FILE_SHARE_READ, - ptr::mut_null(), - libc::OPEN_EXISTING, - libc::FILE_ATTRIBUTE_NORMAL, - ptr::mut_null()) - }) + libc::CreateFileW(p.as_ptr(), + libc::GENERIC_READ, + libc::FILE_SHARE_READ, + ptr::mut_null(), + libc::OPEN_EXISTING, + libc::FILE_ATTRIBUTE_NORMAL, + ptr::mut_null()) }; if handle as int == libc::INVALID_HANDLE_VALUE as int { return Err(super::last_error()) @@ -453,19 +463,19 @@ pub fn readlink(p: &CString) -> IoResult { pub fn symlink(src: &CString, dst: &CString) -> IoResult<()> { use io::c::compat::kernel32::CreateSymbolicLinkW; - super::mkerr_winbool(as_utf16_p(src.as_str().unwrap(), |src| { - as_utf16_p(dst.as_str().unwrap(), |dst| { - unsafe { CreateSymbolicLinkW(dst, src, 0) } - }) as libc::BOOL - })) + let src = try!(to_utf16(src)); + let dst = try!(to_utf16(dst)); + super::mkerr_winbool(unsafe { + CreateSymbolicLinkW(dst.as_ptr(), src.as_ptr(), 0) as libc::BOOL + }) } pub fn link(src: &CString, dst: &CString) -> IoResult<()> { - super::mkerr_winbool(as_utf16_p(src.as_str().unwrap(), |src| { - as_utf16_p(dst.as_str().unwrap(), |dst| { - unsafe { libc::CreateHardLinkW(dst, src, ptr::mut_null()) } - }) - })) + let src = try!(to_utf16(src)); + let dst = try!(to_utf16(dst)); + super::mkerr_winbool(unsafe { + libc::CreateHardLinkW(dst.as_ptr(), src.as_ptr(), ptr::mut_null()) + }) } fn mkstat(stat: &libc::stat) -> rtio::FileStat { @@ -491,12 +501,11 @@ fn mkstat(stat: &libc::stat) -> rtio::FileStat { pub fn stat(p: &CString) -> IoResult { let mut stat: libc::stat = unsafe { mem::zeroed() }; - as_utf16_p(p.as_str().unwrap(), |up| { - match unsafe { libc::wstat(up, &mut stat) } { - 0 => Ok(mkstat(&stat)), - _ => Err(super::last_error()), - } - }) + let p = try!(to_utf16(p)); + match unsafe { libc::wstat(p.as_ptr(), &mut stat) } { + 0 => Ok(mkstat(&stat)), + _ => Err(super::last_error()), + } } pub fn lstat(_p: &CString) -> IoResult { @@ -509,7 +518,8 @@ pub fn utime(p: &CString, atime: u64, mtime: u64) -> IoResult<()> { actime: (atime / 1000) as libc::time64_t, modtime: (mtime / 1000) as libc::time64_t, }; - super::mkerr_libc(as_utf16_p(p.as_str().unwrap(), |p| unsafe { - libc::wutime(p, &buf) - })) + let p = try!(to_utf16(p)); + super::mkerr_libc(unsafe { + libc::wutime(p.as_ptr(), &buf) + }) } diff --git a/src/libnative/io/pipe_win32.rs b/src/libnative/io/pipe_win32.rs index a5694436b97..45b12aa7007 100644 --- a/src/libnative/io/pipe_win32.rs +++ b/src/libnative/io/pipe_win32.rs @@ -88,7 +88,6 @@ use alloc::arc::Arc; use libc; use std::c_str::CString; use std::mem; -use std::os::win32::as_utf16_p; use std::os; use std::ptr; use std::rt::rtio; @@ -98,6 +97,7 @@ use std::rt::mutex; use super::c; use super::util; +use super::file::to_utf16; struct Event(libc::HANDLE); @@ -261,67 +261,66 @@ impl UnixStream { } pub fn connect(addr: &CString, timeout: Option) -> IoResult { - as_utf16_p(addr.as_str().unwrap(), |p| { - let start = ::io::timer::now(); - loop { - match UnixStream::try_connect(p) { - Some(handle) => { - let inner = Inner::new(handle); - let mut mode = libc::PIPE_TYPE_BYTE | - libc::PIPE_READMODE_BYTE | - libc::PIPE_WAIT; - let ret = unsafe { - libc::SetNamedPipeHandleState(inner.handle, - &mut mode, - ptr::mut_null(), - ptr::mut_null()) - }; - return if ret == 0 { - Err(super::last_error()) - } else { - Ok(UnixStream { - inner: Arc::new(inner), - read: None, - write: None, - read_deadline: 0, - write_deadline: 0, - }) - } + let addr = try!(to_utf16(addr)); + let start = ::io::timer::now(); + loop { + match UnixStream::try_connect(addr.as_ptr()) { + Some(handle) => { + let inner = Inner::new(handle); + let mut mode = libc::PIPE_TYPE_BYTE | + libc::PIPE_READMODE_BYTE | + libc::PIPE_WAIT; + let ret = unsafe { + libc::SetNamedPipeHandleState(inner.handle, + &mut mode, + ptr::mut_null(), + ptr::mut_null()) + }; + return if ret == 0 { + Err(super::last_error()) + } else { + Ok(UnixStream { + inner: Arc::new(inner), + read: None, + write: None, + read_deadline: 0, + write_deadline: 0, + }) + } + } + None => {} + } + + // On windows, if you fail to connect, you may need to call the + // `WaitNamedPipe` function, and this is indicated with an error + // code of ERROR_PIPE_BUSY. + let code = unsafe { libc::GetLastError() }; + if code as int != libc::ERROR_PIPE_BUSY as int { + return Err(super::last_error()) + } + + match timeout { + Some(timeout) => { + let now = ::io::timer::now(); + let timed_out = (now - start) >= timeout || unsafe { + let ms = (timeout - (now - start)) as libc::DWORD; + libc::WaitNamedPipeW(addr.as_ptr(), ms) == 0 + }; + if timed_out { + return Err(util::timeout("connect timed out")) } - None => {} } - // On windows, if you fail to connect, you may need to call the - // `WaitNamedPipe` function, and this is indicated with an error - // code of ERROR_PIPE_BUSY. - let code = unsafe { libc::GetLastError() }; - if code as int != libc::ERROR_PIPE_BUSY as int { - return Err(super::last_error()) - } - - match timeout { - Some(timeout) => { - let now = ::io::timer::now(); - let timed_out = (now - start) >= timeout || unsafe { - let ms = (timeout - (now - start)) as libc::DWORD; - libc::WaitNamedPipeW(p, ms) == 0 - }; - if timed_out { - return Err(util::timeout("connect timed out")) - } - } - - // An example I found on microsoft's website used 20 - // seconds, libuv uses 30 seconds, hence we make the - // obvious choice of waiting for 25 seconds. - None => { - if unsafe { libc::WaitNamedPipeW(p, 25000) } == 0 { - return Err(super::last_error()) - } + // An example I found on microsoft's website used 20 + // seconds, libuv uses 30 seconds, hence we make the + // obvious choice of waiting for 25 seconds. + None => { + if unsafe { libc::WaitNamedPipeW(addr.as_ptr(), 25000) } == 0 { + return Err(super::last_error()) } } } - }) + } } fn handle(&self) -> libc::HANDLE { self.inner.handle } @@ -564,14 +563,13 @@ impl UnixListener { // Although we technically don't need the pipe until much later, we // create the initial handle up front to test the validity of the name // and such. - as_utf16_p(addr.as_str().unwrap(), |p| { - let ret = unsafe { pipe(p, true) }; - if ret == libc::INVALID_HANDLE_VALUE as libc::HANDLE { - Err(super::last_error()) - } else { - Ok(UnixListener { handle: ret, name: addr.clone() }) - } - }) + let addr_v = try!(to_utf16(addr)); + let ret = unsafe { pipe(addr_v.as_ptr(), true) }; + if ret == libc::INVALID_HANDLE_VALUE as libc::HANDLE { + Err(super::last_error()) + } else { + Ok(UnixListener { handle: ret, name: addr.clone() }) + } } pub fn native_listen(self) -> IoResult { @@ -639,6 +637,8 @@ impl UnixAcceptor { // using the original server pipe. let handle = self.listener.handle; + let name = try!(to_utf16(&self.listener.name)); + // Once we've got a "server handle", we need to wait for a client to // connect. The ConnectNamedPipe function will block this thread until // someone on the other end connects. This function can "fail" if a @@ -678,9 +678,7 @@ impl UnixAcceptor { // Now that we've got a connected client to our handle, we need to // create a second server pipe. If this fails, we disconnect the // connected client and return an error (see comments above). - let new_handle = as_utf16_p(self.listener.name.as_str().unwrap(), |p| { - unsafe { pipe(p, false) } - }); + let new_handle = unsafe { pipe(name.as_ptr(), false) }; if new_handle == libc::INVALID_HANDLE_VALUE as libc::HANDLE { let ret = Err(super::last_error()); // If our disconnection fails, then there's not really a whole lot diff --git a/src/libnative/io/process.rs b/src/libnative/io/process.rs index 2c2b7cec1de..97b227ae1d8 100644 --- a/src/libnative/io/process.rs +++ b/src/libnative/io/process.rs @@ -296,16 +296,15 @@ fn spawn_process_os(cfg: ProcessConfig, lpSecurityDescriptor: ptr::mut_null(), bInheritHandle: 1, }; - *slot = os::win32::as_utf16_p("NUL", |filename| { - libc::CreateFileW(filename, - access, - libc::FILE_SHARE_READ | - libc::FILE_SHARE_WRITE, - &mut sa, - libc::OPEN_EXISTING, - 0, - ptr::mut_null()) - }); + let filename = "NUL".to_utf16().append_one(0); + *slot = libc::CreateFileW(filename.as_ptr(), + access, + libc::FILE_SHARE_READ | + libc::FILE_SHARE_WRITE, + &mut sa, + libc::OPEN_EXISTING, + 0, + ptr::mut_null()); if *slot == INVALID_HANDLE_VALUE as libc::HANDLE { return Err(super::last_error()) } @@ -338,18 +337,17 @@ fn spawn_process_os(cfg: ProcessConfig, with_envp(cfg.env, |envp| { with_dirp(cfg.cwd, |dirp| { - os::win32::as_mut_utf16_p(cmd_str.as_slice(), |cmdp| { - let created = CreateProcessW(ptr::null(), - cmdp, - ptr::mut_null(), - ptr::mut_null(), - TRUE, - flags, envp, dirp, - &mut si, &mut pi); - if created == FALSE { - create_err = Some(super::last_error()); - } - }) + let mut cmd_str = cmd_str.to_utf16().append_one(0); + let created = CreateProcessW(ptr::null(), + cmd_str.as_mut_ptr(), + ptr::mut_null(), + ptr::mut_null(), + TRUE, + flags, envp, dirp, + &mut si, &mut pi); + if created == FALSE { + create_err = Some(super::last_error()); + } }) }); @@ -740,7 +738,8 @@ fn with_dirp(d: Option<&CString>, cb: |*u16| -> T) -> T { Some(dir) => { let dir_str = dir.as_str() .expect("expected workingdirectory to be utf-8 encoded"); - os::win32::as_utf16_p(dir_str, cb) + let dir_str = dir_str.to_utf16().append_one(0); + cb(dir_str.as_ptr()) }, None => cb(ptr::null()) } diff --git a/src/librustdoc/flock.rs b/src/librustdoc/flock.rs index fea6748660b..8ad10a686e6 100644 --- a/src/librustdoc/flock.rs +++ b/src/librustdoc/flock.rs @@ -135,7 +135,6 @@ mod imp { mod imp { use libc; use std::mem; - use std::os::win32::as_utf16_p; use std::os; use std::ptr; @@ -162,8 +161,9 @@ mod imp { impl Lock { pub fn new(p: &Path) -> Lock { - let handle = as_utf16_p(p.as_str().unwrap(), |p| unsafe { - libc::CreateFileW(p, + let p_16 = p.as_str().unwrap().to_utf16().append_one(0); + let handle = unsafe { + libc::CreateFileW(p_16.as_ptr(), libc::FILE_GENERIC_READ | libc::FILE_GENERIC_WRITE, libc::FILE_SHARE_READ | @@ -173,7 +173,7 @@ mod imp { libc::CREATE_ALWAYS, libc::FILE_ATTRIBUTE_NORMAL, ptr::mut_null()) - }); + }; if handle as uint == libc::INVALID_HANDLE_VALUE as uint { fail!("create file error: {}", os::last_os_error()); } diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 90df18106f0..fa882e7d016 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -133,7 +133,7 @@ pub mod win32 { use os::TMPBUF_SZ; use slice::{MutableVector, ImmutableVector}; use string::String; - use str::{StrSlice, StrAllocating}; + use str::StrSlice; use str; use vec::Vec; @@ -171,17 +171,6 @@ pub mod win32 { return res; } } - - pub fn as_utf16_p(s: &str, f: |*u16| -> T) -> T { - as_mut_utf16_p(s, |t| { f(t as *u16) }) - } - - pub fn as_mut_utf16_p(s: &str, f: |*mut u16| -> T) -> T { - let mut t = s.to_utf16(); - // Null terminate before passing on. - t.push(0u16); - f(t.as_mut_ptr()) - } } /* @@ -356,11 +345,10 @@ pub fn getenv_as_bytes(n: &str) -> Option> { pub fn getenv(n: &str) -> Option { unsafe { with_env_lock(|| { - use os::win32::{as_utf16_p, fill_utf16_buf_and_decode}; - as_utf16_p(n, |u| { - fill_utf16_buf_and_decode(|buf, sz| { - libc::GetEnvironmentVariableW(u, buf, sz) - }) + use os::win32::{fill_utf16_buf_and_decode}; + let n = n.to_utf16().append_one(0); + fill_utf16_buf_and_decode(|buf, sz| { + libc::GetEnvironmentVariableW(n.as_ptr(), buf, sz) }) }) } @@ -398,14 +386,11 @@ pub fn setenv(n: &str, v: &str) { /// Sets the environment variable `n` to the value `v` for the currently running /// process pub fn setenv(n: &str, v: &str) { + let n = n.to_utf16().append_one(0); + let v = v.to_utf16().append_one(0); unsafe { with_env_lock(|| { - use os::win32::as_utf16_p; - as_utf16_p(n, |nbuf| { - as_utf16_p(v, |vbuf| { - libc::SetEnvironmentVariableW(nbuf, vbuf); - }) - }) + libc::SetEnvironmentVariableW(n.as_ptr(), v.as_ptr()); }) } } @@ -428,12 +413,10 @@ pub fn unsetenv(n: &str) { } #[cfg(windows)] fn _unsetenv(n: &str) { + let n = n.to_utf16().append_one(0); unsafe { with_env_lock(|| { - use os::win32::as_utf16_p; - as_utf16_p(n, |nbuf| { - libc::SetEnvironmentVariableW(nbuf, ptr::null()); - }) + libc::SetEnvironmentVariableW(n.as_ptr(), ptr::null()); }) } } @@ -732,11 +715,12 @@ pub fn change_dir(p: &Path) -> bool { #[cfg(windows)] fn chdir(p: &Path) -> bool { + let p = match p.as_str() { + Some(s) => s.to_utf16().append_one(0), + None => return false, + }; unsafe { - use os::win32::as_utf16_p; - return as_utf16_p(p.as_str().unwrap(), |buf| { - libc::SetCurrentDirectoryW(buf) != (0 as libc::BOOL) - }); + libc::SetCurrentDirectoryW(p.as_ptr()) != (0 as libc::BOOL) } } diff --git a/src/libstd/unstable/dynamic_lib.rs b/src/libstd/unstable/dynamic_lib.rs index c05cdc85cc5..6980c5cc0c2 100644 --- a/src/libstd/unstable/dynamic_lib.rs +++ b/src/libstd/unstable/dynamic_lib.rs @@ -272,21 +272,21 @@ pub mod dl { #[cfg(target_os = "win32")] pub mod dl { + use c_str::ToCStr; use libc; use os; use ptr; use result::{Ok, Err, Result}; - use string::String; + use str::StrAllocating; use str; - use c_str::ToCStr; + use string::String; pub unsafe fn open_external(filename: T) -> *u8 { // Windows expects Unicode data let filename_cstr = filename.to_c_str(); let filename_str = str::from_utf8(filename_cstr.as_bytes_no_nul()).unwrap(); - os::win32::as_utf16_p(filename_str, |raw_name| { - LoadLibraryW(raw_name as *libc::c_void) as *u8 - }) + let filename_str = filename_str.to_utf16().append_one(0); + LoadLibraryW(filename_str.as_ptr() as *libc::c_void) as *u8 } pub unsafe fn open_internal() -> *u8 {