On Fri, 5 Apr 2024 11:49:25 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review of this doc-only change which proposes to clean up >> the documentation of `java.net.SocketOptions` interface? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8329733, the existing >> documentation in this legacy interface isn't accurate. The commit in this >> PR, updates the documentation on several of the fields to point to the newer >> java.net.StandardSocketOptions class. While at it, I also did a general >> update of this documentation to use snippets and also reword some of it to >> be a bit more clear. >> >> I have run `make docs-image` on this change and the updated doc looks fine >> to me. > > Jaikiran Pai has updated the pull request incrementally with three additional > commits since the last revision: > > - "timeout value" instead of "timeout" > - missed SO_TIMEOUT review suggestion in previous commit > - Alan's review suggestions src/java.base/share/classes/java/net/SocketOptions.java line 192: > 190: > 191: /** > 192: * See {@link StandardSocketOptions#SO_LINGER} for description of > this socket option. One thing to mention about SO_LINGER is that the value of this socket option is an Integer when enabling and a Boolean when disabling. The getter works the same way. AFAIK, this was never correctly specified in SocketOptions.SO_LINGER but the examples in the setOption/getOption API docs include disabling. To notice this would require something to use a custom SocketImpl of course. So I think we need a follow-up to the change here to say that the value is Boolean.FALSE to disable (or disabled), otherwise an Integer with the linger timeout. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18645#discussion_r1554833351