Skip to content

Commit b7bab20

Browse files
carllerchecarllerche
authored
Split Connection::read_frame for readability (#58)
The `read_frame` function was fairly big. This splits up the function into a separate `parse_frame` function. This should help with breaking up the concept of framing into smaller chunks.
1 parent 98e712a commit b7bab20

File tree

1 file changed

+69
-59
lines changed

1 file changed

+69
-59
lines changed

‎src/connection.rs

Lines changed: 69 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -57,66 +57,11 @@ impl Connection {
5757
/// is closed in a way that doesn't break a frame in half, it returns
5858
/// `None`. Otherwise, an error is returned.
5959
pub async fn read_frame(&mut self) -> crate::Result<Option<Frame>> {
60-
use frame::Error::Incomplete;
61-
6260
loop {
63-
// Cursor is used to track the "current" location in the
64-
// buffer. Cursor also implements `Buf` from the `bytes` crate
65-
// which provides a number of helpful utilities for working
66-
// with bytes.
67-
let mut buf = Cursor::new(&self.buffer[..]);
68-
69-
// The first step is to check if enough data has been buffered to
70-
// parse a single frame. This step is usually much faster than doing
71-
// a full parse of the frame, and allows us to skip allocating data
72-
// structures to hold the frame data unless we know the full frame
73-
// has been received.
74-
match Frame::check(&mut buf) {
75-
Ok(_) => {
76-
// The `check` function will have advanced the cursor until
77-
// the end of the frame. Since the cursor had position set
78-
// to zero before `Frame::check` was called, we obtain the
79-
// length of the frame by checking the cursor position.
80-
let len = buf.position() as usize;
81-
82-
// Reset the position to zero before passing the cursor to
83-
// `Frame::parse`.
84-
buf.set_position(0);
85-
86-
// Parse the frame from the buffer. This allocates the
87-
// necessary structures to represent the frame and returns
88-
// the frame value.
89-
//
90-
// If the encoded frame representation is invalid, an error
91-
// is returned. This should terminate the **current**
92-
// connection but should not impact any other connected
93-
// client.
94-
let frame = Frame::parse(&mut buf)?;
95-
96-
// Discard the parsed data from the read buffer.
97-
//
98-
// When `advance` is called on the read buffer, all of the
99-
// data up to `len` is discarded. The details of how this
100-
// works is left to `BytesMut`. This is often done by moving
101-
// an internal cursor, but it may be done by reallocating
102-
// and copying data.
103-
self.buffer.advance(len);
104-
105-
// Return the parsed frame to the caller.
106-
return Ok(Some(frame));
107-
}
108-
// There is not enough data present in the read buffer to parse
109-
// a single frame. We must wait for more data to be received
110-
// from the socket. Reading from the socket will be done in the
111-
// statement after this `match`.
112-
//
113-
// We do not want to return `Err` from here as this "error" is
114-
// an expected runtime condition.
115-
Err(Incomplete) => {}
116-
// An error was encountered while parsing the frame. The
117-
// connection is now in an invalid state. Returning `Err` from
118-
// here will result in the connection being closed.
119-
Err(e) => return Err(e.into()),
61+
// Attempt to parse a frame from the buffered data. If enough data
62+
// has been buffered, the frame is returned.
63+
if let Some(frame) = self.parse_frame()? {
64+
return Ok(Some(frame));
12065
}
12166

12267
// There is not enough buffered data to read a frame. Attempt to
@@ -138,6 +83,71 @@ impl Connection {
13883
}
13984
}
14085

86+
/// Tries to parse a frame from the buffer. If the buffer contains enough
87+
/// data, the frame is returned and the data removed from the buffer. If not
88+
/// enough data has been buffered yet, `Ok(None)` is returned. If the
89+
/// buffered data does not represent a valid frame, `Err` is returned.
90+
fn parse_frame(&mut self) -> crate::Result<Option<Frame>> {
91+
use frame::Error::Incomplete;
92+
93+
// Cursor is used to track the "current" location in the
94+
// buffer. Cursor also implements `Buf` from the `bytes` crate
95+
// which provides a number of helpful utilities for working
96+
// with bytes.
97+
let mut buf = Cursor::new(&self.buffer[..]);
98+
99+
// The first step is to check if enough data has been buffered to parse
100+
// a single frame. This step is usually much faster than doing a full
101+
// parse of the frame, and allows us to skip allocating data structures
102+
// to hold the frame data unless we know the full frame has been
103+
// received.
104+
match Frame::check(&mut buf) {
105+
Ok(_) => {
106+
// The `check` function will have advanced the cursor until the
107+
// end of the frame. Since the cursor had position set to zero
108+
// before `Frame::check` was called, we obtain the length of the
109+
// frame by checking the cursor position.
110+
let len = buf.position() as usize;
111+
112+
// Reset the position to zero before passing the cursor to
113+
// `Frame::parse`.
114+
buf.set_position(0);
115+
116+
// Parse the frame from the buffer. This allocates the necessary
117+
// structures to represent the frame and returns the frame
118+
// value.
119+
//
120+
// If the encoded frame representation is invalid, an error is
121+
// returned. This should terminate the **current** connection
122+
// but should not impact any other connected client.
123+
let frame = Frame::parse(&mut buf)?;
124+
125+
// Discard the parsed data from the read buffer.
126+
//
127+
// When `advance` is called on the read buffer, all of the data
128+
// up to `len` is discarded. The details of how this works is
129+
// left to `BytesMut`. This is often done by moving an internal
130+
// cursor, but it may be done by reallocating and copying data.
131+
self.buffer.advance(len);
132+
133+
// Return the parsed frame to the caller.
134+
Ok(Some(frame))
135+
}
136+
// There is not enough data present in the read buffer to parse a
137+
// single frame. We must wait for more data to be received from the
138+
// socket. Reading from the socket will be done in the statement
139+
// after this `match`.
140+
//
141+
// We do not want to return `Err` from here as this "error" is an
142+
// expected runtime condition.
143+
Err(Incomplete) => Ok(None),
144+
// An error was encountered while parsing the frame. The connection
145+
// is now in an invalid state. Returning `Err` from here will result
146+
// in the connection being closed.
147+
Err(e) => Err(e.into()),
148+
}
149+
}
150+
141151
/// Write a single `Frame` value to the underlying stream.
142152
///
143153
/// The `Frame` value is written to the socket using the various `write_*`

0 commit comments

Comments
 (0)