| Message ID | 1306140167.2869.8.camel@edumazet-laptop |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2011-05-23 at 10:42 +0200, Eric Dumazet wrote: > net/ipv6/xfrm6_tunnel.c: In function ‘xfrm6_tunnel_rcv’: > net/ipv6/xfrm6_tunnel.c:244:53: warning: the omitted middle operand > in ?: will always be ‘true’, suggest explicit middle operand > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > net/ipv6/xfrm6_tunnel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c > index a6770a0..fb9b0c3 100644 > --- a/net/ipv6/xfrm6_tunnel.c > +++ b/net/ipv6/xfrm6_tunnel.c > @@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb) > __be32 spi; > > spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr); > - return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0; > + return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? 1 : 0; > } > > static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt, I suspect that this was intended to return the result of xfrm6_rcv_spi() if > 0. But if it really is intended to return the result of the inequality, then the '?:' operation is not needed at all. Ben.
From: Ben Hutchings <bhutchings@solarflare.com> Date: Mon, 23 May 2011 08:36:00 -0700 > On Mon, 2011-05-23 at 10:42 +0200, Eric Dumazet wrote: >> net/ipv6/xfrm6_tunnel.c: In function ‘xfrm6_tunnel_rcv’: >> net/ipv6/xfrm6_tunnel.c:244:53: warning: the omitted middle operand >> in ?: will always be ‘true’, suggest explicit middle operand >> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> >> --- >> net/ipv6/xfrm6_tunnel.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c >> index a6770a0..fb9b0c3 100644 >> --- a/net/ipv6/xfrm6_tunnel.c >> +++ b/net/ipv6/xfrm6_tunnel.c >> @@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb) >> __be32 spi; >> >> spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr); >> - return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0; >> + return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? 1 : 0; >> } >> >> static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt, > > I suspect that this was intended to return the result of xfrm6_rcv_spi() > if > 0. I also suspect this was the intent, but I'm not sure why it matters at all. The equivalent code implementing the same operations on the ipv4 side return xfrm4_rcv_spi()'s return value directly. So we need to either decide that we can do the same thing here on the ipv6 side, or document exactly why we can't.
On Mon, May 23, 2011 at 04:33:02PM -0400, David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Mon, 23 May 2011 08:36:00 -0700 > > > On Mon, 2011-05-23 at 10:42 +0200, Eric Dumazet wrote: > >> net/ipv6/xfrm6_tunnel.c: In function ‘xfrm6_tunnel_rcv’: > >> net/ipv6/xfrm6_tunnel.c:244:53: warning: the omitted middle operand > >> in ?: will always be ‘true’, suggest explicit middle operand > >> > >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > >> --- > >> net/ipv6/xfrm6_tunnel.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c > >> index a6770a0..fb9b0c3 100644 > >> --- a/net/ipv6/xfrm6_tunnel.c > >> +++ b/net/ipv6/xfrm6_tunnel.c > >> @@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb) > >> __be32 spi; > >> > >> spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr); > >> - return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0; > >> + return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? 1 : 0; > >> } > >> > >> static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt, > > > > I suspect that this was intended to return the result of xfrm6_rcv_spi() > > if > 0. > > I also suspect this was the intent, but I'm not sure why it matters > at all. > > The equivalent code implementing the same operations on the ipv4 > side return xfrm4_rcv_spi()'s return value directly. > > So we need to either decide that we can do the same thing here on the > ipv6 side, or document exactly why we can't. I think we can return the value directly like ipv4 does it. xfrm6_rcv_spi() returns the return value of xfrm_input() which returns 0 in any case. -- 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/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c index a6770a0..fb9b0c3 100644 --- a/net/ipv6/xfrm6_tunnel.c +++ b/net/ipv6/xfrm6_tunnel.c @@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb) __be32 spi; spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr); - return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0; + return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? 1 : 0; } static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
net/ipv6/xfrm6_tunnel.c: In function ‘xfrm6_tunnel_rcv’: net/ipv6/xfrm6_tunnel.c:244:53: warning: the omitted middle operand in ?: will always be ‘true’, suggest explicit middle operand Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/ipv6/xfrm6_tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 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