This repository has been archived by the owner on Jul 31, 2021. It is now read-only.
forked from AdoptOpenJDK/openjdk-jdk11u
-
Notifications
You must be signed in to change notification settings - Fork 8
FreeBSD/socket: user cookie socket option support. #38
Open
devnexen
wants to merge
2
commits into
battleblow:bsd-port
Choose a base branch
from
devnexen:fbsd_so_user_cookie
base: bsd-port
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...