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(-)
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 <[email protected]>
> ---
> 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:
>> 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
>
>> 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/
>
On Wed, 2014-12-10 at 16:15 +0800, Gu Zheng wrote:
> On 12/10/2014 02:56 PM, Joe Perches wrote:
> > 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
>
> 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 <[email protected]>
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 <[email protected]>
Applied, thanks.
From: David Miller <[email protected]>
Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)
> From: Gu Zheng <[email protected]>
> 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 <[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]
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Hi David,
On 12/11/2014 03:44 AM, David Miller wrote:
> From: David Miller <[email protected]>
> Date: Wed, 10 Dec 2014 13:48:04 -0500 (EST)
>
>> From: Gu Zheng <[email protected]>
>> 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 <[email protected]>
>>
>> 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:
>> On 12/10/2014 02:56 PM, Joe Perches wrote:
>>> 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
>>
>> 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 <[email protected]>
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 <[email protected]>
> 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.
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
> .
>