Skip to content

Commit 912a967

Browse files
aturonalexcrichton
authored andcommitted
Make from_bits in bitflags! safe; add from_bits_truncate
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
1 parent d547de9 commit 912a967

File tree

4 files changed

+33
-16
lines changed

4 files changed

+33
-16
lines changed

src/libnative/io/file_unix.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
493493
io::FileStat {
494494
size: stat.st_size as u64,
495495
kind: kind,
496-
perm: unsafe {
497-
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
498-
},
496+
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
499497
created: mktime(stat.st_ctime as u64, stat.st_ctime_nsec as u64),
500498
modified: mktime(stat.st_mtime as u64, stat.st_mtime_nsec as u64),
501499
accessed: mktime(stat.st_atime as u64, stat.st_atime_nsec as u64),

src/libnative/io/file_win32.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,7 @@ fn mkstat(stat: &libc::stat) -> io::FileStat {
492492
io::FileStat {
493493
size: stat.st_size as u64,
494494
kind: kind,
495-
perm: unsafe {
496-
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
497-
},
495+
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
498496
created: stat.st_ctime as u64,
499497
modified: stat.st_mtime as u64,
500498
accessed: stat.st_atime as u64,

src/librustuv/file.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,7 @@ impl FsRequest {
285285
FileStat {
286286
size: stat.st_size as u64,
287287
kind: kind,
288-
perm: unsafe {
289-
io::FilePermission::from_bits(stat.st_mode as u32) & io::AllPermissions
290-
},
288+
perm: io::FilePermission::from_bits_truncate(stat.st_mode as u32),
291289
created: to_msec(stat.st_birthtim),
292290
modified: to_msec(stat.st_mtim),
293291
accessed: to_msec(stat.st_atim),

src/libstd/bitflags.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,20 @@ macro_rules! bitflags(
136136
self.bits
137137
}
138138

139-
/// Convert from underlying bit representation. Unsafe because the
140-
/// bits are not guaranteed to represent valid flags.
141-
pub unsafe fn from_bits(bits: $T) -> $BitFlags {
142-
$BitFlags { bits: bits }
139+
/// Convert from underlying bit representation, unless that
140+
/// representation contains bits that do not correspond to a flag.
141+
pub fn from_bits(bits: $T) -> ::std::option::Option<$BitFlags> {
142+
if (bits & !$BitFlags::all().bits()) != 0 {
143+
::std::option::None
144+
} else {
145+
::std::option::Some($BitFlags { bits: bits })
146+
}
147+
}
148+
149+
/// Convert from underlying bit representation, dropping any bits
150+
/// that do not correspond to flags.
151+
pub fn from_bits_truncate(bits: $T) -> $BitFlags {
152+
$BitFlags { bits: bits } & $BitFlags::all()
143153
}
144154

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

210220
#[cfg(test)]
211221
mod tests {
222+
use option::{Some, None};
212223
use ops::{BitOr, BitAnd, Sub, Not};
213224

214225
bitflags!(
@@ -231,9 +242,21 @@ mod tests {
231242

232243
#[test]
233244
fn test_from_bits() {
234-
assert!(unsafe { Flags::from_bits(0x00000000) } == Flags::empty());
235-
assert!(unsafe { Flags::from_bits(0x00000001) } == FlagA);
236-
assert!(unsafe { Flags::from_bits(0x00000111) } == FlagABC);
245+
assert!(Flags::from_bits(0) == Some(Flags::empty()));
246+
assert!(Flags::from_bits(0x1) == Some(FlagA));
247+
assert!(Flags::from_bits(0x10) == Some(FlagB));
248+
assert!(Flags::from_bits(0x11) == Some(FlagA | FlagB));
249+
assert!(Flags::from_bits(0x1000) == None);
250+
}
251+
252+
#[test]
253+
fn test_from_bits_truncate() {
254+
assert!(Flags::from_bits_truncate(0) == Flags::empty());
255+
assert!(Flags::from_bits_truncate(0x1) == FlagA);
256+
assert!(Flags::from_bits_truncate(0x10) == FlagB);
257+
assert!(Flags::from_bits_truncate(0x11) == (FlagA | FlagB));
258+
assert!(Flags::from_bits_truncate(0x1000) == Flags::empty());
259+
assert!(Flags::from_bits_truncate(0x1001) == FlagA);
237260
}
238261

239262
#[test]

0 commit comments

Comments
 (0)