diff mbox

[v2] of/base: release the node correctly in of_parse_phandle_with_args()

Message ID 1365564999-24427-1-git-send-email-Yuantian.Tang@freescale.com (mailing list archive)
State Accepted, archived
Delegated to: Grant Likely
Headers show

Commit Message

tang yuantian April 10, 2013, 3:36 a.m. UTC 
From: Tang Yuantian <yuantian.tang@freescale.com>

Call of_node_put() only when the out_args is NULL on success,
or the node's reference count will not be correct because the caller
will call of_node_put() again.

Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
v2:
	- modified the title and description. the 1st patch title is:
	  of: remove the unnecessary of_node_put for of_parse_phandle_with_args()
	  the 1st patch is not good enough.

 drivers/of/base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

tang yuantian April 16, 2013, 6:54 a.m. UTC | #1 
Hi Grant.likely,

I really preciate if you can spend some times to review this patch.

Thanks,
Yuantian

> -----Original Message-----

> From: Tang Yuantian-B29983

> Sent: 2013年4月10日 11:37

> To: grant.likely@secretlab.ca

> Cc: rob.herring@calxeda.com; devicetree-discuss@lists.ozlabs.org; linux-

> kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang Yuantian-

> B29983; Tang Yuantian-B29983

> Subject: [PATCH v2] of/base: release the node correctly in

> of_parse_phandle_with_args()

> 

> From: Tang Yuantian <yuantian.tang@freescale.com>

> 

> Call of_node_put() only when the out_args is NULL on success, or the

> node's reference count will not be correct because the caller will call

> of_node_put() again.

> 

> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>

> ---

> v2:

> 	- modified the title and description. the 1st patch title is:

> 	  of: remove the unnecessary of_node_put for

> of_parse_phandle_with_args()

> 	  the 1st patch is not good enough.

> 

>  drivers/of/base.c | 6 +++---

>  1 file changed, 3 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/of/base.c b/drivers/of/base.c index 321d3ef..ee94f64

> 100644

> --- a/drivers/of/base.c

> +++ b/drivers/of/base.c

> @@ -1158,6 +1158,7 @@ static int __of_parse_phandle_with_args(const

> struct device_node *np,

>  			if (!phandle)

>  				goto err;

> 

> +			/* Found it! return success */

>  			if (out_args) {

>  				int i;

>  				if (WARN_ON(count > MAX_PHANDLE_ARGS)) @@ -

> 1166,11 +1167,10 @@ static int __of_parse_phandle_with_args(const struct

> device_node *np,

>  				out_args->args_count = count;

>  				for (i = 0; i < count; i++)

>  					out_args->args[i] = be32_to_cpup(list++);

> +			} else if (node) {

> +				of_node_put(node);

>  			}

> 

> -			/* Found it! return success */

> -			if (node)

> -				of_node_put(node);

>  			return 0;

>  		}

> 

> --

> 1.8.0
Timur Tabi April 16, 2013, 11:37 a.m. UTC | #2 
On Tue, Apr 9, 2013 at 10:36 PM,  <Yuantian.Tang@freescale.com> wrote:
>
> +                       /* Found it! return success */

I'm pretty sure this comment is in the wrong place.
tang yuantian April 17, 2013, 2:44 a.m. UTC | #3 
> -----Original Message-----

> From: Timur Tabi [mailto:timur@tabi.org]

> Sent: 2013年4月16日 19:37

> To: Tang Yuantian-B29983

> Cc: Grant Likely; devicetree-discuss; linuxppc-dev@lists.ozlabs.org; lkml;

> Rob Herring

> Subject: Re: [PATCH v2] of/base: release the node correctly in

> of_parse_phandle_with_args()

> 

> On Tue, Apr 9, 2013 at 10:36 PM,  <Yuantian.Tang@freescale.com> wrote:

> >

> > +                       /* Found it! return success */

> 

> I'm pretty sure this comment is in the wrong place.


It is not perfect, but acceptable.

-Yuantian
Timur Tabi April 17, 2013, 3:30 a.m. UTC | #4 
Tang Yuantian-B29983 wrote:
>> >On Tue, Apr 9, 2013 at 10:36 PM,<Yuantian.Tang@freescale.com>  wrote:
>>> > >
>>> > >+                       /* Found it! return success */
>> >
>> >I'm pretty sure this comment is in the wrong place.

> It is not perfect, but acceptable.

Like I said, I'm pretty sure it's in the wrong place.
tang yuantian April 17, 2013, 4:49 a.m. UTC | #5 
> -----Original Message-----

> From: Timur Tabi [mailto:timur@tabi.org]

> Sent: 2013年4月17日 11:31

> To: Tang Yuantian-B29983

> Cc: Grant Likely; devicetree-discuss; linuxppc-dev@lists.ozlabs.org; lkml;

> Rob Herring

> Subject: Re: [PATCH v2] of/base: release the node correctly in

> of_parse_phandle_with_args()

> 

> Tang Yuantian-B29983 wrote:

> >> >On Tue, Apr 9, 2013 at 10:36 PM,<Yuantian.Tang@freescale.com>  wrote:

> >>> > >

> >>> > >+                       /* Found it! return success */

> >> >

> >> >I'm pretty sure this comment is in the wrong place.

> 

> > It is not perfect, but acceptable.

> 

> Like I said, I'm pretty sure it's in the wrong place.

> 


It was placed on ELSE statement originally, I moved it to IF statement.
Why is it so wrong?

Thanks,
Yuantian
Timur Tabi April 17, 2013, 11:27 a.m. UTC | #6 
Tang Yuantian-B29983 wrote:
> It was placed on ELSE statement originally, I moved it to IF statement.
> Why is it so wrong?

Because the logic of the comment applies to the else-condition, not the 
if-condtion.
Grant Likely April 17, 2013, 2:57 p.m. UTC | #7 
On Tue, 16 Apr 2013 06:54:40 +0000, Tang Yuantian-B29983 <B29983@freescale.com> wrote:
> Hi Grant.likely,
> 
> I really preciate if you can spend some times to review this patch.

Applied, thanks.

g.
Timur Tabi April 17, 2013, 9:52 p.m. UTC | #8 
On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> I really preciate if you can spend some times to review this patch.
>
> Applied, thanks.

Pff.  Why do I bother?
Grant Likely April 17, 2013, 10 p.m. UTC | #9 
On Wed, Apr 17, 2013 at 10:52 PM, Timur Tabi <timur@tabi.org> wrote:
> On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>>
>>> I really preciate if you can spend some times to review this patch.
>>
>> Applied, thanks.
>
> Pff.  Why do I bother?

Relax Timur:

http://git.secretlab.ca/?p=linux.git;a=commitdiff;h=b855f16b05a697ac1863adabe99bfba56e6d3199

g.



--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
diff mbox

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 321d3ef..ee94f64 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1158,6 +1158,7 @@  static int __of_parse_phandle_with_args(const struct device_node *np,
 			if (!phandle)
 				goto err;
 
+			/* Found it! return success */
 			if (out_args) {
 				int i;
 				if (WARN_ON(count > MAX_PHANDLE_ARGS))
@@ -1166,11 +1167,10 @@  static int __of_parse_phandle_with_args(const struct device_node *np,
 				out_args->args_count = count;
 				for (i = 0; i < count; i++)
 					out_args->args[i] = be32_to_cpup(list++);
+			} else if (node) {
+				of_node_put(node);
 			}
 
-			/* Found it! return success */
-			if (node)
-				of_node_put(node);
 			return 0;
 		}