Yeah, that's my common interaction with Coverity.
IMO, I'd prefer setting up something like findbugs and running that at
build time (alongside checkstyle) for static analysis. But, that's just
me not wanting to be tied to a specific IDE.
Julian Hyde wrote:
We could, I suppose. I receive and read the emails, and previously
everything Coverity has found has been a false positive, so it's
probably wasting people's time.
More useful would be for everyone to switch to Intellij and manually
check every yellow-barred piece of code in each file they are editing.
(Or enable the equivalent feature in Eclipse.)
Intellij found 5 bugs in that file. The 4 other things it found (two
spurious "public"s, an exception being used but not thrown, and an
unnecessary box) are all things I would have fixed too, in the
interests of making boring code look boring.
On Thu, Dec 3, 2015 at 3:21 PM, Josh Elser<[email protected]> 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<[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
