code review round 0 for JVM/TI version fix (6849968) and phase assertion (6648438)
Kelly O'Hair
Kelly.Ohair at Sun.COM
Fri Dec 11 15:26:18 PST 2009
Looks good to me, although I confess to missing the missing else. :^(
-kto
Daniel D. Daugherty wrote:
> David Holmes - Sun Microsystems wrote:
>> Hi Dan,
>>
>> In jvmtiEnv.cpp:
>>
>> 410 return JVMTI_ERROR_NONE;
>> 411 } if (use_version_1_0_semantics()) {
>>
>> I assume stylistically that was meant to be an "else if".
>
> Yup. You won't believe how many times I read the webrev
> before sending it out and still missed that... :-)
>
>
>> Otherwise this seems sound to me.
>
> Thanks for the very fast review!
>
> Dan
>
>
>>
>> David
>>
>> Daniel D. Daugherty said the following on 12/12/09 08:52:
>>> Greetings,
>>>
>>> I have fixes for the following two JVM/TI bugs ready to go:
>>>
>>> 6648438 4/4 src/share/vm/prims/jvmtiEnv.cpp:457
>>> assert(phase == JVMTI_PHASE_LIVE,"sanity check")
>>> 6849968 3/3 JVMTI tests fails on jdk5.0 with hs14
>>>
>>> 6849968 fixes a problem where JVM/TI version 1.0 semantics
>>> were not being used when that version was explicitly
>>> requested. I fixed 6648438 because it happened to be in the
>>> same place that I already had to tweak for 6849968. It also
>>> gets rid of an annoying intermittent fastdebug failure.
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/batch-20091211-webrev/0/
>>>
>>> This webrev is relative to the OpenJDK6 version of the code.
>>> Once approved, these fixes will be pushed to:
>>>
>>> - OpenJDK6 (HSX-14-??)
>>> - JDK6-Update train (HSX-16-??)
>>> - OpenJDK7 (HSX-17-??)
>>>
>>> Comments, questions and suggestions are always welcome!
>>>
>>> Dan
More information about the serviceability-dev
mailing list