Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace more type casts with non-cast equivalents #437

Merged
merged 5 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1 @@
msrv = "1.56"
msrv = "1.57"
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04, windows-2022]
toolchain: [nightly, beta, stable, 1.56]
toolchain: [nightly, beta, stable, 1.57]
# Only Test macOS on stable to reduce macOS CI jobs
include:
# x86_64-apple-darwin.
Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[package]
name = "getrandom"
version = "0.2.15" # Also update html_root_url in lib.rs when bumping this
edition = "2018"
rust-version = "1.56"
edition = "2021"
rust-version = "1.57" # Sync .clippy.toml, tests.yml, and README.md.
authors = ["The Rand Project Developers"]
license = "MIT OR Apache-2.0"
description = "A small cross-platform library for retrieving random data from system source"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ crate features, WASM support and Custom RNGs see the

## Minimum Supported Rust Version

This crate requires Rust 1.56.0 or later.
This crate requires Rust 1.57.0 or later.

## Platform Support

Expand Down
2 changes: 1 addition & 1 deletion benches/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::mem::MaybeUninit;
fn bench_getrandom<const N: usize>() {
let mut buf = [0u8; N];
getrandom::getrandom(&mut buf).unwrap();
test::black_box(&buf as &[u8]);
test::black_box(&buf[..]);
}

// Call getrandom_uninit on an uninitialized stack buffer
Expand Down
15 changes: 7 additions & 8 deletions src/hermit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
use crate::Error;
use core::{mem::MaybeUninit, num::NonZeroU32};

/// Minimum return value which we should get from syscalls in practice,
/// because Hermit uses positive `i32`s for error codes:
/// https://github.com/hermitcore/libhermit-rs/blob/main/src/errno.rs
const MIN_RET_CODE: isize = -(i32::MAX as isize);

extern "C" {
fn sys_read_entropy(buffer: *mut u8, length: usize, flags: u32) -> isize;
}
Expand All @@ -18,9 +13,13 @@ pub fn getrandom_inner(mut dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
if res > 0 && (res as usize) <= dest.len() {
dest = &mut dest[res as usize..];
} else {
let err = match res {
MIN_RET_CODE..=-1 => NonZeroU32::new(-res as u32).unwrap().into(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that if res == -i32::MIN then -res overflows.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MIN_RET_CODE is -i32::MAX, so -res can not overflow. The current one results in a slightly better codegen and prevents accidental creation of errors in the custom range (e.g. if we got a corrupted return code for some reason), but your code is a bit easier to read and does not contain any unwraps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prevents accidental creation of errors in the custom range

Good point. I will point out a couple of other places where this is an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only relevant for Hermit, since it uses isize for return codes, while other targets use i32.

_ => Error::UNEXPECTED,
let err = if res < 0 {
u32::try_from(res.unsigned_abs())
.ok()
.and_then(NonZeroU32::new)
.map_or(Error::UNEXPECTED, Error::from)
} else {
Error::UNEXPECTED
};
return Err(err);
}
Expand Down
2 changes: 1 addition & 1 deletion src/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl LazyBool {
}

pub fn unsync_init(&self, init: impl FnOnce() -> bool) -> bool {
self.0.unsync_init(|| init() as usize) != 0
self.0.unsync_init(|| usize::from(init())) != 0
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/solid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
} else {
// ITRON error numbers are always negative, so we negate it so that it
// falls in the dedicated OS error range (1..INTERNAL_START).
Err(NonZeroU32::new((-ret) as u32).unwrap().into())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unwrap here gets eliminated, but it still may be worth to use:

NonZeroU32::new(res.unsigned_abs()).unwrap_or(Error::UNEXPECTED)

to quickly prevent any concerns about potential panics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I was actually meaning to remove these unwrap()s in a separate PR, but since I did it in the hermit code, I also change it to do the same thing here.

Err(NonZeroU32::new(ret.unsigned_abs()).map_or(Error::UNEXPECTED, Error::from))
}
}
15 changes: 9 additions & 6 deletions src/util_libc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ cfg_if! {
}

pub fn last_os_error() -> Error {
let errno = unsafe { get_errno() };
if errno > 0 {
Error::from(NonZeroU32::new(errno as u32).unwrap())
} else {
Error::ERRNO_NOT_POSITIVE
}
let errno: libc::c_int = unsafe { get_errno() };

// c_int-to-u32 conversion is lossless for nonnegative values if they are the same size.
const _: () = assert!(core::mem::size_of::<libc::c_int>() == core::mem::size_of::<u32>());

u32::try_from(errno)
.ok()
.and_then(NonZeroU32::new)
.map_or(Error::ERRNO_NOT_POSITIVE, Error::from)
}

// Fill a buffer by repeatedly invoking a system call. The `sys_fill` function:
Expand Down
Loading