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

Reply via email to