Skip to contentSkip to conversation

net: introduce helper macro for_each_cmsghdr

PATCH Applied Gu Zheng 3 participants 10 messages Dec 10, 2014 – Dec 11, 2014

Subsystems (10)
Networking Network drivers Documentation Crypto API +6 more subsystems

v2 of 3 Applied Latest is v3 →

lore
Share & tools
lore

Gu Zheng

author
#0 lore raw

PATCH v2 1/1 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 <[email protected]>

---
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(-)
Documentation/networking/timestamping/timestamping.c +1 -3
index 5cdfd74..635a43a 100644
@@ -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:
Documentation/networking/timestamping/txtimestamp.c +1 -3
index b32fc2a..f4fd9a2 100644
@@ -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);
crypto/af_alg.c +1 -1
index 6a3ad80..bc21f52 100644
@@ -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)
include/linux/socket.h +4 -0
index bb9b836..0e71278 100644
@@ -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
net/core/scm.c +1 -2
index b442e7e..3b6899b 100644
@@ -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) */
net/dccp/proto.c +2 -3
index 5ab6627..e7413a9 100644
@@ -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;
net/ipv4/ip_sockglue.c +1 -1
index 9daf217..839db9d 100644
@@ -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)
net/ipv6/datagram.c +1 -1
index 2cdc383..7252965 100644
@@ -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)) {
net/iucv/af_iucv.c +1 -3
index a089b6b..b69d87e 100644
@@ -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;
net/rds/send.c +2 -2
index 0a64541..b23e7b8 100644
@@ -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;
net/rxrpc/ar-output.c +1 -1
index 0b4b9a7..83b4616 100644
@@ -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;
net/sctp/socket.c +1 -2
index 634a2ab..a3802ed 100644
@@ -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;
Signature · 1 line
1.7.7
#1 lore raw
Quoted 2 lines · Gu Zheng

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

Quoted 3 lines
Signed-off-by Gu Zheng <[email protected]>

---
v2: use the lower-case macro name as Joe suggested.

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

author
#2 lore raw

Hi Joe,

Quoted 6 lines · Joe Perches
Quoted 2 lines · Gu Zheng

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 14 lines
Quoted 4 lines
Signed-off-by Gu Zheng <[email protected]>

---
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
Quoted 10 lines · Gu Zheng
Quoted 6 lines · Joe Perches
Quoted 2 lines · Gu Zheng

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.

maintainer
#6 lore raw

From: David Miller <[email protected]>
Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)

Quoted 9 lines

From: Gu Zheng <[email protected]>
Date: Wed, 10 Dec 2014 13:36:25 +0800

Quoted 4 lines

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

Signed-off-by Gu Zheng <[email protected]>

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]

author
#7 lore raw

Hi David,

Quoted 6 lines · David Miller

From: David Miller <[email protected]>
Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)

Quoted 3 lines

From: Gu Zheng <[email protected]>
Date: Wed, 10 Dec 2014 13:36:25 +0800

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

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

Signed-off-by Gu Zheng <[email protected]>

Applied, thanks.

This breaks the build, I'm reverting.

I'm very sorry.

Quoted 5 lines

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.

Quoted 3 lines

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

Quoted 20 lines

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]

author
#4 lore raw
In reply to Joe Perches

Hi Joe,

Quoted 6 lines · Joe Perches
Quoted 5 lines · Gu Zheng
Quoted 4 lines · Joe Perches
Quoted 2 lines · Gu Zheng

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

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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

Quoted 2 lines
maintainer
#8 lore raw
In reply to Gu Zheng

From: Gu Zheng <[email protected]>
Date: Thu, 11 Dec 2014 09:05:59 +0800

Quoted 2 lines

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.

author
#9 lore raw

Hi David,

Quoted 9 lines · David Miller

From: Gu Zheng <[email protected]>
Date: Thu, 11 Dec 2014 09:05:59 +0800

Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 2 lines

.