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

Add SockOptValue and setSockOptValue #588

Conversation

FinleyMcIlwaine
Copy link
Contributor

@FinleyMcIlwaine FinleyMcIlwaine commented Sep 24, 2024

The motivation for this is to enable an interface in network-run similar to the open{Client,Server,TCPServer}SocketWithOptions that allows users to set multiple socket option values of potentially different value types, e.g. NoDelay and Linger. See the corresponding PR in network-run

FinleyMcIlwaine added a commit to FinleyMcIlwaine/network-run that referenced this pull request Sep 24, 2024
The existing interface did not allow one to set both `Linger` and `NoDelay`
socket options. With the `SocketOptionValue` introduced in [this `network`
PR](haskell/network#588), this is now possible.
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

I think that setSocketOptionValue is an extended version of setSockOpt.
So, the name should be setSockOptValue and SockOptValue.
The export list should be setSockOpt, SockOptValue and setSockOptValue in order.

@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/existential-socket-option-values branch from 3893a87 to 2c2a445 Compare September 25, 2024 14:16
@FinleyMcIlwaine
Copy link
Contributor Author

That makes sense to me, I've updated this!

@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/existential-socket-option-values branch from 2c2a445 to 39d6b8d Compare September 25, 2024 14:17
FinleyMcIlwaine added a commit to FinleyMcIlwaine/network-run that referenced this pull request Sep 25, 2024
The existing interface did not allow one to set both `Linger` and `NoDelay`
socket options. With the `SocketOptValue` introduced in [this `network`
PR](haskell/network#588), this is now possible.
@@ -481,6 +500,12 @@ instance Storable StructLinger where
(#poke struct linger, l_linger) p linger
#endif

-- | Existential socket option value
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel that "existential" is too academic to understand for many users.
How about this?
"A data type to hold several different socket option types such as Int and StructLinger".
Please feel free to fix my broken English.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, thanks, I was also a bit worried about that. What you wrote sounds great, I'll update this!

@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/existential-socket-option-values branch from 39d6b8d to 85a4b71 Compare September 26, 2024 14:17
kazu-yamamoto
kazu-yamamoto previously approved these changes Sep 27, 2024
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

I'm sorry for not noticing this yesterday.
Socket should be Sock.
I will merge this PR with this modifications.

@FinleyMcIlwaine
Copy link
Contributor Author

Whoops, sorry about that. Fixed!

See docs for `setSockOptValue`
@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/existential-socket-option-values branch from da15ef8 to f0b472e Compare September 27, 2024 00:13
@FinleyMcIlwaine FinleyMcIlwaine changed the title Add SocketOptionValue and setSocketOptionValue Add SockOptValue and setSockOptValue Sep 27, 2024
FinleyMcIlwaine added a commit to FinleyMcIlwaine/network-run that referenced this pull request Sep 27, 2024
The existing interface did not allow one to set both `Linger` and `NoDelay`
socket options. With the `SockOptValue` introduced in [this `network`
PR](haskell/network#588), this is now possible.
kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this pull request Sep 27, 2024
@kazu-yamamoto
Copy link
Collaborator

Merged.
Please check and close this PR if OK.

@FinleyMcIlwaine
Copy link
Contributor Author

Looks good to me, thank you!

@kazu-yamamoto
Copy link
Collaborator

A new version has been released.

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