Re: [PATCH] Fix for Hashtable contains blowing up the stack

[Mark Wielaard]

Hi,

On Fri, 2003-11-21 at 20:48, Dalibor Topic wrote:
> > this is a typical java library bug, and I've both produced and fixed a 
> > couple of those myself ;) You get an infinite recursion through calling 
> > overridden methods. Class A in library implements public methods b and 
> > c, where b internally (and of course undocumentedly ;) calls c. Class B 
> > from some user of the library extends A and overrides c to call b. When 
> > the user calls the method b of class B it goes straight into an infinite 
> > loop.

Ugh. That is nasty. Especially since when you override Hashtable it is a
natural thing to do since contains and containsValue are specced to do
the same thing.

I created a small Mauve test for this
gnu.testlet.java.util.Hashtable.HashContains.

> Yeah, the ChangeLog doesn;t look Classpath-ish enough.
> 
> 2003-11-21  Dalibor Topic <address@hidden>
> 
>           * libraries/javalib/java/util/Hashtable.java
>           (internalcontainsValue): new method.
>           (contains, containsValue) delegate to internalContainsValue.
> 
>           Reported by: Jim Pick <address@hidden>

Almost :)
Reported by goes on top.
Entires are full sentences starting with a capital.

2003-11-21  Dalibor Topic <address@hidden>

          Reported by: Jim Pick <address@hidden>
          * libraries/javalib/java/util/Hashtable.java
          (internalcontainsValue): New method.
          (contains, containsValue): Delegate to internalContainsValue.

________________________________________________________________________
> --- /var/tmp/PROJECTS/classpath//./java/util/Hashtable.java   Wed Nov 12 
> 21:56:20 2003
> +++ /tmp/topic/kaffe/libraries/javalib/java/util/Hashtable.java       Fri Oct 
> 10 18:49:09 2003
> @@ -333,7 +333,10 @@
>     */
>    public synchronized boolean contains(Object value)
>    {
> -    return containsValue(value);
> +    /* delegate to non-overridable worker method 
> +     * to avoid blowing up the stack.
> +     */
> +    return internalContainsValue(value);
>    }

To be more clear, please add 'blowing up the stack, when called from
overridden contains[Value]() method'.

>    /**
> @@ -349,6 +352,25 @@
>     * @since 1.2
>     */
>    public boolean containsValue(Object value)
> +  {
> +    /* delegate to non-overridable worker method 
> +     * to avoid blowing up the stack.
> +     */
> +    return internalContainsValue(value);
> +  }

Likewise.

Feel free to try out your new cvs commit powers to check this in
yourself.

Thanks,

Mark

signature.asc
Description: This is a digitally signed message part