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

Daniel D. Daugherty daniel.daugherty at oracle.com
Fri Dec 17 12:40:33 PST 2010


On 12/17/2010 12:31 PM, Keith McGuigan wrote:
>
> On Dec 17, 2010, at 1:43 PM, Daniel D. Daugherty wrote:
>> 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.

Sorry for not being clear. Didn't mean the entire else block.

   if (foo) {
       <some code>
       return some_passing_status;
   }
   return failing_status;


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

Thanks for letting me know that it passes jcheck.


>
>> 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 
> <http://cr.openjdk.java.net/%7Ekamg/6436034/webrev.05>

Thumbs up on the latest review.

Dan



More information about the serviceability-dev mailing list