2006-12-31 21:00:23

by Robert P. J. Day

[permalink] [raw]
Subject: [PATCH] Simplify some code to use the container_of() macro.


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

---

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;


2007-01-04 00:50:38

by Andrew Morton

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

On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
"Robert P. J. Day" <[email protected]> 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.

2007-01-04 07:45:56

by Robert P. J. Day

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

On Wed, 3 Jan 2007, Andrew Morton wrote:

> On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
> "Robert P. J. Day" <[email protected]> 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

2007-01-04 08:04:15

by Robert P. J. Day

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

On Wed, 3 Jan 2007, Andrew Morton wrote:

> On Sun, 31 Dec 2006 15:55:22 -0500 (EST)
> "Robert P. J. Day" <[email protected]> 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

2007-01-04 08:42:30

by Thomas Hisch

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

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

2007-01-04 08:53:16

by Robert P. J. Day

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

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

2007-02-04 14:29:30

by Thomas Hisch

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

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

2007-02-04 20:23:50

by David Miller

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

From: Thomas Hisch <[email protected]>
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 <[email protected]>

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 [email protected],
the majority of the networking developers do not read linux-kernel.

Thank you.