Skip to content

Commit

Permalink
sendorder_fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
jesup committed Jun 12, 2023
1 parent f13d785 commit f7db2f9
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 12 deletions.
6 changes: 2 additions & 4 deletions neqo-http3/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,10 +1003,8 @@ impl Http3Connection {
conn: &mut Connection,
stream_id: StreamId,
sendorder: Option<SendOrder>) -> Res<()> {
let send_stream = self.send_streams.get_mut(&stream_id)
.ok_or(Error::InvalidStreamId)?;
send_stream.set_sendorder(conn, sendorder);
Ok(())
Ok(conn.stream_sendorder(stream_id, sendorder)?)

This comment has been minimized.

Copy link
@martinthomson

martinthomson Jun 12, 2023

Member

Also, you can remove the Ok() and ? The return type of these functions is the same.

}
}

pub fn cancel_fetch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl SendStream for WebTransportSendStream {
}

fn set_sendorder(&mut self, conn: &mut Connection, sendorder: Option<i64>) {
conn.stream_sendorder(self.stream_id, sendorder);
conn.stream_sendorder(self.stream_id, sendorder).ok();

This comment has been minimized.

Copy link
@martinthomson

martinthomson Jun 12, 2023

Member

You are still throwing away a return value. Converting from Result to Option doesn't make it better.

}

fn handle_stop_sending(&mut self, close_type: CloseType) {
Expand Down
7 changes: 4 additions & 3 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2947,13 +2947,14 @@ impl Connection {

/// Set the SendOrder of a stream. Re-enqueues to keep the ordering correct
/// # Errors
/// None
/// Returns InvalidStreamId if the stream id doesn't exist
pub fn stream_sendorder(
&mut self,
stream_id: StreamId,
sendorder: Option<SendOrder>,
) {
self.streams.set_sendorder(stream_id, sendorder);
) -> Res<()> {
self.streams.set_sendorder(stream_id, sendorder)
}

This comment has been minimized.

Copy link
@martinthomson

martinthomson Jun 12, 2023

Member

Does this even compile with an extra brace like that?

}

/// Send data on a stream.
Expand Down
2 changes: 1 addition & 1 deletion neqo-transport/src/connection/tests/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn sendorder_test(order_of_sendorder: &[Option<SendOrder>]) {
let id = client.stream_create(StreamType::UniDi).unwrap();
streams.push(id);
ordered.push((id, *sendorder));
client.streams.set_sendorder(id, *sendorder);
client.streams.set_sendorder(id, *sendorder).ok();

This comment has been minimized.

Copy link
@martinthomson

martinthomson Jun 12, 2023

Member

As above, also here.

}
// Write some data to all the streams
for stream_id in streams {
Expand Down
2 changes: 1 addition & 1 deletion neqo-transport/src/send_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ impl SendStreams {
}
}

pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()>{
pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()> {
// don't grab stream here; causes borrow errors
let old_sendorder = self.map.get(&stream_id).unwrap().sendorder();
if old_sendorder != sendorder {
Expand Down
4 changes: 2 additions & 2 deletions neqo-transport/src/streams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,8 @@ impl Streams {
))
}

pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) {
self.send.set_sendorder(stream_id, sendorder).ok();
pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()> {
self.send.set_sendorder(stream_id, sendorder)
}

pub fn stream_create(&mut self, st: StreamType) -> Res<StreamId> {
Expand Down

0 comments on commit f7db2f9

Please sign in to comment.