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

Refactor Socket::process into Socket::would_accept and Socket::process_accepted #38

Closed
wants to merge 4 commits into from

Conversation

batonius
Copy link
Contributor

Problem: Currently the process methods both check whatever the packet can be processed by the socket and do the processing. While it makes sense at the moment, the eventual adoption of a packet dispatching mechanism will make the checks optional in some configurations, thus it's beneficial to split the methods in two preemptively. As a side effect, the refactoring will allow us to get rid of the Error::Rejected error, which doesn't represent a problem with a packet or a socket, but serves as a marker of a failed packet dispatching.

Solution: Split the process methods into would_accept and process_accepted.

Changes introduced by this pull request:

  • TcpSocket::process, UdpSocket::process and RawSocket::process have been refactored.
  • Tests for new methods have been added.
  • Error::Rejected has been removed as not needed anymore.

Other:
This PR is a part of #19 effort.

match raw_socket.process_accepted(&ip_repr, ip_payload) {
// The packet is valid and handled by socket.
Ok(()) => handled_by_raw_socket = true,
// Raw sockets can't fail to process an accepted packet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can if they're out of space.

@whitequark
Copy link
Contributor

Manually merged with stylistic changes.

@whitequark whitequark closed this Sep 8, 2017
@batonius batonius deleted the would_accept branch September 8, 2017 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants