Skip to content

Commit

Permalink
Make from_bits in bitflags! safe; add from_bits_truncate
Browse files Browse the repository at this point in the history
Previously, the `from_bits` function in the `std::bitflags::bitflags`
macro was marked as unsafe, as it did not check that the bits being
converted actually corresponded to flags.

This patch changes the function to check against the full set of
possible flags and return an `Option` which is `None` if a non-flag bit
is set. It also adds a `from_bits_truncate` function which simply zeros
any bits not corresponding to a flag.

This addresses the concern raised in #13897
  • Loading branch information
aturon authored and alexcrichton committed May 15, 2014
1 parent d547de9 commit 912a967
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 16 deletions.
4 changes: 1 addition & 3 deletions src/libnative/io/file_unix.rs
Expand Up @@ -493,9 +493,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
io::FileStat {
size: stat.st_size as u64,
kind: kind,
perm: unsafe {
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
},
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
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),
Expand Down
4 changes: 1 addition & 3 deletions src/libnative/io/file_win32.rs
Expand Up @@ -492,9 +492,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
io::FileStat {
size: stat.st_size as u64,
kind: kind,
perm: unsafe {
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
},
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
created: stat.st_ctime as u64,
modified: stat.st_mtime as u64,
accessed: stat.st_atime as u64,
Expand Down
4 changes: 1 addition & 3 deletions src/librustuv/file.rs
Expand Up @@ -285,9 +285,7 @@ impl FsRequest {
FileStat {
size: stat.st_size as u64,
kind: kind,
perm: unsafe {
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
},
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
created: to_msec(stat.st_birthtim),
modified: to_msec(stat.st_mtim),
accessed: to_msec(stat.st_atim),
Expand Down
37 changes: 30 additions & 7 deletions src/libstd/bitflags.rs
Expand Up @@ -136,10 +136,20 @@ 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 }
/// Convert from underlying bit representation, unless that
/// representation contains bits that do not correspond to a flag.
pub fn from_bits(bits: $T) -> ::std::option::Option<$BitFlags> {
if (bits & !$BitFlags::all().bits()) != 0 {
::std::option::None
} else {
::std::option::Some($BitFlags { bits: bits })
}
}

/// Convert from underlying bit representation, dropping any bits
/// that do not correspond to flags.
pub fn from_bits_truncate(bits: $T) -> $BitFlags {
$BitFlags { bits: bits } & $BitFlags::all()
}

/// Returns `true` if no flags are currently stored.
Expand Down Expand Up @@ -209,6 +219,7 @@ macro_rules! bitflags(

#[cfg(test)]
mod tests {
use option::{Some, None};
use ops::{BitOr, BitAnd, Sub, Not};

bitflags!(
Expand All @@ -231,9 +242,21 @@ mod tests {

#[test]
fn test_from_bits() {
assert!(unsafe { Flags::from_bits(0x00000000) } == Flags::empty());
assert!(unsafe { Flags::from_bits(0x00000001) } == FlagA);
assert!(unsafe { Flags::from_bits(0x00000111) } == FlagABC);
assert!(Flags::from_bits(0) == Some(Flags::empty()));
assert!(Flags::from_bits(0x1) == Some(FlagA));
assert!(Flags::from_bits(0x10) == Some(FlagB));
assert!(Flags::from_bits(0x11) == Some(FlagA | FlagB));
assert!(Flags::from_bits(0x1000) == None);
}

#[test]
fn test_from_bits_truncate() {
assert!(Flags::from_bits_truncate(0) == Flags::empty());
assert!(Flags::from_bits_truncate(0x1) == FlagA);
assert!(Flags::from_bits_truncate(0x10) == FlagB);
assert!(Flags::from_bits_truncate(0x11) == (FlagA | FlagB));
assert!(Flags::from_bits_truncate(0x1000) == Flags::empty());
assert!(Flags::from_bits_truncate(0x1001) == FlagA);
}

#[test]
Expand Down

0 comments on commit 912a967

Please sign in to comment.