calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <els...@apache.org>
Subject Re: New Defects reported by Coverity Scan for Apache Calcite
Date Thu, 03 Dec 2015 23:57:05 GMT
Yeah, I have already registered, Bosco.

I'd rather not become a SPOF though. If we send to the list, the load 
can be spread (these kinds of fixes are great intro-tasks for people).

Don Bosco Durai wrote:
> 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