RFR: 7148497: javax.management.MBeanAttributeInfo.hashCode throws NullPointerException

[Karen Kinnear]

Frederic,

Looks good. And I like the refactoring.

A question - does the Descriptor field have the same potential for being null? e.g.
in MBeanFeatureInfo.java new line 160 - do you need to check getDescriptor() == null as well?
in MBeanConstructorinfo.java line 197, do you need the alternative null checking?
MBeanAttributeInfo.java line 293?
(there may be other lines, I didn't do a thorough search)

thanks,
Karen



On Apr 13, 2012, at 9:01 AM, Staffan Larsen wrote:

> This is much easier to read!
> 
> In MBeanFeatureInfo.hasSameName() this check is not needed:
> 
> 274         if(info.getName() == null) {
> 275            return false;
> 276         }
> 
> since String.equals() with a null argument works as expected (return false). The same is true for the implementation in hasSameDescription().
> 
> When calling hasSameName() and hasSameDescription() the "this." is not needed, but perhaps using it makes the intent clearer?
> 
> Thanks,
> /Staffan
> 
> 
> 
> 
> On 13 apr 2012, at 14:45, Frederic Parain wrote:
> 
>> Thanks Staffan and Dmitry for your feedback,
>> here's a new webrev with a refactored code for
>> name and descriptions fields which are common to
>> most MBean*Info classes:
>> 
>> http://cr.openjdk.java.net/~fparain/7148497/webrev.01/
>> 
>> Thanks,
>> 
>> Fred
>> 
>> On 4/12/12 8:18 AM, Dmitry Samersoff wrote:
>>> Frederic,
>>> 
>>> I'm second to Staffan.
>>> 
>>> Looks good to me but would prefer to have it refactored, e.g. create a
>>> method equalOrNull();
>>> 
>>> -Dmitry
>>> 
>>> 
>>> On 2012-04-10 18:33, Staffan Larsen wrote:
>>>> Looks good! (Although I wish there was a better way to write code like
>>>> this).
>>>> 
>>>> /Staffan
>>>> 
>>>> On 10 apr 2012, at 15:50, Frederic Parain wrote:
>>>> 
>>>>> Greetings,
>>>>> 
>>>>> This is a request for review for CR
>>>>> 7148497: javax.management.MBeanAttributeInfo.hashCode throws
>>>>> NullPointerException
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7148497
>>>>> 
>>>>> Even if the initial CR only refers to MBeanAttributeInfo.hashCode(),
>>>>> the problem is not limited to the MBeanAttributeInfo.hashCode(), but
>>>>> is common to all javax.management.MBean*Info classes:
>>>>> MBeanAttributeInfo, MBeanConstructorInfo, MBeanFeatureInfo,
>>>>> MBeanInfo, MBeanNotificationInfo, MBeanOperationInfo and
>>>>> MBeanParameterInfo.
>>>>> 
>>>>> The root cause of the problem is that these classes can be
>>>>> instantiated with some null fields (mainly, but not limited to, the
>>>>> name and type fields), and these fields are dereferenced
>>>>> unconditionally in some methods. Unconditional dereferencing is used
>>>>> in the hashCode method but also in the equals() method.
>>>>> 
>>>>> The proposed fix improves implementation of equals() and hashCode()
>>>>> method to handle cases where some fields are null without throwing
>>>>> a NPE.
>>>>> 
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~fparain/7148497/webrev.00/
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Fred
>>>>> 
>>>>> --
>>>>> Frederic Parain - Oracle
>>>>> Grenoble Engineering Center - France
>>>>> Phone: +33 4 76 18 81 17
>>>>> Email: Frederic.Parain at oracle.com
>>>>> 
>>>> 
>>> 
>>> 
>> 
>> -- 
>> Frederic Parain - Oracle
>> Grenoble Engineering Center - France
>> Phone: +33 4 76 18 81 17
>> Email: Frederic.Parain at oracle.com
>> 
>