From b733df0fc7942aea205d8fc451d2a4d17d47dfe9 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Fri, 2 May 2014 09:41:34 -0700 Subject: [PATCH 1/3] Allow attributes in std::bitflags::bitflags! The `std::bitflags::bitflags!` macro did not provide support for adding attributes to the generated structure or flags, due to limitations in the parser for macros. This patch works around the parser limitations by requiring a `flags` keyword in the overall `bitflags!` invocation, and a `static` keyword for each flag: bitflags!( #[deriving(Hash)] #[doc="Three flags"] flags Flags: u32 { #[doc="The first flag"] static FlagA = 0x00000001, static FlagB = 0x00000010, static FlagC = 0x00000100 } ) --- src/libstd/bitflags.rs | 65 ++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/src/libstd/bitflags.rs b/src/libstd/bitflags.rs index bf12dd2d94ab1..3c06682eaaf53 100644 --- a/src/libstd/bitflags.rs +++ b/src/libstd/bitflags.rs @@ -17,14 +17,16 @@ //! # Example //! //! ~~~rust -//! bitflags!(Flags: u32 { -//! FlagA = 0x00000001, -//! FlagB = 0x00000010, -//! FlagC = 0x00000100, -//! FlagABC = FlagA.bits -//! | FlagB.bits -//! | FlagC.bits -//! }) +//! bitflags!( +//! flags Flags: u32 { +//! static FlagA = 0x00000001, +//! static FlagB = 0x00000010, +//! static FlagC = 0x00000100, +//! static FlagABC = FlagA.bits +//! | FlagB.bits +//! | FlagC.bits +//! } +//! ) //! //! fn main() { //! let e1 = FlagA | FlagC; @@ -40,10 +42,12 @@ //! ~~~rust //! use std::fmt; //! -//! bitflags!(Flags: u32 { -//! FlagA = 0x00000001, -//! FlagB = 0x00000010 -//! }) +//! bitflags!( +//! flags Flags: u32 { +//! static FlagA = 0x00000001, +//! static FlagB = 0x00000010 +//! } +//! ) //! //! impl Flags { //! pub fn clear(&mut self) { @@ -66,10 +70,16 @@ //! } //! ~~~ //! +//! # Attributes +//! +//! Attributes can be attached to the generated `struct` by placing them +//! before the `flags` keyword. +//! //! # Derived traits //! -//! The `Eq`, `TotalEq`, and `Clone` traits are automatically derived for the -//! `struct` using the `deriving` attribute. +//! The `Eq` and `Clone` traits are automatically derived for the `struct` using +//! the `deriving` attribute. Additional traits can be derived by providing an +//! explicit `deriving` attribute on `flags`. //! //! # Operators //! @@ -91,17 +101,20 @@ //! - `insert`: inserts the specified flags in-place //! - `remove`: removes the specified flags in-place +#![macro_escape] + #[macro_export] macro_rules! bitflags( - ($BitFlags:ident: $T:ty { - $($Flag:ident = $value:expr),+ + ($(#[$attr:meta])* flags $BitFlags:ident: $T:ty { + $($(#[$Flag_attr:meta])* static $Flag:ident = $value:expr),+ }) => ( #[deriving(Eq, TotalEq, Clone)] + $(#[$attr])* pub struct $BitFlags { bits: $T, } - $(pub static $Flag: $BitFlags = $BitFlags { bits: $value };)+ + $($(#[$Flag_attr])* pub static $Flag: $BitFlags = $BitFlags { bits: $value };)+ impl $BitFlags { /// Returns an empty set of flags. @@ -170,14 +183,16 @@ macro_rules! bitflags( mod tests { use ops::{BitOr, BitAnd, Sub}; - bitflags!(Flags: u32 { - FlagA = 0x00000001, - FlagB = 0x00000010, - FlagC = 0x00000100, - FlagABC = FlagA.bits - | FlagB.bits - | FlagC.bits - }) + bitflags!( + flags Flags: u32 { + static FlagA = 0x00000001, + static FlagB = 0x00000010, + static FlagC = 0x00000100, + static FlagABC = FlagA.bits + | FlagB.bits + | FlagC.bits + } + ) #[test] fn test_bits(){ From c00d8fd9a07244a13d1cc7162a80c3d618935ce8 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Fri, 2 May 2014 10:41:07 -0700 Subject: [PATCH 2/3] Add (unsafe) coercion from bits to std::bitflags The intent of `std::bitflags` is to allow building type-safe wrappers around C-style flags APIs. But in addition to construction these flags from the Rust side, we need a way to convert them from the C side. This patch adds a `from_bits` function, which is unsafe since the bits in question may not represent a valid combination of flags. --- src/libstd/bitflags.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/bitflags.rs b/src/libstd/bitflags.rs index 3c06682eaaf53..5737bc772df02 100644 --- a/src/libstd/bitflags.rs +++ b/src/libstd/bitflags.rs @@ -127,6 +127,12 @@ macro_rules! bitflags( self.bits } + /// Convert from underlying bit representation. Unsafe because the + /// bits are not guaranteed to represent valid flags. + pub unsafe fn from_bits(bits: $T) -> $BitFlags { + $BitFlags { bits: bits } + } + /// Returns `true` if no flags are currently stored. pub fn is_empty(&self) -> bool { *self == $BitFlags::empty() From 8d1d7d9b5f3920d70b1edcc258a86106527e83f7 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Fri, 2 May 2014 10:56:26 -0700 Subject: [PATCH 3/3] 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 | 71 ++++++++++++++++++---------------- 6 files changed, 55 insertions(+), 44 deletions(-) diff --git a/src/libnative/io/file_unix.rs b/src/libnative/io/file_unix.rs index 7c19eb5432624..94ca602784112 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 88ba6dcbc7e58..945a1f0d61235 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 665d418ab2ab6..1df68b5cf5c72 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 80c09cc24d60c..999c5ec4e33f2 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 cd304250b1928..6d48b9eee3517 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 d948738ac564a..ff276d0202818 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; - -// 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; - -// 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; - -/// 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; - -/// A mask for all possible permission bits -pub static AllPermissions: FilePermission = 0x1ff; +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, + + static UserRWX = UserRead.bits | UserWrite.bits | UserExecute.bits, + static GroupRWX = GroupRead.bits | GroupWrite.bits | GroupExecute.bits, + static OtherRWX = OtherRead.bits | OtherWrite.bits | OtherExecute.bits, + + #[doc="Permissions for user owned files, equivalent to 0644 on +unix-like systems."] + static UserFile = UserRead.bits | UserWrite.bits | GroupRead.bits | OtherRead.bits, + + #[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, + + #[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 + } +)