Messages in this thread | | | Date | Thu, 23 Apr 2009 07:34:02 +0200 | From | Jens Axboe <> | Subject | Re: [BUG] rbtree bug with mmotm 2009-04-14-17-24 |
| |
On Wed, Apr 22 2009, Steve Wise wrote: > Steve Wise wrote: >> Jens Axboe wrote: >> >> >> >>>> Still crashes with this variant: >>>> >>> >>> OK, so please try and revert a36e71f996e25d6213f57951f7ae1874086ec57e >>> and see if that works? >>> >>> Or, better yet, please try and revert >>> 55a63998b8967615a15e2211ba0ff3a84a565824 first. If that doesn't work, >>> try the above revert. >>> >>> >> >> Reverting 55a63998b8967615a15e2211ba0ff3a84a565824 did the trick. >> >> >> Steve. >> >> > > <snipit from 55a63998b8967615a15e2211ba0ff3a84a565824> > > @@ -200,17 +197,14 @@ static void __rb_erase_color(struct rb_node *node, > struct rb_node *parent, > { > if (!other->rb_left || > rb_is_black(other->rb_left)) > { > - register struct rb_node *o_right; > - if ((o_right = other->rb_right)) > - rb_set_black(o_right); > + rb_set_black(other->rb_right); > rb_set_red(other); > __rb_rotate_left(other, root); > other = parent->rb_left; > } > rb_set_color(other, rb_color(parent)); > rb_set_black(parent); > - if (other->rb_left) > - rb_set_black(other->rb_left); > + rb_set_black(other->rb_left); > __rb_rotate_right(parent, root); > node = root->rb_node; > break; > > > I don't know this code, but isn't the 'if (other->rb_left)' really > needed? Or is it always true that if '!other->rb_left' is true entering > this snipit, then after executing the first 'if' block, then > 'other->rb_left' must be a valid ptr? (how's that for confusing english? > :)
Heh, not sure. What is sure is that the commit in question is problematic. Either because it itself has a bug, or because it exposes a bug elsewhere in the rbtree code. So I'd suggest we just revert that commit ASAP.
-- Jens Axboe
| |