* [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
@ 2012-09-12 15:06 Peter Senna Tschudin
2012-09-13 8:31 ` David Laight
2012-09-13 9:52 ` Tilman Schmidt
0 siblings, 2 replies; 6+ messages in thread
From: Peter Senna Tschudin @ 2012-09-12 15:06 UTC (permalink / raw)
To: Hansjoerg Lipp
Cc: kernel-janitors, Tilman Schmidt, Karsten Keil,
gigaset307x-common, netdev, linux-kernel
From: Peter Senna Tschudin <peter.senna@gmail.com>
Remove useless kfree() and clean up code related to the removal.
The semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
position p1,p2;
expression x;
@@
if (x@p1 == NULL) { ... kfree@p2(x); ... return ...; }
@unchanged exists@
position r.p1,r.p2;
expression e <= r.x,x,e1;
iterator I;
statement S;
@@
if (x@p1 == NULL) { ... when != I(x,...) S
when != e = e1
when != e += e1
when != e -= e1
when != ++e
when != --e
when != e++
when != e--
when != &e
kfree@p2(x); ... return ...; }
@ok depends on unchanged exists@
position any r.p1;
position r.p2;
expression x;
@@
... when != true x@p1 == NULL
kfree@p2(x);
@depends on !ok && unchanged@
position r.p2;
expression x;
@@
*kfree@p2(x);
// </smpl>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
drivers/isdn/gigaset/common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index aa41485..30a6b17 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -1123,7 +1123,6 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
return drv;
error:
- kfree(drv->cs);
kfree(drv);
return NULL;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
2012-09-12 15:06 [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree Peter Senna Tschudin
@ 2012-09-13 8:31 ` David Laight
2012-09-13 9:19 ` Dan Carpenter
2012-09-13 9:27 ` Peter Senna Tschudin
2012-09-13 9:52 ` Tilman Schmidt
1 sibling, 2 replies; 6+ messages in thread
From: David Laight @ 2012-09-13 8:31 UTC (permalink / raw)
To: Peter Senna Tschudin, Hansjoerg Lipp
Cc: kernel-janitors, Tilman Schmidt, Karsten Keil,
gigaset307x-common, netdev, linux-kernel
> Remove useless kfree() and clean up code related to the removal.
...
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index aa41485..30a6b17 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -1123,7 +1123,6 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
> return drv;
>
> error:
> - kfree(drv->cs);
> kfree(drv);
> return NULL;
> }
>
Seems to me that (assuming kfree(NULL) is ok) the kfree()
is best left in - just in case some other error path is
added after drv->cs is assigned.
Better safe than a memory leak.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
2012-09-13 8:31 ` David Laight
@ 2012-09-13 9:19 ` Dan Carpenter
2012-09-13 9:27 ` Peter Senna Tschudin
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-13 9:19 UTC (permalink / raw)
To: David Laight
Cc: Peter Senna Tschudin, Hansjoerg Lipp, kernel-janitors,
Tilman Schmidt, Karsten Keil, gigaset307x-common, netdev,
linux-kernel
On Thu, Sep 13, 2012 at 09:31:45AM +0100, David Laight wrote:
> > Remove useless kfree() and clean up code related to the removal.
> ...
> > diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> > index aa41485..30a6b17 100644
> > --- a/drivers/isdn/gigaset/common.c
> > +++ b/drivers/isdn/gigaset/common.c
> > @@ -1123,7 +1123,6 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
> > return drv;
> >
> > error:
> > - kfree(drv->cs);
> > kfree(drv);
> > return NULL;
> > }
> >
>
> Seems to me that (assuming kfree(NULL) is ok) the kfree()
> is best left in - just in case some other error path is
> added after drv->cs is assigned.
> Better safe than a memory leak.
No. Delete vestigial code. There are all kinds of no-ops we could
add to the unwind bits of code if we wanted to so why is "drv->cs"
better than anything else.
First of all, no one is going to change the ISDN code to add an
allocation, but if they did then they have to make sure they don't
leak memory. That's how it always works. They can't just assume
that there is going to be a forgotten kfree() hanging off to the
side which is going to take care of it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
2012-09-13 8:31 ` David Laight
2012-09-13 9:19 ` Dan Carpenter
@ 2012-09-13 9:27 ` Peter Senna Tschudin
1 sibling, 0 replies; 6+ messages in thread
From: Peter Senna Tschudin @ 2012-09-13 9:27 UTC (permalink / raw)
To: David Laight
Cc: Hansjoerg Lipp, kernel-janitors, Tilman Schmidt, Karsten Keil,
gigaset307x-common, netdev, linux-kernel
> Seems to me that (assuming kfree(NULL) is ok) the kfree()
> is best left in - just in case some other error path is
> added after drv->cs is assigned.
> Better safe than a memory leak.
I'm not sure if I got your point. Now the label "error:" is only
reached if drv->cs is NULL. There is not other way to move to error:
unless drv->cs is NULL. Why wouldn't be safe to remove the
kfree(drv->cs) when drv->cs is NULL?
--
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
2012-09-12 15:06 [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree Peter Senna Tschudin
2012-09-13 8:31 ` David Laight
@ 2012-09-13 9:52 ` Tilman Schmidt
2012-09-13 21:05 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Tilman Schmidt @ 2012-09-13 9:52 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: Hansjoerg Lipp, kernel-janitors, Karsten Keil,
gigaset307x-common, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]
Am 12.09.2012 17:06, schrieb Peter Senna Tschudin:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> Remove useless kfree() and clean up code related to the removal.
>
> The semantic patch that finds this problem is as follows:
[...]
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Acked-by: Tilman Schmidt <tilman@imap.cc>
> ---
> drivers/isdn/gigaset/common.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index aa41485..30a6b17 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -1123,7 +1123,6 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
> return drv;
>
> error:
> - kfree(drv->cs);
> kfree(drv);
> return NULL;
> }
This is indeed vestigial code, left in when another error path
that needed it was removed. Though innocuous, it's better to
remove it.
Thanks,
Tilman
--
Tilman Schmidt E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree
2012-09-13 9:52 ` Tilman Schmidt
@ 2012-09-13 21:05 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-09-13 21:05 UTC (permalink / raw)
To: tilman
Cc: peter.senna, hjlipp, kernel-janitors, isdn, gigaset307x-common,
netdev, linux-kernel
From: Tilman Schmidt <tilman@imap.cc>
Date: Thu, 13 Sep 2012 11:52:27 +0200
> Am 12.09.2012 17:06, schrieb Peter Senna Tschudin:
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>>
>> Remove useless kfree() and clean up code related to the removal.
>>
>> The semantic patch that finds this problem is as follows:
> [...]
>>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>
> Acked-by: Tilman Schmidt <tilman@imap.cc>
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-13 21:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-12 15:06 [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree Peter Senna Tschudin
2012-09-13 8:31 ` David Laight
2012-09-13 9:19 ` Dan Carpenter
2012-09-13 9:27 ` Peter Senna Tschudin
2012-09-13 9:52 ` Tilman Schmidt
2012-09-13 21:05 ` 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).