diff mbox

[RFC] UBI: fix rb_tree node comparison in add_map

Message ID 20140321195403.GA32529@redhat.com
State Accepted
Headers show

Commit Message

Mike Snitzer March 21, 2014, 7:54 p.m. UTC
The comparisons used in add_vol() shouldn't be identical.  Pretty sure
the following is correct but it is completely untested.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/mtd/ubi/fastmap.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

NOTE: I stumbled upon this code while implementing some rb_tree code
      (and looking for some existing rb_tree code as a reference).

Comments

Richard Weinberger March 21, 2014, 11:56 p.m. UTC | #1
Am 21.03.2014 20:54, schrieb Mike Snitzer:
> The comparisons used in add_vol() shouldn't be identical.  Pretty sure
> the following is correct but it is completely untested.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/mtd/ubi/fastmap.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> NOTE: I stumbled upon this code while implementing some rb_tree code
>       (and looking for some existing rb_tree code as a reference).

Thanks a lot for pointing this out!

Acked-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard
Brian Norris May 10, 2014, 1:19 a.m. UTC | #2
Hi Artem,

On Sat, Mar 22, 2014 at 12:56:59AM +0100, Richard Weinberger wrote:
> Am 21.03.2014 20:54, schrieb Mike Snitzer:
> > The comparisons used in add_vol() shouldn't be identical.  Pretty sure
> > the following is correct but it is completely untested.
> > 
> > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > ---
> >  drivers/mtd/ubi/fastmap.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > NOTE: I stumbled upon this code while implementing some rb_tree code
> >       (and looking for some existing rb_tree code as a reference).
> 
> Thanks a lot for pointing this out!
> 
> Acked-by: Richard Weinberger <richard@nod.at>

Bump?

I'm passing through my queue, and I occasionally see some obvious,
reviewed fixes like this one. Typically, you get around to them
eventually, but you never know. Do you prefer the "bump"?

Brian
Artem Bityutskiy May 13, 2014, 9:08 a.m. UTC | #3
On Fri, 2014-05-09 at 18:19 -0700, Brian Norris wrote:
> Hi Artem,
> 
> On Sat, Mar 22, 2014 at 12:56:59AM +0100, Richard Weinberger wrote:
> > Am 21.03.2014 20:54, schrieb Mike Snitzer:
> > > The comparisons used in add_vol() shouldn't be identical.  Pretty sure
> > > the following is correct but it is completely untested.
> > > 
> > > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > > ---
> > >  drivers/mtd/ubi/fastmap.c |    4 ++--
> > >  1 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > NOTE: I stumbled upon this code while implementing some rb_tree code
> > >       (and looking for some existing rb_tree code as a reference).
> > 
> > Thanks a lot for pointing this out!
> > 
> > Acked-by: Richard Weinberger <richard@nod.at>
> 
> Bump?
> 
> I'm passing through my queue, and I occasionally see some obvious,
> reviewed fixes like this one. Typically, you get around to them
> eventually, but you never know. Do you prefer the "bump"?

I'll pick this now, thanks for pointing. And yes, a "bump" is helpful.
E.g., I missed this ones.
Artem Bityutskiy May 13, 2014, 10:48 a.m. UTC | #4
On Fri, 2014-03-21 at 15:54 -0400, Mike Snitzer wrote:
> The comparisons used in add_vol() shouldn't be identical.  Pretty sure
> the following is correct but it is completely untested.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>

Pushed to linux-ubifs.git, thanks!
diff mbox

Patch

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index ead8613..60c7a20 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -125,9 +125,9 @@  static struct ubi_ainf_volume *add_vol(struct ubi_attach_info *ai, int vol_id,
 		parent = *p;
 		av = rb_entry(parent, struct ubi_ainf_volume, rb);
 
-		if (vol_id > av->vol_id)
+		if (vol_id < av->vol_id)
 			p = &(*p)->rb_left;
-		else if (vol_id > av->vol_id)
+		else
 			p = &(*p)->rb_right;
 	}