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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Refactor: assorted code readability improvements in h1/conn.rs #2817

Merged
merged 13 commits into from Apr 26, 2022
Merged

Refactor: assorted code readability improvements in h1/conn.rs #2817

Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a55946c
refactor: use `matches` macro to flatten code
BastiDood Apr 24, 2022
aeda230
refactor: prefer `matches` over explicit matching
BastiDood Apr 24, 2022
5865910
refactor: reuse `can_write_body` method
BastiDood Apr 24, 2022
38601c3
refactor: use `matches` over `if`-`let`
BastiDood Apr 24, 2022
83691fc
refactor: nested `if`-`else` as early return
BastiDood Apr 24, 2022
530ebc2
refactor: move inner `match` logic outside
BastiDood Apr 24, 2022
e1cd9c5
refactor: unneeded `return` in `match`
BastiDood Apr 24, 2022
8530a59
refactor: remove unneeded reference matching
BastiDood Apr 24, 2022
36b62f2
refactor: use early returns for idle check
BastiDood Apr 24, 2022
da23775
refactor: use `matches` macro for Boolean `match`
BastiDood Apr 24, 2022
8817396
fix: remove accidentally introduced semicolon
BastiDood Apr 24, 2022
7aab0f7
fix: revert removal of references
BastiDood Apr 24, 2022
e0f639c
revert: use explicit pattern-matching for some cases
BastiDood Apr 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
162 changes: 69 additions & 93 deletions src/proto/h1/conn.rs
View file
Open in desktop
Expand Up @@ -155,19 +155,15 @@ where
}

pub(crate) fn can_read_head(&self) -> bool {
match self.state.reading {
Reading::Init => {
if T::should_read_first() {
true
} else {
match self.state.writing {
Writing::Init => false,
_ => true,
}
}
}
_ => false,
if !matches!(self.state.reading, Reading::Init) {
return false;
}

if T::should_read_first() {
return true;
}

!matches!(self.state.writing, Writing::Init)
}

pub(crate) fn can_read_body(&self) -> bool {
Expand Down Expand Up @@ -367,10 +363,10 @@ where
}

fn is_mid_message(&self) -> bool {
match (&self.state.reading, &self.state.writing) {
(&Reading::Init, &Writing::Init) => false,
_ => true,
}
!matches!(
(&self.state.reading, &self.state.writing),
(&Reading::Init, &Writing::Init)
)
}

// This will check to make sure the io object read is empty.
Expand Down Expand Up @@ -446,16 +442,12 @@ where
// determined we couldn't keep reading until we knew how writing
// would finish.

match self.state.reading {
Reading::Continue(..) | Reading::Body(..) | Reading::KeepAlive | Reading::Closed => {
return
}
Reading::Init => (),
};
if !matches!(self.state.reading, Reading::Init) {
BastiDood marked this conversation as resolved.
Show resolved Hide resolved
return;
}

match self.state.writing {
Writing::Body(..) => return,
Writing::Init | Writing::KeepAlive | Writing::Closed => (),
if self.can_write_body() {
BastiDood marked this conversation as resolved.
Show resolved Hide resolved
return;
}

if !self.io.is_read_blocked() {
Expand Down Expand Up @@ -493,22 +485,18 @@ where
}

pub(crate) fn can_write_head(&self) -> bool {
if !T::should_read_first() {
if let Reading::Closed = self.state.reading {
return false;
}
if !T::should_read_first() && matches!(self.state.reading, Reading::Closed) {
return false;
}

match self.state.writing {
Writing::Init => self.io.can_headers_buf(),
_ => false,
}
}

pub(crate) fn can_write_body(&self) -> bool {
match self.state.writing {
Writing::Body(..) => true,
Writing::Init | Writing::KeepAlive | Writing::Closed => false,
}
matches!(self.state.writing, Writing::Body(..))
BastiDood marked this conversation as resolved.
Show resolved Hide resolved
}

pub(crate) fn can_buffer_body(&self) -> bool {
Expand Down Expand Up @@ -641,15 +629,15 @@ where
Writing::Body(ref mut encoder) => {
self.io.buffer(encoder.encode(chunk));

if encoder.is_eof() {
if encoder.is_last() {
Writing::Closed
} else {
Writing::KeepAlive
}
} else {
if !encoder.is_eof() {
return;
}

if encoder.is_last() {
Writing::Closed
} else {
Writing::KeepAlive
}
}
_ => unreachable!("write_body invalid state: {:?}", self.state.writing),
};
Expand Down Expand Up @@ -680,32 +668,31 @@ where
pub(crate) fn end_body(&mut self) -> crate::Result<()> {
debug_assert!(self.can_write_body());

let mut res = Ok(());
let state = match self.state.writing {
Writing::Body(ref mut encoder) => {
// end of stream, that means we should try to eof
match encoder.end() {
Ok(end) => {
if let Some(end) = end {
self.io.buffer(end);
}
if encoder.is_last() || encoder.is_close_delimited() {
Writing::Closed
} else {
Writing::KeepAlive
}
}
Err(not_eof) => {
res = Err(crate::Error::new_body_write_aborted().with(not_eof));
Writing::Closed
}
}
}
let encoder = match self.state.writing {
Writing::Body(ref mut enc) => enc,
_ => return Ok(()),
};

self.state.writing = state;
res
// end of stream, that means we should try to eof
match encoder.end() {
Ok(end) => {
if let Some(end) = end {
self.io.buffer(end);
}

self.state.writing = if encoder.is_last() || encoder.is_close_delimited() {
Writing::Closed
} else {
Writing::KeepAlive
};

Ok(())
}
Err(not_eof) => {
self.state.writing = Writing::Closed;
Err(crate::Error::new_body_write_aborted().with(not_eof))
}
}
}

// When we get a parse error, depending on what side we are, we might be able
Expand Down Expand Up @@ -758,10 +745,7 @@ where

// If still in Reading::Body, just give up
match self.state.reading {
Reading::Init | Reading::KeepAlive => {
trace!("body drained");
return;
}
Reading::Init | Reading::KeepAlive => trace!("body drained"),
_ => self.close_read(),
}
}
Expand Down Expand Up @@ -1012,43 +996,35 @@ impl State {

self.method = None;
self.keep_alive.idle();
if self.is_idle() {
self.reading = Reading::Init;
self.writing = Writing::Init;

// !T::should_read_first() means Client.
//
// If Client connection has just gone idle, the Dispatcher
// should try the poll loop one more time, so as to poll the
// pending requests stream.
if !T::should_read_first() {
self.notify_read = true;
}
} else {

if !self.is_idle() {
self.close();
return;
}

self.reading = Reading::Init;
self.writing = Writing::Init;

// !T::should_read_first() means Client.
//
// If Client connection has just gone idle, the Dispatcher
// should try the poll loop one more time, so as to poll the
// pending requests stream.
if !T::should_read_first() {
self.notify_read = true;
}
}

fn is_idle(&self) -> bool {
if let KA::Idle = self.keep_alive.status() {
true
} else {
false
}
matches!(self.keep_alive.status(), KA::Idle)
}

fn is_read_closed(&self) -> bool {
match self.reading {
Reading::Closed => true,
_ => false,
}
matches!(self.reading, Reading::Closed)
}

fn is_write_closed(&self) -> bool {
match self.writing {
Writing::Closed => true,
_ => false,
}
matches!(self.writing, Writing::Closed)
}

fn prepare_upgrade(&mut self) -> crate::upgrade::OnUpgrade {
Expand Down