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

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

[Chris Malley]

From: Chris Malley <mail@chrismalley.co.uk>

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 <mail@chrismalley.co.uk>

---
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(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 839eed8..895813b 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -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);
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index f7e7890..79b8d10 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -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;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 07c7011..06b0a14 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -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;
 		}
 	}
-- 

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

[Jörn Engel]

On Sat, 17 May 2008 18:54:05 +0100, Chris Malley wrote:
> 
> From: Chris Malley <mail@chrismalley.co.uk>
> 
> Should be no functional changes, just a bit of janitorial work
> to remove temporary pointers and make some functions a bit more readable.


> -	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);

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

-- 
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

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

[Chris Malley]

On Sat, 2008-05-17 at 21:49 +0200, Jörn Engel wrote:
> On Sat, 17 May 2008 18:54:05 +0100, Chris Malley wrote:
> > 
> > From: Chris Malley <mail@chrismalley.co.uk>
> > 
> > Should be no functional changes, just a bit of janitorial work
> > to remove temporary pointers and make some functions a bit more readable.
> 
> 
> > -	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);
> 
> 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.

> 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

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

[Chris Malley]

From: Chris Malley <mail@chrismalley.co.uk>

[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 <mail@chrismalley.co.uk>
---
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(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 839eed8..39fefb1 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -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);
 	}
 
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index f7e7890..8c61035 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -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);
 		}
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 07c7011..b7847f2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -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;
-- 

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

[Jörn Engel]

On Sun, 18 May 2008 20:37:13 +0100, Chris Malley wrote:
>
> +	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.

> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 07c7011..b7847f2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -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 <joern@logfs.org>

Jörn

-- 
Victory in war is not repetitious.
-- Sun Tzu

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

[Chris Malley]

From: Chris Malley <mail@chrismalley.co.uk>

[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 <mail@chrismalley.co.uk>
Reviewed-By: Joern Engel <joern@logfs.org>
---
 drivers/mtd/mtd_blkdevs.c |   32 ++++++++++----------------------
 drivers/mtd/mtdcore.c     |   12 ++++--------
 drivers/mtd/mtdpart.c     |   23 +++++++----------------
 3 files changed, 21 insertions(+), 46 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 839eed8..58d80ee 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -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);
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index f7e7890..b06fd3b 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -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;
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 07c7011..c95ceaa 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -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;
-- 

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

[Jörn Engel]

On Mon, 19 May 2008 20:11:50 +0100, Chris Malley wrote:
> 
> [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 <mail@chrismalley.co.uk>
> Reviewed-By: Joern Engel <joern@logfs.org>

Excellent, thanks!

Jörn

-- 
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