[kaffe] Bug in ThreadGroup

[Dalibor Topic]

Hi Hervé,

--- Hervé_Roussain <Herve.Roussain@univ-ubs.fr> wrote:
> Hi!
> 
> The 'parentOf' method doesn't work

thanks for the bug report and the fix.

> The method is
> final public boolean parentOf(ThreadGroup g) {
>          return ((parent == g) ||
> (parentOf(g.getParent())));
> }
> 
> but should be something like this:
> final public boolean parentOf(ThreadGroup g) {
>          return (g != null && this == g.getParent())
>                  || parentOf(g.getParent());
> }

I've expanded your test class a bit to cover two
fringe cases as well: null, and self.

class ThreadGroupTest {
  public static void main(String[] args) {
    ThreadGroup daddy = new ThreadGroup("daddy");
    ThreadGroup child = new ThreadGroup(daddy,
"child");
    System.out.println("daddy.parentOf(child)=" +
daddy.parentOf(child));
    System.out.println("child.parentOf(daddy)=" +
child.parentOf(daddy));
    System.out.println("child.parentOf(child)=" +
child.parentOf(child));
    System.out.println("child.parentOf(null)=" +
child.parentOf(null));
  }
}

I think that your fix fails the null test and the self
test.

A better fix is:

final public boolean parentOf(ThreadGroup g) {
	return (g != null)
	  && ((this == g) ||(parentOf(g.getParent())));
}

which passes all four tests.

By the way, Classpath developers didn't implement the
method in a recursive fashion. In my opinion their
approach to walk the parent group chain in a loop is
better, as it does all the work within a single stack
frame. But that's a matter of taste, I guess.

cheers,
dalibor topic

__________________________________________________
Do you Yahoo!?
Yahoo! Tax Center - File online, calculators, forms, and more
http://tax.yahoo.com