Skip to content

Commit 480d6b0

Browse files
authored
use_file: Use AtomicI32 instead of AtomicUsize to avoid conversions (#480)
All the targets that use `use_file` support `AtomicI32`. Using `AtomicI32` eliminates `as` conversions and thus avoids any possibility of truncation or confusion between `FD_UNINIT` and a valid file descriptor. Use -1 as the sentinel value for `FD_UNINIT` since libstd (only) guarantees that -1 is not a valid file descriptor value. Minimize the scope of `FD_UNINIT`.
1 parent 15c6be8 commit 480d6b0

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

src/use_file.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use core::{
77
cell::UnsafeCell,
88
ffi::c_void,
99
mem::MaybeUninit,
10-
sync::atomic::{AtomicUsize, Ordering::Relaxed},
10+
sync::atomic::{AtomicI32, Ordering::Relaxed},
1111
};
1212

1313
/// For all platforms, we use `/dev/urandom` rather than `/dev/random`.
@@ -17,7 +17,6 @@ use core::{
1717
/// - On AIX, /dev/urandom will "provide cryptographically secure output".
1818
/// - On Haiku and QNX Neutrino they are identical.
1919
const FILE_PATH: &[u8] = b"/dev/urandom\0";
20-
const FD_UNINIT: usize = usize::MAX;
2120

2221
// Do not inline this when it is the fallback implementation, but don't mark it
2322
// `#[cold]` because it is hot when it is actually used.
@@ -33,12 +32,21 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
3332
// bytes. The file will be opened exactly once. All subsequent calls will
3433
// return the same file descriptor. This file descriptor is never closed.
3534
fn get_rng_fd() -> Result<libc::c_int, Error> {
36-
static FD: AtomicUsize = AtomicUsize::new(FD_UNINIT);
35+
// std::os::fd::{BorrowedFd, OwnedFd} guarantee that -1 is not a valid file descriptor.
36+
const FD_UNINIT: libc::c_int = -1;
37+
38+
// In theory `libc::c_int` could be something other than `i32`, but for the
39+
// targets we currently support that use `use_file`, it is always `i32`.
40+
// If/when we add support for a target where that isn't the case, we may
41+
// need to use a different atomic type or make other accomodations. The
42+
// compiler will let us know if/when that is the case, because the
43+
// `FD.store(fd)` would fail to compile.
44+
static FD: AtomicI32 = AtomicI32::new(FD_UNINIT);
3745

3846
fn get_fd() -> Option<libc::c_int> {
3947
match FD.load(Relaxed) {
4048
FD_UNINIT => None,
41-
val => Some(val as libc::c_int),
49+
val => Some(val),
4250
}
4351
}
4452

@@ -59,9 +67,8 @@ fn get_rng_fd() -> Result<libc::c_int, Error> {
5967
wait_until_rng_ready()?;
6068

6169
let fd = open_readonly(FILE_PATH)?;
62-
// The fd always fits in a usize without conflicting with FD_UNINIT.
63-
debug_assert!(fd >= 0 && (fd as usize) < FD_UNINIT);
64-
FD.store(fd as usize, Relaxed);
70+
debug_assert!(fd != FD_UNINIT);
71+
FD.store(fd, Relaxed);
6572

6673
Ok(fd)
6774
}

0 commit comments

Comments
 (0)