code review round 0 for JVM/TI version fix (6849968) and phase assertion (6648438)

Daniel D. Daugherty Daniel.Daugherty at Sun.COM
Fri Dec 11 15:31:35 PST 2009


Thanks for the fast review, Kelly!

Dan


Kelly O'Hair wrote:
> 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