RFR 8202171: Some oopDesc functions compare this with NULL

Harold David Seigel harold.seigel at oracle.com
Mon Jul 30 18:34:07 UTC 2018


Hi Coleen,

Thanks for the reviews!

Harold


On 7/30/2018 2:23 PM, coleen.phillimore at oracle.com wrote:
>
> This change looks good also.
> Thanks,
> Coleen
>
> On 7/25/18 2:56 PM, Harold David Seigel wrote:
>> Hi,
>>
>> Please review this updated webrev:
>>
>> http://cr.openjdk.java.net/~hseigel/bug_8202171.2/webrev/index.html
>>
>> This includes null checks when needed for callers to nonstatic 
>> oopDesc::print() and oopDesc::print_on() functions and changes the 
>> oopDesc verify() functions to static.
>>
>> Thanks, Harold
>>
>>
>> On 7/18/2018 8:44 AM, Harold David Seigel wrote:
>>> Hi Coleen, Kim,
>>>
>>> Thanks for your comments!
>>>
>>> I'll make the changes suggested by Coleen and put out a new webrev.
>>>
>>> Thanks, Harold
>>>
>>> On 7/17/2018 5:38 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>> Hi Harold,
>>>>
>>>> Looking at this change, I would like us to keep the nonstatic 
>>>> print() and print_on(outputStream*) functions because other 
>>>> Metadata and types within the jvm have these functions.  I think 
>>>> the few places where the oop can be NULL at the caller should be 
>>>> checked instead and remove the this == NULL check in the 
>>>> oopDesc::print_on() function.  Most places already do check for 
>>>> NULL.  The verify function seems fine to make a static member 
>>>> function though.
>>>>
>>>> I agree with Kim that there are other places where "this" is 
>>>> compared to NULL which shouldn't be done, and we should file 
>>>> separate RFEs to deal with them, specifically 
>>>> Method::is_valid_method() and 
>>>> Metadata::print_{value_}on_maybe_null() functions.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 7/16/18 3:24 PM, Harold David Seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this JDK-12 fix for bug JDK-8202171.  The fix 
>>>>> changes a few functions in oop.cpp into static functions to avoid 
>>>>> comparisons between 'this' and NULL.
>>>>>
>>>>> Open Webrev: 
>>>>> http://cr.openjdk.java.net/~hseigel/bug_8202171/webrev/index.html
>>>>>
>>>>> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8202171
>>>>>
>>>>> This fix was regression tested by running Mach5 tiers 1 and 2 
>>>>> tests and builds on Linux-X64, Windows, Solaris Sparc, and Mac OS 
>>>>> X, running tiers 3-5 tests on Linux-x64, and by running JCK-11 
>>>>> Lang and VM tests on Linux-x64.
>>>>>
>>>>> Thanks, Harold
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list