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 part 2: change Socket::new to only call socket(2) #110

Merged
merged 5 commits into from
Oct 8, 2020

Conversation

Thomasdezeeuw
Copy link
Collaborator

If socket2 is going to be a fully configuration socket(2) wrapper, then
the user needs to determine if things like setting CLOEXEC is desirable.
The new example shows the common case of setting all those flags.

We should consider adding a Socket::new_with_common_flags(), which would create a socket with all advisable flags set such as in the new example.

These are accessible using the From implementation.
@Thomasdezeeuw Thomasdezeeuw requested a review from a user October 8, 2020 11:58
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I agree it would be good to have a convenient constructor that sets all the advisable flags. Right now a lot of #[cfg(...)] attributes are needed to set them up properly on every platform. But that can be done later...

@Thomasdezeeuw
Copy link
Collaborator Author

Thomasdezeeuw commented Oct 8, 2020

@stjepang created #111 for such a function.

If socket2 is going to be a fully configuration socket(2) wrapper, then
the user needs to determine if things like setting CLOEXEC is desirable.
The new example shows the common case of setting all those flags.
@Thomasdezeeuw Thomasdezeeuw merged commit 5b6eaae into rust-lang:master Oct 8, 2020
@Thomasdezeeuw Thomasdezeeuw deleted the refactor-p2 branch October 8, 2020 12:38
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.

1 participant