RFR 8202171: Some oopDesc functions compare this with NULL

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Jul 30 18:23:25 UTC 2018


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