Skip to content

Make std::env::current_dir work for path names longer than 2048 bytes on non-Windows #26896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 35 additions & 0 deletions src/libstd/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,41 @@ 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 a (pointer, len) pair which can be
// passed to a syscall. The `ptr` is valid for `len` items (i8 in this case).
// The closure is expected to return `None` if the space was insufficient and
// `Some(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(*mut i8, libc::size_t) -> Option<io::Result<T>>,
{
// Start off with a stack buf but then spill over to the heap if we end up
// needing more space.
let mut stack_buf = [0i8; os::BUF_BYTES];
let mut heap_buf = Vec::new();
unsafe {
let mut n = stack_buf.len();
loop {
let buf = if n <= stack_buf.len() {
&mut stack_buf[..]
} else {
heap_buf.set_len(0);
heap_buf.reserve(n);
heap_buf.set_len(n);
&mut heap_buf[..]
};

match f(buf.as_mut_ptr(), n as libc::size_t) {
None => n *= 2,
Some(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
23 changes: 14 additions & 9 deletions src/libstd/sys/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ 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 {
Copy link
Member

Choose a reason for hiding this comment

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

Similar to what I said above, this and the below os2path functions can actually go away now (they're for a time that's long since passed). Could you go ahead and delete them? bytes2path can be replaced with PathBuf::from(OsStr::from_bytes(b)) and os2path can be replaced with PathBuf::from (e.g. 0-copy)

Expand Down Expand Up @@ -102,14 +102,19 @@ 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(|buf, len| {
unsafe {
Some(if !libc::getcwd(buf, len).is_null() {
Ok(bytes2path(CStr::from_ptr(buf).to_bytes()))
} 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 None;
}
Err(error)
})
}
}
})
}

pub fn chdir(p: &path::Path) -> io::Result<()> {
Expand Down