On Fri, 19 Feb 2021 14:53:36 GMT, Conor Cleary <ccle...@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
