RFR 8208399: Metadata methods print_(value_)on_maybe_null() compare 'this' to NULL

David Holmes david.holmes at oracle.com
Mon Jul 30 21:52:48 UTC 2018


On 31/07/2018 4:16 AM, Harold David Seigel wrote:
> Thanks Gerard!
> 
> I'll make that change before pushing the fix.

Hmmm is that part of our coding guidelines? It is not something we 
generally follow. Personally I've never liked the "if (<constant> == 
variable)" form as it just reads wrong to me.

David

> Harold
> 
> 
> On 7/30/2018 2:05 PM, Gerard Ziemski wrote:
>> Looks good.
>>
>> In src/hotspot/share/oops/metadata.hpp tiny nitpick - I was once told 
>> that instead of:
>>
>> +    if (m == NULL)
>>
>> we should be using:
>>
>> +    if (NULL == m)
>>
>> but I guess the entire file would have to use that syntax.
>>
>>
>>
>> cheers
>>
>>> On Jul 27, 2018, at 12:15 PM, Harold David Seigel 
>>> <harold.seigel at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> Please review this JDK-12 fix for bug JDK-8208399.  The fix changes 
>>> function print_value_on_maybe_null() into a static function to avoid 
>>> comparisons between 'this' and NULL.  It also deletes unused methods 
>>> print_maybe_null() and print_on_maybe_null().
>>>
>>> Open Webrev: 
>>> http://cr.openjdk.java.net/~hseigel/bug_8208399/webrev/index.html
>>>
>>> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8208399
>>>
>>> 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