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 May 27, 2023
1 parent 69bd798 commit 0138434
Show file tree
Hide file tree
Showing 65 changed files with 551 additions and 406 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
36 changes: 18 additions & 18 deletions src/dumbo/src/pdu/arp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ impl<'a, T: NetworkBytes> EthIPv4ArpFrame<'a, T> {
}

// We could theoretically skip the hlen and plen checks, since they are kinda implicit.
if maybe.hlen() != MAC_ADDR_LEN as u8 {
if usize::from(maybe.hlen()) != MAC_ADDR_LEN {
return Err(Error::HLen);
}

if maybe.plen() != IPV4_ADDR_LEN as u8 {
if usize::from(maybe.plen()) != IPV4_ADDR_LEN {
return Err(Error::PLen);
}

Expand Down Expand Up @@ -231,8 +231,8 @@ impl<'a, T: NetworkBytesMut> EthIPv4ArpFrame<'a, T> {
buf,
HTYPE_ETHERNET,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REQUEST,
sha,
spa,
Expand All @@ -255,8 +255,8 @@ impl<'a, T: NetworkBytesMut> EthIPv4ArpFrame<'a, T> {
buf,
HTYPE_ETHERNET,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REPLY,
sha,
spa,
Expand Down Expand Up @@ -384,8 +384,8 @@ mod tests {
// This is a bit redundant given the following tests, but assert away!
assert_eq!(f.htype(), HTYPE_ETHERNET);
assert_eq!(f.ptype(), ETHERTYPE_IPV4);
assert_eq!(f.hlen(), MAC_ADDR_LEN as u8);
assert_eq!(f.plen(), IPV4_ADDR_LEN as u8);
assert_eq!(f.hlen() as usize, MAC_ADDR_LEN);
assert_eq!(f.plen() as usize, IPV4_ADDR_LEN);
assert_eq!(f.operation(), OPER_REPLY);
assert_eq!(f.sha(), sha);
assert_eq!(f.spa(), spa);
Expand Down Expand Up @@ -415,8 +415,8 @@ mod tests {
&mut a[..ETH_IPV4_FRAME_LEN],
HTYPE_ETHERNET,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REQUEST,
sha,
spa,
Expand All @@ -433,8 +433,8 @@ mod tests {
&mut a[..ETH_IPV4_FRAME_LEN],
HTYPE_ETHERNET + 1,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REQUEST,
sha,
spa,
Expand All @@ -452,8 +452,8 @@ mod tests {
&mut a[..ETH_IPV4_FRAME_LEN],
HTYPE_ETHERNET,
ETHERTYPE_IPV4 + 1,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REQUEST,
sha,
spa,
Expand All @@ -471,8 +471,8 @@ mod tests {
&mut a[..ETH_IPV4_FRAME_LEN],
HTYPE_ETHERNET,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8 + 1,
IPV4_ADDR_LEN as u8,
u8::try_from(MAC_ADDR_LEN).unwrap() + 1,
u8::try_from(IPV4_ADDR_LEN).unwrap(),
OPER_REQUEST,
sha,
spa,
Expand All @@ -490,8 +490,8 @@ mod tests {
&mut a[..ETH_IPV4_FRAME_LEN],
HTYPE_ETHERNET,
ETHERTYPE_IPV4,
MAC_ADDR_LEN as u8,
IPV4_ADDR_LEN as u8 + 1,
u8::try_from(MAC_ADDR_LEN).unwrap(),
u8::try_from(IPV4_ADDR_LEN).unwrap() + 1,
OPER_REQUEST,
sha,
spa,
Expand Down
11 changes: 6 additions & 5 deletions src/dumbo/src/pdu/ipv4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ impl<'a, T: NetworkBytes> IPv4Packet<'a, T> {
sum = (sum & 0xffff) + (sum >> 16);
}

#[allow(clippy::cast_possible_truncation)] // Checked by "while" condition above
!(sum as u16)
}

Expand Down Expand Up @@ -283,7 +284,7 @@ impl<'a, T: NetworkBytesMut> IPv4Packet<'a, T> {
#[inline]
pub fn set_version_and_header_len(&mut self, version: u8, header_len: usize) -> &mut Self {
let version = version << 4;
let ihl = ((header_len as u8) >> 2) & 0xf;
let ihl = u8::try_from((header_len >> 2) & 0xf).unwrap();
self.bytes[VERSION_AND_IHL_OFFSET] = version | ihl;
self
}
Expand Down Expand Up @@ -404,7 +405,7 @@ impl<'a, T: NetworkBytesMut> Incomplete<IPv4Packet<'a, T>> {
// original slice, which should be the case if our code is not wrong.
packet.bytes.shrink_unchecked(total_len);
// Set the total_len.
packet.set_total_len(total_len as u16);
packet.set_total_len(u16::try_from(total_len).unwrap());
if compute_checksum {
// Ensure this is set to 0 first.
packet.set_header_checksum(0);
Expand Down Expand Up @@ -607,17 +608,17 @@ mod tests {
// Total length smaller than header length.
p(buf.as_mut())
.set_version_and_header_len(IPV4_VERSION, OPTIONS_OFFSET)
.set_total_len(OPTIONS_OFFSET as u16 - 1);
.set_total_len(u16::try_from(OPTIONS_OFFSET).unwrap() - 1);
look_for_error(buf.as_ref(), Error::InvalidTotalLen);

// Total len not matching slice length.
p(buf.as_mut()).set_total_len(buf_len as u16 - 1);
p(buf.as_mut()).set_total_len(u16::try_from(buf_len).unwrap() - 1);
look_for_error(buf.as_ref(), Error::SliceExactLen);

// The original packet header should contain a valid checksum.
assert_eq!(
p(buf.as_mut())
.set_total_len(buf_len as u16)
.set_total_len(u16::try_from(buf_len).unwrap())
.compute_checksum(),
0
);
Expand Down
3 changes: 2 additions & 1 deletion src/dumbo/src/pdu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn compute_checksum<T: NetworkBytes>(

let len = bytes.len();
sum += protocol as u32;
sum += len as u32;
sum += u32::try_from(len).unwrap();

for i in 0..len / 2 {
sum += u32::from(bytes.ntohs_unchecked(i * 2));
Expand All @@ -105,6 +105,7 @@ fn compute_checksum<T: NetworkBytes>(
sum = (sum & 0xffff) + (sum >> 16);
}

#[allow(clippy::cast_possible_truncation)] // Safe because of "while" loop condition above
let mut csum = !(sum as u16);
// If a UDP packet checksum is 0, an all ones value is transmitted
if protocol == ChecksumProto::Udp && csum == 0x0 {
Expand Down
13 changes: 5 additions & 8 deletions src/dumbo/src/pdu/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl<'a, T: NetworkBytesMut> TcpSegment<'a, T> {
// TODO: Check that header_len | 0b11 == 0 and the resulting data_offset is valid?
#[inline]
pub fn set_header_len_rsvd_ns(&mut self, header_len: usize, ns: bool) -> &mut Self {
let mut value = (header_len as u8) << 2;
let mut value = u8::try_from(header_len << 2).unwrap();
if ns {
value |= 1;
}
Expand Down Expand Up @@ -511,7 +511,7 @@ impl<'a, T: NetworkBytesMut> TcpSegment<'a, T> {
// Let's write the MSS option if we have to.
if let Some(value) = mss_option {
segment.bytes[OPTIONS_OFFSET] = OPTION_KIND_MSS;
segment.bytes[OPTIONS_OFFSET + 1] = OPTION_LEN_MSS as u8;
segment.bytes[OPTIONS_OFFSET + 1] = u8::try_from(OPTION_LEN_MSS).unwrap();
segment.bytes.htons_unchecked(OPTIONS_OFFSET + 2, value);
}

Expand Down Expand Up @@ -692,7 +692,7 @@ mod tests {
let options = segment.options_unchecked(header_len);
assert_eq!(options.len(), OPTION_LEN_MSS);
assert_eq!(options[0], OPTION_KIND_MSS);
assert_eq!(options[1], OPTION_LEN_MSS as u8);
assert_eq!(usize::from(options[1]), OPTION_LEN_MSS);
assert_eq!(options.ntohs_unchecked(2), mss_left);
}

Expand Down Expand Up @@ -748,14 +748,11 @@ mod tests {
);
};

// Header length too short.
// Header length too short. (Don't need to test too large because it's bounded by the size
// of the field)
p(a.as_mut()).set_header_len_rsvd_ns(OPTIONS_OFFSET.checked_sub(1).unwrap(), false);
look_for_error(a.as_ref(), Error::HeaderLen);

// Header length too large.
p(a.as_mut()).set_header_len_rsvd_ns(MAX_HEADER_LEN.checked_add(4).unwrap(), false);
look_for_error(a.as_ref(), Error::HeaderLen);

// The previously set checksum should be valid.
assert_eq!(
p(a.as_mut())
Expand Down
4 changes: 2 additions & 2 deletions src/dumbo/src/pdu/udp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl<'a, T: NetworkBytesMut> UdpDatagram<'a, T> {

packet.bytes.shrink_unchecked(len);
packet.payload_mut().copy_from_slice(payload);
packet.set_len(len as u16);
packet.set_len(u16::try_from(len).unwrap());

Ok(Incomplete::new(packet))
}
Expand Down Expand Up @@ -254,7 +254,7 @@ mod tests {
let payload_length = total_len - UDP_HEADER_SIZE;
assert_eq!(p.payload().len(), payload_length);

let payload: Vec<u8> = (0..(payload_length as u8)).collect();
let payload: Vec<u8> = (0..u8::try_from(payload_length).unwrap()).collect();
p.payload_mut().copy_from_slice(&payload);
assert_eq!(*p.payload(), payload[..]);
}
Expand Down
26 changes: 12 additions & 14 deletions src/dumbo/src/tcp/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,7 @@ impl Connection {
// even more useful when we'll support window scaling.
fn local_rwnd(&self) -> u16 {
let rwnd = (self.local_rwnd_edge - self.ack_to_send).0;

if rwnd > u32::from(u16::max_value()) {
u16::max_value()
} else {
rwnd as u16
}
u16::try_from(rwnd).unwrap_or(u16::MAX)
}

// Will actually become meaningful when/if we implement window scaling.
Expand Down Expand Up @@ -642,7 +637,7 @@ impl Connection {
}

let seq = Wrapping(s.sequence_number());
let wrapping_payload_len = Wrapping(payload_len as u32);
let wrapping_payload_len = Wrapping(u32::try_from(payload_len).unwrap());

if payload_len > buf.len() {
return Err(RecvError::BufferTooSmall);
Expand Down Expand Up @@ -877,7 +872,7 @@ impl Connection {
return Err(WriteNextError::PayloadBufTooLarge);
}

let payload_end = payload_seq + Wrapping(read_buf.len() as u32);
let payload_end = payload_seq + Wrapping(u32::try_from(read_buf.len()).unwrap());

let mut rto_triggered = false;

Expand Down Expand Up @@ -958,7 +953,8 @@ impl Connection {
self.dup_ack = false;

let payload_len = segment.inner().payload().len();
let mut first_seq_after = seq_to_send + Wrapping(payload_len as u32);
let mut first_seq_after =
seq_to_send + Wrapping(u32::try_from(payload_len).unwrap());

if let Some(fin_seq) = self.send_fin {
if first_seq_after == fin_seq {
Expand Down Expand Up @@ -1476,7 +1472,9 @@ pub(crate) mod tests {
);

// This is the ack number that should be set/sent.
let expected_ack = t.remote_isn.wrapping_add(data.payload_len() as u32 + 1);
let expected_ack = t
.remote_isn
.wrapping_add(u32::try_from(data.payload_len()).unwrap() + 1);

// Check that internal state gets updated properly.
assert_eq!(c.ack_to_send.0, expected_ack);
Expand All @@ -1492,7 +1490,7 @@ pub(crate) mod tests {
assert!(t.write_next_segment(&mut c, None).unwrap().is_none());

{
let payload_len = data.payload_len() as u32;
let payload_len = u32::try_from(data.payload_len()).unwrap();

// Assuming no one changed the code, the local window size of the connection was 10000,
// so we should be able to successfully receive 9 more segments with 1000 byte payloads.
Expand Down Expand Up @@ -1561,13 +1559,13 @@ pub(crate) mod tests {
.unwrap_or_else(|| panic!("{}", i));

payload_offset += s.payload_len();
response_seq += Wrapping(s.payload_len() as u32);
response_seq += Wrapping(u32::try_from(s.payload_len()).unwrap());

// Again, the 1 accounts for the sequence number taken up by the SYN.
assert_eq!(s.sequence_number(), conn_isn.wrapping_add(1 + i * mss));
assert_eq!(s.ack_number(), remote_isn.wrapping_add(1));
assert_eq!(s.flags_after_ns(), TcpFlags::ACK);
assert_eq!(s.payload_len() as u32, mss);
assert_eq!(s.payload_len(), mss as usize);
}

// No more new data can be sent until the window advances, even though data_buf
Expand Down Expand Up @@ -1774,7 +1772,7 @@ pub(crate) mod tests {
check_control_segment(&s, 0, TcpFlags::FIN | TcpFlags::ACK);
assert_eq!(
s.sequence_number(),
conn_isn.wrapping_add(1 + bytes_sent_by_c as u32)
conn_isn.wrapping_add(1 + u32::try_from(bytes_sent_by_c).unwrap())
);
}

Expand Down
Loading

0 comments on commit 0138434

Please sign in to comment.