On Tue, 2011-05-17 at 09:41 +0200, Ingo Molnar wrote:
> * Sasha Levin <[email protected]> wrote:
> > +#include <kvm/interval-rbtree.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> 
> At first sight i dont think you really need the stdio.h and stlib.h includes 
> - 
> you added these while having debugging printfs in the code?

We can drop either of them, but not both. Added it for size_t
definition.

> > +int rb_int_insert(struct rb_root *root, struct rb_int_node *data)
> 
> i'd suggest to rename 'data' to i_node in other places as well. Here we'd 
> want 
> to use the name i_node_root i suspect.
> 
> (Note, naming it 'inode' would suck for us kernel developers :-)
> 
> > +   struct rb_node **new = &(root->rb_node), *parent = NULL;
> > +
> > +   while (*new) {
> > +           struct rb_int_node *this        = RB_INT(*new);
> 
> So the rb-node iterator is named 'new', while the rb-int-node iterator is 
> called 'this'? That does not make sense.

I actually took that bit from an example in Documentation/rbtree.txt of
how to write an insertion function :)

Maybe it's worth doing another rbtree.txt patch and cleaning up the
samples there?


-- 

Sasha.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to