[PATCH] NETFILTER: replace list_for_each with list_for_each_entry

[PATCH] NETFILTER: replace list_for_each with list_for_each_entry

[Li Zefan]

Replace list_for_each with list_for_each_entry when possible.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
 net/netfilter/nf_sockopt.c         |    8 ++------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 2f544da..311361e 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -109,11 +109,9 @@ clusterip_config_entry_put(struct clusterip_config *c)
 static struct clusterip_config *
 __clusterip_config_find(__be32 clusterip)
 {
-	struct list_head *pos;
+	struct clusterip_config *c;
 
-	list_for_each(pos, &clusterip_configs) {
-		struct clusterip_config *c = list_entry(pos,
-					struct clusterip_config, list);
+	list_for_each_entry(c, &clusterip_configs, list) {
 		if (c->clusterip == clusterip)
 			return c;
 	}
diff --git a/net/netfilter/nf_sockopt.c b/net/netfilter/nf_sockopt.c
index aa28315..06bbccb 100644
--- a/net/netfilter/nf_sockopt.c
+++ b/net/netfilter/nf_sockopt.c
@@ -65,7 +65,6 @@ EXPORT_SYMBOL(nf_unregister_sockopt);
 static int nf_sockopt(struct sock *sk, int pf, int val,
 		      char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;
 
@@ -75,8 +74,7 @@ static int nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;
@@ -124,7 +122,6 @@ EXPORT_SYMBOL(nf_getsockopt);
 static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 			     char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;
 
@@ -135,8 +132,7 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;
-- 
1.5.3.rc7

Re: [PATCH] NETFILTER: replace list_for_each with list_for_each_entry

[Li Zefan]

Li Zefan wrote:
> Replace list_for_each with list_for_each_entry when possible.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> 
> ---
>  net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
>  net/netfilter/nf_sockopt.c         |    8 ++------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 

Please ignore this patch, I'll send a new one.

Li Zefan

[PATCH V2] NETFILTER: replace list_for_each with list_for_each_entry

[Li Zefan]

This patch replaces list_for_each with list_for_each_entry, and
consequently fixes a bug too.

Things like this is wrong:

	list_for_each(i, &nf_sockopts) {
		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;

It makes the assumption that list is the first member of struct
nf_sockopt_ops, so the ptr casting works. But if one day the structure
changes, for example, like this:

	struct nf_sockopt_ops {
		foo_t a_new_member;
		struct list_head list;
	};

Then we get corrupted data.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
 net/netfilter/core.c               |    8 ++++----
 net/netfilter/nf_sockopt.c         |   13 ++++---------
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 2f544da..311361e 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -109,11 +109,9 @@ clusterip_config_entry_put(struct clusterip_config *c)
 static struct clusterip_config *
 __clusterip_config_find(__be32 clusterip)
 {
-	struct list_head *pos;
+	struct clusterip_config *c;
 
-	list_for_each(pos, &clusterip_configs) {
-		struct clusterip_config *c = list_entry(pos,
-					struct clusterip_config, list);
+	list_for_each_entry(c, &clusterip_configs, list) {
 		if (c->clusterip == clusterip)
 			return c;
 	}
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index bed9ba0..ed58922 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -62,17 +62,17 @@ static DEFINE_MUTEX(nf_hook_mutex);
 
 int nf_register_hook(struct nf_hook_ops *reg)
 {
-	struct list_head *i;
+	struct nf_hook_ops *elem;
 	int err;
 
 	err = mutex_lock_interruptible(&nf_hook_mutex);
 	if (err < 0)
 		return err;
-	list_for_each(i, &nf_hooks[reg->pf][reg->hooknum]) {
-		if (reg->priority < ((struct nf_hook_ops *)i)->priority)
+	list_for_each_entry(elem, &nf_hooks[reg->pf][reg->hooknum], list) {
+		if (reg->priority < elem->priority)
 			break;
 	}
-	list_add_rcu(&reg->list, i->prev);
+	list_add_rcu(&reg->list, elem->list.prev);
 	mutex_unlock(&nf_hook_mutex);
 	return 0;
 }
diff --git a/net/netfilter/nf_sockopt.c b/net/netfilter/nf_sockopt.c
index aa28315..2dfac32 100644
--- a/net/netfilter/nf_sockopt.c
+++ b/net/netfilter/nf_sockopt.c
@@ -23,14 +23,13 @@ static inline int overlap(int min1, int max1, int min2, int max2)
 /* Functions to register sockopt ranges (exclusive). */
 int nf_register_sockopt(struct nf_sockopt_ops *reg)
 {
-	struct list_head *i;
+	struct nf_sockopt_ops *ops;
 	int ret = 0;
 
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == reg->pf
 		    && (overlap(ops->set_optmin, ops->set_optmax,
 				reg->set_optmin, reg->set_optmax)
@@ -65,7 +64,6 @@ EXPORT_SYMBOL(nf_unregister_sockopt);
 static int nf_sockopt(struct sock *sk, int pf, int val,
 		      char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;
 
@@ -75,8 +73,7 @@ static int nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;
@@ -124,7 +121,6 @@ EXPORT_SYMBOL(nf_getsockopt);
 static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 			     char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;
 
@@ -135,8 +131,7 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;
-- 
1.5.3.rc7

Re: [PATCH V2] NETFILTER: replace list_for_each with list_for_each_entry

[Patrick McHardy]

Li Zefan wrote:
> This patch replaces list_for_each with list_for_each_entry, and
> consequently fixes a bug too.
> 
> Things like this is wrong:
> 
> 	list_for_each(i, &nf_sockopts) {
> 		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;
> 
> It makes the assumption that list is the first member of struct
> nf_sockopt_ops, so the ptr casting works. But if one day the structure
> changes, for example, like this:

So far its not a bug, so I've applied it without that description.
Thanks.

>  net/netfilter/nf_sockopt.c         |   13 ++++---------

And removed that part since you're the third person within two
weeks to send an identical patch :) Please always base your
patches on Linus' latest -git tree or Dave's networking tree
for the next version once it opens.

Re: [PATCH V2] NETFILTER: replace list_for_each with list_for_each_entry

[Li Zefan]

Patrick McHardy wrote:
> Li Zefan wrote:
>>  net/netfilter/nf_sockopt.c         |   13 ++++---------
> 
> And removed that part since you're the third person within two
> weeks to send an identical patch :) Please always base your
> patches on Linus' latest -git tree or Dave's networking tree
> for the next version once it opens.
> 
> 
> 

OK, I will. :)