request for review: 6436034: Instance filter doesn't filter event if it occurs in native method

Keith McGuigan keith.mcguigan at oracle.com
Fri Dec 17 11:31:28 PST 2010


On Dec 17, 2010, at 1:43 PM, Daniel D. Daugherty wrote:
>
> - don't forget to update the copyright years in the various files
>
> - block comment style is different than existing code. Should be:
> /*
>  * ...
>  */

Fixed and fixed.

>
> src/share/back/debugInit.c
>   Yes, keep the JVM/TI version 0.x compatibility check. It doesn't
>   really cost much and you never know what crazy things we'll try
>   to do when triaging.

Ok thanks, I'll remove my question from the comments.

>   No content comments.
>
> src/share/back/eventFilter.c
>   Don't really need the 'else' on line 292.

Yes, unless we don't bother to check the return value of  
GetVersion().  I dont expect that will ever fail, but you never know.

> src/share/javavm/export/jvmti.h
>   Any idea why there is a new blank line at 2534? Will jcheck
>   complain about that?

I took the jvmti.h generated from a Hotspot build and did only what  
was needed to pass jcheck (mostly removal of spaces at the end of the  
line).

> test/com/sun/jdi/NativeInstanceFilter.java
>   Didn't review closely since I think you'll be tweaking the test.
>
> test/com/sun/jdi/NativeInstanceFilterTarg.java
>   Didn't review closely since I think you'll be tweaking the test.


Thanks for the review, Dan.

Updated webrev: http://cr.openjdk.java.net/~kamg/6436034/webrev.05

--
- Keith
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20101217/8c59a927/attachment.html 


More information about the serviceability-dev mailing list