Skip to content

Commit 38e5810

Browse files
fix(dc): Enable panic clippy lints in s2n-quic-dc (#3116)
1 parent 5c21752 commit 38e5810

58 files changed

Lines changed: 592 additions & 6 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.clippy.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
msrv = "1.88.0"
22
# // https://github.com/aws/s2n-quic/pull/2251
33
too-many-arguments-threshold = 100
4+
# Avoid linting on unwraps in consts and tests where they can't bubble into production panics.
5+
allow-unwrap-in-consts = true
6+
allow-unwrap-in-tests = true
7+
allow-panic-in-tests = true

dc/s2n-quic-dc/Cargo.toml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,12 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
8282
[lints.rust.unexpected_cfgs]
8383
level = "warn"
8484
check-cfg = ['cfg(future)', 'cfg(fuzzing)', 'cfg(kani)', 'cfg(todo)']
85+
86+
[lints.clippy]
87+
panic = "deny"
88+
panicking_unwrap = "deny"
89+
panicking_overflow_checks = "deny"
90+
panic_in_result_fn = "deny"
91+
large_futures = "deny"
92+
unwrap_used = "deny"
93+
unwrap_in_result = "deny"

dc/s2n-quic-dc/src/control.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ pub struct Control {
1111
}
1212

1313
impl Control {
14+
#[expect(
15+
clippy::unwrap_used,
16+
clippy::unwrap_in_result,
17+
reason = "FIXME: local_addr is a fallible socket op and should propagate the error rather than unwrap"
18+
)]
1419
pub fn new(address: SocketAddr, map: Map) -> std::io::Result<Self> {
1520
let socket = Arc::new(std::net::UdpSocket::bind(address)?);
1621
let port = socket.local_addr().unwrap().port();

dc/s2n-quic-dc/src/credentials.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ use s2n_codec::{
1414
pub use s2n_quic_core::varint::VarInt as KeyId;
1515

1616
#[cfg(any(test, feature = "testing"))]
17+
#[allow(
18+
clippy::unwrap_used,
19+
clippy::unwrap_in_result,
20+
clippy::panic,
21+
clippy::panic_in_result_fn,
22+
reason = "test-support helpers may panic to surface setup failures"
23+
)]
1724
pub mod testing;
1825

1926
#[derive(Clone, Copy, Default, PartialEq, Eq, FromBytes, IntoBytes, Unaligned, PartialOrd, Ord)]
@@ -29,6 +36,10 @@ impl Id {
2936
// The ID has very high quality entropy already, so write just one half of it to keep hash
3037
// costs as low as possible. For the main use of the Hash impl in the fixed-size ID map
3138
// this translates to just directly using these bytes for the indexing.
39+
#[expect(
40+
clippy::unwrap_used,
41+
reason = "slicing exactly 8 bytes always converts to [u8; 8]"
42+
)]
3243
u64::from_ne_bytes(self.0[..8].try_into().unwrap())
3344
}
3445
}
@@ -79,6 +90,10 @@ impl s2n_quic_core::probe::Arg for Id {
7990
fn into_usdt(self) -> isize {
8091
// we have to truncate the bytes, but 64 bits should be unique enough for these purposes
8192
let slice = &self.0[..core::mem::size_of::<usize>()];
93+
#[expect(
94+
clippy::unwrap_used,
95+
reason = "slice length equals size_of::<usize>() so the conversion always succeeds"
96+
)]
8297
let bytes = slice.try_into().unwrap();
8398
usize::from_ne_bytes(bytes).into_usdt()
8499
}

dc/s2n-quic-dc/src/crypto/awslc.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ pub mod seal {
2222
impl Application {
2323
#[inline]
2424
pub fn new(key: &[u8], iv: [u8; NONCE_LEN], algorithm: &'static Algorithm) -> Self {
25+
#[expect(
26+
clippy::unwrap_used,
27+
reason = "key sizes are statically known, moving the panic into callers isn't helpful"
28+
)]
2529
let key = UnboundKey::new(algorithm, key).unwrap();
2630
let key = LessSafeKey::new(key);
2731
Self { key, iv: Iv(iv) }
@@ -151,6 +155,10 @@ pub mod open {
151155
impl Application {
152156
#[inline]
153157
pub fn new(key: &[u8], iv: [u8; NONCE_LEN], algorithm: &'static Algorithm) -> Self {
158+
#[expect(
159+
clippy::unwrap_used,
160+
reason = "key sizes are statically known, moving the panic into callers isn't helpful"
161+
)]
154162
let key = UnboundKey::new(algorithm, key).unwrap();
155163
let key = LessSafeKey::new(key);
156164
Self { key, iv: Iv(iv) }

dc/s2n-quic-dc/src/event.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ impl Meta for api::EndpointMeta {
2626
}
2727
}
2828

29+
// The generated code is not currently held to the panic lints; suppress them here at the
30+
// module level rather than annotating each generated site (which would be lost on regeneration).
31+
#[allow(
32+
clippy::panic,
33+
clippy::unwrap_used,
34+
clippy::unwrap_in_result,
35+
clippy::panic_in_result_fn,
36+
reason = "FIXME: generated event code is not yet audited for the panic lints"
37+
)]
2938
mod generated;
3039
pub use generated::*;
3140

dc/s2n-quic-dc/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ pub mod task;
2525
pub mod uds;
2626

2727
#[cfg(any(test, feature = "testing"))]
28+
#[allow(
29+
clippy::unwrap_used,
30+
clippy::unwrap_in_result,
31+
clippy::panic,
32+
clippy::panic_in_result_fn,
33+
reason = "test-support helpers may panic to surface setup failures"
34+
)]
2835
pub mod testing;
2936

3037
pub use s2n_quic_core::dc::{Version, SUPPORTED_VERSIONS};

dc/s2n-quic-dc/src/msg/send.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ impl allocator::Segment for Segment {
6565

6666
#[cfg(debug_assertions)]
6767
impl Drop for Segment {
68+
#[expect(
69+
clippy::panic,
70+
reason = "debug-only leak detector that fires only when a segment was dropped without being freed"
71+
)]
6872
fn drop(&mut self) {
6973
if self.idx != Idx::MAX && !std::thread::panicking() {
7074
panic!("message segment {} leaked", self.idx);
@@ -122,6 +126,10 @@ impl Retransmission {
122126

123127
#[cfg(debug_assertions)]
124128
impl Drop for Retransmission {
129+
#[expect(
130+
clippy::panic,
131+
reason = "debug-only leak detector that fires only when a segment was dropped without being freed"
132+
)]
125133
fn drop(&mut self) {
126134
if self.idx.get() != Idx::MAX && !std::thread::panicking() {
127135
panic!("message segment {} leaked", self.idx.get());

dc/s2n-quic-dc/src/packet/control/encoder.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ where
8080

8181
if cfg!(debug_assertions) {
8282
let decoder = s2n_codec::DecoderBufferMut::new(slice);
83+
#[expect(
84+
clippy::unwrap_used,
85+
reason = "debug-assertions-only round-trip check that the packet we just encoded decodes successfully"
86+
)]
8387
let _ = super::decoder::Packet::decode(decoder, (), crypto.tag_len()).unwrap();
8488
}
8589

dc/s2n-quic-dc/src/packet/datagram/encoder.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ where
9696
if tag.is_connected() || tag.ack_eliciting() {
9797
unsafe {
9898
assume!(encoder.remaining_capacity() >= 8);
99+
#[expect(
100+
clippy::unwrap_used,
101+
reason = "FIXME: change API to enforce both packet number and next_expected_control_packet either both passed or neither is passed"
102+
)]
99103
encoder.encode(&packet_number.unwrap());
100104
}
101105
}

0 commit comments

Comments
 (0)