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.

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

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

5t0n3
Copy link
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
5 checks passed
@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