2014-11-18 20:27:17

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 1/1] net: sched: Deletion of an unnecessary check before the function call "kfree"

From: Markus Elfring <[email protected]>
Date: Tue, 18 Nov 2014 21:21:16 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
net/sched/cls_bpf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 0e30d58..f323944 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -212,8 +212,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,

if (fp_old)
bpf_prog_destroy(fp_old);
- if (bpf_old)
- kfree(bpf_old);
+ kfree(bpf_old);

return 0;

--
2.1.3


2014-11-19 16:48:05

by John Fastabend

[permalink] [raw]
Subject: Re: [PATCH 1/1] net: sched: Deletion of an unnecessary check before the function call "kfree"

On 11/18/2014 12:26 PM, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Tue, 18 Nov 2014 21:21:16 +0100
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> net/sched/cls_bpf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 0e30d58..f323944 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -212,8 +212,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
>
> if (fp_old)
> bpf_prog_destroy(fp_old);
> - if (bpf_old)
> - kfree(bpf_old);
> + kfree(bpf_old);
>
> return 0;
>
>

Maybe I need some coffee but I can't figure out what this
patch is against...

# grep bpf_old ./net/sched/cls_bpf.c
#

Marcus, what tree are you looking at?

--
John Fastabend Intel Corporation

2014-11-19 17:00:56

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 1/1] net: sched: Deletion of an unnecessary check before the function call "kfree"

On 11/19/2014 05:47 PM, John Fastabend wrote:
> On 11/18/2014 12:26 PM, SF Markus Elfring wrote:
>> From: Markus Elfring <[email protected]>
...
>> if (fp_old)
>> bpf_prog_destroy(fp_old);
>> - if (bpf_old)
>> - kfree(bpf_old);
>> + kfree(bpf_old);
>>
>> return 0;
>>
>
> Maybe I need some coffee but I can't figure out what this
> patch is against...
>
> # grep bpf_old ./net/sched/cls_bpf.c
> #

Coffee is always good. :)

Yeah, you actually removed this in commit 1f947bf151e90ec ("net:
sched: rcu'ify cls_bpf"), so looks like Markus's tree is not
up to date ...

2014-11-19 18:50:08

by SF Markus Elfring

[permalink] [raw]
Subject: Re: net: sched: Deletion of an unnecessary check before the function call "kfree"

> Marcus, what tree are you looking at?

I dared to base this update suggestion on the source files
for Linux 3.17.3. Are newer software developments relevant here?

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/net/sched/

Regards,
Markus

2014-11-19 19:09:50

by Daniel Borkmann

[permalink] [raw]
Subject: Re: net: sched: Deletion of an unnecessary check before the function call "kfree"

On 11/19/2014 07:49 PM, SF Markus Elfring wrote:
>> Marcus, what tree are you looking at?
>
> I dared to base this update suggestion on the source files
> for Linux 3.17.3. Are newer software developments relevant here?
>
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/net/sched/

Pointing to linux-next and saying you based your changes on 3.17.3
is confusing, please look correctly when you provide a link ...

Again, John's commit 1f947bf151e90ec0b removed the relevant part in
cls_bpf_modify_existing() that you're trying to modify:

$ git grep -n bpf_old net/sched/cls_bpf.c
$

Therefore, this patch is not needed.

Thanks !

2014-11-20 08:48:14

by Julia Lawall

[permalink] [raw]
Subject: Re: net: sched: Deletion of an unnecessary check before the function call "kfree"

On Wed, 19 Nov 2014, SF Markus Elfring wrote:

> > Marcus, what tree are you looking at?
>
> I dared to base this update suggestion on the source files
> for Linux 3.17.3. Are newer software developments relevant here?

You should always use linux-next. You should update it every day.

julia

> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/net/sched/
>
> Regards,
> Markus
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-11-20 09:23:11

by Daniel Borkmann

[permalink] [raw]
Subject: Re: net: sched: Deletion of an unnecessary check before the function call "kfree"

On 11/20/2014 09:47 AM, Julia Lawall wrote:
> On Wed, 19 Nov 2014, SF Markus Elfring wrote:
>
>>> Marcus, what tree are you looking at?
>>
>> I dared to base this update suggestion on the source files
>> for Linux 3.17.3. Are newer software developments relevant here?
>
> You should always use linux-next. You should update it every day.

Well, if you send in cleanups to netdev, you should always target
the net-next tree:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git