Skip to content

Add std::os::set_errno as a counterpart to std::os::errno #17265

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

Closed
wants to merge 1 commit into from
Closed

Add std::os::set_errno as a counterpart to std::os::errno #17265

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 54 additions & 7 deletions src/libstd/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,23 +902,23 @@ pub fn change_dir(p: &Path) -> bool {

#[cfg(unix)]
/// Returns the platform-specific value of errno
pub fn errno() -> int {
fn errno_location() -> *mut c_int {
#[cfg(target_os = "macos")]
#[cfg(target_os = "ios")]
#[cfg(target_os = "freebsd")]
fn errno_location() -> *const c_int {
fn _errno_location() -> *mut c_int {
extern {
fn __error() -> *const c_int;
fn __error() -> *mut c_int;
}
unsafe {
__error()
}
}

#[cfg(target_os = "dragonfly")]
fn errno_location() -> *const c_int {
fn _errno_location() -> *mut c_int {
extern {
fn __dfly_error() -> *const c_int;
fn __dfly_error() -> *mut c_int;
}
unsafe {
__dfly_error()
Expand All @@ -927,15 +927,21 @@ pub fn errno() -> int {

#[cfg(target_os = "linux")]
#[cfg(target_os = "android")]
fn errno_location() -> *const c_int {
fn _errno_location() -> *mut c_int {
extern {
fn __errno_location() -> *const c_int;
fn __errno_location() -> *mut c_int;
}
unsafe {
__errno_location()
}
}

_errno_location()
}

#[cfg(unix)]
/// Returns the platform-specific value of errno
pub fn errno() -> int {
Copy link
Member

Choose a reason for hiding this comment

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

While you're at it, the return types of errno on unix and windows should be the same, not different. Could you change windows to returning an int as well? I think that most consider errno as being a signed integer as opposed to an unsigned one.

Additionally, set_errno for windows would be updated to take an int instead of a uint.

Copy link
Contributor

Choose a reason for hiding this comment

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

Those should be c_int and not int or uint.

Copy link
Member

Choose a reason for hiding this comment

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

No, they should not. We do not expose C types at the std layer where c_int is a platform-specific definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

errno() is mostly (only?!) useful with the values in libc::consts::os::posix88 which are all c_int. If you don't want c_int in the stdlib then maybe the safe version of errno and set_errno should be moved to libc.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to expose errno() in std as an int, but have it be a wrapper around a c_int-based errno() in libc, and then set_errno() can go into libc.

The reasoning is that reporting the errno in error messages is not an uncommon thing, so having read-only access from std makes sense. Setting errno is much rarer, and presumably if you need that you're going to be doing other FFI work as well, so using libc is fine.

Copy link
Member

Choose a reason for hiding this comment

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

@thestinger Right; that's exactly what I said.

Copy link
Contributor

Choose a reason for hiding this comment

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

If Rust's int type is ever smaller than a C compiler's int when compiling for the same target, then either the C compiler is broken, or Rust is. More generally, under that circumstance, I think we'd have more issues than just errno.

Given that, and since errno is not defined to be any specific bit width but instead defined to be the C int type, I think it's much more natural to expose errno from std as int instead of trying to claim it's some particular bit width. Yes, it's not a perfect match, but it seems like a better choice than i32.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that my above comment is predicated on std only exposing errno as a read-only value, and libc exposing errno as a read/write value using c_int, as stated in my previous comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

A C compiler with a 32-bit int type and 16-bit pointers is not "broken". The same applies to one with a 64-bit int type and 32-bit pointers. If a compiler tied int to general purpose register size, it would be 64-bit on x32 while pointers (and thus int and uint in Rust) are 32-bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexcrichton

We do not expose C types at the std layer

You're already doing this by re-exporting CString which takes c_char.

unsafe {
(*errno_location()) as int
}
Expand All @@ -956,6 +962,29 @@ pub fn errno() -> uint {
}
}

#[cfg(unix)]
/// Set the platform-specific value of errno
pub fn set_errno(e: int) {
unsafe {
*errno_location() = e as c_int
}
}

#[cfg(windows)]
/// Set the platform-specific value of errno
pub fn set_errno(e: uint) {
use libc::types::os::arch::extra::DWORD;

#[link_name = "kernel32"]
extern "system" {
fn SetLastError(dwErrCode: DWORD);
}

unsafe {
SetLastError(e as DWORD)
}
}

/// Return the string corresponding to an `errno()` value of `errnum`.
/// # Example
/// ```rust
Expand Down Expand Up @@ -2031,6 +2060,24 @@ mod tests {
assert!(e.contains(&(n, "VALUE".to_string())));
}

#[test]
#[cfg(unix)]
fn test_errno() {
os::set_errno(100);
assert!(os::errno() == 100);
os::set_errno(0);
assert!(os::errno() == 0);
}

#[test]
#[cfg(windows)]
fn test_errno() {
os::set_errno(100);
assert!(os::errno() == 100);
os::set_errno(0);
assert!(os::errno() == 0);
}

#[test]
fn test() {
assert!((!Path::new("test-path").is_absolute()));
Expand Down