Message ID | 1423275451-3663-1-git-send-email-linux@rasmusvillemoes.dk |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Sat, 2015-02-07 at 03:17 +0100, Rasmus Villemoes wrote: > src_ip is a pointer to a union vxlan_addr, one member of which is a > struct sockaddr. Passing a pointer to src_ip is wrong; one should pass > the value of src_ip itself. Since %pIS formally expects something of > type struct sockaddr*, let's pass a pointer to the appropriate union > member, though this of course doesn't change the generated code. Hello Rasmus Are you finding these mismatches by hand or are you using some tool? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > src_ip is a pointer to a union vxlan_addr, one member of which is a > struct sockaddr. Passing a pointer to src_ip is wrong; one should pass > the value of src_ip itself. Since %pIS formally expects something of > type struct sockaddr*, let's pass a pointer to the appropriate union > member, though this of course doesn't change the generated code. > It is a union, this doesn't harm. > Fixes: e4c7ed415387 ("vxlan: add ipv6 support") > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > drivers/net/vxlan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index a8c755dcab14..11defbb24183 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -991,7 +991,7 @@ static bool vxlan_snoop(struct net_device *dev, > if (net_ratelimit()) > netdev_info(dev, > "%pM migrated from %pIS to %pIS\n", > - src_mac, &rdst->remote_ip, &src_ip); > + src_mac, &rdst->remote_ip.sa, &src_ip->sa); > > rdst->remote_ip = *src_ip; > f->updated = jiffies; Since you are on it, there is another similar place in vxlan too. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 07 2015, Cong Wang <xiyou.wangcong@gmail.com> wrote: > On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes > <linux@rasmusvillemoes.dk> wrote: >> src_ip is a pointer to a union vxlan_addr, one member of which is a >> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass >> the value of src_ip itself. Since %pIS formally expects something of >> type struct sockaddr*, let's pass a pointer to the appropriate union >> member, though this of course doesn't change the generated code. >> > > > It is a union, this doesn't harm. > Just to be clear: This fixes a real bug. The minimal fix had been - src_mac, &rdst->remote_ip, &src_ip); + src_mac, &rdst->remote_ip, src_ip); but I through in the cosmetic improvements while the line needed changing anyway. > Since you are on it, there is another similar place in vxlan too. ... which is why I didn't change that other occurrence. Rasmus -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 07 2015, Joe Perches <joe@perches.com> wrote: > On Sat, 2015-02-07 at 03:17 +0100, Rasmus Villemoes wrote: >> src_ip is a pointer to a union vxlan_addr, one member of which is a >> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass >> the value of src_ip itself. Since %pIS formally expects something of >> type struct sockaddr*, let's pass a pointer to the appropriate union >> member, though this of course doesn't change the generated code. > > Hello Rasmus > > Are you finding these mismatches by hand or > are you using some tool? I've extended smatch to do format checking (mostly for the %p extensions; gcc already does all the other stuff just fine). I'll try and see if I can get it on github sometime in the coming week. Rasmus -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 7, 2015 at 4:34 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > On Sat, Feb 07 2015, Cong Wang <xiyou.wangcong@gmail.com> wrote: > >> On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes >> <linux@rasmusvillemoes.dk> wrote: >>> src_ip is a pointer to a union vxlan_addr, one member of which is a >>> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass >>> the value of src_ip itself. Since %pIS formally expects something of >>> type struct sockaddr*, let's pass a pointer to the appropriate union >>> member, though this of course doesn't change the generated code. >>> >> >> >> It is a union, this doesn't harm. >> > > Just to be clear: This fixes a real bug. The minimal fix had been > > - src_mac, &rdst->remote_ip, &src_ip); > + src_mac, &rdst->remote_ip, src_ip); > > but I through in the cosmetic improvements while the line needed > changing anyway. > Ah, I misread the patch. Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Rasmus Villemoes <linux@rasmusvillemoes.dk> Date: Sat, 7 Feb 2015 03:17:31 +0100 > src_ip is a pointer to a union vxlan_addr, one member of which is a > struct sockaddr. Passing a pointer to src_ip is wrong; one should pass > the value of src_ip itself. Since %pIS formally expects something of > type struct sockaddr*, let's pass a pointer to the appropriate union > member, though this of course doesn't change the generated code. > > Fixes: e4c7ed415387 ("vxlan: add ipv6 support") > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index a8c755dcab14..11defbb24183 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -991,7 +991,7 @@ static bool vxlan_snoop(struct net_device *dev, if (net_ratelimit()) netdev_info(dev, "%pM migrated from %pIS to %pIS\n", - src_mac, &rdst->remote_ip, &src_ip); + src_mac, &rdst->remote_ip.sa, &src_ip->sa); rdst->remote_ip = *src_ip; f->updated = jiffies;
src_ip is a pointer to a union vxlan_addr, one member of which is a struct sockaddr. Passing a pointer to src_ip is wrong; one should pass the value of src_ip itself. Since %pIS formally expects something of type struct sockaddr*, let's pass a pointer to the appropriate union member, though this of course doesn't change the generated code. Fixes: e4c7ed415387 ("vxlan: add ipv6 support") Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/net/vxlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)