Review Request: JDK-8157464: StackWalker.getCallerClass() is not

[serguei.spitsyn at oracle.com]

On 9/14/16 09:50, Mandy Chung wrote:
>
>> On Sep 14, 2016, at 1:18 AM, serguei.spitsyn at oracle.com 
>> <mailto: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
>>
>
> 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/ 
> <http://cr.openjdk.java.net/%7Emchung/jdk9/webrevs/8157464/webrev.03/>

This is better, thanks.


>>
>>   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.

Ok, thanks.

>
>>   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
>>
>>    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.

Ok.


>
>>    At least, one line of two empty lines L78, L79 is useless.
>
> I can take one empty line out.

Thanks,
Serguei

>
> Mandy
>