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 <[email protected]> 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 <[email protected]> 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, "[email protected]" <[email protected]> >> 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 "[email protected]", click >>>https://scan.coverity.com/subscriptions/edit?email=bosco%40apache.org&token=368c97cfe2c99af46cce4f2657f16fe3 >>>
