Skip to content

Commit

Permalink
impl: prefix unsafe macros with 'unsafe_'
Browse files Browse the repository at this point in the history 
If these could be defined as functions, then we would have considered
them to be unsound. Since there is no such thing as an "unsafe macro" in
Rust, we adopt a convention that such a macro start with 'unsafe_'.

There are no changes in the safety properties of the implementation in
this commit. This is only about making the code a bit clearer and more
consistent with Rust's safety story.

Kudos to @tspiteri for the idea:
https://internals.rust-lang.org/t/hidden-unsafe-due-to-unintentionally-abusable-macros-and-include/14107/26
  • Loading branch information
@BurntSushi
BurntSushi committed Feb 24, 2021
1 parent 2819796 commit 5d9d038
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 37 deletions.
3 changes: 2 additions & 1 deletion src/io.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,8 @@ impl<W: io::Write + ?Sized> WriteBytesExt for W {}
/// representation.
///
/// This function is wildly unsafe because it permits arbitrary modification of
/// the binary representation of any `Copy` type. Use with care.
/// the binary representation of any `Copy` type. Use with care. It's intended
/// to be called only where `T` is a numeric type.
unsafe fn slice_to_u8_mut<T: Copy>(slice: &mut [T]) -> &mut [u8] {
use std::mem::size_of;

Expand Down
86 changes: 50 additions & 36 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,9 +1557,7 @@ pub trait ByteOrder:
/// LittleEndian::write_f32_into(&numbers_given, &mut bytes);
///
/// let mut numbers_got = [0.0; 4];
/// unsafe {
/// LittleEndian::read_f32_into(&bytes, &mut numbers_got);
/// }
/// LittleEndian::read_f32_into(&bytes, &mut numbers_got);
/// assert_eq!(numbers_given, numbers_got);
/// ```
fn write_f32_into(src: &[f32], dst: &mut [u8]) {
Expand Down Expand Up @@ -1588,9 +1586,7 @@ pub trait ByteOrder:
/// LittleEndian::write_f64_into(&numbers_given, &mut bytes);
///
/// let mut numbers_got = [0.0; 4];
/// unsafe {
/// LittleEndian::read_f64_into(&bytes, &mut numbers_got);
/// }
/// LittleEndian::read_f64_into(&bytes, &mut numbers_got);
/// assert_eq!(numbers_given, numbers_got);
/// ```
fn write_f64_into(src: &[f64], dst: &mut [u8]) {
Expand Down Expand Up @@ -1899,7 +1895,13 @@ pub type NativeEndian = LittleEndian;
#[cfg(target_endian = "big")]
pub type NativeEndian = BigEndian;

macro_rules! write_num_bytes {
/// Copies $size bytes from a number $n to a &mut [u8] $dst. $ty represents the
/// numeric type of $n and $which must be either to_be or to_le, depending on
/// which endianness one wants to use when writing to $dst.
///
/// This macro is only safe to call when $ty is a numeric type and $size ==
/// size_of::<$ty>() and where $dst is a &mut [u8].
macro_rules! unsafe_write_num_bytes {
($ty:ty, $size:expr, $n:expr, $dst:expr, $which:ident) => {{
assert!($size <= $dst.len());
unsafe {
Expand All @@ -1910,7 +1912,13 @@ macro_rules! write_num_bytes {
}};
}

macro_rules! read_slice {
/// Copies a &[u8] $src into a &mut [<numeric>] $dst for the endianness given
/// by $which (must be either to_be or to_le).
///
/// This macro is only safe to call when $src and $dst are &[u8] and &mut [u8],
/// respectively. The macro will panic if $src.len() != $size * $dst.len(),
/// where $size represents the size of the integers encoded in $src.
macro_rules! unsafe_read_slice {
($src:expr, $dst:expr, $size:expr, $which:ident) => {{
assert_eq!($src.len(), $size * $dst.len());

Expand All @@ -1927,10 +1935,16 @@ macro_rules! read_slice {
}};
}

macro_rules! write_slice_native {
($src:expr, $dst:expr, $ty:ty, $size:expr) => {{
assert!($size == ::core::mem::size_of::<$ty>());
assert_eq!($size * $src.len(), $dst.len());
/// Copies a &[$ty] $src into a &mut [u8] $dst, where $ty must be a numeric
/// type. This panics if size_of::<$ty>() * $src.len() != $dst.len().
///
/// This macro is only safe to call when $src is a slice of numeric types and
/// $dst is a &mut [u8] and where $ty represents the type of the integers in
/// $src.
macro_rules! unsafe_write_slice_native {
($src:expr, $dst:expr, $ty:ty) => {{
let size = core::mem::size_of::<$ty>();
assert_eq!(size * $src.len(), $dst.len());

unsafe {
copy_nonoverlapping(
Expand Down Expand Up @@ -2006,22 +2020,22 @@ impl ByteOrder for BigEndian {

#[inline]
fn write_u16(buf: &mut [u8], n: u16) {
write_num_bytes!(u16, 2, n, buf, to_be);
unsafe_write_num_bytes!(u16, 2, n, buf, to_be);
}

#[inline]
fn write_u32(buf: &mut [u8], n: u32) {
write_num_bytes!(u32, 4, n, buf, to_be);
unsafe_write_num_bytes!(u32, 4, n, buf, to_be);
}

#[inline]
fn write_u64(buf: &mut [u8], n: u64) {
write_num_bytes!(u64, 8, n, buf, to_be);
unsafe_write_num_bytes!(u64, 8, n, buf, to_be);
}

#[inline]
fn write_u128(buf: &mut [u8], n: u128) {
write_num_bytes!(u128, 16, n, buf, to_be);
unsafe_write_num_bytes!(u128, 16, n, buf, to_be);
}

#[inline]
Expand Down Expand Up @@ -2054,28 +2068,28 @@ impl ByteOrder for BigEndian {

#[inline]
fn read_u16_into(src: &[u8], dst: &mut [u16]) {
read_slice!(src, dst, 2, to_be);
unsafe_read_slice!(src, dst, 2, to_be);
}

#[inline]
fn read_u32_into(src: &[u8], dst: &mut [u32]) {
read_slice!(src, dst, 4, to_be);
unsafe_read_slice!(src, dst, 4, to_be);
}

#[inline]
fn read_u64_into(src: &[u8], dst: &mut [u64]) {
read_slice!(src, dst, 8, to_be);
unsafe_read_slice!(src, dst, 8, to_be);
}

#[inline]
fn read_u128_into(src: &[u8], dst: &mut [u128]) {
read_slice!(src, dst, 16, to_be);
unsafe_read_slice!(src, dst, 16, to_be);
}

#[inline]
fn write_u16_into(src: &[u16], dst: &mut [u8]) {
if cfg!(target_endian = "big") {
write_slice_native!(src, dst, u16, 2);
unsafe_write_slice_native!(src, dst, u16);
} else {
write_slice!(src, dst, u16, 2, Self::write_u16);
}
Expand All @@ -2084,7 +2098,7 @@ impl ByteOrder for BigEndian {
#[inline]
fn write_u32_into(src: &[u32], dst: &mut [u8]) {
if cfg!(target_endian = "big") {
write_slice_native!(src, dst, u32, 4);
unsafe_write_slice_native!(src, dst, u32);
} else {
write_slice!(src, dst, u32, 4, Self::write_u32);
}
Expand All @@ -2093,7 +2107,7 @@ impl ByteOrder for BigEndian {
#[inline]
fn write_u64_into(src: &[u64], dst: &mut [u8]) {
if cfg!(target_endian = "big") {
write_slice_native!(src, dst, u64, 8);
unsafe_write_slice_native!(src, dst, u64);
} else {
write_slice!(src, dst, u64, 8, Self::write_u64);
}
Expand All @@ -2102,7 +2116,7 @@ impl ByteOrder for BigEndian {
#[inline]
fn write_u128_into(src: &[u128], dst: &mut [u8]) {
if cfg!(target_endian = "big") {
write_slice_native!(src, dst, u128, 16);
unsafe_write_slice_native!(src, dst, u128);
} else {
write_slice!(src, dst, u128, 16, Self::write_u128);
}
Expand Down Expand Up @@ -2214,22 +2228,22 @@ impl ByteOrder for LittleEndian {

#[inline]
fn write_u16(buf: &mut [u8], n: u16) {
write_num_bytes!(u16, 2, n, buf, to_le);
unsafe_write_num_bytes!(u16, 2, n, buf, to_le);
}

#[inline]
fn write_u32(buf: &mut [u8], n: u32) {
write_num_bytes!(u32, 4, n, buf, to_le);
unsafe_write_num_bytes!(u32, 4, n, buf, to_le);
}

#[inline]
fn write_u64(buf: &mut [u8], n: u64) {
write_num_bytes!(u64, 8, n, buf, to_le);
unsafe_write_num_bytes!(u64, 8, n, buf, to_le);
}

#[inline]
fn write_u128(buf: &mut [u8], n: u128) {
write_num_bytes!(u128, 16, n, buf, to_le);
unsafe_write_num_bytes!(u128, 16, n, buf, to_le);
}

#[inline]
Expand All @@ -2254,28 +2268,28 @@ impl ByteOrder for LittleEndian {

#[inline]
fn read_u16_into(src: &[u8], dst: &mut [u16]) {
read_slice!(src, dst, 2, to_le);
unsafe_read_slice!(src, dst, 2, to_le);
}

#[inline]
fn read_u32_into(src: &[u8], dst: &mut [u32]) {
read_slice!(src, dst, 4, to_le);
unsafe_read_slice!(src, dst, 4, to_le);
}

#[inline]
fn read_u64_into(src: &[u8], dst: &mut [u64]) {
read_slice!(src, dst, 8, to_le);
unsafe_read_slice!(src, dst, 8, to_le);
}

#[inline]
fn read_u128_into(src: &[u8], dst: &mut [u128]) {
read_slice!(src, dst, 16, to_le);
unsafe_read_slice!(src, dst, 16, to_le);
}

#[inline]
fn write_u16_into(src: &[u16], dst: &mut [u8]) {
if cfg!(target_endian = "little") {
write_slice_native!(src, dst, u16, 2);
unsafe_write_slice_native!(src, dst, u16);
} else {
write_slice!(src, dst, u16, 2, Self::write_u16);
}
Expand All @@ -2284,7 +2298,7 @@ impl ByteOrder for LittleEndian {
#[inline]
fn write_u32_into(src: &[u32], dst: &mut [u8]) {
if cfg!(target_endian = "little") {
write_slice_native!(src, dst, u32, 4);
unsafe_write_slice_native!(src, dst, u32);
} else {
write_slice!(src, dst, u32, 4, Self::write_u32);
}
Expand All @@ -2293,7 +2307,7 @@ impl ByteOrder for LittleEndian {
#[inline]
fn write_u64_into(src: &[u64], dst: &mut [u8]) {
if cfg!(target_endian = "little") {
write_slice_native!(src, dst, u64, 8);
unsafe_write_slice_native!(src, dst, u64);
} else {
write_slice!(src, dst, u64, 8, Self::write_u64);
}
Expand All @@ -2302,7 +2316,7 @@ impl ByteOrder for LittleEndian {
#[inline]
fn write_u128_into(src: &[u128], dst: &mut [u8]) {
if cfg!(target_endian = "little") {
write_slice_native!(src, dst, u128, 16);
unsafe_write_slice_native!(src, dst, u128);
} else {
write_slice!(src, dst, u128, 16, Self::write_u128);
}
Expand Down

0 comments on commit 5d9d038

Please sign in to comment.