From 8d1d7d9b5f3920d70b1edcc258a86106527e83f7 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Fri, 2 May 2014 10:56:26 -0700 Subject: [PATCH] Change std::io::FilePermission to a typesafe representation This patch changes `std::io::FilePermissions` from an exposed `u32` representation to a typesafe representation (that only allows valid flag combinations) using the `std::bitflags`, thus ensuring a greater degree of safety on the Rust side. Despite the change to the type, most code should continue to work as-is, sincde the new type provides bit operations in the style of C flags. To get at the underlying integer representation, use the `bits` method; to (unsafely) convert to `FilePermissions`, use `FilePermissions::from_bits`. Closes #6085. [breaking-change] --- src/libnative/io/file_unix.rs | 8 +++-- src/libnative/io/file_win32.rs | 6 ++-- src/librustuv/file.rs | 4 ++- src/librustuv/uvio.rs | 4 +-- src/libstd/io/fs.rs | 6 ++-- src/libstd/io/mod.rs | 63 ++++++++++++++++++---------------- 6 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/libnative/io/file_unix.rs b/src/libnative/io/file_unix.rs index 7c19eb54326..94ca6027841 100644 --- a/src/libnative/io/file_unix.rs +++ b/src/libnative/io/file_unix.rs @@ -335,7 +335,7 @@ pub fn open(path: &CString, fm: io::FileMode, fa: io::FileAccess) pub fn mkdir(p: &CString, mode: io::FilePermission) -> IoResult<()> { super::mkerr_libc(retry(|| unsafe { - libc::mkdir(p.with_ref(|p| p), mode as libc::mode_t) + libc::mkdir(p.with_ref(|p| p), mode.bits() as libc::mode_t) })) } @@ -392,7 +392,7 @@ pub fn rename(old: &CString, new: &CString) -> IoResult<()> { pub fn chmod(p: &CString, mode: io::FilePermission) -> IoResult<()> { super::mkerr_libc(retry(|| unsafe { - libc::chmod(p.with_ref(|p| p), mode as libc::mode_t) + libc::chmod(p.with_ref(|p| p), mode.bits() as libc::mode_t) })) } @@ -470,7 +470,9 @@ fn mkstat(stat: &libc::stat, path: &CString) -> io::FileStat { path: Path::new(path), size: stat.st_size as u64, kind: kind, - perm: (stat.st_mode) as io::FilePermission & io::AllPermissions, + perm: unsafe { + io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions + }, created: mktime(stat.st_ctime as u64, stat.st_ctime_nsec as u64), modified: mktime(stat.st_mtime as u64, stat.st_mtime_nsec as u64), accessed: mktime(stat.st_atime as u64, stat.st_atime_nsec as u64), diff --git a/src/libnative/io/file_win32.rs b/src/libnative/io/file_win32.rs index 88ba6dcbc7e..945a1f0d612 100644 --- a/src/libnative/io/file_win32.rs +++ b/src/libnative/io/file_win32.rs @@ -391,7 +391,7 @@ pub fn rename(old: &CString, new: &CString) -> IoResult<()> { pub fn chmod(p: &CString, mode: io::FilePermission) -> IoResult<()> { super::mkerr_libc(as_utf16_p(p.as_str().unwrap(), |p| unsafe { - libc::wchmod(p, mode as libc::c_int) + libc::wchmod(p, mode.bits() as libc::c_int) })) } @@ -470,7 +470,9 @@ fn mkstat(stat: &libc::stat, path: &CString) -> io::FileStat { path: Path::new(path), size: stat.st_size as u64, kind: kind, - perm: (stat.st_mode) as io::FilePermission & io::AllPermissions, + perm: unsafe { + io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions + }, created: stat.st_ctime as u64, modified: stat.st_mtime as u64, accessed: stat.st_atime as u64, diff --git a/src/librustuv/file.rs b/src/librustuv/file.rs index 665d418ab2a..1df68b5cf5c 100644 --- a/src/librustuv/file.rs +++ b/src/librustuv/file.rs @@ -283,7 +283,9 @@ impl FsRequest { path: path, size: stat.st_size as u64, kind: kind, - perm: (stat.st_mode as io::FilePermission) & io::AllPermissions, + perm: unsafe { + io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions + }, created: to_msec(stat.st_birthtim), modified: to_msec(stat.st_mtim), accessed: to_msec(stat.st_atim), diff --git a/src/librustuv/uvio.rs b/src/librustuv/uvio.rs index 80c09cc24d6..999c5ec4e33 100644 --- a/src/librustuv/uvio.rs +++ b/src/librustuv/uvio.rs @@ -224,7 +224,7 @@ impl IoFactory for UvIoFactory { } fn fs_mkdir(&mut self, path: &CString, perm: io::FilePermission) -> Result<(), IoError> { - let r = FsRequest::mkdir(&self.loop_, path, perm as c_int); + let r = FsRequest::mkdir(&self.loop_, path, perm.bits() as c_int); r.map_err(uv_error_to_io_error) } fn fs_rmdir(&mut self, path: &CString) -> Result<(), IoError> { @@ -237,7 +237,7 @@ impl IoFactory for UvIoFactory { } fn fs_chmod(&mut self, path: &CString, perm: io::FilePermission) -> Result<(), IoError> { - let r = FsRequest::chmod(&self.loop_, path, perm as c_int); + let r = FsRequest::chmod(&self.loop_, path, perm.bits() as c_int); r.map_err(uv_error_to_io_error) } fn fs_readdir(&mut self, path: &CString, flags: c_int) diff --git a/src/libstd/io/fs.rs b/src/libstd/io/fs.rs index cd304250b19..6d48b9eee35 100644 --- a/src/libstd/io/fs.rs +++ b/src/libstd/io/fs.rs @@ -1119,7 +1119,7 @@ mod test { check!(File::create(&input)); check!(chmod(&input, io::UserRead)); check!(copy(&input, &out)); - assert!(check!(out.stat()).perm & io::UserWrite == 0); + assert!(!check!(out.stat()).perm.intersects(io::UserWrite)); check!(chmod(&input, io::UserFile)); check!(chmod(&out, io::UserFile)); @@ -1193,9 +1193,9 @@ mod test { let file = tmpdir.join("in.txt"); check!(File::create(&file)); - assert!(check!(stat(&file)).perm & io::UserWrite == io::UserWrite); + assert!(check!(stat(&file)).perm.contains(io::UserWrite)); check!(chmod(&file, io::UserRead)); - assert!(check!(stat(&file)).perm & io::UserWrite == 0); + assert!(!check!(stat(&file)).perm.contains(io::UserWrite)); match chmod(&tmpdir.join("foo"), io::UserRWX) { Ok(..) => fail!("wanted a failure"), diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index d948738ac56..ff276d02028 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -224,6 +224,7 @@ use fmt; use int; use iter::Iterator; use libc; +use ops::{BitOr, BitAnd, Sub}; use os; use option::{Option, Some, None}; use path::Path; @@ -1558,36 +1559,40 @@ pub struct UnstableFileStat { pub gen: u64, } -/// A set of permissions for a file or directory is represented by a set of -/// flags which are or'd together. -pub type FilePermission = u32; +bitflags!( + #[doc="A set of permissions for a file or directory is represented +by a set of flags which are or'd together."] + #[deriving(Hash)] + #[deriving(Show)] + flags FilePermission: u32 { + static UserRead = 0o400, + static UserWrite = 0o200, + static UserExecute = 0o100, + static GroupRead = 0o040, + static GroupWrite = 0o020, + static GroupExecute = 0o010, + static OtherRead = 0o004, + static OtherWrite = 0o002, + static OtherExecute = 0o001, -// Each permission bit -pub static UserRead: FilePermission = 0x100; -pub static UserWrite: FilePermission = 0x080; -pub static UserExecute: FilePermission = 0x040; -pub static GroupRead: FilePermission = 0x020; -pub static GroupWrite: FilePermission = 0x010; -pub static GroupExecute: FilePermission = 0x008; -pub static OtherRead: FilePermission = 0x004; -pub static OtherWrite: FilePermission = 0x002; -pub static OtherExecute: FilePermission = 0x001; + static UserRWX = UserRead.bits | UserWrite.bits | UserExecute.bits, + static GroupRWX = GroupRead.bits | GroupWrite.bits | GroupExecute.bits, + static OtherRWX = OtherRead.bits | OtherWrite.bits | OtherExecute.bits, -// Common combinations of these bits -pub static UserRWX: FilePermission = UserRead | UserWrite | UserExecute; -pub static GroupRWX: FilePermission = GroupRead | GroupWrite | GroupExecute; -pub static OtherRWX: FilePermission = OtherRead | OtherWrite | OtherExecute; + #[doc="Permissions for user owned files, equivalent to 0644 on +unix-like systems."] + static UserFile = UserRead.bits | UserWrite.bits | GroupRead.bits | OtherRead.bits, -/// A set of permissions for user owned files, this is equivalent to 0644 on -/// unix-like systems. -pub static UserFile: FilePermission = UserRead | UserWrite | GroupRead | OtherRead; -/// A set of permissions for user owned directories, this is equivalent to 0755 -/// on unix-like systems. -pub static UserDir: FilePermission = UserRWX | GroupRead | GroupExecute | - OtherRead | OtherExecute; -/// A set of permissions for user owned executables, this is equivalent to 0755 -/// on unix-like systems. -pub static UserExec: FilePermission = UserDir; + #[doc="Permissions for user owned directories, equivalent to 0755 on +unix-like systems."] + static UserDir = UserRWX.bits | GroupRead.bits | GroupExecute.bits | + OtherRead.bits | OtherExecute.bits, -/// A mask for all possible permission bits -pub static AllPermissions: FilePermission = 0x1ff; + #[doc="Permissions for user owned executables, equivalent to 0755 +on unix-like systems."] + static UserExec = UserDir.bits, + + #[doc="All possible permissions enabled."] + static AllPermissions = UserRWX.bits | GroupRWX.bits | OtherRWX.bits + } +)