[PATCH] Simplify some code to use the container_of() macro.

[PATCH] Simplify some code to use the container_of() macro.

[Robert P. J. Day]

  Simplify a number of code snippets in source and header files to use
the kernel.h "container_of()" macro.

Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

---

  and while we're at it, everybody can stop re-inventing the
container_of() macro.  :-)


 drivers/net/ppp_generic.c        |    2 +-
 drivers/s390/net/lcs.c           |    6 ++----
 drivers/video/sa1100fb.h         |    4 +---
 include/linux/security.h         |    2 +-
 net/ipv4/netfilter/nf_nat_core.c |    2 +-
 security/selinux/hooks.c         |    2 +-
 6 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index c6de566..0986f6c 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -83,7 +83,7 @@ struct ppp_file {
 	int		dead;		/* unit/channel has been shut down */
 };

-#define PF_TO_X(pf, X)		((X *)((char *)(pf) - offsetof(X, file)))
+#define PF_TO_X(pf, X)		container_of(pf, X, file)

 #define PF_TO_PPP(pf)		PF_TO_X(pf, struct ppp)
 #define PF_TO_CHANNEL(pf)	PF_TO_X(pf, struct channel)
diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
index e5665b6..732cd9f 100644
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -1511,8 +1511,7 @@ lcs_txbuffer_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
 	LCS_DBF_TEXT(5, trace, "txbuffcb");
 	/* Put buffer back to pool. */
 	lcs_release_buffer(channel, buffer);
-	card = (struct lcs_card *)
-		((char *) channel - offsetof(struct lcs_card, write));
+	card = container_of(channel, struct lcs_card, write);
 	if (netif_queue_stopped(card->dev) && netif_carrier_ok(card->dev))
 		netif_wake_queue(card->dev);
 	spin_lock(&card->lock);
@@ -1810,8 +1809,7 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
 		LCS_DBF_TEXT(4, trace, "-eiogpkt");
 		return;
 	}
-	card = (struct lcs_card *)
-		((char *) channel - offsetof(struct lcs_card, read));
+	card = container_of(channel, struct lcs_card, write);
 	offset = 0;
 	while (lcs_hdr->offset != 0) {
 		if (lcs_hdr->offset <= 0 ||
diff --git a/drivers/video/sa1100fb.h b/drivers/video/sa1100fb.h
index 0b07f6a..48066ef 100644
--- a/drivers/video/sa1100fb.h
+++ b/drivers/video/sa1100fb.h
@@ -110,9 +110,7 @@ struct sa1100fb_info {
 #endif
 };

-#define __type_entry(ptr,type,member) ((type *)((char *)(ptr)-offsetof(type,member)))
-
-#define TO_INF(ptr,member)	__type_entry(ptr,struct sa1100fb_info,member)
+#define TO_INF(ptr,member)	container_of(ptr,struct sa1100fb_info,member)

 #define SA1100_PALETTE_MODE_VAL(bpp)    (((bpp) & 0x018) << 9)

diff --git a/include/linux/security.h b/include/linux/security.h
index 83cdefa..c554f60 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -492,7 +492,7 @@ struct request_sock;
  *	Note that the fown_struct, @fown, is never outside the context of a
  *	struct file, so the file structure (and associated security information)
  *	can always be obtained:
- *		(struct file *)((long)fown - offsetof(struct file,f_owner));
+ *		container_of(fown, struct file, f_owner)
  * 	@tsk contains the structure of task receiving signal.
  *	@fown contains the file owner information.
  *	@sig is the signal that will be sent.  When 0, kernel sends SIGIO.
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 86a9227..26c8f69 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -168,7 +168,7 @@ find_appropriate_src(const struct nf_conntrack_tuple *tuple,

 	read_lock_bh(&nf_nat_lock);
 	list_for_each_entry(nat, &bysource[h], info.bysource) {
-		ct = (struct nf_conn *)((char *)nat - offsetof(struct nf_conn, data));
+		ct = container_of(nat, struct nf_conn, data);
 		if (same_src(ct, tuple)) {
 			/* Copy source part from reply tuple. */
 			nf_ct_invert_tuplepr(result,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65fb5e8..778ceb9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2655,7 +2655,7 @@ static int selinux_file_send_sigiotask(struct task_struct *tsk,
 	struct file_security_struct *fsec;

 	/* struct fown_struct is never outside the context of a struct file */
-        file = (struct file *)((long)fown - offsetof(struct file,f_owner));
+        file = container_of(fown, struct file, f_owner);

 	tsec = tsk->security;
 	fsec = file->f_security;

Re: [PATCH] Simplify some code to use the container_of() macro.

[Andrew Morton]

On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
"Robert P. J. Day" <rpjday@mindspring.com> wrote:

> --- a/net/ipv4/netfilter/nf_nat_core.c
> +++ b/net/ipv4/netfilter/nf_nat_core.c
> @@ -168,7 +168,7 @@ find_appropriate_src(const struct nf_conntrack_tuple *tuple,
> 
>  	read_lock_bh(&nf_nat_lock);
>  	list_for_each_entry(nat, &bysource[h], info.bysource) {
> -		ct = (struct nf_conn *)((char *)nat - offsetof(struct nf_conn, data));
> +		ct = container_of(nat, struct nf_conn, data);

This one isn't right.  Please always carefully compile-test these things.

Re: [PATCH] Simplify some code to use the container_of() macro.

[Robert P. J. Day]

On Wed, 3 Jan 2007, Andrew Morton wrote:

> On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
> "Robert P. J. Day" <rpjday@mindspring.com> wrote:
>
> > --- a/net/ipv4/netfilter/nf_nat_core.c
> > +++ b/net/ipv4/netfilter/nf_nat_core.c
> > @@ -168,7 +168,7 @@ find_appropriate_src(const struct nf_conntrack_tuple *tuple,
> >
> >  	read_lock_bh(&nf_nat_lock);
> >  	list_for_each_entry(nat, &bysource[h], info.bysource) {
> > -		ct = (struct nf_conn *)((char *)nat - offsetof(struct nf_conn, data));
> > +		ct = container_of(nat, struct nf_conn, data);
>
> This one isn't right.  Please always carefully compile-test these things.

i was quite sure i had.  i'll take another look at it.

rday

Re: [PATCH] Simplify some code to use the container_of() macro.

[Robert P. J. Day]

On Wed, 3 Jan 2007, Andrew Morton wrote:

> On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
> "Robert P. J. Day" <rpjday@mindspring.com> wrote:
>
> > --- a/net/ipv4/netfilter/nf_nat_core.c
> > +++ b/net/ipv4/netfilter/nf_nat_core.c
> > @@ -168,7 +168,7 @@ find_appropriate_src(const struct nf_conntrack_tuple *tuple,
> >
> >  	read_lock_bh(&nf_nat_lock);
> >  	list_for_each_entry(nat, &bysource[h], info.bysource) {
> > -		ct = (struct nf_conn *)((char *)nat - offsetof(struct nf_conn, data));
> > +		ct = container_of(nat, struct nf_conn, data);
>
> This one isn't right.  Please always carefully compile-test these
> things.

whoops, you're right, i'm not sure why i didn't catch that.  my
apologies.

rday

Re: [PATCH] Simplify some code to use the container_of() macro.

[Thomas Hisch]

On Sun, Dec 31, 2006 at 03:55:22PM -0500, Robert P. J. Day wrote:
> @@ -1810,8 +1809,7 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
>  		LCS_DBF_TEXT(4, trace, "-eiogpkt");
>  		return;
>  	}
> -	card = (struct lcs_card *)
> -		((char *) channel - offsetof(struct lcs_card, read));
> +	card = container_of(channel, struct lcs_card, write);
the last argument in container_of should be read instead of write.

>  	offset = 0;
>  	while (lcs_hdr->offset != 0) {

-- 
Thomas Hisch
e0625874@stud.tuwien.ac.at

Re: [PATCH] Simplify some code to use the container_of() macro.

[Robert P. J. Day]

On Thu, 4 Jan 2007, Thomas Hisch wrote:

> On Sun, Dec 31, 2006 at 03:55:22PM -0500, Robert P. J. Day wrote:
> > @@ -1810,8 +1809,7 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
> >  		LCS_DBF_TEXT(4, trace, "-eiogpkt");
> >  		return;
> >  	}
> > -	card = (struct lcs_card *)
> > -		((char *) channel - offsetof(struct lcs_card, read));
> > +	card = container_of(channel, struct lcs_card, write);
> the last argument in container_of should be read instead of write.

gack, i really made a mess of that one, didn't i?  sorry, i'll send
andrew a revised and healthy patch.

rday

[PATCH] ipv4: remove a call to skb_queue_len() in inet_diag.c

[Thomas Hisch]

remove unneeded call to skb_queue_len (skb_dequeue already checks queuelen) and
replace a sizeof() by a Netlink Macro

Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
---
 net/ipv4/inet_diag.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 77761ac..ccd7fab 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -846,7 +846,7 @@ static inline void inet_diag_rcv_skb(struct sk_buff *skb)
 		int err;
 		struct nlmsghdr *nlh = (struct nlmsghdr *)skb->data;
 
-		if (nlh->nlmsg_len < sizeof(*nlh) ||
+		if (nlh->nlmsg_len < NLMSG_HDRLEN ||
 		    skb->len < nlh->nlmsg_len)
 			return;
 		err = inet_diag_rcv_msg(skb, nlh);
@@ -858,9 +858,8 @@ static inline void inet_diag_rcv_skb(struct sk_buff *skb)
 static void inet_diag_rcv(struct sock *sk, int len)
 {
 	struct sk_buff *skb;
-	unsigned int qlen = skb_queue_len(&sk->sk_receive_queue);
 
-	while (qlen-- && (skb = skb_dequeue(&sk->sk_receive_queue))) {
+	while (skb = skb_dequeue(&sk->sk_receive_queue)) {
 		inet_diag_rcv_skb(skb);
 		kfree_skb(skb);
 	}
-- 
1.5.0.rc3.22.g5057

Re: [PATCH] ipv4: remove a call to skb_queue_len() in inet_diag.c

[David Miller]

From: Thomas Hisch <t.hisch@gmail.com>
Date: Sun, 4 Feb 2007 15:29:21 +0100

> remove unneeded call to skb_queue_len (skb_dequeue already checks queuelen) and
> replace a sizeof() by a Netlink Macro
> 
> Signed-off-by: Thomas Hisch <t.hisch@gmail.com>

You don't understand the code you are editing :-)

We want to process the number of packets present when we
start the function, other threads can add more packets to
the queue meanwhile and we don't want to keep dequeueing
in that case or else we can theoretically run forever with
a fast enough producer.

Also, please post all networking patches to netdev@vger.kernel.org,
the majority of the networking developers do not read linux-kernel.

Thank you.

[PATCH] ipv4: remove a call to skb_queue_len() in inet_diag.c

[Thomas Hisch]

remove unneeded call to skb_queue_len (skb_dequeue already checks queuelen) and
replace a sizeof() by a Netlink Macro

Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
---

sorry, my previous version of this patch didn't conform to the Codingstyle document.
now everything should be fine.

 net/ipv4/inet_diag.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 77761ac..9cf27c7 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -846,7 +846,7 @@ static inline void inet_diag_rcv_skb(struct sk_buff *skb)
 		int err;
 		struct nlmsghdr *nlh = (struct nlmsghdr *)skb->data;
 
-		if (nlh->nlmsg_len < sizeof(*nlh) ||
+		if (nlh->nlmsg_len < NLMSG_HDRLEN ||
 		    skb->len < nlh->nlmsg_len)
 			return;
 		err = inet_diag_rcv_msg(skb, nlh);
@@ -858,9 +858,8 @@ static inline void inet_diag_rcv_skb(struct sk_buff *skb)
 static void inet_diag_rcv(struct sock *sk, int len)
 {
 	struct sk_buff *skb;
-	unsigned int qlen = skb_queue_len(&sk->sk_receive_queue);
 
-	while (qlen-- && (skb = skb_dequeue(&sk->sk_receive_queue))) {
+	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
 		inet_diag_rcv_skb(skb);
 		kfree_skb(skb);
 	}
-- 
1.5.0.rc3.22.g5057