From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sasha Levin <levinsasha928@gmail.com>
Subject: Re: [PATCH 1/2] kvm tools: Add interval red-black tree helper
Date: Tue, 17 May 2011 10:55:29 +0300
Message-ID: <1305618929.12150.23.camel@sasha>
References: <1305615515-13913-1-git-send-email-levinsasha928@gmail.com>
	 <20110517074115.GG22305@elte.hu>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: penberg@kernel.org, asias.hejun@gmail.com,
	prasadjoshi124@gmail.com, gorcunov@gmail.com, kvm@vger.kernel.org,
	john@jfloren.net
To: Ingo Molnar <mingo@elte.hu>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-bw0-f46.google.com ([209.85.214.46]:61373 "EHLO
	mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752565Ab1EQHze (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 17 May 2011 03:55:34 -0400
Received: by bwz15 with SMTP id 15so279988bwz.19
        for <kvm@vger.kernel.org>; Tue, 17 May 2011 00:55:33 -0700 (PDT)
In-Reply-To: <20110517074115.GG22305@elte.hu>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, 2011-05-17 at 09:41 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> 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.