* [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
@ 2009-02-24 7:45 Wei Yongjun
2009-02-24 9:33 ` Yang Hongyang
2009-02-25 7:38 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Wei Yongjun @ 2009-02-24 7:45 UTC (permalink / raw)
To: netdev, David Miller
Remove some pointless conditionals before kfree_skb().
The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@@
expression E;
@@
- if (E)
- kfree_skb(E);
+ kfree_skb(E);
// </smpl>
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
net/ipv6/ipv6_sockglue.c | 3 +--
net/ipv6/tcp_ipv6.c | 6 ++----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 40f3246..d31df0f 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -218,8 +218,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
if (opt)
sock_kfree_s(sk, opt, opt->tot_len);
pktopt = xchg(&np->pktoptions, NULL);
- if (pktopt)
- kfree_skb(pktopt);
+ kfree_skb(pktopt);
sk->sk_destruct = inet_sock_destruct;
/*
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e5b85d4..273a5dd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -533,8 +533,7 @@ static inline void syn_flood_warning(struct sk_buff *skb)
static void tcp_v6_reqsk_destructor(struct request_sock *req)
{
- if (inet6_rsk(req)->pktopts)
- kfree_skb(inet6_rsk(req)->pktopts);
+ kfree_skb(inet6_rsk(req)->pktopts);
}
#ifdef CONFIG_TCP_MD5SIG
@@ -1611,8 +1610,7 @@ ipv6_pktoptions:
}
}
- if (opt_skb)
- kfree_skb(opt_skb);
+ kfree_skb(opt_skb);
return 0;
}
--
1.5.3.8
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
2009-02-24 7:45 [PATCH] ipv6: Remove some pointless conditionals before kfree_skb() Wei Yongjun
@ 2009-02-24 9:33 ` Yang Hongyang
2009-02-24 9:47 ` Herbert Xu
2009-02-25 7:38 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Yang Hongyang @ 2009-02-24 9:33 UTC (permalink / raw)
To: Wei Yongjun; +Cc: netdev, David Miller
Wei Yongjun wrote:
> Remove some pointless conditionals before kfree_skb().
>
> The semantic match that finds the problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E;
> @@
> - if (E)
> - kfree_skb(E);
> + kfree_skb(E);
> // </smpl>
I think kfree_skb() suppose that you have handled NULL pointer
because it uses `unlikely` to check the pointer.
So I don't think these conditions are pointless...
437 void kfree_skb(struct sk_buff *skb)
438 {
439 if (unlikely(!skb))
440 return;
>
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
> net/ipv6/ipv6_sockglue.c | 3 +--
> net/ipv6/tcp_ipv6.c | 6 ++----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 40f3246..d31df0f 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -218,8 +218,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> if (opt)
> sock_kfree_s(sk, opt, opt->tot_len);
> pktopt = xchg(&np->pktoptions, NULL);
> - if (pktopt)
> - kfree_skb(pktopt);
> + kfree_skb(pktopt);
>
> sk->sk_destruct = inet_sock_destruct;
> /*
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e5b85d4..273a5dd 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -533,8 +533,7 @@ static inline void syn_flood_warning(struct sk_buff *skb)
>
> static void tcp_v6_reqsk_destructor(struct request_sock *req)
> {
> - if (inet6_rsk(req)->pktopts)
> - kfree_skb(inet6_rsk(req)->pktopts);
> + kfree_skb(inet6_rsk(req)->pktopts);
> }
>
> #ifdef CONFIG_TCP_MD5SIG
> @@ -1611,8 +1610,7 @@ ipv6_pktoptions:
> }
> }
>
> - if (opt_skb)
> - kfree_skb(opt_skb);
> + kfree_skb(opt_skb);
> return 0;
> }
>
--
Regards
Yang Hongyang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
2009-02-24 9:33 ` Yang Hongyang
@ 2009-02-24 9:47 ` Herbert Xu
2009-02-24 9:54 ` Yang Hongyang
0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2009-02-24 9:47 UTC (permalink / raw)
To: Yang Hongyang; +Cc: yjwei, netdev, davem
Yang Hongyang <yanghy@cn.fujitsu.com> wrote:
>
> I think kfree_skb() suppose that you have handled NULL pointer
> because it uses `unlikely` to check the pointer.
> So I don't think these conditions are pointless...
> 437 void kfree_skb(struct sk_buff *skb)
> 438 {
> 439 if (unlikely(!skb))
> 440 return;
These checks were specifically added so that callers didn't have
to check for NULL.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
2009-02-24 9:47 ` Herbert Xu
@ 2009-02-24 9:54 ` Yang Hongyang
2009-02-24 9:55 ` Herbert Xu
0 siblings, 1 reply; 6+ messages in thread
From: Yang Hongyang @ 2009-02-24 9:54 UTC (permalink / raw)
To: Herbert Xu; +Cc: yjwei, netdev, davem
Herbert Xu wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> wrote:
>> I think kfree_skb() suppose that you have handled NULL pointer
>> because it uses `unlikely` to check the pointer.
>> So I don't think these conditions are pointless...
>> 437 void kfree_skb(struct sk_buff *skb)
>> 438 {
>> 439 if (unlikely(!skb))
>> 440 return;
>
> These checks were specifically added so that callers didn't have
> to check for NULL.
Yes,but if the the callers checked for NULL,it's OK right?
>
> Cheers,
--
Regards
Yang Hongyang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
2009-02-24 9:54 ` Yang Hongyang
@ 2009-02-24 9:55 ` Herbert Xu
0 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2009-02-24 9:55 UTC (permalink / raw)
To: Yang Hongyang; +Cc: yjwei, netdev, davem
On Tue, Feb 24, 2009 at 05:54:25PM +0800, Yang Hongyang wrote:
>
> > These checks were specifically added so that callers didn't have
> > to check for NULL.
>
> Yes,but if the the callers checked for NULL,it's OK right?
Well people will continue to send patches to remove them until
they're all gone :)
Really this isn't even worth discussing.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipv6: Remove some pointless conditionals before kfree_skb()
2009-02-24 7:45 [PATCH] ipv6: Remove some pointless conditionals before kfree_skb() Wei Yongjun
2009-02-24 9:33 ` Yang Hongyang
@ 2009-02-25 7:38 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2009-02-25 7:38 UTC (permalink / raw)
To: yjwei; +Cc: netdev
From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Tue, 24 Feb 2009 15:45:33 +0800
> Remove some pointless conditionals before kfree_skb().
>
> The semantic match that finds the problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
...
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-02-25 7:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-24 7:45 [PATCH] ipv6: Remove some pointless conditionals before kfree_skb() Wei Yongjun
2009-02-24 9:33 ` Yang Hongyang
2009-02-24 9:47 ` Herbert Xu
2009-02-24 9:54 ` Yang Hongyang
2009-02-24 9:55 ` Herbert Xu
2009-02-25 7:38 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).