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

Increase MSRV to 1.38 and use ptr.cast::<T>() whenever practical to clarify casts. #425

Merged
merged 3 commits into from
May 26, 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.36"
msrv = "1.38"
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.36]
toolchain: [nightly, beta, stable, 1.38]
# Only Test macOS on stable to reduce macOS CI jobs
include:
# x86_64-apple-darwin.
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Breaking Changes
- Update MSRV to 1.38 [#425]

[#425]: https://github.com/rust-random/getrandom/pull/425

## [0.2.15] - 2024-05-06
### Added
- Apple visionOS support [#410]
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.36.0 or later.
This crate requires Rust 1.38.0 or later.

## Platform Support

Expand Down
2 changes: 1 addition & 1 deletion src/apple-other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let ret = unsafe { CCRandomGenerateBytes(dest.as_mut_ptr() as *mut c_void, dest.len()) };
let ret = unsafe { CCRandomGenerateBytes(dest.as_mut_ptr().cast::<c_void>(), dest.len()) };
Copy link
Member

Choose a reason for hiding this comment

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

I think we can rely on type inference and use just .cast().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intentionally avoid using type inference so that we ensure we're casting to the expected type. That is one of the goals of this change.

Copy link
Member

@newpavlov newpavlov May 20, 2024

Choose a reason for hiding this comment

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

I don't think it helps in any practical way in this case. In the first place, pointer types do not matter on the ABI level, so we are doing casting to just satisfy conventional signature of external functions. Also, IIRC std also uses a lot of pointer casts which rely on type inference in cases like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem to help in our code because our code does seem to be correct. But this style of explicitly naming the type has helped prevent bugs in practice in other code, and it makes the code easier to audit (from my own experience).

Copy link
Member

Choose a reason for hiding this comment

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

@josephlr WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I personally like having the type explicit in the cast. It makes it easier for me to understand what's going on. It's not a huge deal, but it is nice to confirm that nothing unexpected is happening with the cast.

// kCCSuccess (from CommonCryptoError.h) is always zero.
if ret != 0 {
Err(Error::IOS_SEC_RANDOM)
Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Error {
cfg_if! {
if #[cfg(unix)] {
fn os_err(errno: i32, buf: &mut [u8]) -> Option<&str> {
let buf_ptr = buf.as_mut_ptr() as *mut libc::c_char;
let buf_ptr = buf.as_mut_ptr().cast::<libc::c_char>();
if unsafe { libc::strerror_r(errno, buf_ptr, buf.len()) } != 0 {
return None;
}
Expand Down
2 changes: 1 addition & 1 deletion src/fuchsia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
unsafe { zx_cprng_draw(dest.as_mut_ptr() as *mut u8, dest.len()) }
unsafe { zx_cprng_draw(dest.as_mut_ptr().cast::<u8>(), dest.len()) }
Ok(())
}
4 changes: 2 additions & 2 deletions src/getentropy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
//!
//! For these targets, we use getentropy(2) because getrandom(2) doesn't exist.
use crate::{util_libc::last_os_error, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
for chunk in dest.chunks_mut(256) {
let ret = unsafe { libc::getentropy(chunk.as_mut_ptr() as *mut libc::c_void, chunk.len()) };
let ret = unsafe { libc::getentropy(chunk.as_mut_ptr().cast::<c_void>(), chunk.len()) };
if ret != 0 {
return Err(last_os_error());
}
Expand Down
4 changes: 2 additions & 2 deletions src/getrandom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
//! nothing. On illumos, the default pool is used to implement getentropy(2),
//! so we assume it is acceptable here.
use crate::{util_libc::sys_fill_exact, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
sys_fill_exact(dest, |buf| unsafe {
libc::getrandom(buf.as_mut_ptr() as *mut libc::c_void, buf.len(), 0)
libc::getrandom(buf.as_mut_ptr().cast::<c_void>(), buf.len(), 0)
})
}
2 changes: 1 addition & 1 deletion src/hermit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extern "C" {

pub fn getrandom_inner(mut dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
while !dest.is_empty() {
let res = unsafe { sys_read_entropy(dest.as_mut_ptr() as *mut u8, dest.len(), 0) };
let res = unsafe { sys_read_entropy(dest.as_mut_ptr().cast::<u8>(), dest.len(), 0) };
// Positive `isize`s can be safely casted to `usize`
if res > 0 && (res as usize) <= dest.len() {
dest = &mut dest[res as usize..];
Expand Down
4 changes: 2 additions & 2 deletions src/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error>
// have a notion of "uninitialized memory", this is purely
// a Rust/C/C++ concept.
let res = n.random_fill_sync(unsafe {
Uint8Array::view_mut_raw(chunk.as_mut_ptr() as *mut u8, chunk.len())
Uint8Array::view_mut_raw(chunk.as_mut_ptr().cast::<u8>(), chunk.len())
});
if res.is_err() {
return Err(Error::NODE_RANDOM_FILL_SYNC);
Expand All @@ -60,7 +60,7 @@ pub(crate) fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error>
}

// SAFETY: `sub_buf`'s length is the same length as `chunk`
unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr() as *mut u8) };
unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr().cast::<u8>()) };
}
}
};
Expand Down
6 changes: 3 additions & 3 deletions src/netbsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn kern_arnd(buf: &mut [MaybeUninit<u8>]) -> libc::ssize_t {
libc::sysctl(
MIB.as_ptr(),
MIB.len() as libc::c_uint,
buf.as_mut_ptr() as *mut _,
buf.as_mut_ptr().cast::<c_void>(),
&mut len,
ptr::null(),
0,
Expand All @@ -29,7 +29,7 @@ static GETRANDOM: LazyPtr = LazyPtr::new();

fn dlsym_getrandom() -> *mut c_void {
static NAME: &[u8] = b"getrandom\0";
let name_ptr = NAME.as_ptr() as *const libc::c_char;
let name_ptr = NAME.as_ptr().cast::<libc::c_char>();
unsafe { libc::dlsym(libc::RTLD_DEFAULT, name_ptr) }
}

Expand All @@ -38,7 +38,7 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
if !fptr.is_null() {
let func: GetRandomFn = unsafe { core::mem::transmute(fptr) };
return sys_fill_exact(dest, |buf| unsafe {
func(buf.as_mut_ptr() as *mut u8, buf.len(), 0)
func(buf.as_mut_ptr().cast::<u8>(), buf.len(), 0)
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/solaris.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
//! https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2
//! which also explains why this crate should not use getentropy(2).
use crate::{util_libc::last_os_error, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

const MAX_BYTES: usize = 1024;

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
for chunk in dest.chunks_mut(MAX_BYTES) {
let ptr = chunk.as_mut_ptr() as *mut libc::c_void;
let ptr = chunk.as_mut_ptr().cast::<c_void>();
let ret = unsafe { libc::getrandom(ptr, chunk.len(), libc::GRND_RANDOM) };
// In case the man page has a typo, we also check for negative ret.
if ret <= 0 {
Expand Down
2 changes: 1 addition & 1 deletion src/solid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let ret = unsafe { SOLID_RNG_SampleRandomBytes(dest.as_mut_ptr() as *mut u8, dest.len()) };
let ret = unsafe { SOLID_RNG_SampleRandomBytes(dest.as_mut_ptr().cast::<u8>(), dest.len()) };
if ret >= 0 {
Ok(())
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/use_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
};
use core::{
cell::UnsafeCell,
ffi::c_void,
mem::MaybeUninit,
sync::atomic::{AtomicUsize, Ordering::Relaxed},
};
Expand All @@ -21,7 +22,7 @@ const FD_UNINIT: usize = usize::max_value();
pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let fd = get_rng_fd()?;
sys_fill_exact(dest, |buf| unsafe {
libc::read(fd, buf.as_mut_ptr() as *mut libc::c_void, buf.len())
libc::read(fd, buf.as_mut_ptr().cast::<c_void>(), buf.len())
})
}

Expand Down
7 changes: 5 additions & 2 deletions src/util_libc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ pub fn sys_fill_exact(
pub unsafe fn open_readonly(path: &str) -> Result<libc::c_int, Error> {
debug_assert_eq!(path.as_bytes().last(), Some(&0));
loop {
let fd = libc::open(path.as_ptr() as *const _, libc::O_RDONLY | libc::O_CLOEXEC);
let fd = libc::open(
path.as_ptr().cast::<libc::c_char>(),
briansmith marked this conversation as resolved.
Show resolved Hide resolved
libc::O_RDONLY | libc::O_CLOEXEC,
);
if fd >= 0 {
return Ok(fd);
}
Expand All @@ -92,7 +95,7 @@ pub fn getrandom_syscall(buf: &mut [MaybeUninit<u8>]) -> libc::ssize_t {
unsafe {
libc::syscall(
libc::SYS_getrandom,
buf.as_mut_ptr() as *mut libc::c_void,
buf.as_mut_ptr().cast::<core::ffi::c_void>(),
buf.len(),
0,
) as libc::ssize_t
Expand Down
2 changes: 1 addition & 1 deletion src/vxworks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {

// Prevent overflow of i32
for chunk in dest.chunks_mut(i32::max_value() as usize) {
let ret = unsafe { libc::randABytes(chunk.as_mut_ptr() as *mut u8, chunk.len() as i32) };
let ret = unsafe { libc::randABytes(chunk.as_mut_ptr().cast::<u8>(), chunk.len() as i32) };
if ret != 0 {
return Err(last_os_error());
}
Expand Down
2 changes: 1 addition & 1 deletion src/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::{
use wasi::random_get;

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
unsafe { random_get(dest.as_mut_ptr() as *mut u8, dest.len()) }.map_err(|e| {
unsafe { random_get(dest.as_mut_ptr().cast::<u8>(), dest.len()) }.map_err(|e| {
// The WASI errno will always be non-zero, but we check just in case.
match NonZeroU16::new(e.raw()) {
Some(r) => Error::from(NonZeroU32::from(r)),
Expand Down
7 changes: 4 additions & 3 deletions src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let ret = unsafe {
BCryptGenRandom(
ptr::null_mut(),
chunk.as_mut_ptr() as *mut u8,
chunk.as_mut_ptr().cast::<u8>(),
chunk.len() as u32,
BCRYPT_USE_SYSTEM_PREFERRED_RNG,
)
Expand All @@ -39,8 +39,9 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// Failed. Try RtlGenRandom as a fallback.
#[cfg(not(target_vendor = "uwp"))]
{
let ret =
unsafe { RtlGenRandom(chunk.as_mut_ptr() as *mut c_void, chunk.len() as u32) };
let ret = unsafe {
RtlGenRandom(chunk.as_mut_ptr().cast::<c_void>(), chunk.len() as u32)
};
if ret != 0 {
continue;
}
Expand Down
Loading