* [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree
@ 2014-06-20 20:36 Fabian Frederick
2014-06-21 8:37 ` Bjørn Mork
0 siblings, 1 reply; 5+ messages in thread
From: Fabian Frederick @ 2014-06-20 20:36 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, Grant Likely, netdev
Fix checkpatch warning:
WARNING: kfree(NULL) is safe this check is probably not required
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dsa/dsa.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 5db37ce..0a49632 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -351,8 +351,7 @@ static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
for (i = 0; i < pd->nr_chips; i++) {
port_index = 0;
while (port_index < DSA_MAX_PORTS) {
- if (pd->chip[i].port_names[port_index])
- kfree(pd->chip[i].port_names[port_index]);
+ kfree(pd->chip[i].port_names[port_index]);
port_index++;
}
kfree(pd->chip[i].rtable);
--
1.8.4.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree
2014-06-20 20:36 [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree Fabian Frederick
@ 2014-06-21 8:37 ` Bjørn Mork
2014-06-21 9:36 ` Fabian Frederick
0 siblings, 1 reply; 5+ messages in thread
From: Bjørn Mork @ 2014-06-21 8:37 UTC (permalink / raw)
To: Fabian Frederick, linux-kernel; +Cc: David S. Miller, Grant Likely, netdev
On 20 June 2014 22:36:47 CEST, Fabian Frederick <fabf@skynet.be> wrote:
>Fix checkpatch warning:
>WARNING: kfree(NULL) is safe this check is probably not required
"probably not" implies that there are cases where the check *is* required. That means that your commit message should explain why this particular check is redundant.
I haven't analyzed your changes here, so they could be fine for all I know. My point is that such analysis is your job when submitting cleanups like this one.
Bjørn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree
2014-06-21 8:37 ` Bjørn Mork
@ 2014-06-21 9:36 ` Fabian Frederick
2014-06-21 14:10 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Fabian Frederick @ 2014-06-21 9:36 UTC (permalink / raw)
To: Bjørn Mork
Cc: linux-kernel, David S. Miller, Grant Likely, netdev, Joe Perches
On Sat, 21 Jun 2014 10:37:24 +0200
Bjørn Mork <bjorn@mork.no> wrote:
>
>
> On 20 June 2014 22:36:47 CEST, Fabian Frederick <fabf@skynet.be> wrote:
> >Fix checkpatch warning:
> >WARNING: kfree(NULL) is safe this check is probably not required
>
> "probably not" implies that there are cases where the check *is* required. That means that your commit message should explain why this particular check is redundant.
>
> I haven't analyzed your changes here, so they could be fine for all I know. My point is that such analysis is your job when submitting cleanups like this one.
>
>
>
AFAIK, any
if(foo)
kfree(foo)
can be updated to kfree(foo) but
if (foo){
kfree(foo)
do something else
}
has to be evaluated ; reason for the "probably" in warning message.
If I'm wrong maybe we could be more verbose in checkpatch :)
(I added Joe Perches in Cc list ; maybe he can help here)
Fabian
> Bjørn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree
2014-06-21 9:36 ` Fabian Frederick
@ 2014-06-21 14:10 ` Joe Perches
2014-06-22 2:22 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2014-06-21 14:10 UTC (permalink / raw)
To: Fabian Frederick
Cc: Bjørn Mork, linux-kernel, David S. Miller, Grant Likely, netdev
On Sat, 2014-06-21 at 11:36 +0200, Fabian Frederick wrote:
> On Sat, 21 Jun 2014 10:37:24 +0200 Bjørn Mork <bjorn@mork.no> wrote:
> > On 20 June 2014 22:36:47 CEST, Fabian Frederick <fabf@skynet.be> wrote:
> > > Fix checkpatch warning:
> > > WARNING: kfree(NULL) is safe this check is probably not required
> >
> > "probably not" implies that there are cases where the check *is*
> > required. That means that your commit message should explain why
> > this particular check is redundant.
> >
> > I haven't analyzed your changes here, so they could be fine for all
> > I know. My point is that such analysis is your job when submitting
> > cleanups like this one.
> AFAIK, any
>
> if(foo)
> kfree(foo)
>
> can be updated to kfree(foo) but
>
> if (foo){
> kfree(foo)
> do something else
> }
>
> has to be evaluated ; reason for the "probably" in warning message.
> If I'm wrong maybe we could be more verbose in checkpatch :)
I think Bjørn is correct here.
Just because checkpatch bleats some message or another,
it's still the submitter's job to validate the code.
In this case, it seems the simple substitution of
"unnecessary null" to the subject would have been
enough validation.
I don't think checkpatch needs updating for this, but
maybe you could propose better language.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree
2014-06-21 14:10 ` Joe Perches
@ 2014-06-22 2:22 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2014-06-22 2:22 UTC (permalink / raw)
To: joe; +Cc: fabf, bjorn, linux-kernel, grant.likely, netdev
From: Joe Perches <joe@perches.com>
Date: Sat, 21 Jun 2014 07:10:06 -0700
> In this case, it seems the simple substitution of
> "unnecessary null" to the subject would have been
> enough validation.
Agreed.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-22 2:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-20 20:36 [PATCH 1/1] net/dsa/dsa.c: remove null test before kfree Fabian Frederick
2014-06-21 8:37 ` Bjørn Mork
2014-06-21 9:36 ` Fabian Frederick
2014-06-21 14:10 ` Joe Perches
2014-06-22 2:22 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.