Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: ignore more broken clippy lints #1591

Merged
merged 3 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ jobs:
command: clippy
# deriving Eq may break API compatibility so we disable it
# See https://github.com/rust-lang/rust-clippy/issues/9063
args: --all-features --all-targets -- -A clippy::derive_partial_eq_without_eq ${{ matrix.args }}

# manual_clamp will panic when min > max
# See https://github.com/rust-lang/rust-clippy/pull/10101
args: --all-features --all-targets -- -A clippy::derive_partial_eq_without_eq -A clippy::manual_clamp -A clippy::uninlined_format_args ${{ matrix.args }}

udeps:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion netbench/netbench/src/scenario.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub(crate) mod pkcs12_format {
{
if deserializer.is_human_readable() {
let s = String::deserialize(deserializer)?;
let out = base64::decode(&s).map_err(serde::de::Error::custom)?;
let out = base64::decode(s).map_err(serde::de::Error::custom)?;
Ok(out)
} else {
Vec::deserialize(deserializer)
Expand Down
16 changes: 8 additions & 8 deletions quic/s2n-quic-core/src/packet/decoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ impl<'a> HeaderDecoder<'a> {
}
}

pub fn decode_destination_connection_id<'b>(
pub fn decode_destination_connection_id(
&mut self,
buffer: &DecoderBufferMut<'b>,
buffer: &DecoderBufferMut<'_>,
) -> Result<CheckedRange, DecoderError> {
let destination_connection_id =
self.decode_checked_range::<DestinationConnectionIdLen>(buffer)?;
validate_destination_connection_id_range(&destination_connection_id)?;
Ok(destination_connection_id)
}

pub fn decode_short_destination_connection_id<'b, Validator: connection::id::Validator>(
pub fn decode_short_destination_connection_id<Validator: connection::id::Validator>(
&mut self,
buffer: &DecoderBufferMut<'b>,
buffer: &DecoderBufferMut<'_>,
connection_info: &ConnectionInfo,
connection_id_validator: &Validator,
) -> Result<CheckedRange, DecoderError> {
Expand All @@ -78,18 +78,18 @@ impl<'a> HeaderDecoder<'a> {
Ok(destination_connection_id)
}

pub fn decode_source_connection_id<'b>(
pub fn decode_source_connection_id(
&mut self,
buffer: &DecoderBufferMut<'b>,
buffer: &DecoderBufferMut<'_>,
) -> Result<CheckedRange, DecoderError> {
let source_connection_id = self.decode_checked_range::<SourceConnectionIdLen>(buffer)?;
validate_source_connection_id_range(&source_connection_id)?;
Ok(source_connection_id)
}

pub fn decode_checked_range<'b, Len: DecoderValue<'a> + TryInto<usize>>(
pub fn decode_checked_range<Len: DecoderValue<'a> + TryInto<usize>>(
&mut self,
buffer: &DecoderBufferMut<'b>,
buffer: &DecoderBufferMut<'_>,
) -> Result<CheckedRange, DecoderError> {
let (value, peek) = self.peek.skip_into_range_with_len_prefix::<Len>(buffer)?;
self.peek = peek;
Expand Down
20 changes: 10 additions & 10 deletions quic/s2n-quic-core/src/packet/interceptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,23 @@ pub trait Interceptor: 'static + Send {
}

#[inline(always)]
fn intercept_tx_datagram<'a>(
fn intercept_tx_datagram(
&mut self,
subject: &Subject,
datagram: &Datagram,
payload: &mut EncoderBuffer<'a>,
payload: &mut EncoderBuffer,
) {
let _ = subject;
let _ = datagram;
let _ = payload;
}

#[inline(always)]
fn intercept_tx_payload<'a>(
fn intercept_tx_payload(
&mut self,
subject: &Subject,
packet: &Packet,
payload: &mut EncoderBuffer<'a>,
payload: &mut EncoderBuffer,
) {
let _ = subject;
let _ = packet;
Expand Down Expand Up @@ -111,22 +111,22 @@ where
}

#[inline(always)]
fn intercept_tx_datagram<'a>(
fn intercept_tx_datagram(
&mut self,
subject: &Subject,
datagram: &Datagram,
payload: &mut EncoderBuffer<'a>,
payload: &mut EncoderBuffer,
) {
self.0.intercept_tx_datagram(subject, datagram, payload);
self.1.intercept_tx_datagram(subject, datagram, payload);
}

#[inline(always)]
fn intercept_tx_payload<'a>(
fn intercept_tx_payload(
&mut self,
subject: &Subject,
packet: &Packet,
payload: &mut EncoderBuffer<'a>,
payload: &mut EncoderBuffer,
) {
self.0.intercept_tx_payload(subject, packet, payload);
self.1.intercept_tx_payload(subject, packet, payload);
Expand Down Expand Up @@ -175,11 +175,11 @@ where
}

#[inline]
fn intercept_tx_payload<'a>(
fn intercept_tx_payload(
&mut self,
_subject: &Subject,
_packet: &Packet,
payload: &mut EncoderBuffer<'a>,
payload: &mut EncoderBuffer,
) {
self.tx.havoc(&mut self.random, payload);
}
Expand Down
4 changes: 2 additions & 2 deletions quic/s2n-quic-core/src/packet/interceptor/loss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,11 @@ where
}

#[inline]
fn intercept_tx_datagram<'a>(
fn intercept_tx_datagram(
&mut self,
_subject: &crate::event::api::Subject,
_datagram: &super::Datagram,
payload: &mut s2n_codec::EncoderBuffer<'a>,
payload: &mut s2n_codec::EncoderBuffer,
) {
if !self.tx.should_pass(&mut self.random) {
payload.set_position(0);
Expand Down
2 changes: 1 addition & 1 deletion quic/s2n-quic-core/src/recovery/cubic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ impl Cubic {
//# where beta_cubic is the CUBIC multiplication decrease factor
#[inline]
fn w_cubic(&self, t: Duration) -> f32 {
C * (t.as_secs_f32() - self.k.as_secs_f32()).powi(3) + self.w_max as f32
C * (t.as_secs_f32() - self.k.as_secs_f32()).powi(3) + self.w_max
}

//= https://www.rfc-editor.org/rfc/rfc8312#section-4.2
Expand Down
2 changes: 1 addition & 1 deletion quic/s2n-quic-core/src/recovery/rtt_estimator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ mod test {
let weight = 8;

// perform the unoptimized version
let expected = ((weight - 1) as u32 * a) / weight + b / weight;
let expected = ((weight - 1) * a) / weight + b / weight;
let actual = super::weighted_average(a, b, weight as _);

// assert that the unoptimized result matches the optimized to the nearest `weight` nanos
Expand Down
2 changes: 2 additions & 0 deletions quic/s2n-quic-platform/src/message/cmsg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

#![allow(clippy::unnecessary_cast)] // some platforms encode lengths as `u32` so we cast everything to be safe
Copy link
Contributor

@toidiu toidiu Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇 dang sharp edges. Wondering if some tests failed or you foresaw this on your own?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we ran into issues with it in the past. i believe it was macos so our current tests should catch it


use core::mem::{align_of, size_of};
use s2n_quic_core::inet::{AncillaryData, ExplicitCongestionNotification};

Expand Down
12 changes: 7 additions & 5 deletions quic/s2n-quic-platform/src/message/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,13 @@ impl MessageTrait for msghdr {
}

// reset the control messages if it isn't set to the default value
if self.msg_controllen as usize != cmsg::MAX_LEN {
let cmsg = core::slice::from_raw_parts_mut(
self.msg_control as *mut u8,
self.msg_controllen as _,
);

// some platforms encode lengths as `u32` so we cast everything to be safe
#[allow(clippy::unnecessary_cast)]
let msg_controllen = self.msg_controllen as usize;

if msg_controllen != cmsg::MAX_LEN {
let cmsg = core::slice::from_raw_parts_mut(self.msg_control as *mut u8, msg_controllen);

for byte in cmsg.iter_mut() {
*byte = 0;
Expand Down
1 change: 1 addition & 0 deletions quic/s2n-quic-platform/src/message/queue/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ impl<
}

#[inline]
#[allow(unknown_lints, clippy::misnamed_getters)] // this slice is made up of two halves and uses the primary for unfilled data
fn capacity(&self) -> usize {
self.primary.len
}
Expand Down
2 changes: 1 addition & 1 deletion quic/s2n-quic-platform/src/message/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl MessageTrait for Message {
}

fn payload_len(&self) -> usize {
self.payload_len as usize
self.payload_len
}

unsafe fn set_payload_len(&mut self, len: usize) {
Expand Down
4 changes: 2 additions & 2 deletions quic/s2n-quic-transport/src/connection/connection_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,12 +415,12 @@ impl<Config: endpoint::Config> ConnectionImpl<Config> {
/// Since non-probing frames can only be sent on the active path, a separate
/// transmission context with Mode::PathValidationOnly is used to send on
/// other paths.
fn path_validation_only_transmission<'a, 'sub, Tx: tx::Queue<Handle = Config::PathHandle>>(
fn path_validation_only_transmission<'a, Tx: tx::Queue<Handle = Config::PathHandle>>(
&mut self,
queue: &mut Tx,
timestamp: Timestamp,
outcome: &'a mut transmission::Outcome,
subscriber: &'sub mut Config::EventSubscriber,
subscriber: &mut Config::EventSubscriber,
packet_interceptor: &'a mut Config::PacketInterceptor,
) -> usize {
let mut count = 0;
Expand Down
4 changes: 2 additions & 2 deletions quic/s2n-quic-transport/src/stream/receive_stream/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ fn flow_control_window_update_is_only_sent_when_minimum_data_size_is_consumed()
test_env.stream.get_stream_interests()
);

let expected_window = absolute_threshold as u64 - 1;
let expected_window = absolute_threshold - 1;
assert_eq!(
expected_window,
Into::<u64>::into(
Expand Down Expand Up @@ -1156,7 +1156,7 @@ fn connection_flow_control_window_update_is_only_sent_when_minimum_data_size_is_
.get_transmission_interest()
);

let expected_window = absolute_threshold as u64 - 1;
let expected_window = absolute_threshold - 1;
assert_eq!(
expected_window,
Into::<u64>::into(
Expand Down
4 changes: 2 additions & 2 deletions quic/s2n-quic-transport/src/stream/send_stream/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ fn can_not_enqueue_data_if_max_buffer_size_has_been_reached() {
// as long as we are below the flow control window. We subtract 2
// in the second instruction so that even the the third instruction
// would not cause us a rejection based on the flow control size.
Instruction::EnqueueData(VarInt::from_u32(0), MAX_BUFFER_SIZE as usize - 1, true),
Instruction::EnqueueData(VarInt::from_u32(0), MAX_BUFFER_SIZE - 1, true),
Instruction::EnqueueData(
max_buffer_size_varint - 1,
RECEIVE_WINDOW as usize - MAX_BUFFER_SIZE - 2,
Expand Down Expand Up @@ -488,7 +488,7 @@ fn multiple_stream_frames_are_sent_in_a_packet() {
// as long as we are below the flow control window. We subtract 2
// in the second instruction so that even the the third instruction
// would not cause us a rejection based on the flow control size.
Instruction::EnqueueData(VarInt::from_u32(0), MAX_BUFFER_SIZE as usize - 1, true),
Instruction::EnqueueData(VarInt::from_u32(0), MAX_BUFFER_SIZE - 1, true),
Instruction::EnqueueData(
max_buffer_size_varint - 1,
RECEIVE_WINDOW as usize - MAX_BUFFER_SIZE - 2,
Expand Down
2 changes: 1 addition & 1 deletion scripts/local_test
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@
set -e

cargo +nightly fmt --all -- --check
cargo +stable clippy --all-features --all-targets -- -D warnings -A clippy::derive_partial_eq_without_eq
cargo +stable clippy --all-features --all-targets -- -D warnings -A clippy::derive_partial_eq_without_eq -A clippy::manual_clamp -A clippy::uninlined_format_args
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe also create an issue to track removing it in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo test