Mailing-List: contact dev-help@calcite.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@calcite.apache.org
MIME-Version: 1.0
In-Reply-To: <5B7DD02C-0E0F-4007-88D5-7F3D85C0DBDD@apache.org>
References: <56609508dcacd_2dd3885338366b5@ss1435.mail>
	<5B7DD02C-0E0F-4007-88D5-7F3D85C0DBDD@apache.org>
Date: Thu, 3 Dec 2015 14:59:47 -0800
Message-ID: 
 <CAPSgeETZ_-LgaC27ktUYUOj_XGDSHCwEJxTbBFMwYa55qY8pLQ@mail.gmail.com>
Subject: Re: FW: New Defects reported by Coverity Scan for Apache Calcite
From: Julian Hyde <jhyde@apache.org>
To: Bosco Durai <bdurai@hortonworks.com>, Josh Elser <elserj@apache.org>
Cc: dev@calcite.apache.org
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

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 doe=
s

  if (obj =3D=3D 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 =3D=3D obj) {
    return true;
  }

  if (obj =3D=3D null || !(obj instanceof RpcMetadataResponse)) {
    return false;
  }

  RpcMetadataResponse other =3D (RpcMetadataResponse) obj;
  if (serverAddress =3D=3D null) {
    if (other.serverAddress !=3D null) {
      return false;
    }
  }
  return serverAddress.equals(other.serverAddress);
}

there is a case to be made that

public boolean equals(Object obj) {
  return this =3D=3D obj
    || obj instanceof RpcMetadataResponse
    && Objects.equals(((RpcMetadataResponse) obj).serverAddress, serverAddr=
ess);
}

is more understandable as well as more concise.

Elsewhere in the file there is -- not written by you -- the following:

  if (null =3D=3D connectionId) {
    if (null !=3D 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 i=
t=E2=80=A6
>
> 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 Calci=
te 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: 31=
95 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: 31=
95 in org.apache.calcite.avatica.remote.Service$RpcMetadataResponse.equals(=
java.lang.Object)()
>>3189           RpcMetadataResponse other =3D (RpcMetadataResponse) obj;
>>3190           if (serverAddress =3D=3D null) {
>>3191             if (other.serverAddress !=3D 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/pro=
jects/apache-calcite?tab=3Doverview
>>
>>To manage Coverity Scan email notifications for "bosco@apache.org", click=
 https://scan.coverity.com/subscriptions/edit?email=3Dbosco%40apache.org&to=
ken=3D368c97cfe2c99af46cce4f2657f16fe3
>>