Skip to content

fix: non-exhaustive pattern in is_connection_open on old Rust versions#42

Merged
5t0n3 merged 2 commits intocPacketNetworks:mainfrom
5t0n3:non-exhaustive-read-check-fix
Sep 11, 2024
Merged

fix: non-exhaustive pattern in is_connection_open on old Rust versions#42
5t0n3 merged 2 commits intocPacketNetworks:mainfrom
5t0n3:non-exhaustive-read-check-fix

Conversation

@5t0n3
Copy link
Copy Markdown
Contributor

@5t0n3 5t0n3 commented Sep 11, 2024

#39 broke compilation on Rust versions below 1.75 due to a match in is_connection_open(), namely:

Poll::Ready(ready) => match ready {
// read of length 0 indicates an EOF, which happens when the other side closes a TCP connection
Ok(0) => Ok(false),
Err(e) => match e.kind() {
// these errors indicate that the connection is closed, which is the exact
// situation we're trying to recover from
//
// BrokenPipe seems to be Linux-specific (?), ConnectionReset is more general though
// (checked TCP & read(2) man pages for MacOS/FreeBSD/Linux)
io::ErrorKind::BrokenPipe | io::ErrorKind::ConnectionReset => Ok(false),
// bubble up any other errors to the caller
_ => Err(e),
},
// if there's data still available, the connection is still open, although
// this shouldn't happen in the context of TACACS+
Ok(1..) => Ok(true),
},

The behavior of matching usizes against open ranges landed in rust-lang/rust#116692, which was first included in 1.75.0.

To keep compatibility with older versions of Rust, this PR changes the range to an _ instead, which has the same behavior and unbreaks builds on older compilers.

Something changed in how the compiler determines whether all cases of a
match were covered in 1.75, so using the range pattern broke building
on 1.74.
@5t0n3 5t0n3 self-assigned this Sep 11, 2024
@5t0n3 5t0n3 merged commit 42eb2ee into cPacketNetworks:main Sep 11, 2024
@5t0n3 5t0n3 deleted the non-exhaustive-read-check-fix branch September 11, 2024 20:14
@5t0n3 5t0n3 removed their assignment Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants