exynos4-is: Add missing endpoint of_node_put on error paths
Subsystems (2)
PATCH 1/2 exynos4-is: Add missing endpoint of_node_put on error paths
In fimc_md_parse_port_node() endpoint node is get with of_get_next_child() but it is not put on error path.
---
Not tested on hardware, only built+static checkers.
--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/media/platform/exynos4-is/media-dev.c +3 -1
@@ -339,8 +339,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,return 0 ;v4l2_of_parse_endpoint (ep, &endpoint); -if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) +if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) { +of_node_put (ep);return -EINVAL ; + } pd->mux_id = (endpoint.base.port -1 ) &0x1 ;
Signature · 1 line
1.9.1
PATCH 2/2 exynos4-is: Add missing port parent of_node_put on error paths
In fimc_md_parse_port_node() remote port parent node is get with of_graph_get_remote_port_parent() but it is not put on error path.
---
Not tested on hardware, only built+static checkers.
--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/media/platform/exynos4-is/media-dev.c +3 -1
@@ -385,8 +385,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,else pd->fimc_bus_type = pd->sensor_bus_type; -if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) +if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) { +of_node_put (rem);return -EINVAL ; + } fmd->sensor[index].asd.match_type =V4L2_ASYNC_MATCH_OF ; fmd->sensor[index].asd.match.of.node = rem;
Signature · 1 line
1.9.1
Re: [RFT 2/2] [media] exynos4-is: Add missing port parent of_node_put on error paths
Hello Krzysztof,
Quoted 20 lines · Krzysztof Kozlowski
In
fimc_md_parse_port_node()remote port parent node is get withof_graph_get_remote_port_parent()but it is not put on error path.Fixes fa91f1056f17 ("[media] exynos4-is: Add support for asynchronous subdevices registration")Signed-off-by Krzysztof Kozlowski <[email protected]>---
Not tested on hardware, only built+static checkers.
--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/media/platform/exynos4-is/media-dev.c +0
-0@@ -385,8 +385,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,else
drivers/media/platform/exynos4-is/media-dev.c static int fimc_md_parse_port_node(struct fimc_md *fmd, · line 394
Quoted 10 lines
drivers/media/platform/exynos4-is/media-dev.c +3
-1pd->fimc_bus_type = pd->sensor_bus_type; -if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) +if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) { +of_node_put (rem);return -EINVAL ; + } fmd->sensor[index].asd.match_type =V4L2_ASYNC_MATCH_OF ; fmd->sensor[index].asd.match.of.node = rem;
Best regards,
Signature · 3 lines
Javier Martinez Canillas Open Source Group Samsung Research America
PATCH 2/2 exynos4-is: Add missing port parent of_node_put on error paths
In reply to Krzysztof KozlowskiQuoted 6 lines · Krzysztof Kozlowski
In
fimc_md_parse_port_node()remote port parent node is get withof_graph_get_remote_port_parent()but it is not put on error path.Fixes fa91f1056f17 ("[media] exynos4-is: Add support forasynchronous subdevices registration")
Signed-off-by Krzysztof Kozlowski <[email protected]>
Thanks, patch applied with s/is get/is acquired/.
Quoted 24 lines
---
Not tested on hardware, only built+static checkers.
--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/media/platform/exynos4-is/media-dev.c +3
-1@@ -385,8 +385,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,else pd->fimc_bus_type = pd->sensor_bus_type; -if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) +if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) { +of_node_put (rem);return -EINVAL ; + } fmd->sensor[index].asd.match_type =V4L2_ASYNC_MATCH_OF ; fmd->sensor[index].asd.match.of.node = rem;
Signature · 2 lines
Regards, Sylwester
Hello Krzysztof,
Quoted 20 lines · Krzysztof Kozlowski
In
fimc_md_parse_port_node()endpoint node is get withof_get_next_child()but it is not put on error path.Fixes 56fa1a6a6a7d ("[media] s5p-fimc: Change the driver directory name to exynos4-is")Signed-off-by Krzysztof Kozlowski <[email protected]>---
Not tested on hardware, only built+static checkers.
--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/media/platform/exynos4-is/media-dev.c +0
-0@@ -339,8 +339,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,return 0 ;
drivers/media/platform/exynos4-is/media-dev.c static int fimc_md_parse_port_node(struct fimc_md *fmd, · line 348
Quoted 10 lines
drivers/media/platform/exynos4-is/media-dev.c +3
-1v4l2_of_parse_endpoint (ep, &endpoint); -if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) +if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) { +of_node_put (ep);return -EINVAL ; + } pd->mux_id = (endpoint.base.port -1 ) &0x1 ;
Best regards,
Signature · 3 lines
Javier Martinez Canillas Open Source Group Samsung Research America
PATCH 1/2 exynos4-is: Add missing endpoint of_node_put on error paths
In reply to Krzysztof KozlowskiQuoted 2 lines · Krzysztof Kozlowski
In
fimc_md_parse_port_node()endpoint node is get withof_get_next_child()but it is not put on error path.
"is get" doesn't sound right to me, how about rephrasing this to:
"In fimc_md_parse_port_node() reference count of the endpoint node "is incremented by of_get_next_child() but it is not decremented
on error path."
Quoted 14 lines
Fixes 56fa1a6a6a7d ("[media] s5p-fimc: Change the driver directory name to exynos4-is")Signed-off-by Krzysztof Kozlowski <[email protected]>---
Not tested on hardware, only built+static checkers.--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/media/platform/exynos4-is/media-dev.c +0
-0
drivers/media/platform/exynos4-is/media-dev.c static int fimc_md_parse_port_node(struct fimc_md *fmd, · line 345
Quoted 8 lines
drivers/media/platform/exynos4-is/media-dev.c +3
-1@@ -339,8 +339,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,return 0 ;v4l2_of_parse_endpoint (ep, &endpoint); -if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) +if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) { +of_node_put (ep);return -EINVAL ; + }
Thanks for the patch, it looks correct but it doesn't apply cleanly due to patches already in media master branch [1]. Could you refresh this patch and resend?
Also I don't quite like multiple calls to of_node_put(), how about doing something like this instead:
---------------------------8<----------------------------------
(quoted diff) +0 -0
b/drivers/media/platform/exynos4-is/media-dev.c
index feb521f..663d32e 100644
drivers/media/platform/exynos4-is/media-dev.c +11 -10
@@ -397,18 +397,19 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,return 0 ; ret =v4l2_of_parse_endpoint (ep, &endpoint); -if (ret) { -of_node_put (ep); -return ret; +if (!ret) { +if (WARN_ON (endpoint.base.port ==0 ) || + index >=FIMC_MAX_SENSORS ) { + ret = -EINVAL ; + }else { + pd->mux_id = (endpoint.base.port -1 ) &0x1 ; + rem =of_graph_get_remote_port_parent (ep); + } } - -if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) -return -EINVAL ; - - pd->mux_id = (endpoint.base.port -1 ) &0x1 ; - - rem =of_graph_get_remote_port_parent (ep);of_node_put (ep); +if (ret <0 ) +return ret; +if (rem ==NULL ) {v4l2_info (&fmd->v4l2_dev,"Remote device at %s not found\n" , ep->full_name);
---------------------------8<----------------------------------
Signature · 4 lines
Thanks, Sylwester [1] git://linuxtv.org/media_tree.git
PATCH 1/2 exynos4-is: Add missing endpoint of_node_put on error paths
Quoted 26 lines · Sylwester Nawrocki
Quoted 2 lines · Krzysztof Kozlowski
In
fimc_md_parse_port_node()endpoint node is get withof_get_next_child()but it is not put on error path."is get" doesn't sound right to me, how about rephrasing this to:
"In
fimc_md_parse_port_node()reference count of the endpoint node "is incremented byof_get_next_child()but it is not decremented
on error path."Quoted 16 lines
Fixes 56fa1a6a6a7d ("[media] s5p-fimc: Change the driver directory name to exynos4-is")Signed-off-by Krzysztof Kozlowski <[email protected]>---
Not tested on hardware, only built+static checkers.--- drivers/media/platform/exynos4-is/media-dev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/media/platform/exynos4-is/media-dev.c +0
-0@@ -339,8 +339,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,return 0 ;
Quoted 12 lines
drivers/media/platform/exynos4-is/media-dev.c static int fimc_md_parse_port_node(struct fimc_md *fmd, · line 345
Quoted 6 lines
drivers/media/platform/exynos4-is/media-dev.c +3
-1v4l2_of_parse_endpoint (ep, &endpoint); -if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) +if (WARN_ON (endpoint.base.port ==0 ) || index >=FIMC_MAX_SENSORS ) { +of_node_put (ep);return -EINVAL ; + }Thanks for the patch, it looks correct but it doesn't apply cleanly due to patches already in media master branch [1]. Could you refresh this patch and resend?
Also I don't quite like multiple calls toof_node_put(), how about doing something like this instead:
How about sending your patch then with my reported-by?
Best regards,
Krzysztof