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

[Chris Hegarty]

On Fri, 19 Feb 2021 14:53:36 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

>> src/java.base/unix/native/libnet/Inet4AddressImpl.c line 375:
>> 
>>> 373:         icmp->icmp_type = ICMP_ECHO;
>>> 374:         icmp->icmp_code = 0;
>>> 375:         // same result as downcasting the little-endian pid, although we are not longer
>> 
>> I don't think this is true. When downcasting the pid (which is at this stage in Nework/Big-Endian Order), the host order will be considered. Assuming that the downcast is to `uint16_t`, which it looks like is what icmp_id takes, the 16 least significant bits will be considered (if the host machine is little-endian of course). This is different from the 16 bit right shift. Here is some example output from a short C program I wrote (output is in hex to more easily demonstrate bitwise ops):
>> 
>> pid size: 4
>> int size: 4
>> 
>> getpid() testing:
>> pid   = be67f
>> htonl = 7fe60b00
>> cast  = b00
>>>> 16 = 7fe6
>
> On that note, is it ok to ignore any bits that are shifted out? For example, `7fe60b00 >> 16` results in  `00007fe6` while the trailing `0b00` is ignored.
> 
> Edit: Upon further analysis, the data encoded in the most significant bytes above reflects what was originally the least significant bytes of the `pid` and so I believe this to be the correct solution

Unless I'm mistaken, this shifting will result in a different ident (icmp_id) value when run in big-endian systems (which appears unintentional).  While it is largely not all that significant any more, I still feel that it would be simpler and more straightforward to just retain the ident value, and introduce a new local variable for the full-pid. ( the comparison of the reply will continue to check against the payload pid value, rather than that of the ident ).

Alternatively (and better still), retain the pid value and do the narrowing cast and htoXX at the use-sites.

The comment can be simplified also.

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

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