diff mbox series

[v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()

Message ID 20231116122424.80136-1-biju.das.jz@bp.renesas.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series [v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init() | expand

Commit Message

Biju Das Nov. 16, 2023, 12:24 p.m. UTC
The rcar_du_vsps_init() doesn't free the np allocated by
of_parse_phandle_with_fixed_args() for the non-error case.

Fix memory leak for the non-error case.

While at it, replace the label 'error'->'done' as it applies to non-error
case as well and update the error check condition for rcar_du_vsp_init()
to avoid breakage in future, if it returns positive value.

Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the same VSP")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Replaced the label 'error'->'done' as it applies to non-error case as
   well.
 * Update the error check condition for rcar_du_vsp_init() to avoid
   breakage in future, if it returns positive value.
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Biju Das Dec. 15, 2023, 9:15 a.m. UTC | #1
Hi All,

Gentle ping.

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Thursday, November 16, 2023 12:24 PM
> Subject: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> 
> The rcar_du_vsps_init() doesn't free the np allocated by
> of_parse_phandle_with_fixed_args() for the non-error case.
> 
> Fix memory leak for the non-error case.
> 
> While at it, replace the label 'error'->'done' as it applies to non-error
> case as well and update the error check condition for rcar_du_vsp_init()
> to avoid breakage in future, if it returns positive value.
> 
> Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the same
> VSP")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Replaced the label 'error'->'done' as it applies to non-error case as
>    well.
>  * Update the error check condition for rcar_du_vsp_init() to avoid
>    breakage in future, if it returns positive value.
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> index 70d8ad065bfa..4c8fe83dd610 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device
> *rcdu)
>  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
>  						       cells, i, &args);
>  		if (ret < 0)
> -			goto error;
> +			goto done;
> 
>  		/*
>  		 * Add the VSP to the list or update the corresponding
> existing @@ -743,13 +743,11 @@ static int rcar_du_vsps_init(struct
> rcar_du_device *rcdu)
>  		vsp->dev = rcdu;
> 
>  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> -		if (ret < 0)
> -			goto error;
> +		if (ret)
> +			goto done;
>  	}
> 
> -	return 0;
> -
> -error:
> +done:
>  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
>  		of_node_put(vsps[i].np);
> 
> --
> 2.25.1

Cheers,
Biju
Laurent Pinchart Dec. 18, 2023, 10:39 a.m. UTC | #2
Hi Biju,

Thank you for the patch.

On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> The rcar_du_vsps_init() doesn't free the np allocated by
> of_parse_phandle_with_fixed_args() for the non-error case.
> 
> Fix memory leak for the non-error case.
> 
> While at it, replace the label 'error'->'done' as it applies to non-error
> case as well and update the error check condition for rcar_du_vsp_init()
> to avoid breakage in future, if it returns positive value.
> 
> Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the same VSP")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> v1->v2:
>  * Replaced the label 'error'->'done' as it applies to non-error case as
>    well.
>  * Update the error check condition for rcar_du_vsp_init() to avoid
>    breakage in future, if it returns positive value.
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> index 70d8ad065bfa..4c8fe83dd610 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
>  						       cells, i, &args);
>  		if (ret < 0)
> -			goto error;
> +			goto done;
>  
>  		/*
>  		 * Add the VSP to the list or update the corresponding existing
> @@ -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  		vsp->dev = rcdu;
>  
>  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> -		if (ret < 0)
> -			goto error;
> +		if (ret)
> +			goto done;
>  	}
>  
> -	return 0;
> -
> -error:
> +done:
>  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
>  		of_node_put(vsps[i].np);
>
Biju Das Jan. 4, 2024, 11:04 a.m. UTC | #3
Hi Laurent,

> Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in
> rcar_du_vsps_init()
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > The rcar_du_vsps_init() doesn't free the np allocated by
> > of_parse_phandle_with_fixed_args() for the non-error case.
> >
> > Fix memory leak for the non-error case.
> >
> > While at it, replace the label 'error'->'done' as it applies to
> > non-error case as well and update the error check condition for
> > rcar_du_vsp_init() to avoid breakage in future, if it returns positive
> value.
> >
> > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the
> > same VSP")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Can it be applied to rcar-du tree as in MAINTAINERS entry? Or it has to go through drm-misc-next?

Cheers,
Biju

> 
> > ---
> > v1->v2:
> >  * Replaced the label 'error'->'done' as it applies to non-error case as
> >    well.
> >  * Update the error check condition for rcar_du_vsp_init() to avoid
> >    breakage in future, if it returns positive value.
> > ---
> >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > index 70d8ad065bfa..4c8fe83dd610 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device
> *rcdu)
> >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> >  						       cells, i, &args);
> >  		if (ret < 0)
> > -			goto error;
> > +			goto done;
> >
> >  		/*
> >  		 * Add the VSP to the list or update the corresponding
> existing @@
> > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device
> *rcdu)
> >  		vsp->dev = rcdu;
> >
> >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > -		if (ret < 0)
> > -			goto error;
> > +		if (ret)
> > +			goto done;
> >  	}
> >
> > -	return 0;
> > -
> > -error:
> > +done:
> >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> >  		of_node_put(vsps[i].np);
> >
> 
> --
> Regards,
> 
> Laurent Pinchart
Biju Das Aug. 27, 2024, 4:43 p.m. UTC | #4
Hi Laurent and Tomi,

Gentle ping. Are you happy with this patch?

Cheers,
Biju

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, December 18, 2023 10:39 AM
> Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > The rcar_du_vsps_init() doesn't free the np allocated by
> > of_parse_phandle_with_fixed_args() for the non-error case.
> >
> > Fix memory leak for the non-error case.
> >
> > While at it, replace the label 'error'->'done' as it applies to
> > non-error case as well and update the error check condition for
> > rcar_du_vsp_init() to avoid breakage in future, if it returns positive value.
> >
> > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the
> > same VSP")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> > ---
> > v1->v2:
> >  * Replaced the label 'error'->'done' as it applies to non-error case as
> >    well.
> >  * Update the error check condition for rcar_du_vsp_init() to avoid
> >    breakage in future, if it returns positive value.
> > ---
> >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > index 70d8ad065bfa..4c8fe83dd610 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> >  						       cells, i, &args);
> >  		if (ret < 0)
> > -			goto error;
> > +			goto done;
> >
> >  		/*
> >  		 * Add the VSP to the list or update the corresponding existing @@
> > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> >  		vsp->dev = rcdu;
> >
> >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > -		if (ret < 0)
> > -			goto error;
> > +		if (ret)
> > +			goto done;
> >  	}
> >
> > -	return 0;
> > -
> > -error:
> > +done:
> >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> >  		of_node_put(vsps[i].np);
> >
> 
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Aug. 27, 2024, 10:22 p.m. UTC | #5
Hi Biju,

On Tue, Aug 27, 2024 at 04:43:12PM +0000, Biju Das wrote:
> Hi Laurent and Tomi,
> 
> Gentle ping. Are you happy with this patch?

I've sent a pull request last Friday, see
https://lore.kernel.org/r/20240822234445.GA23541@pendragon.ideasonboard.com

> > -----Original Message-----
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: Monday, December 18, 2023 10:39 AM
> > Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> > 
> > Hi Biju,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > > The rcar_du_vsps_init() doesn't free the np allocated by
> > > of_parse_phandle_with_fixed_args() for the non-error case.
> > >
> > > Fix memory leak for the non-error case.
> > >
> > > While at it, replace the label 'error'->'done' as it applies to
> > > non-error case as well and update the error check condition for
> > > rcar_du_vsp_init() to avoid breakage in future, if it returns positive value.
> > >
> > > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the
> > > same VSP")
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > > ---
> > > v1->v2:
> > >  * Replaced the label 'error'->'done' as it applies to non-error case as
> > >    well.
> > >  * Update the error check condition for rcar_du_vsp_init() to avoid
> > >    breakage in future, if it returns positive value.
> > > ---
> > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > index 70d8ad065bfa..4c8fe83dd610 100644
> > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> > >  						       cells, i, &args);
> > >  		if (ret < 0)
> > > -			goto error;
> > > +			goto done;
> > >
> > >  		/*
> > >  		 * Add the VSP to the list or update the corresponding existing @@
> > > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > >  		vsp->dev = rcdu;
> > >
> > >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > > -		if (ret < 0)
> > > -			goto error;
> > > +		if (ret)
> > > +			goto done;
> > >  	}
> > >
> > > -	return 0;
> > > -
> > > -error:
> > > +done:
> > >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> > >  		of_node_put(vsps[i].np);
> > >
Biju Das Aug. 28, 2024, 5:54 a.m. UTC | #6
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Tuesday, August 27, 2024 11:23 PM
> Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> 
> Hi Biju,
> 
> On Tue, Aug 27, 2024 at 04:43:12PM +0000, Biju Das wrote:
> > Hi Laurent and Tomi,
> >
> > Gentle ping. Are you happy with this patch?
> 
> I've sent a pull request last Friday, see
> https://lore.kernel.org/r/20240822234445.GA23541@pendragon.ideasonboard.com

That is nice.

Thanks,
Biju

> 
> > > -----Original Message-----
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: Monday, December 18, 2023 10:39 AM
> > > Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in
> > > rcar_du_vsps_init()
> > >
> > > Hi Biju,
> > >
> > > Thank you for the patch.
> > >
> > > On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > > > The rcar_du_vsps_init() doesn't free the np allocated by
> > > > of_parse_phandle_with_fixed_args() for the non-error case.
> > > >
> > > > Fix memory leak for the non-error case.
> > > >
> > > > While at it, replace the label 'error'->'done' as it applies to
> > > > non-error case as well and update the error check condition for
> > > > rcar_du_vsp_init() to avoid breakage in future, if it returns positive value.
> > > >
> > > > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from
> > > > the same VSP")
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Reviewed-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > >
> > > > ---
> > > > v1->v2:
> > > >  * Replaced the label 'error'->'done' as it applies to non-error case as
> > > >    well.
> > > >  * Update the error check condition for rcar_du_vsp_init() to avoid
> > > >    breakage in future, if it returns positive value.
> > > > ---
> > > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > index 70d8ad065bfa..4c8fe83dd610 100644
> > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> > > >  						       cells, i, &args);
> > > >  		if (ret < 0)
> > > > -			goto error;
> > > > +			goto done;
> > > >
> > > >  		/*
> > > >  		 * Add the VSP to the list or update the corresponding existing
> > > > @@
> > > > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > >  		vsp->dev = rcdu;
> > > >
> > > >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > > > -		if (ret < 0)
> > > > -			goto error;
> > > > +		if (ret)
> > > > +			goto done;
> > > >  	}
> > > >
> > > > -	return 0;
> > > > -
> > > > -error:
> > > > +done:
> > > >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> > > >  		of_node_put(vsps[i].np);
> > > >
> 
> --
> Regards,
> 
> Laurent Pinchart
Biju Das Aug. 28, 2024, 7:21 a.m. UTC | #7
Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Tuesday, August 27, 2024 11:23 PM
> Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> 
> Hi Biju,
> 
> On Tue, Aug 27, 2024 at 04:43:12PM +0000, Biju Das wrote:
> > Hi Laurent and Tomi,
> >
> > Gentle ping. Are you happy with this patch?
> 
> I've sent a pull request last Friday, see
> https://lore.kernel.org/r/20240822234445.GA23541@pendragon.ideasonboard.com

I believe the commit[1] won't apply any more as the source location is moved.
Shall I resend the patch or You will take care, please let me know.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git/commit/?h=drm-next-20240823&id=866dfbc953d46cf7682c6a434a4dd6249677fd68

Cheers,
Biju

> 
> > > -----Original Message-----
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: Monday, December 18, 2023 10:39 AM
> > > Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in
> > > rcar_du_vsps_init()
> > >
> > > Hi Biju,
> > >
> > > Thank you for the patch.
> > >
> > > On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > > > The rcar_du_vsps_init() doesn't free the np allocated by
> > > > of_parse_phandle_with_fixed_args() for the non-error case.
> > > >
> > > > Fix memory leak for the non-error case.
> > > >
> > > > While at it, replace the label 'error'->'done' as it applies to
> > > > non-error case as well and update the error check condition for
> > > > rcar_du_vsp_init() to avoid breakage in future, if it returns positive value.
> > > >
> > > > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from
> > > > the same VSP")
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Reviewed-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > >
> > > > ---
> > > > v1->v2:
> > > >  * Replaced the label 'error'->'done' as it applies to non-error case as
> > > >    well.
> > > >  * Update the error check condition for rcar_du_vsp_init() to avoid
> > > >    breakage in future, if it returns positive value.
> > > > ---
> > > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > index 70d8ad065bfa..4c8fe83dd610 100644
> > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> > > >  						       cells, i, &args);
> > > >  		if (ret < 0)
> > > > -			goto error;
> > > > +			goto done;
> > > >
> > > >  		/*
> > > >  		 * Add the VSP to the list or update the corresponding existing
> > > > @@
> > > > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > >  		vsp->dev = rcdu;
> > > >
> > > >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > > > -		if (ret < 0)
> > > > -			goto error;
> > > > +		if (ret)
> > > > +			goto done;
> > > >  	}
> > > >
> > > > -	return 0;
> > > > -
> > > > -error:
> > > > +done:
> > > >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> > > >  		of_node_put(vsps[i].np);
> > > >
> 
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Aug. 29, 2024, 1:52 p.m. UTC | #8
On Wed, Aug 28, 2024 at 07:21:32AM +0000, Biju Das wrote:
> On Tuesday, August 27, 2024 11:23 PM, Laurent Pinchart wrote:
> > Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in rcar_du_vsps_init()
> > 
> > Hi Biju,
> > 
> > On Tue, Aug 27, 2024 at 04:43:12PM +0000, Biju Das wrote:
> > > Hi Laurent and Tomi,
> > >
> > > Gentle ping. Are you happy with this patch?
> > 
> > I've sent a pull request last Friday, see
> > https://lore.kernel.org/r/20240822234445.GA23541@pendragon.ideasonboard.com
> 
> I believe the commit[1] won't apply any more as the source location is moved.
> Shall I resend the patch or You will take care, please let me know.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git/commit/?h=drm-next-20240823&id=866dfbc953d46cf7682c6a434a4dd6249677fd68

I think git will be able to handle that nicely.

> > > > -----Original Message-----
> > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Sent: Monday, December 18, 2023 10:39 AM
> > > > Subject: Re: [PATCH v2] drm: rcar-du: Fix memory leak in
> > > > rcar_du_vsps_init()
> > > >
> > > > Hi Biju,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > On Thu, Nov 16, 2023 at 12:24:24PM +0000, Biju Das wrote:
> > > > > The rcar_du_vsps_init() doesn't free the np allocated by
> > > > > of_parse_phandle_with_fixed_args() for the non-error case.
> > > > >
> > > > > Fix memory leak for the non-error case.
> > > > >
> > > > > While at it, replace the label 'error'->'done' as it applies to
> > > > > non-error case as well and update the error check condition for
> > > > > rcar_du_vsp_init() to avoid breakage in future, if it returns positive value.
> > > > >
> > > > > Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from
> > > > > the same VSP")
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Reviewed-by: Laurent Pinchart
> > > > <laurent.pinchart+renesas@ideasonboard.com>
> > > >
> > > > > ---
> > > > > v1->v2:
> > > > >  * Replaced the label 'error'->'done' as it applies to non-error case as
> > > > >    well.
> > > > >  * Update the error check condition for rcar_du_vsp_init() to avoid
> > > > >    breakage in future, if it returns positive value.
> > > > > ---
> > > > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 10 ++++------
> > > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > > index 70d8ad065bfa..4c8fe83dd610 100644
> > > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> > > > > @@ -705,7 +705,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > > >  		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> > > > >  						       cells, i, &args);
> > > > >  		if (ret < 0)
> > > > > -			goto error;
> > > > > +			goto done;
> > > > >
> > > > >  		/*
> > > > >  		 * Add the VSP to the list or update the corresponding existing
> > > > > @@
> > > > > -743,13 +743,11 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> > > > >  		vsp->dev = rcdu;
> > > > >
> > > > >  		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
> > > > > -		if (ret < 0)
> > > > > -			goto error;
> > > > > +		if (ret)
> > > > > +			goto done;
> > > > >  	}
> > > > >
> > > > > -	return 0;
> > > > > -
> > > > > -error:
> > > > > +done:
> > > > >  	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
> > > > >  		of_node_put(vsps[i].np);
> > > > >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
index 70d8ad065bfa..4c8fe83dd610 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
@@ -705,7 +705,7 @@  static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
 		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
 						       cells, i, &args);
 		if (ret < 0)
-			goto error;
+			goto done;
 
 		/*
 		 * Add the VSP to the list or update the corresponding existing
@@ -743,13 +743,11 @@  static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
 		vsp->dev = rcdu;
 
 		ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask);
-		if (ret < 0)
-			goto error;
+		if (ret)
+			goto done;
 	}
 
-	return 0;
-
-error:
+done:
 	for (i = 0; i < ARRAY_SIZE(vsps); ++i)
 		of_node_put(vsps[i].np);