Thank you very much for the review, JC. My comments inline:
On 1/8/2019 12:22 AM, JC Beyler wrote:
http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java.udiff.html
+ // by appending 'sudo' to it. Check now if sudo
passes in this
+ // environment with a simple 'echo' command.
-> Really shouldn't provide implementation details about what is
being done by that method; if we change that, this comment will rot :-)
canAddPrivileges is explicit enough in my mind
Done. I have removed the comment.
http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/lib/jdk/test/lib/SA/SATestUtils.java.html
-> You put some System.out.println; are they intended to still be
there or were they for debug?
I guess you meant this one:
System.out.println("Adding 'sudo -E' to the command.");
This one is meant to be there.
77 for (String val: cmdStringList) {
78 outStringList.add(val);
79 }
Couldn't we use addAll ?
Done.
The modified webrev is at:
http://cr.openjdk.java.net/~jgeorge/8215544/webrev.04/index.html
Thanks,
Jini.
