Skip to content

Commit

Permalink
queue: check queue size at compile time
Browse files Browse the repository at this point in the history
Emit a compile-time error if a queue size is invalid, instead of a
runtime error. This allows catching mistakes earlier, since the queue
size is always defined at compile time.

Unfortunately this means we must remove the corresponding test, as the
only way to test a compile-time failure (without using external
crates) is via doctests, which do not have access to private items
(which is the case for VirtQueue).

Signed-off-by: Carlos López <[email protected]>
  • Loading branch information
00xc committed May 14, 2024
1 parent 5a213ae commit eea1aef
Showing 1 changed file with 7 additions and 16 deletions.
23 changes: 7 additions & 16 deletions src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use zerocopy::{AsBytes, FromBytes, FromZeroes};
/// Each device can have zero or more virtqueues.
///
/// * `SIZE`: The size of the queue. This is both the number of descriptors, and the number of slots
/// in the available and used rings.
/// in the available and used rings. It must be a power of 2 and fit in a [`u16`].
#[derive(Debug)]
pub struct VirtQueue<H: Hal, const SIZE: usize> {
/// DMA guard
Expand Down Expand Up @@ -61,6 +61,8 @@ pub struct VirtQueue<H: Hal, const SIZE: usize> {
}

impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
const SIZE_OK: () = assert!(SIZE.is_power_of_two() && SIZE <= u16::MAX as usize);

/// Creates a new VirtQueue.
///
/// * `indirect`: Whether to use indirect descriptors. This should be set if the
Expand All @@ -74,13 +76,13 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
indirect: bool,

Check warning on line 76 in src/queue.rs

View workflow job for this annotation

GitHub Actions / build

unused variable: `indirect`

Check warning on line 76 in src/queue.rs

View workflow job for this annotation

GitHub Actions / build

unused variable: `indirect`
event_idx: bool,
) -> Result<Self> {
#[allow(clippy::let_unit_value)]
let _ = Self::SIZE_OK;

if transport.queue_used(idx) {
return Err(Error::AlreadyUsed);
}
if !SIZE.is_power_of_two()
|| SIZE > u16::MAX.into()
|| transport.max_queue_size(idx) < SIZE as u32
{
if transport.max_queue_size(idx) < SIZE as u32 {
return Err(Error::InvalidParam);
}
let size = SIZE as u16;
Expand Down Expand Up @@ -976,17 +978,6 @@ mod tests {
use core::ptr::NonNull;
use std::sync::{Arc, Mutex};

#[test]
fn invalid_queue_size() {
let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4);
let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap();
// Size not a power of 2.
assert_eq!(
VirtQueue::<FakeHal, 3>::new(&mut transport, 0, false, false).unwrap_err(),
Error::InvalidParam
);
}

#[test]
fn queue_too_big() {
let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4);
Expand Down

0 comments on commit eea1aef

Please sign in to comment.