[PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree

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;
 }

Remove useless kfree() and clean up code related to the removal.

...

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

On Thu, Sep 13, 2012 at 09:31:45AM +0100, David Laight wrote:

quoted

Remove useless kfree() and clean up code related to the removal.

...

quoted

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

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

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 <redacted>

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)

From: Tilman Schmidt <redacted>
Date: Thu, 13 Sep 2012 11:52:27 +0200

Am 12.09.2012 17:06, schrieb Peter Senna Tschudin:

quoted

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:

[...]

quoted

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

Acked-by: Tilman Schmidt <redacted>

Applied to net-next, thanks.