Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1243886 > unrolled thread
| Started by | Julia Lawall <Julia.Lawall@lip6.fr> |
|---|---|
| First post | 2015-10-10 14:50 +0200 |
| Last post | 2015-10-12 15:00 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 4/5] regulator: of: add missing of_node_put Julia Lawall <Julia.Lawall@lip6.fr> - 2015-10-10 14:50 +0200
Re: [PATCH 4/5] regulator: of: add missing of_node_put Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-12 02:40 +0200
Re: [PATCH 4/5] regulator: of: add missing of_node_put Julia Lawall <julia.lawall@lip6.fr> - 2015-10-12 07:40 +0200
Re: [PATCH 4/5] regulator: of: add missing of_node_put Julia Lawall <julia.lawall@lip6.fr> - 2015-10-12 14:50 +0200
Re: [PATCH 4/5] regulator: of: add missing of_node_put Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-12 15:00 +0200
| From | Julia Lawall <Julia.Lawall@lip6.fr> |
|---|---|
| Date | 2015-10-10 14:50 +0200 |
| Subject | [PATCH 4/5] regulator: of: add missing of_node_put |
| Message-ID | <qi1L4-18e-25@gated-at.bofh.it> |
for_each_child_of_node performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):
// <smpl>
@@
expression root,e;
local idexpression child;
@@
for_each_child_of_node(root, child) {
... when != of_node_put(child)
when != e = child
(
return child;
|
+ of_node_put(child);
? return ...;
)
...
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/regulator/of_regulator.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 499e437..f9d77b4 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -274,6 +274,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
dev_err(dev,
"failed to parse DT for regulator %s\n",
child->name);
+ of_node_put(child);
return -EINVAL;
}
match->of_node = of_node_get(child);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-10-12 02:40 +0200 |
| Message-ID | <qizjI-7Uc-7@gated-at.bofh.it> |
| In reply to | #1243886 |
2015-10-10 21:30 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>:
>
> for_each_child_of_node performs an of_node_get on each iteration, so
> a break out of the loop requires an of_node_put.
>
> The semantic patch that fixes this problem is as follows
> (http://coccinelle.lip6.fr):
>
> // <smpl>
> @@
> expression root,e;
> local idexpression child;
> @@
>
> for_each_child_of_node(root, child) {
> ... when != of_node_put(child)
> when != e = child
> (
> return child;
> |
> + of_node_put(child);
> ? return ...;
> )
> ...
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> drivers/regulator/of_regulator.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> index 499e437..f9d77b4 100644
> --- a/drivers/regulator/of_regulator.c
> +++ b/drivers/regulator/of_regulator.c
> @@ -274,6 +274,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
> dev_err(dev,
> "failed to parse DT for regulator %s\n",
> child->name);
> + of_node_put(child);
This looks good.
> return -EINVAL;
> }
> match->of_node = of_node_get(child);
But what about 'break' few lines below? The reference from last
of_get_next_child() should be also dropped because... or we should
remove this of_node_get() call.
How about fixing also usage of for_each_available_child_of_node() in
regulator_of_get_init_data()?
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2015-10-12 07:40 +0200 |
| Message-ID | <qiE02-6lf-9@gated-at.bofh.it> |
| In reply to | #1244315 |
On Mon, 12 Oct 2015, Krzysztof Kozlowski wrote:
> 2015-10-10 21:30 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>:
> >
> > for_each_child_of_node performs an of_node_get on each iteration, so
> > a break out of the loop requires an of_node_put.
> >
> > The semantic patch that fixes this problem is as follows
> > (http://coccinelle.lip6.fr):
> >
> > // <smpl>
> > @@
> > expression root,e;
> > local idexpression child;
> > @@
> >
> > for_each_child_of_node(root, child) {
> > ... when != of_node_put(child)
> > when != e = child
> > (
> > return child;
> > |
> > + of_node_put(child);
> > ? return ...;
> > )
> > ...
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > drivers/regulator/of_regulator.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> > index 499e437..f9d77b4 100644
> > --- a/drivers/regulator/of_regulator.c
> > +++ b/drivers/regulator/of_regulator.c
> > @@ -274,6 +274,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
> > dev_err(dev,
> > "failed to parse DT for regulator %s\n",
> > child->name);
> > + of_node_put(child);
>
> This looks good.
>
> > return -EINVAL;
> > }
> > match->of_node = of_node_get(child);
>
> But what about 'break' few lines below? The reference from last
> of_get_next_child() should be also dropped because... or we should
> remove this of_node_get() call.
>
> How about fixing also usage of for_each_available_child_of_node() in
> regulator_of_get_init_data()?
I'll check on all of this and resend. Thanks for the feedback.
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2015-10-12 14:50 +0200 |
| Message-ID | <qiKI9-7EQ-5@gated-at.bofh.it> |
| In reply to | #1244315 |
On Mon, 12 Oct 2015, Krzysztof Kozlowski wrote:
> 2015-10-10 21:30 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>:
> >
> > for_each_child_of_node performs an of_node_get on each iteration, so
> > a break out of the loop requires an of_node_put.
> >
> > The semantic patch that fixes this problem is as follows
> > (http://coccinelle.lip6.fr):
> >
> > // <smpl>
> > @@
> > expression root,e;
> > local idexpression child;
> > @@
> >
> > for_each_child_of_node(root, child) {
> > ... when != of_node_put(child)
> > when != e = child
> > (
> > return child;
> > |
> > + of_node_put(child);
> > ? return ...;
> > )
> > ...
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > drivers/regulator/of_regulator.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> > index 499e437..f9d77b4 100644
> > --- a/drivers/regulator/of_regulator.c
> > +++ b/drivers/regulator/of_regulator.c
> > @@ -274,6 +274,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
> > dev_err(dev,
> > "failed to parse DT for regulator %s\n",
> > child->name);
> > + of_node_put(child);
>
> This looks good.
>
> > return -EINVAL;
> > }
> > match->of_node = of_node_get(child);
>
> But what about 'break' few lines below? The reference from last
> of_get_next_child() should be also dropped because... or we should
> remove this of_node_get() call.
Actually, the break is OK. It's on the inner for loop, not the
for_each_child_of_node loop. The for_each_child_of_node will still
decrement the reference count in the normal way.
julia
> How about fixing also usage of for_each_available_child_of_node() in
> regulator_of_get_init_data()?
>
> Best regards,
> Krzysztof
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-10-12 15:00 +0200 |
| Message-ID | <qiKRQ-7QK-15@gated-at.bofh.it> |
| In reply to | #1244654 |
W dniu 12.10.2015 o 21:44, Julia Lawall pisze:
>
>
> On Mon, 12 Oct 2015, Krzysztof Kozlowski wrote:
>
>> 2015-10-10 21:30 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>:
>>>
>>> for_each_child_of_node performs an of_node_get on each iteration, so
>>> a break out of the loop requires an of_node_put.
>>>
>>> The semantic patch that fixes this problem is as follows
>>> (http://coccinelle.lip6.fr):
>>>
>>> // <smpl>
>>> @@
>>> expression root,e;
>>> local idexpression child;
>>> @@
>>>
>>> for_each_child_of_node(root, child) {
>>> ... when != of_node_put(child)
>>> when != e = child
>>> (
>>> return child;
>>> |
>>> + of_node_put(child);
>>> ? return ...;
>>> )
>>> ...
>>> }
>>> // </smpl>
>>>
>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>> drivers/regulator/of_regulator.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
>>> index 499e437..f9d77b4 100644
>>> --- a/drivers/regulator/of_regulator.c
>>> +++ b/drivers/regulator/of_regulator.c
>>> @@ -274,6 +274,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
>>> dev_err(dev,
>>> "failed to parse DT for regulator %s\n",
>>> child->name);
>>> + of_node_put(child);
>>
>> This looks good.
>>
>>> return -EINVAL;
>>> }
>>> match->of_node = of_node_get(child);
>>
>> But what about 'break' few lines below? The reference from last
>> of_get_next_child() should be also dropped because... or we should
>> remove this of_node_get() call.
>
> Actually, the break is OK. It's on the inner for loop, not the
> for_each_child_of_node loop. The for_each_child_of_node will still
> decrement the reference count in the normal way.
>
> julia
Yes, you're right. The patch looks good:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Best regards,
Krzysztof
>> How about fixing also usage of for_each_available_child_of_node() in
>> regulator_of_get_init_data()?
>>
>> Best regards,
>> Krzysztof
>> --
>> 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
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web