BBA/HLE: Don't assume connect is successful #12555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sepalani
force-pushed
the
bba-connecting
branch
3 times, most recently
from
89161ca
to
104f15b
Compare
|
I switched from draft to PR and added a test case. It successfully errors on |
|
Rebase, please. |
AdmiralCurtiss
changed the title
sepalani
force-pushed
the
bba-connecting
branch
from
104f15b
to
da86c2b
Compare
|
No one seems to want to give this a detailed review and this looks fine and has a hardware test, so I'll just merge it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While writing a hardware test, I found that
connectwas implemented as always being successful. The previous implementation assumed that the connection succeeded (whereas it hasn't yet). Then, a game might callsend/recvwhich will fail but will be reported as successful as well. So the game will assume the data is properly handled when in reality it wasn't since the socket wasn't properly connected yet. It also means sent data is lost since it was assumed as successful so the game has no way to know it failed.This PR defers the successful connect packet and depends on #12533 for the time being. Here is a test case (based on the previous hardware test) but with an unreachable IP address hardcoded instead of example's one: net_connect_failure.zip
Ready to be reviewed & merged.