All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Prisk <linux@prisktech.co.nz>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [Bulk] [PATCH] drivers/video/wm8505fb.c: use devm_ functions
Date: Sun, 09 Dec 2012 23:44:38 +0000	[thread overview]
Message-ID: <1355096678.6821.3.camel@gitbox> (raw)
In-Reply-To: <1355076491-2246-1-git-send-email-Julia.Lawall@lip6.fr>

On Sun, 2012-12-09 at 19:08 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
> 
> The patch makes some other cleanups.  First, the original code used
> devm_kzalloc, but kfree.  This would lead to a double free.  The problem
> was found using the following semantic match (http://coccinelle.lip6.fr/):
> 
> // <smpl>
> @@
> expression x,e;
> @@
> x = devm_kzalloc(...)
> ... when != x = e
> ?-kfree(x,...);
> // </smpl>
> 
> The error-handing code of devm_request_and_ioremap does not print any
> warning message, because devm_request_and_ioremap does this.
> 
> The call to dma_alloc_coherent is converted to its devm equivalent,
> dmam_alloc_coherent.  This implicitly introduces a call to
> dmam_free_coherent, which was completly missing in the original code.
> 
> A semicolon is removed at the end of the error-handling code for the call
> to dma_alloc_coherent.
> 
> The block of code calling fb_alloc_cmap is moved below the block of code
> calling wm8505fb_set_par, so that the error-handing code of the call to
> wm8505fb_set_par can just return ret.  This way there is only one block of
> error-handling code that needs to call fb_dealloc_cmap, and so this is
> moved up to the place where it is needed, eliminating the need for all
> gotos and labels in the function.  This was suggested by Tony Prisk.
> 
> The initializations of fbi and ret at the beginning of the function are not
> necessary and are removed.  The call platform_set_drvdata(pdev, NULL); at
> the end of the function is also removed.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Only compiled.  Not tested.


Thanks Julia,

I will include this with the other patches we have for fbdev. I have
tested this on WM8650 and all appears fine.

Regards
Tony P


WARNING: multiple messages have this Message-ID (diff)
From: linux@prisktech.co.nz (Tony Prisk)
To: linux-arm-kernel@lists.infradead.org
Subject: [Bulk] [PATCH] drivers/video/wm8505fb.c: use devm_ functions
Date: Mon, 10 Dec 2012 12:44:38 +1300	[thread overview]
Message-ID: <1355096678.6821.3.camel@gitbox> (raw)
In-Reply-To: <1355076491-2246-1-git-send-email-Julia.Lawall@lip6.fr>

On Sun, 2012-12-09 at 19:08 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
> 
> The patch makes some other cleanups.  First, the original code used
> devm_kzalloc, but kfree.  This would lead to a double free.  The problem
> was found using the following semantic match (http://coccinelle.lip6.fr/):
> 
> // <smpl>
> @@
> expression x,e;
> @@
> x = devm_kzalloc(...)
> ... when != x = e
> ?-kfree(x,...);
> // </smpl>
> 
> The error-handing code of devm_request_and_ioremap does not print any
> warning message, because devm_request_and_ioremap does this.
> 
> The call to dma_alloc_coherent is converted to its devm equivalent,
> dmam_alloc_coherent.  This implicitly introduces a call to
> dmam_free_coherent, which was completly missing in the original code.
> 
> A semicolon is removed at the end of the error-handling code for the call
> to dma_alloc_coherent.
> 
> The block of code calling fb_alloc_cmap is moved below the block of code
> calling wm8505fb_set_par, so that the error-handing code of the call to
> wm8505fb_set_par can just return ret.  This way there is only one block of
> error-handling code that needs to call fb_dealloc_cmap, and so this is
> moved up to the place where it is needed, eliminating the need for all
> gotos and labels in the function.  This was suggested by Tony Prisk.
> 
> The initializations of fbi and ret at the beginning of the function are not
> necessary and are removed.  The call platform_set_drvdata(pdev, NULL); at
> the end of the function is also removed.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Only compiled.  Not tested.


Thanks Julia,

I will include this with the other patches we have for fbdev. I have
tested this on WM8650 and all appears fine.

Regards
Tony P

WARNING: multiple messages have this Message-ID (diff)
From: Tony Prisk <linux@prisktech.co.nz>
To: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: linux-fbdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Subject: Re: [Bulk] [PATCH] drivers/video/wm8505fb.c: use devm_ functions
Date: Mon, 10 Dec 2012 12:44:38 +1300	[thread overview]
Message-ID: <1355096678.6821.3.camel@gitbox> (raw)
In-Reply-To: <1355076491-2246-1-git-send-email-Julia.Lawall@lip6.fr>

On Sun, 2012-12-09 at 19:08 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
> 
> The patch makes some other cleanups.  First, the original code used
> devm_kzalloc, but kfree.  This would lead to a double free.  The problem
> was found using the following semantic match (http://coccinelle.lip6.fr/):
> 
> // <smpl>
> @@
> expression x,e;
> @@
> x = devm_kzalloc(...)
> ... when != x = e
> ?-kfree(x,...);
> // </smpl>
> 
> The error-handing code of devm_request_and_ioremap does not print any
> warning message, because devm_request_and_ioremap does this.
> 
> The call to dma_alloc_coherent is converted to its devm equivalent,
> dmam_alloc_coherent.  This implicitly introduces a call to
> dmam_free_coherent, which was completly missing in the original code.
> 
> A semicolon is removed at the end of the error-handling code for the call
> to dma_alloc_coherent.
> 
> The block of code calling fb_alloc_cmap is moved below the block of code
> calling wm8505fb_set_par, so that the error-handing code of the call to
> wm8505fb_set_par can just return ret.  This way there is only one block of
> error-handling code that needs to call fb_dealloc_cmap, and so this is
> moved up to the place where it is needed, eliminating the need for all
> gotos and labels in the function.  This was suggested by Tony Prisk.
> 
> The initializations of fbi and ret at the beginning of the function are not
> necessary and are removed.  The call platform_set_drvdata(pdev, NULL); at
> the end of the function is also removed.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Only compiled.  Not tested.


Thanks Julia,

I will include this with the other patches we have for fbdev. I have
tested this on WM8650 and all appears fine.

Regards
Tony P


  reply	other threads:[~2012-12-09 23:44 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-08 15:31 [PATCH] drivers/video/wm8505fb.c: use devm_ functions Julia Lawall
2012-12-08 16:31 ` Julia Lawall
2012-12-08 16:31 ` Julia Lawall
2012-12-08 15:31 ` Julia Lawall
2012-12-08 21:42 ` Tony Prisk
2012-12-08 21:42   ` Tony Prisk
2012-12-08 21:42   ` Tony Prisk
2012-12-09  7:21   ` Julia Lawall
2012-12-09  7:21     ` Julia Lawall
2012-12-09  7:21     ` Julia Lawall
2012-12-09 10:21     ` Tony Prisk
2012-12-09 10:21       ` Tony Prisk
2012-12-09 10:21       ` Tony Prisk
2012-12-09 17:08 ` Julia Lawall
2012-12-09 18:08   ` Julia Lawall
2012-12-09 18:08   ` Julia Lawall
2012-12-09 17:08   ` Julia Lawall
2012-12-09 23:44   ` Tony Prisk [this message]
2012-12-09 23:44     ` [Bulk] " Tony Prisk
2012-12-09 23:44     ` Tony Prisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1355096678.6821.3.camel@gitbox \
    --to=linux@prisktech.co.nz \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.