Re: [PATCH 3/9] drm: rcar-du: Drop unneeded encoder cleanup in error path

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

On Mon, Dec 14, 2020 at 11:11:08AM +0100, Jacopo Mondi wrote:
> On Sat, Dec 05, 2020 at 12:01:33AM +0200, Laurent Pinchart wrote:
> > The encoder->name field can never be non-null in the error path, as that
> > can only be possible after a successful call to
> > drm_simple_encoder_init(). Drop the cleanup.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > index 2d40da98144b..0edce24f2053 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > @@ -124,11 +124,8 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> >  	}
> >
> >  done:
> > -	if (ret < 0) {
> > -		if (encoder->name)
> > -			encoder->funcs->destroy(encoder);
> 
> This is probably worth a Fixes tag, as accessing encoder->func if
> drm_simple_encoder_init() has not completed might lead to a NULL
> pointer dereference.

But in that case encoder->name would be NULL, so encoder->funcs won't be
dereferenced. And encoder->name can never be non-NULL here, as explained
in the commit message, so this is dead code. I don't think it requires a
Fixes: tag.

> Apart from this, patch looks good
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > +	if (ret < 0)
> >  		devm_kfree(rcdu->dev, renc);
> > -	}
> >
> >  	return ret;
> >  }

-- 
Regards,

Laurent Pinchart