Skip to content

Commit

Permalink
Address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@larseggert
larseggert committed May 2, 2024
1 parent 44c0447 commit f1dddab
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 26 deletions.
6 changes: 3 additions & 3 deletions neqo-transport/src/frame.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,11 +614,11 @@ impl<'a> Frame<'a> {
0
};
// We can tolerate this copy for now.
let reason_phrase = d(dec.decode_vvec())?.to_vec();
let reason_phrase = String::from_utf8_lossy(d(dec.decode_vvec())?).to_string();
Ok(Self::ConnectionClose {
error_code,
frame_type,
reason_phrase: String::from_utf8_lossy(&reason_phrase).to_string(),
reason_phrase,
})
}
FRAME_TYPE_HANDSHAKE_DONE => Ok(Self::HandshakeDone),
Expand Down Expand Up @@ -936,7 +936,7 @@ mod tests {
let f = Frame::ConnectionClose {
error_code: CloseError::Application(0x5678),
frame_type: 0,
reason_phrase: String::from_utf8(vec![0x01, 0x02, 0x03]).unwrap(),
reason_phrase: String::from("\x01\x02\x03"),
};

just_dec(&f, "1d8000567803010203");
Expand Down
46 changes: 23 additions & 23 deletions neqo-transport/src/qlog.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub fn packet_sent(
let mut frames = SmallVec::new();
while d.remaining() > 0 {
if let Ok(f) = Frame::decode(&mut d) {
frames.push(QuicFrame::from(&f));
frames.push(QuicFrame::from(f));
} else {
qinfo!("qlog: invalid frame");
break;
Expand Down Expand Up @@ -293,7 +293,7 @@ pub fn packet_received(

while d.remaining() > 0 {
if let Ok(f) = Frame::decode(&mut d) {
frames.push(QuicFrame::from(&f));
frames.push(QuicFrame::from(f));
} else {
qinfo!("qlog: invalid frame");
break;
Expand Down Expand Up @@ -387,12 +387,12 @@ pub fn metrics_updated(qlog: &mut NeqoQlog, updated_metrics: &[QlogMetric]) {

#[allow(clippy::too_many_lines)] // Yeah, but it's a nice match.
#[allow(clippy::cast_possible_truncation, clippy::cast_precision_loss)] // No choice here.
impl From<&Frame<'_>> for QuicFrame {
fn from(frame: &Frame) -> Self {
impl From<Frame<'_>> for QuicFrame {
fn from(frame: Frame) -> Self {
match frame {
Frame::Padding(len) => QuicFrame::Padding {
length: None,
payload_length: u32::from(*len),
payload_length: u32::from(len),
},
Frame::Ping => QuicFrame::Ping {
length: None,
Expand All @@ -406,7 +406,7 @@ impl From<&Frame<'_>> for QuicFrame {
ecn_count,
} => {
let ranges =
Frame::decode_ack_frame(*largest_acknowledged, *first_ack_range, ack_ranges)
Frame::decode_ack_frame(largest_acknowledged, first_ack_range, &ack_ranges)
.ok();

let acked_ranges = ranges.map(|all| {
Expand All @@ -418,7 +418,7 @@ impl From<&Frame<'_>> for QuicFrame {
});

QuicFrame::Ack {
ack_delay: Some(*ack_delay as f32 / 1000.0),
ack_delay: Some(ack_delay as f32 / 1000.0),
acked_ranges,
ect1: ecn_count.map(|c| c[IpTosEcn::Ect1]),
ect0: ecn_count.map(|c| c[IpTosEcn::Ect0]),
Expand All @@ -433,8 +433,8 @@ impl From<&Frame<'_>> for QuicFrame {
final_size,
} => QuicFrame::ResetStream {
stream_id: stream_id.as_u64(),
error_code: *application_error_code,
final_size: *final_size,
error_code: application_error_code,
final_size,
length: None,
payload_length: None,
},
Expand All @@ -443,12 +443,12 @@ impl From<&Frame<'_>> for QuicFrame {
application_error_code,
} => QuicFrame::StopSending {
stream_id: stream_id.as_u64(),
error_code: *application_error_code,
error_code: application_error_code,
length: None,
payload_length: None,
},
Frame::Crypto { offset, data } => QuicFrame::Crypto {
offset: *offset,
offset,
length: data.len() as u64,
},
Frame::NewToken { token } => QuicFrame::NewToken {
Expand All @@ -470,20 +470,20 @@ impl From<&Frame<'_>> for QuicFrame {
..
} => QuicFrame::Stream {
stream_id: stream_id.as_u64(),
offset: *offset,
offset,
length: data.len() as u64,
fin: Some(*fin),
fin: Some(fin),
raw: None,
},
Frame::MaxData { maximum_data } => QuicFrame::MaxData {
maximum: *maximum_data,
maximum: maximum_data,
},
Frame::MaxStreamData {
stream_id,
maximum_stream_data,
} => QuicFrame::MaxStreamData {
stream_id: stream_id.as_u64(),
maximum: *maximum_stream_data,
maximum: maximum_stream_data,
},
Frame::MaxStreams {
stream_type,
Expand All @@ -493,15 +493,15 @@ impl From<&Frame<'_>> for QuicFrame {
NeqoStreamType::BiDi => StreamType::Bidirectional,
NeqoStreamType::UniDi => StreamType::Unidirectional,
},
maximum: *maximum_streams,
maximum: maximum_streams,
},
Frame::DataBlocked { data_limit } => QuicFrame::DataBlocked { limit: *data_limit },
Frame::DataBlocked { data_limit } => QuicFrame::DataBlocked { limit: data_limit },
Frame::StreamDataBlocked {
stream_id,
stream_data_limit,
} => QuicFrame::StreamDataBlocked {
stream_id: stream_id.as_u64(),
limit: *stream_data_limit,
limit: stream_data_limit,
},
Frame::StreamsBlocked {
stream_type,
Expand All @@ -511,22 +511,22 @@ impl From<&Frame<'_>> for QuicFrame {
NeqoStreamType::BiDi => StreamType::Bidirectional,
NeqoStreamType::UniDi => StreamType::Unidirectional,
},
limit: *stream_limit,
limit: stream_limit,
},
Frame::NewConnectionId {
sequence_number,
retire_prior,
connection_id,
stateless_reset_token,
} => QuicFrame::NewConnectionId {
sequence_number: *sequence_number as u32,
retire_prior_to: *retire_prior as u32,
sequence_number: sequence_number as u32,
retire_prior_to: retire_prior as u32,
connection_id_length: Some(connection_id.len() as u8),
connection_id: hex(connection_id),
stateless_reset_token: Some(hex(stateless_reset_token)),
},
Frame::RetireConnectionId { sequence_number } => QuicFrame::RetireConnectionId {
sequence_number: *sequence_number as u32,
sequence_number: sequence_number as u32,
},
Frame::PathChallenge { data } => QuicFrame::PathChallenge {
data: Some(hex(data)),
Expand All @@ -546,7 +546,7 @@ impl From<&Frame<'_>> for QuicFrame {
error_code: Some(error_code.code()),
error_code_value: Some(0),
reason: Some(reason_phrase),
trigger_frame_type: Some(*frame_type),
trigger_frame_type: Some(frame_type),
},
Frame::HandshakeDone => QuicFrame::HandshakeDone,
Frame::AckFrequency { .. } => QuicFrame::Unknown {
Expand Down

0 comments on commit f1dddab

Please sign in to comment.