diff mbox

[v2,7/8] drivers/media/platform/davinci/vpbe.c: Removes useless kfree()

Message ID 1347454564-5178-2-git-send-email-peter.senna@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Senna Tschudin Sept. 12, 2012, 12:55 p.m. UTC
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/media/platform/davinci/vpbe.c |    1 -
 1 file changed, 1 deletion(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marcos Paulo de Souza Sept. 12, 2012, 1:34 p.m. UTC | #1
2012/9/12 Peter Senna Tschudin <peter.senna@gmail.com>:
> 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/media/platform/davinci/vpbe.c |    1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
> index c4a82a1..1125a87 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -771,7 +771,6 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
>         return 0;
>
>  vpbe_fail_amp_register:
> -       kfree(vpbe_dev->amp);

Now that you removed this kfree, you could remove this label too. Very
nice your cleanup :)

>  vpbe_fail_sd_register:
>         kfree(vpbe_dev->encoders);
>  vpbe_fail_v4l2_device:
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Senna Tschudin Sept. 12, 2012, 3:50 p.m. UTC | #2
Marcos,

> Now that you removed this kfree, you could remove this label too. Very
> nice your cleanup :)
Thanks!

>
>>  vpbe_fail_sd_register:
>>         kfree(vpbe_dev->encoders);
>>  vpbe_fail_v4l2_device:

The problem removing the label is that it will require some more work
naming the labels. See:
if (!vpbe_dev->amp) {
...
	goto vpbe_fail_amp_register;

If I just remove the label vpbe_fail_amp_register, the label names
will not make sense any more as the next label is
vpbe_fail_sd_register. So I will need to change the name to something
different or rename all labels to out1, out2, out3 or err1, err2,
err3, or ....

Any suggestions?
Marcos Paulo de Souza Sept. 12, 2012, 4:10 p.m. UTC | #3
Hi Peter,

2012/9/12 Peter Senna Tschudin <peter.senna@gmail.com>:
> Marcos,
>
>> Now that you removed this kfree, you could remove this label too. Very
>> nice your cleanup :)
> Thanks!
>
>>
>>>  vpbe_fail_sd_register:
>>>         kfree(vpbe_dev->encoders);
>>>  vpbe_fail_v4l2_device:
>
> The problem removing the label is that it will require some more work
> naming the labels. See:
> if (!vpbe_dev->amp) {
> ...
>         goto vpbe_fail_amp_register;
>
> If I just remove the label vpbe_fail_amp_register, the label names
> will not make sense any more as the next label is
> vpbe_fail_sd_register. So I will need to change the name to something
> different or rename all labels to out1, out2, out3 or err1, err2,
> err3, or ....

I was looking at the code here, but this code is under
drivers/media/video/davince/vpbe.c....

Are  you using the Linus tree?

BTW, this label is only used once. AFAICS, you can GOTO to the next
label, vpbe_fail_sd_register in this case, who frees another member of
the vpbe_dev.

This make sense to you?

> Any suggestions?
>
> --
> Peter
Dan Carpenter Sept. 12, 2012, 5:25 p.m. UTC | #4
On Wed, Sep 12, 2012 at 05:50:54PM +0200, Peter Senna Tschudin wrote:
> Marcos,
> 
> > Now that you removed this kfree, you could remove this label too. Very
> > nice your cleanup :)
> Thanks!
> 
> >
> >>  vpbe_fail_sd_register:
> >>         kfree(vpbe_dev->encoders);
> >>  vpbe_fail_v4l2_device:
> 
> The problem removing the label is that it will require some more work
> naming the labels. See:
> if (!vpbe_dev->amp) {
> ...
> 	goto vpbe_fail_amp_register;
> 
> If I just remove the label vpbe_fail_amp_register, the label names
> will not make sense any more as the next label is
> vpbe_fail_sd_register. So I will need to change the name to something
> different or rename all labels to out1, out2, out3 or err1, err2,
> err3, or ....
> 
> Any suggestions?

Labal names should not be numbers because this is not GW-BASIC.  The
label should reflect what happens on the next line.  Labeling the
place after the goto location where you started from is always
nonsense.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index c4a82a1..1125a87 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -771,7 +771,6 @@  static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
 	return 0;
 
 vpbe_fail_amp_register:
-	kfree(vpbe_dev->amp);
 vpbe_fail_sd_register:
 	kfree(vpbe_dev->encoders);
 vpbe_fail_v4l2_device: