CORE: Use list_for_each_entry[_safe] where appropriate.

PATCH In review Chris Malley 2 participants 7 messages May 17, 2008 – May 19, 2008 Memory technology devices (MTD)

lore

Share & tools

lore

Chris Malley

author

2008-05-17 17:54 UTC #0 lore raw

PATCH 1/1 CORE: Use list_for_each_entry[_safe] where appropriate.

From: Chris Malley <[email protected]>

Should be no functional changes, just a bit of janitorial work to remove temporary pointers and make some functions a bit more readable.

Signed-off-by Chris Malley <[email protected]>

---
Tested on 2.6.26-rc2 with mtdram but would appreciate someone testing on a partition-capable device (this is my first MTD patch). Also applies cleanly to latest MTD git.

 drivers/mtd/mtd_blkdevs.c |   43 ++++++++++++++++++-------------------------
 drivers/mtd/mtdcore.c     |   14 ++++++--------
 drivers/mtd/mtdpart.c     |   24 ++++++++----------------
 3 files changed, 32 insertions(+), 49 deletions(-)

drivers/mtd/mtd_blkdevs.c +18 ~~-25~~

index 839eed8..895813b 100644
@@ -212,7 +212,7 @@ static struct block_device_operations mtd_blktrans_ops = {
 int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 {
 	struct mtd_blktrans_ops *tr = new->tr;
-	struct list_head *this;
+	struct mtd_blktrans_dev *this;
 	int last_devnum = -1;
 	struct gendisk *gd;

@@ -221,25 +221,24 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 		BUG();
 	}
 
-	list_for_each(this, &tr->devs) {
-		struct mtd_blktrans_dev *d = list_entry(this, struct mtd_blktrans_dev, list);
+	list_for_each_entry(this, &tr->devs, list) {
 		if (new->devnum == -1) {
 			/* Use first free number */
-			if (d->devnum != last_devnum+1) {
+			if (this->devnum != last_devnum+1) {
 				/* Found a free devnum. Plug it in here */
 				new->devnum = last_devnum+1;
-				list_add_tail(&new->list, &d->list);
+				list_add_tail(&new->list, &this->list);
 				goto added;
 			}
-		} else if (d->devnum == new->devnum) {
+		} else if (this->devnum == new->devnum) {
 			/* Required number taken */
 			return -EBUSY;
-		} else if (d->devnum > new->devnum) {
+		} else if (this->devnum > new->devnum) {
 			/* Required number was free */
-			list_add_tail(&new->list, &d->list);
+			list_add_tail(&new->list, &this->list);
 			goto added;
 		}
-		last_devnum = d->devnum;
+		last_devnum = this->devnum;
 	}
 	if (new->devnum == -1)
 		new->devnum = last_devnum+1;

@@ -309,14 +308,11 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 
 static void blktrans_notify_remove(struct mtd_info *mtd)
 {
-	struct list_head *this, *this2, *next;
-
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
-		list_for_each_safe(this2, next, &tr->devs) {
-			struct mtd_blktrans_dev *dev = list_entry(this2, struct mtd_blktrans_dev, list);
+	struct mtd_blktrans_ops *tr;
+	struct mtd_blktrans_dev *dev, *next;
 
+	list_for_each_entry(tr, &blktrans_majors, list) {
+		list_for_each_entry_safe(dev, next, &tr->devs, list) {
 			if (dev->mtd == mtd)
 				tr->remove_dev(dev);
 		}

@@ -325,15 +321,13 @@ static void blktrans_notify_remove(struct mtd_info *mtd)
 
 static void blktrans_notify_add(struct mtd_info *mtd)
 {
-	struct list_head *this;
+	struct mtd_blktrans_ops *this;
 
 	if (mtd->type == MTD_ABSENT)
 		return;
 
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
-		tr->add_mtd(tr, mtd);
+	list_for_each_entry(this, &blktrans_majors, list) {
+		this->add_mtd(this, mtd);
 	}
 
 }

@@ -406,7 +400,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 
 int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 {
-	struct list_head *this, *next;
+	struct mtd_blktrans_dev *this, *next;
 
 	mutex_lock(&mtd_table_mutex);

@@ -416,9 +410,8 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 	/* Remove it from the list of active majors */
 	list_del(&tr->list);
 
-	list_for_each_safe(this, next, &tr->devs) {
-		struct mtd_blktrans_dev *dev = list_entry(this, struct mtd_blktrans_dev, list);
-		tr->remove_dev(dev);
+	list_for_each_entry_safe(this, next, &tr->devs, list) {
+		tr->remove_dev(this);
 	}
 
 	blk_cleanup_queue(tr->blkcore_priv->rq);

drivers/mtd/mtdcore.c +6 -8

index f7e7890..79b8d10 100644
@@ -53,7 +53,7 @@ int add_mtd_device(struct mtd_info *mtd)
 
 	for (i=0; i < MAX_MTD_DEVICES; i++)
 		if (!mtd_table[i]) {
-			struct list_head *this;
+			struct mtd_notifier *this;
 
 			mtd_table[i] = mtd;
 			mtd->index = i;

@@ -72,9 +72,8 @@ int add_mtd_device(struct mtd_info *mtd)
 			DEBUG(0, "mtd: Giving out device %d to %s\n",i, mtd->name);
 			/* No need to get a refcount on the module containing
 			   the notifier, since we hold the mtd_table_mutex */
-			list_for_each(this, &mtd_notifiers) {
-				struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
-				not->add(mtd);
+			list_for_each_entry(this, &mtd_notifiers, list) {
+				this->add(mtd);
 			}
 
 			mutex_unlock(&mtd_table_mutex);

@@ -113,13 +112,12 @@ int del_mtd_device (struct mtd_info *mtd)
 		       mtd->index, mtd->name, mtd->usecount);
 		ret = -EBUSY;
 	} else {
-		struct list_head *this;
+		struct mtd_notifier *this;
 
 		/* No need to get a refcount on the module containing
 		   the notifier, since we hold the mtd_table_mutex */
-		list_for_each(this, &mtd_notifiers) {
-			struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
-			not->remove(mtd);
+		list_for_each_entry(this, &mtd_notifiers, list) {
+			this->remove(mtd);
 		}
 
 		mtd_table[mtd->index] = NULL;

drivers/mtd/mtdpart.c +8 ~~-16~~

index 07c7011..06b0a14 100644
@@ -302,20 +302,14 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs)
 
 int del_mtd_partitions(struct mtd_info *master)
 {
-	struct list_head *node;
-	struct mtd_part *slave;
+	struct mtd_part *slave, *safe;
 
-	for (node = mtd_partitions.next;
-	     node != &mtd_partitions;
-	     node = node->next) {
-		slave = list_entry(node, struct mtd_part, list);
+	list_for_each_entry_safe(slave, safe, &mtd_partitions, list) {
 		if (slave->master == master) {
-			struct list_head *prev = node->prev;
-			__list_del(prev, node->next);
+			list_del(&slave->list);
 			if(slave->registered)
 				del_mtd_device(&slave->mtd);
 			kfree(slave);
-			node = prev;
 		}
 	}

@@ -513,15 +507,13 @@ static LIST_HEAD(part_parsers);
 
 static struct mtd_part_parser *get_partition_parser(const char *name)
 {
-	struct list_head *this;
-	void *ret = NULL;
-	spin_lock(&part_parser_lock);
+	struct mtd_part_parser *this, *ret = NULL;
 
-	list_for_each(this, &part_parsers) {
-		struct mtd_part_parser *p = list_entry(this, struct mtd_part_parser, list);
+	spin_lock(&part_parser_lock);
 
-		if (!strcmp(p->name, name) && try_module_get(p->owner)) {
-			ret = p;
+	list_for_each_entry(this, &part_parsers, list) {
+		if (!strcmp(this->name, name) && try_module_get(this->owner)) {
+			ret = this;
 			break;
 		}
 	}

Jörn Engel

2008-05-17 19:49 UTC #1 lore raw

Quoted 13 lines · Chris Malley

From: Chris Malley <[email protected]>
Should be no functional changes, just a bit of janitorial work to remove temporary pointers and make some functions a bit more readable.
drivers/mtd/mtd_blkdevs.c +1 -1
-	struct list_head *this;
+	struct mtd_blktrans_dev *this;
 	int last_devnum = -1;
 	struct gendisk *gd;

drivers/mtd/mtd_blkdevs.c int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) · line 225

Quoted 5 lines

drivers/mtd/mtd_blkdevs.c +0 -2

@@ -221,25 +221,24 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 		BUG();
 	}
 
-	list_for_each(this, &tr->devs) {
-		struct mtd_blktrans_dev *d = list_entry(this, struct mtd_blktrans_dev, list);

I guess whoever originally wrote the code used "d" for ..._dev. And in spite of being just one letter, I find it more descriptive than "this". Would you mind killing "this" instead of "d"? And likewise in the other hunks, always nuke "this", as it has zero descriptiveness.

Otherwise a nice cleanup. Did you check that the resulting object code remains identical?

Jörn

Signature · 5 lines

In America you can have either a flimsy box banged together out of two
by fours and drywall, or a McMansion -- a flimsy box banged together
out of two by fours and drywall, but larger, more dramatic-looking,
and full of expensive fittings.
-- Paul Graham

Chris Malley

author

2008-05-17 20:08 UTC #2 lore raw

Quoted 17 lines · Jörn Engel

Quoted 16 lines · Chris Malley
From: Chris Malley <[email protected]>
Should be no functional changes, just a bit of janitorial work to remove temporary pointers and make some functions a bit more readable.
drivers/mtd/mtd_blkdevs.c +1 -1
-	struct list_head *this;
+	struct mtd_blktrans_dev *this;
 	int last_devnum = -1;
 	struct gendisk *gd;
@@ -221,25 +221,24 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 		BUG();
 	}
 

Quoted 7 lines

drivers/mtd/mtd_blkdevs.c int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) · line 225
Quoted 2 lines
drivers/mtd/mtd_blkdevs.c +0 -2
-	list_for_each(this, &tr->devs) {
-		struct mtd_blktrans_dev *d = list_entry(this, struct mtd_blktrans_dev, list);
I guess whoever originally wrote the code used "d" for ..._dev. And in spite of being just one letter, I find it more descriptive than "this". Would you mind killing "this" instead of "d"? And likewise in the other hunks, always nuke "this", as it has zero descriptiveness.

OK, I wasn't sure what people preferred; I'll change to use the original identifiers.

Quoted 2 lines

Otherwise a nice cleanup. Did you check that the resulting object code remains identical?

Not yet, I'll do this before posting an updated version. Thanks for the feedback.

Chris

Chris Malley

author

2008-05-18 19:37 UTC #3 lore raw

Re: [PATCH] [MTD] CORE: Use list_for_each_entry[_safe] where appropriate (2nd attempt)

In reply to Jörn Engel

From: Chris Malley <[email protected]>

[MTD] CORE: Use list_for_each_entry[_safe] where appropriate.

Janitorial work to remove temporary pointers and make some functions a bit more readable.

Signed-off-by Chris Malley <[email protected]>

---
Compile tested against mtd git. I compared the object output as Jörn suggested and there are a number of differences on my machine, perhaps due to the different way the library functions are implemented and moving the relevant struct vars outside the loop. I don't think these should make any material difference but it's worth giving them a good testing before sending to Linus.

 drivers/mtd/mtd_blkdevs.c |   27 ++++++++++-----------------
 drivers/mtd/mtdcore.c     |   10 ++++------
 drivers/mtd/mtdpart.c     |   20 ++++++--------------
 3 files changed, 20 insertions(+), 37 deletions(-)

drivers/mtd/mtd_blkdevs.c +10 ~~-17~~

index 839eed8..39fefb1 100644
@@ -212,7 +212,7 @@ static struct block_device_operations mtd_blktrans_ops = {
 int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 {
 	struct mtd_blktrans_ops *tr = new->tr;
-	struct list_head *this;
+	struct mtd_blktrans_dev *d;
 	int last_devnum = -1;
 	struct gendisk *gd;

@@ -221,8 +221,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 		BUG();
 	}
 
-	list_for_each(this, &tr->devs) {
-		struct mtd_blktrans_dev *d = list_entry(this, struct mtd_blktrans_dev, list);
+	list_for_each_entry(d, &tr->devs, list) {
 		if (new->devnum == -1) {
 			/* Use first free number */
 			if (d->devnum != last_devnum+1) {

@@ -309,14 +308,11 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 
 static void blktrans_notify_remove(struct mtd_info *mtd)
 {
-	struct list_head *this, *this2, *next;
-
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
-		list_for_each_safe(this2, next, &tr->devs) {
-			struct mtd_blktrans_dev *dev = list_entry(this2, struct mtd_blktrans_dev, list);
+	struct mtd_blktrans_ops *tr;
+	struct mtd_blktrans_dev *dev, *next;
 
+	list_for_each_entry(tr, &blktrans_majors, list) {
+		list_for_each_entry_safe(dev, next, &tr->devs, list) {
 			if (dev->mtd == mtd)
 				tr->remove_dev(dev);
 		}

@@ -325,14 +321,12 @@ static void blktrans_notify_remove(struct mtd_info *mtd)
 
 static void blktrans_notify_add(struct mtd_info *mtd)
 {
-	struct list_head *this;
+	struct mtd_blktrans_ops *tr;
 
 	if (mtd->type == MTD_ABSENT)
 		return;
 
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
+	list_for_each_entry(tr, &blktrans_majors, list) {
 		tr->add_mtd(tr, mtd);
 	}

@@ -406,7 +400,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 
 int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 {
-	struct list_head *this, *next;
+	struct mtd_blktrans_dev *dev, *next;
 
 	mutex_lock(&mtd_table_mutex);

@@ -416,8 +410,7 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 	/* Remove it from the list of active majors */
 	list_del(&tr->list);
 
-	list_for_each_safe(this, next, &tr->devs) {
-		struct mtd_blktrans_dev *dev = list_entry(this, struct mtd_blktrans_dev, list);
+	list_for_each_entry_safe(dev, next, &tr->devs, list) {
 		tr->remove_dev(dev);
 	}

drivers/mtd/mtdcore.c +4 -6

index f7e7890..8c61035 100644
@@ -53,7 +53,7 @@ int add_mtd_device(struct mtd_info *mtd)
 
 	for (i=0; i < MAX_MTD_DEVICES; i++)
 		if (!mtd_table[i]) {
-			struct list_head *this;
+			struct mtd_notifier *not;
 
 			mtd_table[i] = mtd;
 			mtd->index = i;

@@ -72,8 +72,7 @@ int add_mtd_device(struct mtd_info *mtd)
 			DEBUG(0, "mtd: Giving out device %d to %s\n",i, mtd->name);
 			/* No need to get a refcount on the module containing
 			   the notifier, since we hold the mtd_table_mutex */
-			list_for_each(this, &mtd_notifiers) {
-				struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
+			list_for_each_entry(not, &mtd_notifiers, list) {
 				not->add(mtd);
 			}

@@ -113,12 +112,11 @@ int del_mtd_device (struct mtd_info *mtd)
 		       mtd->index, mtd->name, mtd->usecount);
 		ret = -EBUSY;
 	} else {
-		struct list_head *this;
+		struct mtd_notifier *not;
 
 		/* No need to get a refcount on the module containing
 		   the notifier, since we hold the mtd_table_mutex */
-		list_for_each(this, &mtd_notifiers) {
-			struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
+		list_for_each_entry(not, &mtd_notifiers, list) {
 			not->remove(mtd);
 		}

drivers/mtd/mtdpart.c +6 ~~-14~~

index 07c7011..b7847f2 100644
@@ -302,20 +302,14 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs)
 
 int del_mtd_partitions(struct mtd_info *master)
 {
-	struct list_head *node;
-	struct mtd_part *slave;
+	struct mtd_part *slave, *safe;
 
-	for (node = mtd_partitions.next;
-	     node != &mtd_partitions;
-	     node = node->next) {
-		slave = list_entry(node, struct mtd_part, list);
+	list_for_each_entry_safe(slave, safe, &mtd_partitions, list) {
 		if (slave->master == master) {
-			struct list_head *prev = node->prev;
-			__list_del(prev, node->next);
+			list_del(&slave->list);
 			if(slave->registered)
 				del_mtd_device(&slave->mtd);
 			kfree(slave);
-			node = prev;
 		}
 	}

@@ -513,13 +507,11 @@ static LIST_HEAD(part_parsers);
 
 static struct mtd_part_parser *get_partition_parser(const char *name)
 {
-	struct list_head *this;
-	void *ret = NULL;
-	spin_lock(&part_parser_lock);
+	struct mtd_part_parser *p, *ret = NULL;
 
-	list_for_each(this, &part_parsers) {
-		struct mtd_part_parser *p = list_entry(this, struct mtd_part_parser, list);
+	spin_lock(&part_parser_lock);
 
+	list_for_each_entry(p, &part_parsers, list) {
 		if (!strcmp(p->name, name) && try_module_get(p->owner)) {
 			ret = p;
 			break;

Jörn Engel

Reviewed-by

2008-05-19 11:39 UTC #4 lore raw

Re: [PATCH] [MTD] CORE: Use list_for_each_entry[_safe] where appropriate (2nd attempt)

drivers/mtd/mtd_blkdevs.c static void blktrans_notify_remove(struct mtd_info *mtd) · line 331

Quoted 4 lines · Chris Malley

drivers/mtd/mtd_blkdevs.c +1 -0

+	list_for_each_entry(tr, &blktrans_majors, list) {
 		tr->add_mtd(tr, mtd);
 	}

In a couple of places like this the loop body became a single line. So you could remove the braces as well, while at it.

drivers/mtd/mtdpart.c static int part_block_markbad (struct mtd_info *mtd, loff_t ofs) · line 305

Quoted 11 lines

drivers/mtd/mtdpart.c +1 -2

index 07c7011..b7847f2 100644
@@ -302,20 +302,14 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs)
 
 int del_mtd_partitions(struct mtd_info *master)
 {
-	struct list_head *node;
-	struct mtd_part *slave;
+	struct mtd_part *slave, *safe;

"next" seems a bit better than "safe".

Rest looks fine to me. So I leave it up to you whether to add those trivial cosmetics or just leave the patch as-is. Either way, it is an improvement. Also, having dwmw2 on Cc: improves the chances of this patch not being ignored. ;)

Reviewed-by Joern Engel <[email protected]>

Jörn

Signature · 2 lines

Victory in war is not repetitious.
-- Sun Tzu

Chris Malley

author Reviewed-by Joern Engel

2008-05-19 19:11 UTC #5 lore raw

Re: [PATCH] [MTD] CORE: Use list_for_each_entry[_safe] where appropriate (3rd attempt)

From: Chris Malley <[email protected]>

[MTD] CORE: Use list_for_each_entry[_safe] where appropriate.

Janitorial work to remove temporary pointers and make some functions a bit more readable.

Signed-off-by Chris Malley <[email protected]>

Reviewed-by Joern Engel <[email protected]>

---
 drivers/mtd/mtd_blkdevs.c |   32 ++++++++++----------------------
 drivers/mtd/mtdcore.c     |   12 ++++--------
 drivers/mtd/mtdpart.c     |   23 +++++++----------------
 3 files changed, 21 insertions(+), 46 deletions(-)

drivers/mtd/mtd_blkdevs.c +10 ~~-22~~

index 839eed8..58d80ee 100644
@@ -212,7 +212,7 @@ static struct block_device_operations mtd_blktrans_ops = {
 int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 {
 	struct mtd_blktrans_ops *tr = new->tr;
-	struct list_head *this;
+	struct mtd_blktrans_dev *d;
 	int last_devnum = -1;
 	struct gendisk *gd;

@@ -221,8 +221,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 		BUG();
 	}
 
-	list_for_each(this, &tr->devs) {
-		struct mtd_blktrans_dev *d = list_entry(this, struct mtd_blktrans_dev, list);
+	list_for_each_entry(d, &tr->devs, list) {
 		if (new->devnum == -1) {
 			/* Use first free number */
 			if (d->devnum != last_devnum+1) {

@@ -309,33 +308,24 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 
 static void blktrans_notify_remove(struct mtd_info *mtd)
 {
-	struct list_head *this, *this2, *next;
-
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
-		list_for_each_safe(this2, next, &tr->devs) {
-			struct mtd_blktrans_dev *dev = list_entry(this2, struct mtd_blktrans_dev, list);
+	struct mtd_blktrans_ops *tr;
+	struct mtd_blktrans_dev *dev, *next;
 
+	list_for_each_entry(tr, &blktrans_majors, list)
+		list_for_each_entry_safe(dev, next, &tr->devs, list)
 			if (dev->mtd == mtd)
 				tr->remove_dev(dev);
-		}
-	}
 }
 
 static void blktrans_notify_add(struct mtd_info *mtd)
 {
-	struct list_head *this;
+	struct mtd_blktrans_ops *tr;
 
 	if (mtd->type == MTD_ABSENT)
 		return;
 
-	list_for_each(this, &blktrans_majors) {
-		struct mtd_blktrans_ops *tr = list_entry(this, struct mtd_blktrans_ops, list);
-
+	list_for_each_entry(tr, &blktrans_majors, list)
 		tr->add_mtd(tr, mtd);
-	}
-
 }
 
 static struct mtd_notifier blktrans_notifier = {

@@ -406,7 +396,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 
 int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 {
-	struct list_head *this, *next;
+	struct mtd_blktrans_dev *dev, *next;
 
 	mutex_lock(&mtd_table_mutex);

@@ -416,10 +406,8 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 	/* Remove it from the list of active majors */
 	list_del(&tr->list);
 
-	list_for_each_safe(this, next, &tr->devs) {
-		struct mtd_blktrans_dev *dev = list_entry(this, struct mtd_blktrans_dev, list);
+	list_for_each_entry_safe(dev, next, &tr->devs, list)
 		tr->remove_dev(dev);
-	}
 
 	blk_cleanup_queue(tr->blkcore_priv->rq);
 	unregister_blkdev(tr->major, tr->name);

drivers/mtd/mtdcore.c +4 -8

index f7e7890..b06fd3b 100644
@@ -53,7 +53,7 @@ int add_mtd_device(struct mtd_info *mtd)
 
 	for (i=0; i < MAX_MTD_DEVICES; i++)
 		if (!mtd_table[i]) {
-			struct list_head *this;
+			struct mtd_notifier *not;
 
 			mtd_table[i] = mtd;
 			mtd->index = i;

@@ -72,10 +72,8 @@ int add_mtd_device(struct mtd_info *mtd)
 			DEBUG(0, "mtd: Giving out device %d to %s\n",i, mtd->name);
 			/* No need to get a refcount on the module containing
 			   the notifier, since we hold the mtd_table_mutex */
-			list_for_each(this, &mtd_notifiers) {
-				struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
+			list_for_each_entry(not, &mtd_notifiers, list)
 				not->add(mtd);
-			}
 
 			mutex_unlock(&mtd_table_mutex);
 			/* We _know_ we aren't being removed, because

@@ -113,14 +111,12 @@ int del_mtd_device (struct mtd_info *mtd)
 		       mtd->index, mtd->name, mtd->usecount);
 		ret = -EBUSY;
 	} else {
-		struct list_head *this;
+		struct mtd_notifier *not;
 
 		/* No need to get a refcount on the module containing
 		   the notifier, since we hold the mtd_table_mutex */
-		list_for_each(this, &mtd_notifiers) {
-			struct mtd_notifier *not = list_entry(this, struct mtd_notifier, list);
+		list_for_each_entry(not, &mtd_notifiers, list)
 			not->remove(mtd);
-		}
 
 		mtd_table[mtd->index] = NULL;

drivers/mtd/mtdpart.c +7 ~~-16~~

index 07c7011..c95ceaa 100644
@@ -302,22 +302,15 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs)
 
 int del_mtd_partitions(struct mtd_info *master)
 {
-	struct list_head *node;
-	struct mtd_part *slave;
+	struct mtd_part *slave, *next;
 
-	for (node = mtd_partitions.next;
-	     node != &mtd_partitions;
-	     node = node->next) {
-		slave = list_entry(node, struct mtd_part, list);
+	list_for_each_entry_safe(slave, next, &mtd_partitions, list)
 		if (slave->master == master) {
-			struct list_head *prev = node->prev;
-			__list_del(prev, node->next);
+			list_del(&slave->list);
 			if(slave->registered)
 				del_mtd_device(&slave->mtd);
 			kfree(slave);
-			node = prev;
 		}
-	}
 
 	return 0;
 }

@@ -513,18 +506,16 @@ static LIST_HEAD(part_parsers);
 
 static struct mtd_part_parser *get_partition_parser(const char *name)
 {
-	struct list_head *this;
-	void *ret = NULL;
-	spin_lock(&part_parser_lock);
+	struct mtd_part_parser *p, *ret = NULL;
 
-	list_for_each(this, &part_parsers) {
-		struct mtd_part_parser *p = list_entry(this, struct mtd_part_parser, list);
+	spin_lock(&part_parser_lock);
 
+	list_for_each_entry(p, &part_parsers, list)
 		if (!strcmp(p->name, name) && try_module_get(p->owner)) {
 			ret = p;
 			break;
 		}
-	}
+
 	spin_unlock(&part_parser_lock);
 
 	return ret;

Jörn Engel

2008-05-19 19:27 UTC #6 lore raw

Re: [PATCH] [MTD] CORE: Use list_for_each_entry[_safe] where appropriate (3rd attempt)

Quoted 8 lines · Chris Malley

[MTD] CORE: Use list_for_each_entry[_safe] where appropriate.
Janitorial work to remove temporary pointers and make some functions a bit more readable.
Signed-off-by Chris Malley <[email protected]>
Reviewed-by Joern Engel <[email protected]>

Excellent, thanks!

Jörn

Signature · 4 lines

Data dominates. If you've chosen the right data structures and organized
things well, the algorithms will almost always be self-evident. Data
structures, not algorithms, are central to programming.
-- Rob Pike