Skip to content
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.

Sign up for GitHub

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

Jump to bottom

os::change_dir is outdated #16315

Closed
carllerche opened this issue Aug 7, 2014 · 1 comment
Closed

os::change_dir is outdated #16315

carllerche opened this issue Aug 7, 2014 · 1 comment

Comments

@carllerche
Copy link
Member

It seems to me that os::change_dir is an outdated function. At the very least, it should return an IoResult with information about what caused the failure.
@aturon
Copy link
Member

aturon commented Aug 7, 2014

cc me

@aturon aturon added the A-libs label Aug 7, 2014
@barosl barosl mentioned this issue Nov 11, 2014
Closed
bors added a commit that referenced this issue Nov 18, 2014
@bors
auto merge of #18870 : barosl/rust/os-ioresult, r=alexcrichton
1628b98 
Make old-fashioned functions in the `std::os` module utilize `IoResult`.

I'm still investigating the possibility to include more functions in this pull request. Currently, it covers `getcwd()`, `make_absolute()`, and `change_dir()`. The issues covered by this PR are #16946 and #16315.

A few concerns:

- Should we provide `OsError` in distinction from `IoError`? I'm saying this because in Python, those two are distinguished. One advantage that we keep using `IoError` is that we can make the error cascade down other functions whose return type also includes `IoError`. An example of such functions is `std::io::TempDir::new_in()`, which uses `os::make_absolute()` as well as returns `IoResult<TempDir>`.
- `os::getcwd()` uses an internal buffer whose size is 2048 bytes, which is passed to `getcwd(3)`. There is no upper limitation of file paths in the POSIX standard, but typically it is set to 4096 bytes such as in Linux. Should we increase the buffer size? One thing that makes me nervous is that the size of 2048 bytes already seems a bit excessive, thinking that in normal cases, there would be no filenames that even exceeds 512 bytes.

Fixes #16946.
Fixes #16315.

Any ideas are welcomed. Thanks!
@bors bors closed this as completed in 5de56b3 Nov 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use IoResult in the std::os module barosl/rust
2 participants
@carllerche @aturon