calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Don Bosco Durai <bo...@apache.org>
Subject Re: New Defects reported by Coverity Scan for Apache Calcite
Date Thu, 03 Dec 2015 23:35:28 GMT
Josher

If you are a committer in Calcite, then you can register yourself at https://scan.coverity.com/projects/apache-calcite?tab=overview.
The scans are done twice a week and if there are any issues found, then the email is automatically
sent.

Bosco





On 12/3/15, 3:21 PM, "Josh Elser" <elserj@apache.org> wrote:

>Alright, will do. Thanks for bringing it up.
>
>Do we want to just set up Coverity so that we get these emails directly?
>
>Julian Hyde wrote:
>> Bosco,
>>
>> Yes, this is a problem. Thanks for forwarding.
>>
>> Josh,
>>
>> You made this change yesterday. Can you please fix?
>>
>> I see quite a few examples in that file that should be boilerplate. Why does
>>
>>    if (obj == null || !(obj instanceof RpcMetadataResponse)) {
>>      return false;
>>    }
>>
>> depart from
>>
>>    if (!(obj instanceof RpcMetadataResponse)) {
>>      return false;
>>    }
>>
>> which is sufficient and is used elsewhere in the file? Boring stuff
>> should look boring.
>>
>> In fact the whole method could be simplified. Rather than
>>
>> public boolean equals(Object obj) {
>>    if (this == obj) {
>>      return true;
>>    }
>>
>>    if (obj == null || !(obj instanceof RpcMetadataResponse)) {
>>      return false;
>>    }
>>
>>    RpcMetadataResponse other = (RpcMetadataResponse) obj;
>>    if (serverAddress == null) {
>>      if (other.serverAddress != null) {
>>        return false;
>>      }
>>    }
>>    return serverAddress.equals(other.serverAddress);
>> }
>>
>> there is a case to be made that
>>
>> public boolean equals(Object obj) {
>>    return this == obj
>>      || obj instanceof RpcMetadataResponse
>>      &&  Objects.equals(((RpcMetadataResponse) obj).serverAddress, serverAddress);
>> }
>>
>> is more understandable as well as more concise.
>>
>> Elsewhere in the file there is -- not written by you -- the following:
>>
>>    if (null == connectionId) {
>>      if (null != other.connectionId) {
>>        return false;
>>      }
>>    } else if (!connectionId.equals(other.connectionId)) {
>>      return false;
>>    }
>>
>>    return true;
>>
>> It works, but not obviously so.
>>
>> Julian
>>
>>
>> On Thu, Dec 3, 2015 at 11:21 AM, Bosco Durai<bdurai@hortonworks.com>  wrote:
>>>> 3195           return serverAddress.equals(other.serverAddress);
>>> Good catch. I know you are monitoring it. Just wanted double sure about it…
>>>
>>> Thanks
>>>
>>> Bosco
>>>
>>>
>>>
>>>
>>> On 12/3/15, 11:16 AM, "scan-admin@coverity.com"<scan-admin@coverity.com>
 wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find the latest report on new defect(s) introduced to Apache Calcite
found with Coverity Scan.
>>>>
>>>> 1 new defect(s) introduced to Apache Calcite found with Coverity Scan.
>>>>
>>>>
>>>> New defect(s) Reported-by: Coverity Scan
>>>> Showing 1 of 1 defect(s)
>>>>
>>>>
>>>> ** CID 120367:  Null pointer dereferences  (FORWARD_NULL)
>>>> /avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java: 3195
in org.apache.calcite.avatica.remote.Service$RpcMetadataResponse.equals(java.lang.Object)()
>>>>
>>>>
>>>> ________________________________________________________________________________________________________
>>>> *** CID 120367:  Null pointer dereferences  (FORWARD_NULL)
>>>> /avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java: 3195
in org.apache.calcite.avatica.remote.Service$RpcMetadataResponse.equals(java.lang.Object)()
>>>> 3189           RpcMetadataResponse other = (RpcMetadataResponse) obj;
>>>> 3190           if (serverAddress == null) {
>>>> 3191             if (other.serverAddress != null) {
>>>> 3192               return false;
>>>> 3193             }
>>>> 3194           }
>>>>>>>      CID 120367:  Null pointer dereferences  (FORWARD_NULL)
>>>>>>>      Calling a method on null object "serverAddress".
>>>> 3195           return serverAddress.equals(other.serverAddress);
>>>> 3196         }
>>>> 3197       }
>>>> 3198     }
>>>> 3199
>>>>
>>>>
>>>> ________________________________________________________________________________________________________
>>>> To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/apache-calcite?tab=overview
>>>>
>>>> To manage Coverity Scan email notifications for "bosco@apache.org", click
https://scan.coverity.com/subscriptions/edit?email=bosco%40apache.org&token=368c97cfe2c99af46cce4f2657f16fe3
>>>>
>


Mime
View raw message