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

Reply via email to