2013-04-10 04:16:28

by Tang Yuantian

[permalink] [raw]
Subject: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

From: Tang Yuantian <[email protected]>

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 <[email protected]>
---
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


2013-04-16 06:54:43

by Tang Yuantian-B29983

[permalink] [raw]
Subject: RE: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

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: [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Tang Yuantian-
> B29983; Tang Yuantian-B29983
> Subject: [PATCH v2] of/base: release the node correctly in
> of_parse_phandle_with_args()
>
> From: Tang Yuantian <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-04-16 11:37:24

by Timur Tabi

[permalink] [raw]
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, <[email protected]> wrote:
>
> + /* Found it! return success */

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

2013-04-17 02:45:42

by Tang Yuantian-B29983

[permalink] [raw]
Subject: RE: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

> -----Original Message-----
> From: Timur Tabi [mailto:[email protected]]
> Sent: 2013??4??16?? 19:37
> To: Tang Yuantian-B29983
> Cc: Grant Likely; devicetree-discuss; [email protected]; 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, <[email protected]> wrote:
> >
> > + /* Found it! return success */
>
> I'm pretty sure this comment is in the wrong place.

It is not perfect, but acceptable.

-Yuantian

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-04-17 03:30:48

by Timur Tabi

[permalink] [raw]
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,<[email protected]> 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.

--
Timur Tabi

2013-04-17 04:49:24

by Tang Yuantian-B29983

[permalink] [raw]
Subject: RE: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()



> -----Original Message-----
> From: Timur Tabi [mailto:[email protected]]
> Sent: 2013??4??17?? 11:31
> To: Tang Yuantian-B29983
> Cc: Grant Likely; devicetree-discuss; [email protected]; 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,<[email protected]> 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-04-17 11:27:12

by Timur Tabi

[permalink] [raw]
Subject: Re: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

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.

--
Timur Tabi

2013-04-17 14:57:26

by Grant Likely

[permalink] [raw]
Subject: RE: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

On Tue, 16 Apr 2013 06:54:40 +0000, Tang Yuantian-B29983 <[email protected]> wrote:
> Hi Grant.likely,
>
> I really preciate if you can spend some times to review this patch.

Applied, thanks.

g.

2013-04-17 21:52:54

by Timur Tabi

[permalink] [raw]
Subject: Re: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <[email protected]> wrote:
>
>> I really preciate if you can spend some times to review this patch.
>
> Applied, thanks.

Pff. Why do I bother?

2013-04-17 22:00:45

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()

On Wed, Apr 17, 2013 at 10:52 PM, Timur Tabi <[email protected]> wrote:
> On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <[email protected]> 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.