* [PATCH] [media] exynos4-is: Add missing 'of_node_put'
@ 2017-01-23 21:16 Christophe JAILLET
2017-02-24 21:19 ` Javier Martinez Canillas
0 siblings, 1 reply; 3+ messages in thread
From: Christophe JAILLET @ 2017-01-23 21:16 UTC (permalink / raw)
To: kyungmin.park, s.nawrocki, mchehab, kgene, krzk, javier
Cc: linux-media, linux-arm-kernel, linux-samsung-soc, linux-kernel,
kernel-janitors, Christophe JAILLET
It is likely that a "of_node_put(ep)" is missing here.
There is one in the previous error handling code, and one a few lines
below in the normal case as well.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/media/platform/exynos4-is/media-dev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index e3a8709138fa..da5b76c1df98 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -402,8 +402,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
return ret;
}
- 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;
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] exynos4-is: Add missing 'of_node_put'
2017-01-23 21:16 [PATCH] [media] exynos4-is: Add missing 'of_node_put' Christophe JAILLET
@ 2017-02-24 21:19 ` Javier Martinez Canillas
2017-02-25 8:01 ` Christophe JAILLET
0 siblings, 1 reply; 3+ messages in thread
From: Javier Martinez Canillas @ 2017-02-24 21:19 UTC (permalink / raw)
To: Christophe JAILLET, kyungmin.park, s.nawrocki, mchehab, kgene, krzk
Cc: linux-media, linux-arm-kernel, linux-samsung-soc, linux-kernel,
kernel-janitors
Hello Christophe,
On 01/23/2017 06:16 PM, Christophe JAILLET wrote:
> It is likely that a "of_node_put(ep)" is missing here.
> There is one in the previous error handling code, and one a few lines
> below in the normal case as well.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/media/platform/exynos4-is/media-dev.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index e3a8709138fa..da5b76c1df98 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -402,8 +402,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> return ret;
> }
>
> - 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;
>
>
Thanks for the patch, but Krzysztof sent the exact same patch before [0]. There
was feedback from Sylwester at the time that you can also look at [0]. Could you
please take that into account and post a patch according to what he suggested?
[0]: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/415207.html
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] exynos4-is: Add missing 'of_node_put'
2017-02-24 21:19 ` Javier Martinez Canillas
@ 2017-02-25 8:01 ` Christophe JAILLET
0 siblings, 0 replies; 3+ messages in thread
From: Christophe JAILLET @ 2017-02-25 8:01 UTC (permalink / raw)
To: Javier Martinez Canillas, kyungmin.park, s.nawrocki, mchehab,
kgene, krzk
Cc: linux-media, linux-arm-kernel, linux-samsung-soc, linux-kernel,
kernel-janitors
Le 24/02/2017 à 22:19, Javier Martinez Canillas a écrit :
> Thanks for the patch, but Krzysztof sent the exact same patch before
> [0]. There
> was feedback from Sylwester at the time that you can also look at [0]. Could you
> please take that into account and post a patch according to what he suggested?
>
> [0]: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/415207.html
>
> Best regards,
Hi,
apparently, the patch has already been taken into -next by Mauro
Carvalho Chehab.
Moreover, I personally don't think that what is proposed in [0] is more
readable.
There is not that much code between 'of_get_next_child' and
'of_node_put' in the normal path and not so many error handling paths
between the 2 function calls.
Adding some indentation level is not an improvement, IMHO.
If you really want, I could propose something like the code below (not
carefully checked, just to give an idea), but honestly, I don't think it
is needed.
It avoids some new indent and avoids duplicating 'of_node_put'.
Tell me if you think it is an improvement and I'll send a patch.
CJ
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c
index e3a8709138fa..da5b76c1df98 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -385,38 +385,35 @@ static void fimc_md_pipelines_free(struct fimc_md
*fmd)
static int fimc_md_parse_port_node(struct fimc_md *fmd,
struct device_node *port,
unsigned int index)
{
struct fimc_source_info *pd = &fmd->sensor[index].pdata;
- struct device_node *rem, *ep, *np;
+ struct device_node *rem = NULL, *ep = NULL, *np;
struct v4l2_of_endpoint endpoint;
int ret;
/* Assume here a port node can have only one endpoint node. */
ep = of_get_next_child(port, NULL);
if (!ep)
return 0;
ret = v4l2_of_parse_endpoint(ep, &endpoint);
- if (ret) {
- of_node_put(ep);
- return ret;
- }
+ if (ret)
+ goto put_ep;
if (WARN_ON(endpoint.base.port == 0) || index >= FIMC_MAX_SENSORS) {
- of_node_put(ep);
- return -EINVAL;
+ ret = -EINVAL;
+ goto put_ep;
}
pd->mux_id = (endpoint.base.port - 1) & 0x1;
rem = of_graph_get_remote_port_parent(ep);
- of_node_put(ep);
if (rem == NULL) {
v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
ep->full_name);
- return 0;
+ goto out;
}
if (fimc_input_is_parallel(endpoint.base.port)) {
if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
@@ -447,22 +444,28 @@ static int fimc_md_parse_port_node(struct fimc_md
*fmd,
pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK;
else
pd->fimc_bus_type = pd->sensor_bus_type;
if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) {
- of_node_put(rem);
- return -EINVAL;
+ ret = -EINVAL;
+ goto put_rem;
}
fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF;
fmd->sensor[index].asd.match.of.node = rem;
fmd->async_subdevs[index] = &fmd->sensor[index].asd;
fmd->num_sensors++;
+out:
+ ret = 0;
+put_rem:
of_node_put(rem);
- return 0;
+
+put_ep:
+ of_node_put(ep);
+ return ret;
}
/* Register all SoC external sub-devices */
static int fimc_md_register_sensor_entities(struct fimc_md *fmd)
{
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-25 8:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 21:16 [PATCH] [media] exynos4-is: Add missing 'of_node_put' Christophe JAILLET
2017-02-24 21:19 ` Javier Martinez Canillas
2017-02-25 8:01 ` Christophe JAILLET
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).