On Fri, 5 Mar 2021 15:08:03 GMT, Michael McMahon <[email protected]> wrote:
>> Jayashree S Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code Review: Incorporated Indention correction, added getIP and updated
>> try-catch inside for loop
>
> src/java.base/share/classes/java/net/SocketPermission.java line 706:
>
>> 704: for (String cname : cnames) {
>> 705: if (Objects.nonNull(cname) && match(cname, hname)) {
>> 706: return true;
>
> Sorry, forgot to mention, I think I'd prefer just to have:
> if (cname != null && match(..)))
>
> I think Objects.nonNull exists more for use as a method reference.
Done.
> test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 32:
>
>> 30: } while (testPass <= 2);
>> 31: }
>> 32:
>
> Formatting looks off in the file. The do statement in 22 should be aligned
> with SocketPermission in line 120.
>
> The implementation changes look fine.
Corrected the alignment.
> test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 18:
>
>> 16: String hostsFileName = System.getProperty("test.src", ".") +
>> File.separator + "Host.txt";
>> 17: System.setProperty("jdk.net.hosts.file", hostsFileName);
>> 18:
>
> It's better to specify this system property on the command line because (we
> discovered recently) there are some environments where the name service gets
> initialized before main() is called in which case setting the property here
> is too late. So, you need to add a -Djdk.net.hosts.file=${test.src}....
Removed the system property and added this as a command-line option.
> test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 6:
>
>> 4: * @summary SocketPermission implies(Permission p) spec allows, If the
>> object was initialized with a single IP address and one of p's IP addresses
>> is equal to this object's IP addr
>> 5: * @run java -Dsun.net.inetaddr.ttl=0 SocketPermissionIm
>> 6: */
>
> Instead of:
> @run java
> you need:
> @run main/othervm
>
> Also see further comment below.
Done
-------------
PR: https://git.openjdk.java.net/jdk/pull/1916
