Skip to content

Commit

Permalink
Warn on clippy::cast_possible_truncation
Browse files Browse the repository at this point in the history
Fixes #3197

Signed-off-by: Jonathan Browne <[email protected]>
  • Loading branch information
JBYoshi committed Apr 27, 2023
1 parent 4f576dd commit 99f2ec8
Show file tree
Hide file tree
Showing 67 changed files with 542 additions and 402 deletions.
1 change: 1 addition & 0 deletions .cargo/config
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ rustflags = [
"-Dclippy::ptr_as_ptr",
"-Dclippy::undocumented_unsafe_blocks",
"-Dclippy::cast_lossless",
"-Dclippy::cast_possible_truncation",
"-Dclippy::cast_sign_loss"
]

Expand Down
4 changes: 2 additions & 2 deletions src/devices/src/bus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,13 @@ mod tests {
impl BusDevice for ConstantDevice {
fn read(&mut self, offset: u64, data: &mut [u8]) {
for (i, v) in data.iter_mut().enumerate() {
*v = (offset as u8) + (i as u8);
*v = u8::try_from(offset).unwrap() + u8::try_from(i).unwrap();
}
}

fn write(&mut self, offset: u64, data: &[u8]) {
for (i, v) in data.iter().enumerate() {
assert_eq!(*v, (offset as u8) + (i as u8))
assert_eq!(*v, u8::try_from(offset).unwrap() + u8::try_from(i).unwrap())
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/devices/src/legacy/i8042.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ mod tests {

// Test buffer full.
for i in 0..BUF_SIZE {
i8042.push_byte(i as u8).unwrap();
i8042.push_byte(u8::try_from(i).unwrap()).unwrap();
assert_eq!(i8042.buf_len(), i + 1);
}
assert_eq!(i8042.push_byte(0).unwrap_err(), Error::InternalBufferFull);
Expand Down
4 changes: 2 additions & 2 deletions src/devices/src/legacy/rtc_pl031.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl BusDevice for RTCDevice {
if data.len() == 4 {
// read() function from RTC implementation expects a slice of
// len 4, and we just validated that this is the data lengt
self.read(offset as u16, data.try_into().unwrap())
self.read(u16::try_from(offset).unwrap(), data.try_into().unwrap())
} else {
warn!(
"Found invalid data length while trying to read from the RTC: {}",
Expand All @@ -30,7 +30,7 @@ impl BusDevice for RTCDevice {
if data.len() == 4 {
// write() function from RTC implementation expects a slice of
// len 4, and we just validated that this is the data length
self.write(offset as u16, data.try_into().unwrap())
self.write(u16::try_from(offset).unwrap(), data.try_into().unwrap())
} else {
warn!(
"Found invalid data length while trying to write to the RTC: {}",
Expand Down
4 changes: 2 additions & 2 deletions src/devices/src/legacy/serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,15 @@ impl<W: Write + Send + 'static> BusDevice
self.serial.events().metrics.missed_read_count.inc();
return;
}
data[0] = self.serial.read(offset as u8);
data[0] = self.serial.read(u8::try_from(offset).unwrap());
}

fn write(&mut self, offset: u64, data: &[u8]) {
if data.len() != 1 {
self.serial.events().metrics.missed_write_count.inc();
return;
}
if let Err(err) = self.serial.write(offset as u8, data[0]) {
if let Err(err) = self.serial.write(u8::try_from(offset).unwrap(), data[0]) {
// Counter incremented for any handle_write() error.
error!("Failed the write to serial: {:?}", err);
self.serial.events().metrics.error_count.inc();
Expand Down
58 changes: 47 additions & 11 deletions src/devices/src/virtio/balloon/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,8 @@ impl VirtioDevice for Balloon {
if let Some(end) = offset.checked_add(data.len() as u64) {
// This write can't fail, offset and end are checked against config_len.
data.write_all(
&config_space_bytes[offset as usize..cmp::min(end, config_len) as usize],
&config_space_bytes[usize::try_from(offset).unwrap()
..usize::try_from(cmp::min(end, config_len)).unwrap()],
)
.unwrap();
}
Expand All @@ -559,7 +560,9 @@ impl VirtioDevice for Balloon {
error!("Failed to write config space");
return;
}
config_space_bytes[offset as usize..(offset + data_len) as usize].copy_from_slice(data);
config_space_bytes
[usize::try_from(offset).unwrap()..usize::try_from(offset + data_len).unwrap()]
.copy_from_slice(data);
}

fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
Expand Down Expand Up @@ -689,7 +692,10 @@ pub(crate) mod tests {
| (u64::from(*deflate_on_oom) << VIRTIO_BALLOON_F_DEFLATE_ON_OOM)
| ((u64::from(*stats_interval)) << VIRTIO_BALLOON_F_STATS_VQ);

assert_eq!(balloon.avail_features_by_page(0), features as u32);
assert_eq!(
balloon.avail_features_by_page(0),
(features & 0xffff_ffff) as u32
);
assert_eq!(balloon.avail_features_by_page(1), (features >> 32) as u32);
for i in 2..10 {
assert_eq!(balloon.avail_features_by_page(i), 0u32);
Expand Down Expand Up @@ -780,7 +786,7 @@ pub(crate) mod tests {
&infq,
0,
page_addr,
SIZE_OF_U32 as u32,
u32::try_from(SIZE_OF_U32).unwrap(),
VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE,
);

Expand All @@ -800,7 +806,7 @@ pub(crate) mod tests {
&infq,
1,
page_addr,
SIZE_OF_U32 as u32 + 1,
u32::try_from(SIZE_OF_U32).unwrap() + 1,
VIRTQ_DESC_F_NEXT,
);

Expand Down Expand Up @@ -835,7 +841,13 @@ pub(crate) mod tests {
// to trigger the inflate event queue.
{
mem.write_obj::<u32>(0x1, GuestAddress(page_addr)).unwrap();
set_request(&infq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
set_request(
&infq,
0,
page_addr,
u32::try_from(SIZE_OF_U32).unwrap(),
VIRTQ_DESC_F_NEXT,
);

check_metric_after_block!(
METRICS.balloon.event_fails,
Expand All @@ -856,7 +868,13 @@ pub(crate) mod tests {
// Test the happy case.
{
mem.write_obj::<u32>(0x1, GuestAddress(page_addr)).unwrap();
set_request(&infq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
set_request(
&infq,
0,
page_addr,
u32::try_from(SIZE_OF_U32).unwrap(),
VIRTQ_DESC_F_NEXT,
);

check_metric_after_block!(
METRICS.balloon.inflate_count,
Expand Down Expand Up @@ -884,7 +902,13 @@ pub(crate) mod tests {

// Error case: forgot to trigger deflate event queue.
{
set_request(&defq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
set_request(
&defq,
0,
page_addr,
u32::try_from(SIZE_OF_U32).unwrap(),
VIRTQ_DESC_F_NEXT,
);
check_metric_after_block!(
METRICS.balloon.event_fails,
1,
Expand All @@ -898,7 +922,13 @@ pub(crate) mod tests {

// Happy case.
{
set_request(&defq, 1, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
set_request(
&defq,
1,
page_addr,
u32::try_from(SIZE_OF_U32).unwrap(),
VIRTQ_DESC_F_NEXT,
);
check_metric_after_block!(
METRICS.balloon.deflate_count,
1,
Expand All @@ -920,7 +950,13 @@ pub(crate) mod tests {

// Error case: forgot to trigger stats event queue.
{
set_request(&statsq, 0, 0x1000, SIZE_OF_STAT as u32, VIRTQ_DESC_F_NEXT);
set_request(
&statsq,
0,
0x1000,
u32::try_from(SIZE_OF_STAT).unwrap(),
VIRTQ_DESC_F_NEXT,
);
check_metric_after_block!(
METRICS.balloon.event_fails,
1,
Expand Down Expand Up @@ -956,7 +992,7 @@ pub(crate) mod tests {
&statsq,
0,
page_addr,
2 * SIZE_OF_STAT as u32,
u32::try_from(2 * SIZE_OF_STAT).unwrap(),
VIRTQ_DESC_F_NEXT,
);
check_metric_after_block!(METRICS.balloon.stats_updates_count, 1, {
Expand Down
8 changes: 4 additions & 4 deletions src/devices/src/virtio/balloon/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {

pub fn set_request(queue: &VirtQueue, idx: usize, addr: u64, len: u32, flags: u16) {
// Set the index of the next request.
queue.avail.idx.set((idx + 1) as u16);
queue.avail.idx.set(u16::try_from(idx + 1).unwrap());
// Set the current descriptor table entry index.
queue.avail.ring[idx].set(idx as u16);
queue.avail.ring[idx].set(u16::try_from(idx).unwrap());
// Set the current descriptor table entry.
queue.dtable[idx].set(addr, len, flags, 1);
}

pub fn check_request_completion(queue: &VirtQueue, idx: usize) {
// Check that the next used will be idx + 1.
assert_eq!(queue.used.idx.get(), (idx + 1) as u16);
assert_eq!(usize::from(queue.used.idx.get()), idx + 1);
// Check that the current used is idx.
assert_eq!(queue.used.ring[idx].get().id, idx as u32);
assert_eq!(usize::try_from(queue.used.ring[idx].get().id).unwrap(), idx);
// The length of the completed request is 0.
assert_eq!(queue.used.ring[idx].get().len, 0);
}
4 changes: 2 additions & 2 deletions src/devices/src/virtio/balloon/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub(crate) fn remove_range(
let ret = unsafe {
libc::mmap(
phys_address.cast(),
range_len as usize,
usize::try_from(range_len).unwrap(),
libc::PROT_READ | libc::PROT_WRITE,
libc::MAP_FIXED | libc::MAP_ANONYMOUS | libc::MAP_PRIVATE,
-1,
Expand All @@ -103,7 +103,7 @@ pub(crate) fn remove_range(
// Madvise the region in order to mark it as not used.
// SAFETY: The address and length are known to be valid.
let ret = unsafe {
let range_len = range_len as usize;
let range_len = usize::try_from(range_len).unwrap();
libc::madvise(phys_address.cast(), range_len, libc::MADV_DONTNEED)
};
if ret < 0 {
Expand Down
46 changes: 29 additions & 17 deletions src/devices/src/virtio/block/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ impl DiskProperties {
// The config space is little endian.
let mut config = Vec::with_capacity(CONFIG_SPACE_SIZE);
for i in 0..CONFIG_SPACE_SIZE {
// `as` truncates large integers into smaller integers, here this is intentional
#[allow(clippy::cast_possible_truncation)]
config.push((self.nsectors >> (8 * i)) as u8);
}
config
Expand Down Expand Up @@ -569,8 +571,11 @@ impl VirtioDevice for Block {
}
if let Some(end) = offset.checked_add(data.len() as u64) {
// This write can't fail, offset and end are checked against config_len.
data.write_all(&self.config_space[offset as usize..cmp::min(end, config_len) as usize])
.unwrap();
data.write_all(
&self.config_space[usize::try_from(offset).unwrap()
..usize::try_from(cmp::min(end, config_len)).unwrap()],
)
.unwrap();
}
}

Expand All @@ -583,7 +588,9 @@ impl VirtioDevice for Block {
return;
}

self.config_space[offset as usize..(offset + data_len) as usize].copy_from_slice(data);
self.config_space
[usize::try_from(offset).unwrap()..usize::try_from(offset + data_len).unwrap()]
.copy_from_slice(data);
}

fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
Expand Down Expand Up @@ -666,7 +673,7 @@ pub(crate) mod tests {
let cfg = disk_properties.virtio_block_config_space();
assert_eq!(cfg.len(), CONFIG_SPACE_SIZE);
for (i, byte) in cfg.iter().enumerate() {
assert_eq!(*byte, (num_sectors >> (8 * i)) as u8);
assert_eq!(*byte, ((num_sectors >> (8 * i)) & 0xff) as u8);
}
// Testing `backing_file.virtio_block_disk_image_id()` implies
// duplicating that logic in tests, so skipping it.
Expand All @@ -688,7 +695,10 @@ pub(crate) mod tests {

let features: u64 = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_RING_F_EVENT_IDX);

assert_eq!(block.avail_features_by_page(0), features as u32);
assert_eq!(
block.avail_features_by_page(0),
(features & 0xffff_ffff) as u32
);
assert_eq!(block.avail_features_by_page(1), (features >> 32) as u32);

for i in 2..10 {
Expand Down Expand Up @@ -808,8 +818,8 @@ pub(crate) mod tests {
let status_addr = GuestAddress(vq.dtable[2].addr.get());
assert_eq!(used.len, 1);
assert_eq!(
mem.read_obj::<u8>(status_addr).unwrap(),
VIRTIO_BLK_S_IOERR as u8
u32::from(mem.read_obj::<u8>(status_addr).unwrap()),
VIRTIO_BLK_S_IOERR
);
}

Expand All @@ -833,8 +843,8 @@ pub(crate) mod tests {
let status_addr = GuestAddress(vq.dtable[2].addr.get());
assert_eq!(used.len, 1);
assert_eq!(
mem.read_obj::<u8>(status_addr).unwrap(),
VIRTIO_BLK_S_IOERR as u8
u32::from(mem.read_obj::<u8>(status_addr).unwrap()),
VIRTIO_BLK_S_IOERR
);
}
}
Expand Down Expand Up @@ -1020,8 +1030,8 @@ pub(crate) mod tests {

let status_addr = GuestAddress(vq.dtable[2].addr.get());
assert_eq!(
mem.read_obj::<u8>(status_addr).unwrap(),
VIRTIO_BLK_S_IOERR as u8
u32::from(mem.read_obj::<u8>(status_addr).unwrap()),
VIRTIO_BLK_S_IOERR
);
}

Expand Down Expand Up @@ -1241,8 +1251,8 @@ pub(crate) mod tests {

let status_addr = GuestAddress(vq.dtable[2].addr.get());
assert_eq!(
mem.read_obj::<u8>(status_addr).unwrap(),
VIRTIO_BLK_S_IOERR as u8
u32::from(mem.read_obj::<u8>(status_addr).unwrap()),
VIRTIO_BLK_S_IOERR
);
}
}
Expand Down Expand Up @@ -1402,10 +1412,12 @@ pub(crate) mod tests {
let status_addr = vq.dtable[used.id as usize + 1].addr.get();
assert_eq!(used.len, 1);
assert_eq!(
vq.memory()
.read_obj::<u8>(GuestAddress(status_addr))
.unwrap(),
VIRTIO_BLK_S_OK as u8
u32::from(
vq.memory()
.read_obj::<u8>(GuestAddress(status_addr))
.unwrap()
),
VIRTIO_BLK_S_OK
);
}
}
Expand Down
15 changes: 6 additions & 9 deletions src/devices/src/virtio/block/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,14 @@ pub mod tests {
fn check_dirty_mem(mem: &GuestMemoryMmap, addr: GuestAddress, len: u32) {
let bitmap = mem.find_region(addr).unwrap().bitmap().as_ref().unwrap();
for offset in addr.0..addr.0 + u64::from(len) {
assert!(bitmap.dirty_at(offset as usize));
assert!(bitmap.dirty_at(usize::try_from(offset).unwrap()));
}
}

fn check_clean_mem(mem: &GuestMemoryMmap, addr: GuestAddress, len: u32) {
let bitmap = mem.find_region(addr).unwrap().bitmap().as_ref().unwrap();
for offset in addr.0..addr.0 + u64::from(len) {
assert!(!bitmap.dirty_at(offset as usize));
assert!(!bitmap.dirty_at(usize::try_from(offset).unwrap()));
}
}

Expand Down Expand Up @@ -287,16 +287,13 @@ pub mod tests {
.to_vec();

// Partial write
let partial_len = 50;
let addr = GuestAddress(MEM_LEN as u64 - partial_len);
let partial_len = 50u32;
let addr = GuestAddress(u64::try_from(MEM_LEN - partial_len as usize).unwrap());
mem.write(&data, addr).unwrap();
assert_sync_execution!(
engine.write(0, &mem, addr, FILE_LEN, ()),
partial_len as u32
);
assert_sync_execution!(engine.write(0, &mem, addr, FILE_LEN, ()), partial_len);
// Partial read
let mem = create_mem();
assert_sync_execution!(engine.read(0, &mem, addr, FILE_LEN, ()), partial_len as u32);
assert_sync_execution!(engine.read(0, &mem, addr, FILE_LEN, ()), partial_len);
// Check data
let mut buf = vec![0u8; partial_len as usize];
mem.read_slice(&mut buf, addr).unwrap();
Expand Down
Loading

0 comments on commit 99f2ec8

Please sign in to comment.