Skip to content

Commit

Permalink
Forbid use of as_slice when unaligned feature is enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
as-com authored and dwrensha committed May 12, 2024
1 parent edac915 commit 1c57b0c
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 3 deletions.
22 changes: 20 additions & 2 deletions capnp/src/primitive_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,14 @@ impl<'a, T: PrimitiveElement> Reader<'a, T> {
}
}

#[cfg(target_endian = "little")]
const _CHECK_SLICE: () = check_slice_supported::<T>();

/// Returns something if the slice is as expected in memory.
///
/// If the target is not little-endian or the `unaligned` feature is enabled, this function
/// will only be available for types that are 1 byte or smaller.
pub fn as_slice(&self) -> Option<&[T]> {
let () = Self::_CHECK_SLICE;

Check warning on line 122 in capnp/src/primitive_list.rs

View workflow job for this annotation

GitHub Actions / lint

this let-binding has unit value

warning: this let-binding has unit value --> capnp/src/primitive_list.rs:122:9 | 122 | let () = Self::_CHECK_SLICE; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `Self::_CHECK_SLICE;` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value = note: `#[warn(clippy::let_unit_value)]` on by default

This comment has been minimized.

Copy link
@dwrensha

dwrensha May 27, 2024

Member

@as-com do you know how to get rid of these clippy warnings? They are showing up in later PRs too:
/~https://github.com/capnproto/capnproto-rust/pull/502/files

This comment has been minimized.

Copy link
@as-com

as-com May 27, 2024

Author Contributor

It’s a false positive that should have been fixed: rust-lang/rust-clippy#10844

This comment has been minimized.

Copy link
@dwrensha

dwrensha May 27, 2024

Member

Ah, maybe this will fix it: #503

if self.reader.get_element_size() == T::element_size() {
let bytes = self.reader.into_raw_bytes();
let bits_per_element = data_bits_per_element(T::element_size()) as usize;
Expand All @@ -137,6 +142,17 @@ impl<'a, T: PrimitiveElement> Reader<'a, T> {
}
}

const fn check_slice_supported<T: PrimitiveElement>() {
if core::mem::size_of::<T>() > 1 {
if !cfg!(target_endian = "little") {
panic!("cannot call as_slice on primitive list of multi-byte elements on non-little endian targets");
}
if cfg!(feature = "unaligned") {
panic!("cannot call as_slice on primitive list of multi-byte elements when unaligned feature is enabled");
}
}
}

impl<'a, T> crate::traits::IntoInternalListReader<'a> for Reader<'a, T>
where
T: PrimitiveElement,
Expand Down Expand Up @@ -178,8 +194,10 @@ where
PrimitiveElement::set(&self.builder, index, value);
}

#[cfg(target_endian = "little")]
const _CHECK_SLICE: () = check_slice_supported::<T>();

pub fn as_slice(&mut self) -> Option<&mut [T]> {
let () = Self::_CHECK_SLICE;

Check warning on line 200 in capnp/src/primitive_list.rs

View workflow job for this annotation

GitHub Actions / lint

this let-binding has unit value

warning: this let-binding has unit value --> capnp/src/primitive_list.rs:200:9 | 200 | let () = Self::_CHECK_SLICE; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `Self::_CHECK_SLICE;` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
if self.builder.get_element_size() == T::element_size() {
let bytes = self.builder.as_raw_bytes();
let bits_per_element = data_bits_per_element(T::element_size()) as usize;
Expand Down
6 changes: 5 additions & 1 deletion capnp/tests/primitive_list_as_slice.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#![cfg(all(target_endian = "little", feature = "alloc"))]
#![cfg(all(
target_endian = "little",
feature = "alloc",
not(feature = "unaligned")
))]

use capnp::{message, primitive_list};

Expand Down
37 changes: 37 additions & 0 deletions capnp/tests/primitive_list_as_slice_unaligned.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#![cfg(feature = "alloc")]

use capnp::{message, primitive_list};

#[test]
pub fn primitive_list_as_slice_small_values() {
let mut msg = message::Builder::new_default();

{
let mut void_list = msg.initn_root::<primitive_list::Builder<()>>(0);
assert_eq!(void_list.as_slice().unwrap().len(), 0);
assert_eq!(void_list.into_reader().as_slice().unwrap().len(), 0);
}

{
let mut void_list = msg.initn_root::<primitive_list::Builder<()>>(5);
assert_eq!(void_list.as_slice().unwrap(), &[(), (), (), (), ()]);
assert_eq!(
void_list.into_reader().as_slice().unwrap(),
&[(), (), (), (), ()]
);
}

{
let mut u8list = msg.initn_root::<primitive_list::Builder<u8>>(0);
assert_eq!(u8list.as_slice().unwrap().len(), 0);
assert_eq!(u8list.into_reader().as_slice().unwrap().len(), 0);
}

{
let mut u8list = msg.initn_root::<primitive_list::Builder<u8>>(3);
u8list.set(0, 0);
u8list.set(1, 1);
u8list.set(2, 2);
assert_eq!(u8list.as_slice().unwrap(), &[0, 1, 2]);
}
}

0 comments on commit 1c57b0c

Please sign in to comment.