RFR: JDK-8257235: [PATCH] InetAddress.isReachable: Try to use an IPPROTO_ICMP socket type before attempting RAW_SOCK [v2]

[Conor Cleary]

On Tue, 16 Feb 2021 13:31:13 GMT, Jamie Le Tual <github.com+55101029+jamieletual at openjdk.org> wrote:

>> src/java.base/unix/native/libnet/Inet6AddressImpl.c line 713:
>> 
>>> 711:     This usually requires "root" privileges, so it's likely to fail.
>>> 712:     If all else fails, fall back to TCP and implement tcp echo
>>> 713: */
>> 
>> This is one of the block comments that needs a tidy, same thing in Inet4Address.c. Also check the // comments and you'll see some of the inconsistencies there. It's just nit picking, the patch itself is good, just hard to test.
>
> OK, is it the fact that it's a block comment instead of a line comment? Is the indentation not right? 
> I don't see what needs to be cleaned up

I think what Alan said was in reference to the overall styling of the block comment. For example, something like this with more consistent spacing may be more appropriate (note the minimal spacing at the end of sentences where possible). Perhaps the block comment could be indented also to match the block of code its concerning.

/*
    First we see if the OS supports ICMP as a protocol. In the event 
    that this is an older kernel without IPPROTO_ICMP or that the
    net.ipv4.ping_group_range kernel parameter is not set, then we
    fall back to trying to create a RAW socket to send ICMP packets.
    This usually requires "root" privileges, so it's likely to fail.
    If all else fails, fall back to TCP and implement tcp echo
*/

Also, is there any scope for simplifying the comments a bit so that they could be shortened to 2-3 line comments? For example
    // Check OS supports ICMP. If IPPROTO_ICMP is not present or
    // the net.ipv4.ping_group_range kernel parameter is not set, fall
    // back to trying to create a RAW socket to send ICMP packets.
    // If all else fails, fall back to TCP and implement tcp echo.
The second comment is more of a suggestion concerning formatting and not what I think should be present. Having simple and concise comments is always beneficial for future maintenance in my opinion.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1502