* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
@ 2016-01-25 15:21 Amitoj Kaur Chawla
2016-01-26 0:24 ` Krzysztof Kozlowski
0 siblings, 1 reply; 6+ messages in thread
From: Amitoj Kaur Chawla @ 2016-01-25 15:21 UTC (permalink / raw)
To: linux-arm-kernel
for_each_available_child_of_node and for_each_child_of_node perform an
of_node_get on each iteration, so to break out of the loop an of_node_put is
required.
Found using Coccinelle. The simplified version of the semantic patch
that is used for this is as follows:
// <smpl>
@@
local idexpression n;
expression e,r;
@@
for_each_available_child_of_node(r,n) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? break;
)
...
}
... when != n
// </smpl>
Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
drivers/media/platform/exynos4-is/media-dev.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 4f5586a..09f6e54 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -430,8 +430,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
continue;
ret = fimc_md_parse_port_node(fmd, port, index);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(node);
goto rpm_put;
+ }
index++;
}
@@ -442,8 +444,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
for_each_child_of_node(ports, node) {
ret = fimc_md_parse_port_node(fmd, node, index);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(node);
break;
+ }
index++;
}
rpm_put:
@@ -651,8 +655,10 @@ static int fimc_md_register_platform_entities(struct fimc_md *fmd,
ret = fimc_md_register_platform_entity(fmd, pdev,
plat_entity);
put_device(&pdev->dev);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(node);
break;
+ }
}
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
2016-01-25 15:21 [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put Amitoj Kaur Chawla
@ 2016-01-26 0:24 ` Krzysztof Kozlowski
2016-01-26 6:24 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-26 0:24 UTC (permalink / raw)
To: linux-arm-kernel
On 26.01.2016 00:21, Amitoj Kaur Chawla wrote:
> for_each_available_child_of_node and for_each_child_of_node perform an
> of_node_get on each iteration, so to break out of the loop an of_node_put is
> required.
>
> Found using Coccinelle. The simplified version of the semantic patch
> that is used for this is as follows:
>
> // <smpl>
> @@
> local idexpression n;
> expression e,r;
> @@
>
> for_each_available_child_of_node(r,n) {
> ...
> (
> of_node_put(n);
> |
> e = n
> |
> + of_node_put(n);
> ? break;
> )
> ...
> }
> ... when != n
> // </smpl>
Patch iselft looks correct but why are you pasting coccinelle script
into the message?
The script is already present in Linux kernel:
scripts/coccinelle/iterators/device_node_continue.cocci
This just extends the commit message without any meaningful data so with
removal of coccinelle script above:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Best regards,
Krzysztof
>
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---
> drivers/media/platform/exynos4-is/media-dev.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 4f5586a..09f6e54 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -430,8 +430,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
> continue;
>
> ret = fimc_md_parse_port_node(fmd, port, index);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(node);
> goto rpm_put;
> + }
> index++;
> }
>
> @@ -442,8 +444,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
>
> for_each_child_of_node(ports, node) {
> ret = fimc_md_parse_port_node(fmd, node, index);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(node);
> break;
> + }
> index++;
> }
> rpm_put:
> @@ -651,8 +655,10 @@ static int fimc_md_register_platform_entities(struct fimc_md *fmd,
> ret = fimc_md_register_platform_entity(fmd, pdev,
> plat_entity);
> put_device(&pdev->dev);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(node);
> break;
> + }
> }
>
> return ret;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
2016-01-26 0:24 ` Krzysztof Kozlowski
@ 2016-01-26 6:24 ` Julia Lawall
2016-01-26 6:55 ` Krzysztof Kozlowski
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-01-26 6:24 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
> On 26.01.2016 00:21, Amitoj Kaur Chawla wrote:
> > for_each_available_child_of_node and for_each_child_of_node perform an
> > of_node_get on each iteration, so to break out of the loop an of_node_put is
> > required.
> >
> > Found using Coccinelle. The simplified version of the semantic patch
> > that is used for this is as follows:
> >
> > // <smpl>
> > @@
> > local idexpression n;
> > expression e,r;
> > @@
> >
> > for_each_available_child_of_node(r,n) {
> > ...
> > (
> > of_node_put(n);
> > |
> > e = n
> > |
> > + of_node_put(n);
> > ? break;
> > )
> > ...
> > }
> > ... when != n
> > // </smpl>
>
> Patch iselft looks correct but why are you pasting coccinelle script
> into the message?
>
> The script is already present in Linux kernel:
> scripts/coccinelle/iterators/device_node_continue.cocci
I don't think so. The continue one takes care of the case where there is
an extraneous of_node_put before a continue, not a missing one before a
break. But OK to drop it if it doesn't seem useful.
julia
> This just extends the commit message without any meaningful data so with
> removal of coccinelle script above:
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
> Best regards,
> Krzysztof
>
> >
> > Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> > ---
> > drivers/media/platform/exynos4-is/media-dev.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> > index 4f5586a..09f6e54 100644
> > --- a/drivers/media/platform/exynos4-is/media-dev.c
> > +++ b/drivers/media/platform/exynos4-is/media-dev.c
> > @@ -430,8 +430,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
> > continue;
> >
> > ret = fimc_md_parse_port_node(fmd, port, index);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + of_node_put(node);
> > goto rpm_put;
> > + }
> > index++;
> > }
> >
> > @@ -442,8 +444,10 @@ static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
> >
> > for_each_child_of_node(ports, node) {
> > ret = fimc_md_parse_port_node(fmd, node, index);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + of_node_put(node);
> > break;
> > + }
> > index++;
> > }
> > rpm_put:
> > @@ -651,8 +655,10 @@ static int fimc_md_register_platform_entities(struct fimc_md *fmd,
> > ret = fimc_md_register_platform_entity(fmd, pdev,
> > plat_entity);
> > put_device(&pdev->dev);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + of_node_put(node);
> > break;
> > + }
> > }
> >
> > return ret;
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
2016-01-26 6:24 ` Julia Lawall
@ 2016-01-26 6:55 ` Krzysztof Kozlowski
2016-01-26 7:12 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-26 6:55 UTC (permalink / raw)
To: linux-arm-kernel
On 26.01.2016 15:24, Julia Lawall wrote:
>
>
> On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
>
>> On 26.01.2016 00:21, Amitoj Kaur Chawla wrote:
>>> for_each_available_child_of_node and for_each_child_of_node perform an
>>> of_node_get on each iteration, so to break out of the loop an of_node_put is
>>> required.
>>>
>>> Found using Coccinelle. The simplified version of the semantic patch
>>> that is used for this is as follows:
>>>
>>> // <smpl>
>>> @@
>>> local idexpression n;
>>> expression e,r;
>>> @@
>>>
>>> for_each_available_child_of_node(r,n) {
>>> ...
>>> (
>>> of_node_put(n);
>>> |
>>> e = n
>>> |
>>> + of_node_put(n);
>>> ? break;
>>> )
>>> ...
>>> }
>>> ... when != n
>>> // </smpl>
>>
>> Patch iselft looks correct but why are you pasting coccinelle script
>> into the message?
>>
>> The script is already present in Linux kernel:
>> scripts/coccinelle/iterators/device_node_continue.cocci
>
> I don't think so. The continue one takes care of the case where there is
> an extraneous of_node_put before a continue, not a missing one before a
> break. But OK to drop it if it doesn't seem useful.
>
> julia
You are right - this is not covered by that cocci patch... but I think
is covered by scripts/coccinelle/iterators/fen.cocci, isn't it?
BR,
Krzysztof
>
>> This just extends the commit message without any meaningful data so with
>> removal of coccinelle script above:
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> Best regards,
>> Krzysztof
>>
>>>
>>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>>> ---
>>> drivers/media/platform/exynos4-is/media-dev.c | 12 +++++++++---
>>> 1 file changed, 9 insertions(+), 3 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
2016-01-26 6:55 ` Krzysztof Kozlowski
@ 2016-01-26 7:12 ` Julia Lawall
2016-01-26 9:12 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-01-26 7:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
> On 26.01.2016 15:24, Julia Lawall wrote:
> >
> >
> > On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
> >
> >> On 26.01.2016 00:21, Amitoj Kaur Chawla wrote:
> >>> for_each_available_child_of_node and for_each_child_of_node perform an
> >>> of_node_get on each iteration, so to break out of the loop an of_node_put is
> >>> required.
> >>>
> >>> Found using Coccinelle. The simplified version of the semantic patch
> >>> that is used for this is as follows:
> >>>
> >>> // <smpl>
> >>> @@
> >>> local idexpression n;
> >>> expression e,r;
> >>> @@
> >>>
> >>> for_each_available_child_of_node(r,n) {
> >>> ...
> >>> (
> >>> of_node_put(n);
> >>> |
> >>> e = n
> >>> |
> >>> + of_node_put(n);
> >>> ? break;
> >>> )
> >>> ...
> >>> }
> >>> ... when != n
> >>> // </smpl>
> >>
> >> Patch iselft looks correct but why are you pasting coccinelle script
> >> into the message?
> >>
> >> The script is already present in Linux kernel:
> >> scripts/coccinelle/iterators/device_node_continue.cocci
> >
> > I don't think so. The continue one takes care of the case where there is
> > an extraneous of_node_put before a continue, not a missing one before a
> > break. But OK to drop it if it doesn't seem useful.
> >
> > julia
>
> You are right - this is not covered by that cocci patch... but I think
> is covered by scripts/coccinelle/iterators/fen.cocci, isn't it?
Not quite. That is for of_node_puts after normal loop completion (not
sure that this problem comes up any more, but at one point there were a
number of them). There are indeed a lot of ways in which the management of
reference counts can go wrong...
Anyway, the rule that Amitoj used seems to be pretty reliable, so I'll try
to get it into the kernel source tree some day soon.
julia
>
> BR,
> Krzysztof
>
> >
> >> This just extends the commit message without any meaningful data so with
> >> removal of coccinelle script above:
> >> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >>>
> >>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> >>> ---
> >>> drivers/media/platform/exynos4-is/media-dev.c | 12 +++++++++---
> >>> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put
2016-01-26 7:12 ` Julia Lawall
@ 2016-01-26 9:12 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2016-01-26 9:12 UTC (permalink / raw)
To: linux-arm-kernel
Em Tue, 26 Jan 2016 08:12:15 +0100
Julia Lawall <julia.lawall@lip6.fr> escreveu:
> On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
>
> > On 26.01.2016 15:24, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 26 Jan 2016, Krzysztof Kozlowski wrote:
> > >
> > >> On 26.01.2016 00:21, Amitoj Kaur Chawla wrote:
> > >>> for_each_available_child_of_node and for_each_child_of_node perform an
> > >>> of_node_get on each iteration, so to break out of the loop an of_node_put is
> > >>> required.
> > >>>
> > >>> Found using Coccinelle. The simplified version of the semantic patch
> > >>> that is used for this is as follows:
> > >>>
> > >>> // <smpl>
> > >>> @@
> > >>> local idexpression n;
> > >>> expression e,r;
> > >>> @@
> > >>>
> > >>> for_each_available_child_of_node(r,n) {
> > >>> ...
> > >>> (
> > >>> of_node_put(n);
> > >>> |
> > >>> e = n
> > >>> |
> > >>> + of_node_put(n);
> > >>> ? break;
> > >>> )
> > >>> ...
> > >>> }
> > >>> ... when != n
> > >>> // </smpl>
> > >>
> > >> Patch iselft looks correct but why are you pasting coccinelle script
> > >> into the message?
> > >>
> > >> The script is already present in Linux kernel:
> > >> scripts/coccinelle/iterators/device_node_continue.cocci
> > >
> > > I don't think so. The continue one takes care of the case where there is
> > > an extraneous of_node_put before a continue, not a missing one before a
> > > break. But OK to drop it if it doesn't seem useful.
> > >
> > > julia
> >
> > You are right - this is not covered by that cocci patch... but I think
> > is covered by scripts/coccinelle/iterators/fen.cocci, isn't it?
>
> Not quite.
If the script is not part of the Kernel, please keep the script in the
patch, as it could be useful in some future.
Regards,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-26 9:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-25 15:21 [PATCH] media: platform: exynos4-is: media-dev: Add missing of_node_put Amitoj Kaur Chawla
2016-01-26 0:24 ` Krzysztof Kozlowski
2016-01-26 6:24 ` Julia Lawall
2016-01-26 6:55 ` Krzysztof Kozlowski
2016-01-26 7:12 ` Julia Lawall
2016-01-26 9:12 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).