Review Request: JDK-8157464: StackWalker.getCallerClass() is not
Mandy Chung
mandy.chung at oracle.com
Wed Sep 14 16:50:18 UTC 2016
> On Sep 14, 2016, at 1:18 AM, serguei.spitsyn at oracle.com wrote:
>
>
> Hi Mandy,
>
> A couple of minor comments.
>
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.02/hotspot/src/share/vm/prims/stackwalk.cpp.frames.html
> <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.02/hotspot/src/share/vm/prims/stackwalk.cpp.frames.html>
The assert has been adjusted to make it clear in webrev.03 posted yesterday:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.03/
>
> Also, it seems that the condition at L146 needs a short comment to clarify that
> the (index == start_index) part is for optimization (is it correct?).
No, it’s not optimizaton. It checks the first caller calling SW::getCallerClass is not a caller-sensitive method.
> If this optimization is not that important then it could be removed for clarity.
>
>
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.02/jdk/test/java/lang/StackWalker/CallerSensitiveMethod/csm/jdk/test/CallerSensitiveTest.java.html <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.02/jdk/test/java/lang/StackWalker/CallerSensitiveMethod/csm/jdk/test/CallerSensitiveTest.java.html>
>
> 49 boolean sm = false;
> 50 if (args.length > 0 && args[0].equals("sm")) {
>
> Better to add a comment that "sm" means Security Manager but not "Sensitive Method" :).
>
sm is used in many tests as the variable name for security manager.
> At least, one line of two empty lines L78, L79 is useless.
I can take one empty line out.
Mandy
More information about the hotspot-runtime-dev
mailing list