Skip to content
This repository has been archived by the owner on Jul 31, 2021. It is now read-only.

FreeBSD/socket: user cookie socket option support. #38

Open
wants to merge 2 commits into
base: bsd-port
Choose a base branch
from

Conversation

devnexen
Copy link

giving here an identifier to the socket for DTrace's context,
ipfw filter and so on.

giving here an identifier to the socket for DTrace's context,
ipfw filter and so on.
Copy link
Owner

@battleblow battleblow left a comment

Choose a reason for hiding this comment

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

Thanks! Code change looks good to me. I'd like to read a little about this socket option before merging. I'm going to be away a few days, so will get back to this Wednesday/Thursday next week. Sorry for the delay.

@@ -76,6 +76,7 @@ int getTcpKeepAliveIntvl(int fd) throws SocketException {
private static native int getTcpKeepAliveTime0(int fd) throws SocketException;
private static native int getTcpKeepAliveIntvl0(int fd) throws SocketException;
private static native boolean keepAliveOptionsSupported0();
private static native void setUserCookie(int fd, int cookie) throws SocketException;
Copy link
Owner

Choose a reason for hiding this comment

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

I might have been a bit hasty earlier. Please allow me to make some further comments now that I've had a longer look at it.

A minor comment is that all other native methods in this class end with 0. This is mostly because the non-native versions of the methods which call them take up the same name without the 0, but I think it's reasonable to want to be consistent. However, this leads me to the larger concern.

The method you're adding here is a private method. Since it isn't exposed through the public methods of the class it can never be called (other than internally, which is not done in this change). I question the value of adding code that can't actually be called (note that I am ignoring byte manipulation hacks that some mocking packages, for instance, employ that might allow that). Note that I'm not suggesting here that you immediately go and add a public method to call into this. What I'm suggesting is a discussion of how developers may want to utilise this and what changes might make sense to allow for that.

Copy link
Author

Choose a reason for hiding this comment

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

Yes in fact I wanted to test the "appeal" of this idea first and also to see if this is better to call it indirectly from another higher java class or since it is pretty "unique feature" to make it simply public finally...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants