Re: [PATCH v2] drm: rcar-du: fix possible object reference leak
> > The call to of_get_parent returns a node pointer with refcount
> > incremented thus it must be explicitly decremented after the last
> > usage.
> >
> > Detected by coccinelle with the following warnings:
> > drivers/gpu/drm/rcar-du/rcar_du_of.c:235:2-8: ERROR: missing of_node_put;
> > acquired a node pointer with refcount incremented on line 216, but without
> > a corresponding object release within this function.
> > drivers/gpu/drm/rcar-du/rcar_du_of.c:236:2-8: ERROR: missing of_node_put;
> > acquired a node pointer with refcount incremented on line 209, but without
> > a corresponding object release within this function.
> >
> > Signed-off-by: Wen Yang
> > Suggested-by: Laurent Pinchart
> > Cc: Laurent Pinchart
> > Cc: Kieran Bingham
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-renesas-...@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org (open list)
> > ---
> > v2->v1: turn the return into a goto done.
> >
> > drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > index afef696..782bfc7 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > @@ -232,7 +232,7 @@ static void __init rcar_du_of_lvds_patch(const struct
> > of_device_id *of_ids)
> > lvds_node = of_find_compatible_node(NULL, NULL, compatible);
> > if (lvds_node) {
> > of_node_put(lvds_node);
> > -return;
> > +goto done;
> > }
> >
>
>
> you might have to create multiple level to do handling it.. there are
> unnecessary put being done on "done" which is not valid
>
> for this case.
Hi Mukesh,
Thank you for your comments.
lvds_data has been initialized to 0,
so the put operation in the done tag may have no effect:
199 struct lvds_of_data lvds_data[2] = { };
...
309 done:
310 for (i = 0; i < info->num_lvds; ++i) {
311 of_node_put(lvds_data[i].clkspec.np);
312 of_node_put(lvds_data[i].local);
313 of_node_put(lvds_data[i].remote);
314 }
315
316 of_node_put(soc_node);
317 of_node_put(du_node);
---
Thanks and regards,
Wen___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/meson: fix possible object reference leak
> > v2->v1: convert a if statement into a ternary statement.
>
> Would you like to omit the arrow in such version information?
Thank you for your comments
The information about the previous versions goes below the ---,
and only the reviewers can see it.
It does not appear in the git log log.
Thanks.
> > @@ -720,15 +720,10 @@ static bool meson_hdmi_connector_is_available(struct
> > device *dev)
> >
> > /* If the endpoint node exists, consider it enabled */
> > remote = of_graph_get_remote_port(ep);
> > -if (remote) {
> > -of_node_put(ep);
> > -return true;
> > -}
> > -
> > of_node_put(ep);
> > of_node_put(remote);
>
> Can a reordering of the passed variables be useful for such function calls?
>
> + of_node_put(remote);
> + of_node_put(ep);
>
Thank you.
But considering the multiprocessor concurrent execution,
the reverse of the order of these two statements may not have additional
benefits.
The previous code implementation also maintains this order,
and we will keep the original order unless we can prove that the reverse order
can indeed bring real benefits.
--
Regards,
Wen___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/7] drm/omap: fix possible object reference leak
> > The call to of_find_matching_node returns a node pointer with refcount > > incremented thus it must be explicitly decremented after the last > > usage. > > > > Detected by coccinelle with the following warnings: > > drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing > > of_node_put; acquired a node pointer with refcount incremented on line 209, > > but without a corresponding object release within this function. > > drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing > > of_node_put; acquired a node pointer with refcount incremented on line 209, > > but without a corresponding object release within this function. > > > > Signed-off-by: Wen Yang > > Cc: Tomi Valkeinen > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Sebastian Reichel > > Cc: Laurent Pinchart > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-ker...@vger.kernel.org > > Reviewed-by: Laurent Pinchart > > Would you like to get the series merged in one go, or individual patches > picked by the respective maintainer ? Thank you. We are sorry that we did not respond in time because of the Ching Ming Festival holiday. In our opinion, both ways are fine. In addition, we just checked and found that some patches have been selected by the maintainers. -- Regards, Wen___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
答复: Re: [PATCH 6/7] drm: rcar-du: fix possible object reference leak
> > The call to of_get_parent returns a node pointer with refcount
> > incremented thus it must be explicitly decremented after the last
> > usage.
> >
> > Detected by coccinelle with the following warnings:
> > drivers/gpu/drm/rcar-du/rcar_du_of.c:235:2-8: ERROR: missing of_node_put;
> > acquired a node pointer with refcount incremented on line 216, but without
> > a corresponding object release within this function.
> > drivers/gpu/drm/rcar-du/rcar_du_of.c:236:2-8: ERROR: missing of_node_put;
> > acquired a node pointer with refcount incremented on line 209, but without
> > a corresponding object release within this function.
> >
> > Signed-off-by: Wen Yang
> > Cc: Laurent Pinchart
> > Cc: Kieran Bingham
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-renesas-...@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org (open list)
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > index afef696..30bceca 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > @@ -232,6 +232,8 @@ static void __init rcar_du_of_lvds_patch(const struct
> > of_device_id *of_ids)
> > lvds_node = of_find_compatible_node(NULL, NULL, compatible);
> > if (lvds_node) {
> > of_node_put(lvds_node);
> > +of_node_put(soc_node);
> > +of_node_put(du_node);
> > return;
>
> Wouldn't it be simpler to just turn the return into a goto done ?
Hello, thank you for your comments.
Adding a "a goto done" here is indeed clearer and simpler, and we will fix it
soon.
Thank you.
--
Best wishes,
Wen___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
