| Message ID | 1497451026-3923-1-git-send-email-cugyly@163.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 14 Jun 2017 22:37:06 +0800 yuan linyu <cugyly@163.com> wrote: > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > --- > net/ipv6/mcast.c | 3 +-- > net/ipv6/ndisc.c | 13 ++----------- > 2 files changed, 3 insertions(+), 13 deletions(-) > > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c > index 07403fa..b186c67 100644 > --- a/net/ipv6/mcast.c > +++ b/net/ipv6/mcast.c > @@ -2008,8 +2008,7 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type) > > memcpy(skb_put(skb, sizeof(ra)), ra, sizeof(ra)); > > - hdr = (struct mld_msg *) skb_put(skb, sizeof(struct mld_msg)); > - memset(hdr, 0, sizeof(struct mld_msg)); > + hdr = (struct mld_msg *) skb_put_zero(skb, sizeof(struct mld_msg)); Why does skb_put_zero return char * instead of void *? If returned void * it would save having to add lots of casts. One could even go farther by making skb_put_zero a macro and use typeof().
On Wed, 2017-06-14 at 08:44 -0700, Stephen Hemminger wrote: > > > memcpy(skb_put(skb, sizeof(ra)), ra, sizeof(ra)); > > > > - hdr = (struct mld_msg *) skb_put(skb, sizeof(struct > > mld_msg)); > > - memset(hdr, 0, sizeof(struct mld_msg)); > > + hdr = (struct mld_msg *) skb_put_zero(skb, sizeof(struct > > mld_msg)); > > Why does skb_put_zero return char * instead of void *? > If returned void * it would save having to add lots of casts. > > One could even go farther by making skb_put_zero a macro and > use typeof(). I just copied it from skb_put() - you could ask the same there? :) No objection to changing it though. johannes
On Wed, 14 Jun 2017 21:01:32 +0200 Johannes Berg <johannes@sipsolutions.net> wrote: > On Wed, 2017-06-14 at 08:44 -0700, Stephen Hemminger wrote: > > > > > memcpy(skb_put(skb, sizeof(ra)), ra, sizeof(ra)); > > > > > > - hdr = (struct mld_msg *) skb_put(skb, sizeof(struct > > > mld_msg)); > > > - memset(hdr, 0, sizeof(struct mld_msg)); > > > + hdr = (struct mld_msg *) skb_put_zero(skb, sizeof(struct > > > mld_msg)); > > > > Why does skb_put_zero return char * instead of void *? > > If returned void * it would save having to add lots of casts. > > > > One could even go farther by making skb_put_zero a macro and > > use typeof(). > > I just copied it from skb_put() - you could ask the same there? :) My taste is to have less casts. Never understood why so many skb_ functions returned char *, probably a leftover from older Unix style.
On Wed, 2017-06-14 at 12:14 -0700, Stephen Hemminger wrote: > > > I just copied it from skb_put() - you could ask the same there? :) > > My taste is to have less casts. Never understood why so many skb_ > functions returned char *, probably a leftover from older Unix style. I agree, for many of them it's awkward and I often just put a (void *) cast in there ... I guess we can just make them void, and if anyone wants it I can easily cook up an spatch to remove casts. johannes
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index 07403fa..b186c67 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -2008,8 +2008,7 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type) memcpy(skb_put(skb, sizeof(ra)), ra, sizeof(ra)); - hdr = (struct mld_msg *) skb_put(skb, sizeof(struct mld_msg)); - memset(hdr, 0, sizeof(struct mld_msg)); + hdr = (struct mld_msg *) skb_put_zero(skb, sizeof(struct mld_msg)); hdr->mld_type = type; hdr->mld_mca = *addr; diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index d310dc4..e6586ed 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -144,21 +144,12 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data, int data_len, int pad) { int space = __ndisc_opt_addr_space(data_len, pad); - u8 *opt = skb_put(skb, space); + u8 *opt = skb_put_zero(skb, space); opt[0] = type; opt[1] = space>>3; - memset(opt + 2, 0, pad); - opt += pad; - space -= pad; - - memcpy(opt+2, data, data_len); - data_len += 2; - opt += data_len; - space -= data_len; - if (space > 0) - memset(opt, 0, space); + memcpy(opt+2+pad, data, data_len); } EXPORT_SYMBOL_GPL(__ndisc_fill_addr_option);