calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <els...@apache.org>
Subject Re: FW: New Defects reported by Coverity Scan for Apache Calcite
Date Thu, 03 Dec 2015 23:21:51 GMT
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