You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Leo Sutic <le...@inspireinfrastructure.com> on 2003/02/17 17:20:11 UTC
Bug in StaticBucketMap
I've found a bug in the StaticBucketMap class, the remove(Object)
method:
/**
* Implements {@link Map#remove(Object)}.
*/
public Object remove( Object key )
{
int hash = getHash( key );
synchronized( m_locks[ hash ] )
{
Node n = m_buckets[ hash ];
Node prev = null;
while( n != null )
{
HERE>>>>>>>>> if( n.key == null || ( n.key != null && n.key.equals(
key ) ) ) <<<<<<<<<<<<<<<<<<
{
// Remove this node from the linked list of nodes.
if( null == prev )
{
// This node was the head, set the next node to
be the new head.
m_buckets[ hash ] = n.next;
}
else
{
// Set the next node of the previous node to be
the node after this one.
prev.next = n.next;
}
m_locks[hash].size--;
return n.value;
}
prev = n;
n = n.next;
}
}
return null;
}
The test is:
if( n.key == null || ( n.key != null && n.key.equals(
key ) ) )
should be:
if( n.key == key || ( n.key != null && n.key.equals( key
) ) )
which is how it works in get(Object), containsKey(Object) etc. and which
is correct. We have a match if the keys match using == OR if they are
equal according to equals().
/LS
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] Bug in StaticBucketMap
Posted by Stephen Colebourne <sc...@btopenworld.com>.
I have made the change to CVS head, thanks
Stephen
----- Original Message -----
From: "Leo Sutic" <le...@inspireinfrastructure.com>
To: <co...@jakarta.apache.org>
Cc: "Avalon Developer's List" <de...@avalon.apache.org>
Sent: Monday, February 17, 2003 4:20 PM
Subject: Bug in StaticBucketMap
> I've found a bug in the StaticBucketMap class, the remove(Object)
> method:
>
> /**
> * Implements {@link Map#remove(Object)}.
> */
> public Object remove( Object key )
> {
> int hash = getHash( key );
>
> synchronized( m_locks[ hash ] )
> {
> Node n = m_buckets[ hash ];
> Node prev = null;
>
> while( n != null )
> {
> HERE>>>>>>>>> if( n.key == null || ( n.key != null && n.key.equals(
> key ) ) ) <<<<<<<<<<<<<<<<<<
> {
> // Remove this node from the linked list of nodes.
> if( null == prev )
> {
> // This node was the head, set the next node to
> be the new head.
> m_buckets[ hash ] = n.next;
> }
> else
> {
> // Set the next node of the previous node to be
> the node after this one.
> prev.next = n.next;
> }
> m_locks[hash].size--;
> return n.value;
> }
>
> prev = n;
> n = n.next;
> }
> }
>
> return null;
> }
>
> The test is:
>
> if( n.key == null || ( n.key != null && n.key.equals(
> key ) ) )
>
> should be:
>
> if( n.key == key || ( n.key != null && n.key.equals( key
> ) ) )
>
> which is how it works in get(Object), containsKey(Object) etc. and which
> is correct. We have a match if the keys match using == OR if they are
> equal according to equals().
>
> /LS
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@avalon.apache.org
For additional commands, e-mail: dev-help@avalon.apache.org
Re: [collections] Bug in StaticBucketMap
Posted by Stephen Colebourne <sc...@btopenworld.com>.
I have made the change to CVS head, thanks
Stephen
----- Original Message -----
From: "Leo Sutic" <le...@inspireinfrastructure.com>
To: <co...@jakarta.apache.org>
Cc: "Avalon Developer's List" <de...@avalon.apache.org>
Sent: Monday, February 17, 2003 4:20 PM
Subject: Bug in StaticBucketMap
> I've found a bug in the StaticBucketMap class, the remove(Object)
> method:
>
> /**
> * Implements {@link Map#remove(Object)}.
> */
> public Object remove( Object key )
> {
> int hash = getHash( key );
>
> synchronized( m_locks[ hash ] )
> {
> Node n = m_buckets[ hash ];
> Node prev = null;
>
> while( n != null )
> {
> HERE>>>>>>>>> if( n.key == null || ( n.key != null && n.key.equals(
> key ) ) ) <<<<<<<<<<<<<<<<<<
> {
> // Remove this node from the linked list of nodes.
> if( null == prev )
> {
> // This node was the head, set the next node to
> be the new head.
> m_buckets[ hash ] = n.next;
> }
> else
> {
> // Set the next node of the previous node to be
> the node after this one.
> prev.next = n.next;
> }
> m_locks[hash].size--;
> return n.value;
> }
>
> prev = n;
> n = n.next;
> }
> }
>
> return null;
> }
>
> The test is:
>
> if( n.key == null || ( n.key != null && n.key.equals(
> key ) ) )
>
> should be:
>
> if( n.key == key || ( n.key != null && n.key.equals( key
> ) ) )
>
> which is how it works in get(Object), containsKey(Object) etc. and which
> is correct. We have a match if the keys match using == OR if they are
> equal according to equals().
>
> /LS
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org