[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-java] [PATCH 29/65] Implement equals and hashCode methods for Connect and Domain



On Thu, Feb 13, 2014 at 04:22:37PM +0100, Claudio Bley wrote:
> 
> Signed-off-by: Claudio Bley <cbley av-test de>
> ---
>  src/main/java/org/libvirt/Connect.java          |   35 +++++++++++++++++
>  src/main/java/org/libvirt/Domain.java           |   48 ++++++++++++++++++++++-
>  src/test/java/org/libvirt/TestJavaBindings.java |    4 ++
>  3 files changed, 86 insertions(+), 1 deletion(-)

NACK

> +    /* (non-Javadoc)
> +     * @see java.lang.Object#hashCode()
> +     */
> +    @Override
> +    public int hashCode() {
> +        final int prime = 31;
> +        int result = 1;
> +        result = prime * result + ((VCP == null) ? 0 : VCP.hashCode());
> +        return result;
> +    }
> +
> +    /* (non-Javadoc)
> +     * @see java.lang.Object#equals(java.lang.Object)
> +     */
> +    @Override
> +    public boolean equals(Object obj) {
> +        if (this == obj)
> +            return true;
> +        if (obj == null)
> +            return false;
> +        if (!(obj instanceof Connect))
> +            return false;
> +        Connect other = (Connect) obj;
> +        if (VCP == null)
> +            return (other.VCP == null);
> +        else if (VCP.equals(other.VCP))
> +            return true;
> +
> +        try {
> +            return getURI().equals(other.getURI());
> +        } catch (LibvirtException e) {
> +            throw new RuntimeException("libvirt error testing connect equality", e);
> +        }
> +    }

This causes a violation of the hashCode API contract. Per the
java api docs for java.lang.Object

   "If two objects are equal according to the equals(Object) method,
    then calling the hashCode method on each of the two objects must
    produce the same integer result."

If the equals method falls through to checking equality of the URIs
and returns true, the hashCode method is going to return different
results since they have different VCP pointers.

Either you have to remove the comparison of getURI in the equals
method, or make the hashCode method use the URI to build up the
hash code.

> diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java
> index 047ac33..ab6f9f0 100644
> --- a/src/main/java/org/libvirt/Domain.java
> +++ b/src/main/java/org/libvirt/Domain.java
> @@ -21,6 +21,8 @@ import com.sun.jna.NativeLong;
>  import com.sun.jna.Pointer;
>  import com.sun.jna.ptr.IntByReference;
>  
> +import java.util.Arrays;
> +
>  /**
>   * A virtual machine defined within libvirt.
>   */
> @@ -135,10 +137,52 @@ public class Domain {
>       */
>      DomainPointer VDP;
>  
> +    /* (non-Javadoc)
> +     * @see java.lang.Object#hashCode()
> +     */
> +    @Override
> +    public int hashCode() {
> +        final int prime = 31;
> +        int result = 1;
> +        result = prime * result + ((VDP == null) ? 0 : VDP.hashCode());
> +        result = prime * result
> +            + ((virConnect == null) ? 0 : virConnect.hashCode());
> +        return result;
> +    }
> +
> +    /* (non-Javadoc)
> +     * @see java.lang.Object#equals(java.lang.Object)
> +     */
> +    @Override
> +    public boolean equals(Object obj) {
> +        if (this == obj)
> +            return true;
> +        if (obj == null)
> +            return false;
> +        if (!(obj instanceof Domain))
> +            return false;
> +        Domain other = (Domain) obj;
> +
> +        // return false when this domain belongs to
> +        // a different hypervisor than the other
> +        if (!this.virConnect.equals(other.virConnect))
> +            return false;
> +
> +        if (VDP == null) return (other.VDP == null);
> +
> +        if (VDP.equals(other.VDP)) return true;
> +
> +        try {
> +            return Arrays.equals(getUUID(), other.getUUID());
> +        } catch (LibvirtException e) {
> +            throw new RuntimeException("libvirt error testing domain equality", e);
> +        }
> +    }

Again, violation of hashCode API contract due to use of UUID comparison


Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]