Hi Daniil,

It looks okay to me in general.
But I'm puzzled with this part:

http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/src/jdk.jstatd/share/classes/sun/tools/jstatd/Jstatd.java.udiff.html
+            } else if (arg.startsWith("-r")) {
+                if (arg.compareTo("-r") != 0) {
+                    port = Integer.parseInt(arg.substring(2));
+                } else {
+                    argc++;
+                    if (argc >= args.length) {
+                        printUsage();
+                        System.exit(1);
+                    }
+                    rmiPort = Integer.parseInt(args[argc]);
+                }

The option -r is for rmi connection port number.
Why does this code set the RMI registry port? :
+                if (arg.compareTo("-r") != 0) {
+                    port = Integer.parseInt(arg.substring(2));

Thanks,
Serguei


On 1/31/20 13:08, Daniil Titov wrote:
Please review change [1] that adds a new command line option to jstatd tool to specify a RMI connector port.

Currently a random port is used that prevents this tool from being used behind a firewall or in a container.

New CSR [3] was created for this change and it needs to be reviewed as well.

Man pages for jstatd will be updated in a separate issue.

Testing: Mach5 tier1-tier3 and sun/tools/jstatd/* tests  succeeded. Mach5 tier5 tests are in progress.
 
[1] Webrev: http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/  
[2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196729  
[3] CSR : https://bugs.openjdk.java.net/browse/JDK-8238357

Thank you,
Daniil




Reply via email to