From 59ae1d127ac0ae404baf414c434ba2651b793f46 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 16 Jun 2017 14:29:20 +0200 Subject: networking: introduce and use skb_put_data() A common pattern with skb_put() is to just want to memcpy() some data into the new space, introduce skb_put_data() for this. An spatch similar to the one for skb_put_zero() converts many of the places using it: @@ identifier p, p2; expression len, skb, data; type t, t2; @@ ( -p = skb_put(skb, len); +p = skb_put_data(skb, data, len); | -p = (t)skb_put(skb, len); +p = skb_put_data(skb, data, len); ) ( p2 = (t2)p; -memcpy(p2, data, len); | -memcpy(p, data, len); ) @@ type t, t2; identifier p, p2; expression skb, data; @@ t *p; ... ( -p = skb_put(skb, sizeof(t)); +p = skb_put_data(skb, data, sizeof(t)); | -p = (t *)skb_put(skb, sizeof(t)); +p = skb_put_data(skb, data, sizeof(t)); ) ( p2 = (t2)p; -memcpy(p2, data, sizeof(*p)); | -memcpy(p, data, sizeof(*p)); ) @@ expression skb, len, data; @@ -memcpy(skb_put(skb, len), data, len); +skb_put_data(skb, data, len); (again, manually post-processed to retain some comments) Reviewed-by: Stephen Hemminger Signed-off-by: Johannes Berg Signed-off-by: David S. Miller --- include/linux/mISDNif.h | 2 +- include/linux/skbuff.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/mISDNif.h b/include/linux/mISDNif.h index ac02c54520e9..a7330eb3ec64 100644 --- a/include/linux/mISDNif.h +++ b/include/linux/mISDNif.h @@ -554,7 +554,7 @@ _alloc_mISDN_skb(u_int prim, u_int id, u_int len, void *dp, gfp_t gfp_mask) if (!skb) return NULL; if (len) - memcpy(skb_put(skb, len), dp, len); + skb_put_data(skb, dp, len); hh = mISDN_HEAD_P(skb); hh->prim = prim; hh->id = id; diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 01ea64d0783a..5af5385a0e72 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1913,6 +1913,16 @@ static inline void *skb_put_zero(struct sk_buff *skb, unsigned int len) return tmp; } +static inline void *skb_put_data(struct sk_buff *skb, const void *data, + unsigned int len) +{ + void *tmp = skb_put(skb, len); + + memcpy(tmp, data, len); + + return tmp; +} + unsigned char *skb_push(struct sk_buff *skb, unsigned int len); static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) { -- cgit v1.2.3 From 4df864c1d9afb46e2461a9f808d9f11a42d31bad Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 16 Jun 2017 14:29:21 +0200 Subject: networking: make skb_put & friends return void pointers It seems like a historic accident that these return unsigned char *, and in many places that means casts are required, more often than not. Make these functions (skb_put, __skb_put and pskb_put) return void * and remove all the casts across the tree, adding a (u8 *) cast only where the unsigned char pointer was used directly, all done with the following spatch: @@ expression SKB, LEN; typedef u8; identifier fn = { skb_put, __skb_put }; @@ - *(fn(SKB, LEN)) + *(u8 *)fn(SKB, LEN) @@ expression E, SKB, LEN; identifier fn = { skb_put, __skb_put }; type T; @@ - E = ((T *)(fn(SKB, LEN))) + E = fn(SKB, LEN) which actually doesn't cover pskb_put since there are only three users overall. A handful of stragglers were converted manually, notably a macro in drivers/isdn/i4l/isdn_bsdcomp.c and, oddly enough, one of the many instances in net/bluetooth/hci_sock.c. In the former file, I also had to fix one whitespace problem spatch introduced. Signed-off-by: Johannes Berg Signed-off-by: David S. Miller --- include/linux/skbuff.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 5af5385a0e72..454ea37dddbb 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1893,11 +1893,11 @@ static inline void skb_set_tail_pointer(struct sk_buff *skb, const int offset) /* * Add data to an sk_buff */ -unsigned char *pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len); -unsigned char *skb_put(struct sk_buff *skb, unsigned int len); -static inline unsigned char *__skb_put(struct sk_buff *skb, unsigned int len) +void *pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len); +void *skb_put(struct sk_buff *skb, unsigned int len); +static inline void *__skb_put(struct sk_buff *skb, unsigned int len) { - unsigned char *tmp = skb_tail_pointer(skb); + void *tmp = skb_tail_pointer(skb); SKB_LINEAR_ASSERT(skb); skb->tail += len; skb->len += len; -- cgit v1.2.3 From af72868b9070d1b843c829f0d0d0b22c04a20815 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 16 Jun 2017 14:29:22 +0200 Subject: networking: make skb_pull & friends return void pointers It seems like a historic accident that these return unsigned char *, and in many places that means casts are required, more often than not. Make these functions return void * and remove all the casts across the tree, adding a (u8 *) cast only where the unsigned char pointer was used directly, all done with the following spatch: @@ expression SKB, LEN; typedef u8; identifier fn = { skb_pull, __skb_pull, skb_pull_inline, __pskb_pull_tail, __pskb_pull, pskb_pull }; @@ - *(fn(SKB, LEN)) + *(u8 *)fn(SKB, LEN) @@ expression E, SKB, LEN; identifier fn = { skb_pull, __skb_pull, skb_pull_inline, __pskb_pull_tail, __pskb_pull, pskb_pull }; type T; @@ - E = ((T *)(fn(SKB, LEN))) + E = fn(SKB, LEN) Signed-off-by: Johannes Berg Signed-off-by: David S. Miller --- include/linux/skbuff.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'include/linux') diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 454ea37dddbb..ac9d10dadd1a 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1931,22 +1931,22 @@ static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) return skb->data; } -unsigned char *skb_pull(struct sk_buff *skb, unsigned int len); -static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len) +void *skb_pull(struct sk_buff *skb, unsigned int len); +static inline void *__skb_pull(struct sk_buff *skb, unsigned int len) { skb->len -= len; BUG_ON(skb->len < skb->data_len); return skb->data += len; } -static inline unsigned char *skb_pull_inline(struct sk_buff *skb, unsigned int len) +static inline void *skb_pull_inline(struct sk_buff *skb, unsigned int len) { return unlikely(len > skb->len) ? NULL : __skb_pull(skb, len); } -unsigned char *__pskb_pull_tail(struct sk_buff *skb, int delta); +void *__pskb_pull_tail(struct sk_buff *skb, int delta); -static inline unsigned char *__pskb_pull(struct sk_buff *skb, unsigned int len) +static inline void *__pskb_pull(struct sk_buff *skb, unsigned int len) { if (len > skb_headlen(skb) && !__pskb_pull_tail(skb, len - skb_headlen(skb))) @@ -1955,7 +1955,7 @@ static inline unsigned char *__pskb_pull(struct sk_buff *skb, unsigned int len) return skb->data += len; } -static inline unsigned char *pskb_pull(struct sk_buff *skb, unsigned int len) +static inline void *pskb_pull(struct sk_buff *skb, unsigned int len) { return unlikely(len > skb->len) ? NULL : __pskb_pull(skb, len); } @@ -2938,7 +2938,7 @@ static inline void skb_postpush_rcsum(struct sk_buff *skb, __skb_postpush_rcsum(skb, start, len, 0); } -unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len); +void *skb_pull_rcsum(struct sk_buff *skb, unsigned int len); /** * skb_push_rcsum - push skb and update receive checksum -- cgit v1.2.3 From d58ff35122847a83ba55394e2ae3a1527b6febf5 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 16 Jun 2017 14:29:23 +0200 Subject: networking: make skb_push & __skb_push return void pointers It seems like a historic accident that these return unsigned char *, and in many places that means casts are required, more often than not. Make these functions return void * and remove all the casts across the tree, adding a (u8 *) cast only where the unsigned char pointer was used directly, all done with the following spatch: @@ expression SKB, LEN; typedef u8; identifier fn = { skb_push, __skb_push, skb_push_rcsum }; @@ - *(fn(SKB, LEN)) + *(u8 *)fn(SKB, LEN) @@ expression E, SKB, LEN; identifier fn = { skb_push, __skb_push, skb_push_rcsum }; type T; @@ - E = ((T *)(fn(SKB, LEN))) + E = fn(SKB, LEN) @@ expression SKB, LEN; identifier fn = { skb_push, __skb_push, skb_push_rcsum }; @@ - fn(SKB, LEN)[0] + *(u8 *)fn(SKB, LEN) Note that the last part there converts from push(...)[0] to the more idiomatic *(u8 *)push(...). Signed-off-by: Johannes Berg Signed-off-by: David S. Miller --- include/linux/if_vlan.h | 2 +- include/linux/skbuff.h | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index 283dc2f5364d..5e6a2d4dc366 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -318,7 +318,7 @@ static inline int __vlan_insert_tag(struct sk_buff *skb, if (skb_cow_head(skb, VLAN_HLEN) < 0) return -ENOMEM; - veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN); + veth = skb_push(skb, VLAN_HLEN); /* Move the mac addresses to the beginning of the new header. */ memmove(skb->data, skb->data + VLAN_HLEN, 2 * ETH_ALEN); diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index ac9d10dadd1a..46bd514e719c 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1923,8 +1923,8 @@ static inline void *skb_put_data(struct sk_buff *skb, const void *data, return tmp; } -unsigned char *skb_push(struct sk_buff *skb, unsigned int len); -static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) +void *skb_push(struct sk_buff *skb, unsigned int len); +static inline void *__skb_push(struct sk_buff *skb, unsigned int len) { skb->data -= len; skb->len += len; @@ -2951,8 +2951,7 @@ void *skb_pull_rcsum(struct sk_buff *skb, unsigned int len); * that the checksum difference is zero (e.g., a valid IP header) * or you are setting ip_summed to CHECKSUM_NONE. */ -static inline unsigned char *skb_push_rcsum(struct sk_buff *skb, - unsigned int len) +static inline void *skb_push_rcsum(struct sk_buff *skb, unsigned int len) { skb_push(skb, len); skb_postpush_rcsum(skb, skb->data, len); -- cgit v1.2.3 From 634fef61076d644b989b86abc2f560d81a089a31 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 16 Jun 2017 14:29:24 +0200 Subject: networking: add and use skb_put_u8() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Joe and Bjørn suggested that it'd be nicer to not have the cast in the fairly common case of doing *(u8 *)skb_put(skb, 1) = c; Add skb_put_u8() for this case, and use it across the code, using the following spatch: @@ expression SKB, C, S; typedef u8; identifier fn = {skb_put}; fresh identifier fn2 = fn ## "_u8"; @@ - *(u8 *)fn(SKB, S) = C; + fn2(SKB, C); Note that due to the "S", the spatch isn't perfect, it should have checked that S is 1, but there's also places that use a sizeof expression like sizeof(var) or sizeof(u8) etc. Turns out that nobody ever did something like *(u8 *)skb_put(skb, 2) = c; which would be wrong anyway since the second byte wouldn't be initialized. Suggested-by: Joe Perches Suggested-by: Bjørn Mork Signed-off-by: Johannes Berg Signed-off-by: David S. Miller --- include/linux/skbuff.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'include/linux') diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 46bd514e719c..852feacf4bbf 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1923,6 +1923,11 @@ static inline void *skb_put_data(struct sk_buff *skb, const void *data, return tmp; } +static inline void skb_put_u8(struct sk_buff *skb, u8 val) +{ + *(u8 *)skb_put(skb, 1) = val; +} + void *skb_push(struct sk_buff *skb, unsigned int len); static inline void *__skb_push(struct sk_buff *skb, unsigned int len) { -- cgit v1.2.3