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