<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Alex,<br>
<br>
It looks great and very solid in general!<br>
<br>
Some minor comments are below.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c.frames.html">http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c.frames.html</a><br>
<br>
<pre><span class="new"> 263 * Result must be release with dbgsysFreeAddrInfo.</span></pre>
 A typo: "must be release" => "must be released"<br>
<br>
<pre> 80 typedef struct {
<span class="changed"> 81 struct in6_addr subnet;</span>
<span class="changed"> 82 struct in6_addr netmask;</span>
83 } AllowedPeerInfo;
. . .
<span class="changed"> 431 parseAllowedPeersInternal(char *buffer) {
. . .</span>
<span class="changed"><span class="changed"> 483 }
. . .
</span></span><span class="changed"> 524 isPeerAllowed(struct sockaddr_storage *peer) {</span>
<span class="changed"> 525 struct in6_addr tmp;</span>
<span class="changed"> 526 struct in6_addr *addr6;</span>
<span class="changed"> 527 if (peer->ss_family == AF_INET) {</span>
<span class="changed"> 528 convertIPv4ToIPv6((struct sockaddr *)peer, &tmp);</span>
<span class="changed"> 529 addr6 = &tmp;</span>
<span class="changed"> 530 } else {</span>
<span class="changed"> 531 addr6 = &(((struct sockaddr_in6 *)peer)->sin6_addr);</span>
<span class="changed"> 532 }</span>
<span class="changed"> 533 </span>
<span class="changed"> 534 for (int i = 0; i < _peers_cnt; ++i) {</span>
<span class="changed"> 535 if (isAddressInSubnet(addr6, &(_peers[i].subnet), &(_peers[i].netmask))) {</span>
536 return 1;
537 }
538 }</pre>
 The allowed pears are converted into and used/checked in the IPv6
format.<br>
 Some short comments about it in all three places above would be
helpful.<br>
 I'd consider to do the same in parseAllowedAddr() before this
fragment:<br>
<pre><span class="changed"> 367 if (addrInfo->ai_family == AF_INET6) {</span>
<span class="changed"> 368 memcpy(result, &(((struct sockaddr_in6 *)(addrInfo->ai_addr))->sin6_addr), sizeof(*result));</span>
<span class="changed"> 369 *isIPv4 = 0;</span>
<span class="changed"> 370 } else { // IPv4 address</span>
<span class="changed"> 371 struct in6_addr addr6;</span>
<span class="changed"> 372 convertIPv4ToIPv6(addrInfo->ai_addr, &addr6);</span>
<span class="changed"> 373 memcpy(result, &addr6, sizeof(*result));</span>
<span class="changed"> 374 *isIPv4 = 1;</span>
375 }
</pre>
 and in parseAllowedMask() before the line:<br>
<pre><span class="changed"> 420 result->s6_addr[i] = (char)(0xFF << (8 - prefixLen));
</span></pre>
<br>
<pre><span class="changed"> This fragment needs a comment:
402 if (isIPv4) {</span>
<span class="changed"> 403 prefixLen += 96;</span>
404 }
</pre>
We have to find out at least one more reviewer for this fix!<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 5/3/19 18:14, <a class="moz-txt-link-abbreviated" href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:6d6a4b53-7841-197e-96b3-7770ca8bdff2@oracle.com">Hi
Alex,
<br>
<br>
Thank you for creating the CSR!
<br>
I've added myself as a reviewer and corrected a couple of places.
<br>
Feel free to change my changes if necessary. :)
<br>
It would be nice to get one more CSR review if possible, so I've
added the net-dev mailing list.
<br>
<br>
I hope to finish the webrev review soon.
<br>
<br>
Thanks,
<br>
Serguei
<br>
<br>
On 5/1/19 10:54 AM, Alex Menkov wrote:
<br>
<blockquote type="cite">Hi all,
<br>
<br>
I created CSR for the feature:
<br>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8223104">https://bugs.openjdk.java.net/browse/JDK-8223104</a>
<br>
<br>
The latest webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/">http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/</a>
<br>
<br>
--alex
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>