calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jh...@apache.org>
Subject Re: FW: New Defects reported by Coverity Scan for Apache Calcite
Date Thu, 03 Dec 2015 23:24:20 GMT
Intellij highlights several other bugs in that file: lines 398, 1036,
1246, 1326 (possible), 2266. Please fix these also.

On Thu, Dec 3, 2015 at 2:59 PM, Julian Hyde <jhyde@apache.org> 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