Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/libstd/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ use sys::os as os_imp;
///
/// * Current directory does not exist.
/// * There are insufficient permissions to access the current directory.
/// * The internal buffer is not large enough to hold the path.
///
/// # Examples
///
Expand Down
22 changes: 22 additions & 0 deletions src/libstd/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind {
}
}

// Some system functions expect the user to pass a appropiately-sized buffer
// without specifying its size. They will only report back whether the buffer
// was large enough or not.
//
// The callback is yielded an uninitialized vector which can be passed to a
// syscall. The closure is expected to return `Err(v)` with the passed vector
// if the space was insufficient and `Ok(r)` if the syscall did not fail due to
// insufficient space.
fn fill_bytes_buf<F, T>(mut f: F) -> io::Result<T>
where F: FnMut(Vec<u8>) -> Result<io::Result<T>,Vec<u8>>,
{
let mut buf = Vec::new();
let mut n = os::BUF_BYTES;
loop {
buf.reserve(n);
match f(buf) {
Err(b) => { buf = b; n *= 2; }
Ok(r) => return r,
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function actually primarily exists on Windows due to the utf-16 to wtf-8 conversion because it allows us to use a u16 stack buffer first and avoid the extra heap allocation unless necessary. On Unix, however, we shouldn't need this function because the same byte buffer filled in by the OS can be converted directly into an OsString.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There aren't any other functions using this on the unix side, so I think it'd be best to just inline the logic into getcwd which may help simpilify it a bit as well.


pub fn cvt<T: One + PartialEq + Neg<Output=T>>(t: T) -> io::Result<T> {
let one: T = T::one();
if t == -one {
Expand Down
43 changes: 23 additions & 20 deletions src/libstd/sys/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,17 @@ use io;
use iter;
use libc::{self, c_int, c_char, c_void};
use mem;
use ptr;
use path::{self, PathBuf};
use ptr;
use slice;
use str;
use sys::c;
use sys::fd;
use vec;

const BUF_BYTES: usize = 2048;
pub const BUF_BYTES: usize = 2048;
const TMPBUF_SZ: usize = 128;

fn bytes2path(b: &[u8]) -> PathBuf {
PathBuf::from(<OsStr as OsStrExt>::from_bytes(b))
}

fn os2path(os: OsString) -> PathBuf {
bytes2path(os.as_bytes())
}

/// Returns the platform-specific value of errno
pub fn errno() -> i32 {
#[cfg(any(target_os = "macos",
Expand Down Expand Up @@ -102,14 +94,22 @@ pub fn error_string(errno: i32) -> String {
}

pub fn getcwd() -> io::Result<PathBuf> {
let mut buf = [0 as c_char; BUF_BYTES];
unsafe {
if libc::getcwd(buf.as_mut_ptr(), buf.len() as libc::size_t).is_null() {
Err(io::Error::last_os_error())
} else {
Ok(bytes2path(CStr::from_ptr(buf.as_ptr()).to_bytes()))
super::fill_bytes_buf(|mut buf| {
unsafe {
let ptr = buf.as_mut_ptr() as *mut libc::c_char;
Ok(if !libc::getcwd(ptr, buf.capacity() as libc::size_t).is_null() {
let len = CStr::from_ptr(buf.as_ptr() as *const libc::c_char).to_bytes().len();
buf.set_len(len);
Ok(PathBuf::from(OsString::from_bytes(buf).unwrap()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the OsString::from_vec function which is the infallible unix-specific version to avoid the unwrap

} else {
let error = io::Error::last_os_error();
if error.raw_os_error().unwrap() == libc::ERANGE {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can also compare against Some(libc::ERANGE) to avoid the unwrap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a program error if raw_os_error returns None, so the unwrap doesn't hurt here IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While unwrap() will always succeed, it's not idiomatic, so let's compare against Some(libc::ERANGE)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abandoning sanity-checking for idomatic code is not something I'd like to do, but since you insist, I'll change it.

return Err(buf);
}
Err(error)
})
}
}
})
}

pub fn chdir(p: &path::Path) -> io::Result<()> {
Expand All @@ -129,11 +129,14 @@ pub struct SplitPaths<'a> {
}

pub fn split_paths<'a>(unparsed: &'a OsStr) -> SplitPaths<'a> {
fn bytes_to_path(b: &[u8]) -> PathBuf {
PathBuf::from(<OsStr as OsStrExt>::from_bytes(b))
}
fn is_colon(b: &u8) -> bool { *b == b':' }
let unparsed = unparsed.as_bytes();
SplitPaths {
iter: unparsed.split(is_colon as fn(&u8) -> bool)
.map(bytes2path as fn(&'a [u8]) -> PathBuf)
.map(bytes_to_path as fn(&'a [u8]) -> PathBuf)
}
}

Expand Down Expand Up @@ -444,7 +447,7 @@ pub fn page_size() -> usize {
}

pub fn temp_dir() -> PathBuf {
getenv("TMPDIR".as_ref()).map(os2path).unwrap_or_else(|| {
getenv("TMPDIR".as_ref()).map(PathBuf::from).unwrap_or_else(|| {
if cfg!(target_os = "android") {
PathBuf::from("/data/local/tmp")
} else {
Expand All @@ -456,7 +459,7 @@ pub fn temp_dir() -> PathBuf {
pub fn home_dir() -> Option<PathBuf> {
return getenv("HOME".as_ref()).or_else(|| unsafe {
fallback()
}).map(os2path);
}).map(PathBuf::from);

#[cfg(any(target_os = "android",
target_os = "ios"))]
Expand Down