[PATCH v2] net: introduce helper macro for_each_cmsghdr

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

Signed-off-by: Gu Zheng <redacted>
---
v2: use the lower-case macro name as Joe suggested.
---
 .../networking/timestamping/timestamping.c         |    4 +---
 .../networking/timestamping/txtimestamp.c          |    4 +---
 crypto/af_alg.c                                    |    2 +-
 include/linux/socket.h                             |    4 ++++
 net/core/scm.c                                     |    3 +--
 net/dccp/proto.c                                   |    5 ++---
 net/ipv4/ip_sockglue.c                             |    2 +-
 net/ipv6/datagram.c                                |    2 +-
 net/iucv/af_iucv.c                                 |    4 +---
 net/rds/send.c                                     |    4 ++--
 net/rxrpc/ar-output.c                              |    2 +-
 net/sctp/socket.c                                  |    3 +--
 12 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/Documentation/networking/timestamping/timestamping.c b/Documentation/networking/timestamping/timestamping.c
index 5cdfd74..635a43a 100644
--- a/Documentation/networking/timestamping/timestamping.c
+++ b/Documentation/networking/timestamping/timestamping.c

@@ -169,9 +169,7 @@ static void printpacket(struct msghdr *msg, int res,
 	       res,
 	       inet_ntoa(from_addr->sin_addr),
 	       msg->msg_controllen);
-	for (cmsg = CMSG_FIRSTHDR(msg);
-	     cmsg;
-	     cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		printf("   cmsg len %zu: ", cmsg->cmsg_len);
 		switch (cmsg->cmsg_level) {
 		case SOL_SOCKET:

diff --git a/Documentation/networking/timestamping/txtimestamp.c b/Documentation/networking/timestamping/txtimestamp.c
index b32fc2a..f4fd9a2 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c

@@ -149,9 +149,7 @@ static void __recv_errmsg_cmsg(struct msghdr *msg, int payload_len)
 	struct scm_timestamping *tss = NULL;
 	struct cmsghdr *cm;
 
-	for (cm = CMSG_FIRSTHDR(msg);
-	     cm && cm->cmsg_len;
-	     cm = CMSG_NXTHDR(msg, cm)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (cm->cmsg_level == SOL_SOCKET &&
 		    cm->cmsg_type == SCM_TIMESTAMPING) {
 			tss = (void *) CMSG_DATA(cm);

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 6a3ad80..bc21f52 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c

@@ -399,7 +399,7 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
 {
 	struct cmsghdr *cmsg;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 		if (cmsg->cmsg_level != SOL_ALG)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index bb9b836..0e71278 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h

@@ -94,6 +94,10 @@ struct cmsghdr {
 			     (cmsg)->cmsg_len <= (unsigned long) \
 			     ((mhdr)->msg_controllen - \
 			      ((char *)(cmsg) - (char *)(mhdr)->msg_control)))
+#define for_each_cmsghdr(cmsg, msg) \
+	for (cmsg = CMSG_FIRSTHDR(msg); \
+	     cmsg; \
+	     cmsg = CMSG_NXTHDR(msg, cmsg))
 
 /*
  *	Get the next cmsg header

diff --git a/net/core/scm.c b/net/core/scm.c
index b442e7e..3b6899b 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c

@@ -129,8 +129,7 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
 	struct cmsghdr *cmsg;
 	int err;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg))
-	{
+	for_each_cmsghdr(cmsg, msg) {
 		err = -EINVAL;
 
 		/* Verify that cmsg_len is at least sizeof(struct cmsghdr) */

diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 5ab6627..e7413a9 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c

@@ -703,7 +703,7 @@ EXPORT_SYMBOL_GPL(compat_dccp_getsockopt);
 
 static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
 {
-	struct cmsghdr *cmsg = CMSG_FIRSTHDR(msg);
+	struct cmsghdr *cmsg;
 
 	/*
 	 * Assign an (opaque) qpolicy priority value to skb->priority.

@@ -717,8 +717,7 @@ static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
 	 */
 	skb->priority = 0;
 
-	for (; cmsg != NULL; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 9daf217..839db9d 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c

@@ -192,7 +192,7 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc,
 	int err, val;
 	struct cmsghdr *cmsg;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 #if IS_ENABLED(CONFIG_IPV6)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 2cdc383..7252965 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c

@@ -640,7 +640,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 	int len;
 	int err = 0;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		int addr_type;
 
 		if (!CMSG_OK(msg, cmsg)) {

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index a089b6b..b69d87e 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c

@@ -1070,9 +1070,7 @@ static int iucv_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	txmsg.class = 0;
 
 	/* iterate over control messages */
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg;
-		cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg)) {
 			err = -EINVAL;
 			goto out;

diff --git a/net/rds/send.c b/net/rds/send.c
index 0a64541..b23e7b8 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c

@@ -826,7 +826,7 @@ static int rds_rm_size(struct msghdr *msg, int data_len)
 	int cmsg_groups = 0;
 	int retval;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 

@@ -878,7 +878,7 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
 	struct cmsghdr *cmsg;
 	int ret = 0;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 

diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 0b4b9a7..83b4616 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c

@@ -45,7 +45,7 @@ static int rxrpc_sendmsg_cmsg(struct rxrpc_sock *rx, struct msghdr *msg,
 	if (msg->msg_controllen == 0)
 		return -EINVAL;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+	for_each_cmsghdr(cmsg, msg) {
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
 

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 634a2ab..a3802ed 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c

@@ -6592,8 +6592,7 @@ static int sctp_msghdr_parse(const struct msghdr *msg, sctp_cmsgs_t *cmsgs)
 	struct cmsghdr *cmsg;
 	struct msghdr *my_msg = (struct msghdr *)msg;
 
-	for (cmsg = CMSG_FIRSTHDR(msg); cmsg != NULL;
-	     cmsg = CMSG_NXTHDR(my_msg, cmsg)) {
+	for_each_cmsghdr(cmsg, my_msg) {
 		if (!CMSG_OK(my_msg, cmsg))
 			return -EINVAL;
 

-- 
1.7.7

On Wed, 2014-12-10 at 13:36 +0800, Gu Zheng wrote:

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

This looks nicer.
Ideally this would have used: [PATCH V3] as the subject

Signed-off-by: Gu Zheng <redacted>
---
v2: use the lower-case macro name as Joe suggested.

And a description of the v2->v3 change here.

Hi Joe,
On 12/10/2014 02:56 PM, Joe Perches wrote:

On Wed, 2014-12-10 at 13:36 +0800, Gu Zheng wrote:

quoted

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

This looks nicer.
Ideally this would have used: [PATCH V3] as the subject

Thanks for your review.
The previous v2 thread was marked as mistake, so this is the really active
v2 version.

Regards,
Gu

quoted

Signed-off-by: Gu Zheng <redacted>
---
v2: use the lower-case macro name as Joe suggested.

And a description of the v2->v3 change here.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

On Wed, 2014-12-10 at 16:15 +0800, Gu Zheng wrote:

On 12/10/2014 02:56 PM, Joe Perches wrote:

quoted

On Wed, 2014-12-10 at 13:36 +0800, Gu Zheng wrote:

quoted

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

This looks nicer.
Ideally this would have used: [PATCH V3] as the subject

The previous v2 thread was marked as mistake, so this is the really active
v2 version.

A buggy submitted version is still a version.
Using an incremented version number helps.

From: Gu Zheng <redacted>
Date: Wed, 10 Dec 2014 13:36:25 +0800

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

Signed-off-by: Gu Zheng <redacted>

Applied, thanks.

From: David Miller <davem@davemloft.net>
Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)

From: Gu Zheng <redacted>
Date: Wed, 10 Dec 2014 13:36:25 +0800

quoted

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

Signed-off-by: Gu Zheng <redacted>

Applied, thanks.

This breaks the build, I'm reverting.

You cannot use your new macros in
Documentation/networking/timestamping/txtimestamp.c, that is a
userland program and the header you are adding your helper to is not
available to userspace.

This also means you didn't sufficiently test the build of your
changes.

Documentation/networking/timestamping/timestamping.c: In function ‘printpacket’:
Documentation/networking/timestamping/timestamping.c:172:2: warning: implicit declaration of function ‘for_each_cmsghdr’ [-Wimplicit-function-declaration]
Documentation/networking/timestamping/timestamping.c:172:30: error: expected ‘;’ before ‘{’ token
Documentation/networking/timestamping/timestamping.c:161:18: warning: unused variable ‘ts’ [-Wunused-variable]
Documentation/networking/timestamping/timestamping.c:160:17: warning: unused variable ‘tv’ [-Wunused-variable]
make[3]: *** [Documentation/networking/timestamping/timestamping] Error 1
make[3]: *** Waiting for unfinished jobs....
Documentation/networking/timestamping/txtimestamp.c: In function ‘__recv_errmsg_cmsg’:
Documentation/networking/timestamping/txtimestamp.c:187:2: warning: implicit declaration of function ‘for_each_cmsghdr’ [-Wimplicit-function-declaration]
Documentation/networking/timestamping/txtimestamp.c:187:19: error: ‘cmsg’ undeclared (first use in this function)
Documentation/networking/timestamping/txtimestamp.c:187:19: note: each undeclared identifier is reported only once for each function it appears in
Documentation/networking/timestamping/txtimestamp.c:187:30: error: expected ‘;’ before ‘{’ token
Documentation/networking/timestamping/txtimestamp.c:185:18: warning: unused variable ‘cm’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c:184:27: warning: unused variable ‘tss’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c:183:28: warning: unused variable ‘serr’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c: At top level:
Documentation/networking/timestamping/txtimestamp.c:123:13: warning: ‘print_timestamp’ defined but not used [-Wunused-function]
Documentation/networking/timestamping/txtimestamp.c:159:13: warning: ‘print_pktinfo’ defined but not used [-Wunused-function]

Hi David,
On 12/11/2014 03:44 AM, David Miller wrote:

From: David Miller <davem@davemloft.net>
Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)

quoted

From: Gu Zheng <redacted>
Date: Wed, 10 Dec 2014 13:36:25 +0800

quoted

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

Signed-off-by: Gu Zheng <redacted>

Applied, thanks.

This breaks the build, I'm reverting.

I'm very sorry.

You cannot use your new macros in
Documentation/networking/timestamping/txtimestamp.c, that is a
userland program and the header you are adding your helper to is not
available to userspace.

IMO, the user-land programs need to be build with the matched headers separately.
I split the kernel codes and the user-land programs, build and install the kernel first,
and then build the user-land ones. 

This also means you didn't sufficiently test the build of your
changes.

To be honest, I do the test as I mentioned above.

Thanks,
Gu

Documentation/networking/timestamping/timestamping.c: In function ‘printpacket’:
Documentation/networking/timestamping/timestamping.c:172:2: warning: implicit declaration of function ‘for_each_cmsghdr’ [-Wimplicit-function-declaration]
Documentation/networking/timestamping/timestamping.c:172:30: error: expected ‘;’ before ‘{’ token
Documentation/networking/timestamping/timestamping.c:161:18: warning: unused variable ‘ts’ [-Wunused-variable]
Documentation/networking/timestamping/timestamping.c:160:17: warning: unused variable ‘tv’ [-Wunused-variable]
make[3]: *** [Documentation/networking/timestamping/timestamping] Error 1
make[3]: *** Waiting for unfinished jobs....
Documentation/networking/timestamping/txtimestamp.c: In function ‘__recv_errmsg_cmsg’:
Documentation/networking/timestamping/txtimestamp.c:187:2: warning: implicit declaration of function ‘for_each_cmsghdr’ [-Wimplicit-function-declaration]
Documentation/networking/timestamping/txtimestamp.c:187:19: error: ‘cmsg’ undeclared (first use in this function)
Documentation/networking/timestamping/txtimestamp.c:187:19: note: each undeclared identifier is reported only once for each function it appears in
Documentation/networking/timestamping/txtimestamp.c:187:30: error: expected ‘;’ before ‘{’ token
Documentation/networking/timestamping/txtimestamp.c:185:18: warning: unused variable ‘cm’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c:184:27: warning: unused variable ‘tss’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c:183:28: warning: unused variable ‘serr’ [-Wunused-variable]
Documentation/networking/timestamping/txtimestamp.c: At top level:
Documentation/networking/timestamping/txtimestamp.c:123:13: warning: ‘print_timestamp’ defined but not used [-Wunused-function]
Documentation/networking/timestamping/txtimestamp.c:159:13: warning: ‘print_pktinfo’ defined but not used [-Wunused-function]

Hi Joe,
On 12/10/2014 10:51 PM, Joe Perches wrote:

On Wed, 2014-12-10 at 16:15 +0800, Gu Zheng wrote:

quoted

On 12/10/2014 02:56 PM, Joe Perches wrote:

quoted

On Wed, 2014-12-10 at 13:36 +0800, Gu Zheng wrote:

quoted

Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup. 

This looks nicer.
Ideally this would have used: [PATCH V3] as the subject

The previous v2 thread was marked as mistake, so this is the really active
v2 version.

A buggy submitted version is still a version.
Using an incremented version number helps.

Got it, this can also avoid some unnecessary misreading.
Thanks for your suggestion.

Regards,
Gu

From: Gu Zheng <redacted>
Date: Thu, 11 Dec 2014 09:05:59 +0800

IMO, the user-land programs need to be build with the matched
headers separately.

You absolutely cannot provide this new interface to userland and
expect programs to be able to just use it, it's non-standard and does
not exist in older headers.

I do not want this new interface available to userland programs.
They can just open-code the list traversal just like every program
in history has had to do since BSD UNIX.

Hi David,
On 12/11/2014 09:47 AM, David Miller wrote:

From: Gu Zheng <redacted>
Date: Thu, 11 Dec 2014 09:05:59 +0800

quoted

IMO, the user-land programs need to be build with the matched
headers separately.

You absolutely cannot provide this new interface to userland and
expect programs to be able to just use it, it's non-standard and does
not exist in older headers.

Got it, sorry for my mistake, and thanks for your correction.

I do not want this new interface available to userland programs.
They can just open-code the list traversal just like every program
in history has had to do since BSD UNIX.

Agree. Let's leave the user-land ones as they are.
I'll send a new version (without changing user-land ones) soon.

Best regards,
Gu

.