Skip to contentSkip to conversation

UBI: fix rb_tree node comparison in add_map

RFC PATCH Applied Mike Snitzer 4 participants 5 messages Mar 21, 2014 – May 13, 2014

Subsystems (3)
Memory technology devices (MTD) Arm STM32 Unsorted block images (UBI)
lore
Share & tools
lore

Mike Snitzer

author
#0 lore raw

RFC PATCH 1/1 UBI: fix rb_tree node comparison in add_map

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 <[email protected]>
---
 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).

drivers/mtd/ubi/fastmap.c +2 -2
index ead8613..60c7a20 100644
@@ -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;
 	}
maintainer Acked-by on v1 1/1
#1 lore raw

Am 21.03.2014 20:54, schrieb Mike Snitzer:

Quoted 10 lines

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 <[email protected]>
---
 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 <[email protected]>

Thanks,
//richard

#2 lore raw

Hi Artem,

Quoted 9 lines · Richard Weinberger

Am 21.03.2014 20:54, schrieb Mike Snitzer:

Quoted 8 lines

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 <[email protected]>
---
 drivers/mtd/ubi/fastmap.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Quoted 6 lines
Quoted 2 lines

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 <[email protected]>

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

#3 lore raw
Quoted 17 lines · Brian Norris

Hi Artem,

Quoted 14 lines · Richard Weinberger

Am 21.03.2014 20:54, schrieb Mike Snitzer:

Quoted 10 lines

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 <[email protected]>
---
 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!

Quoted 7 lines
Quoted 1 line
Acked-by Richard Weinberger <[email protected]>

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.

Signature · 2 lines
Best Regards,
Artem Bityutskiy