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@xxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
>
> > + if (ret < 0)
> > devm_kfree(rcdu->dev, renc);
> > - }
> >
> > return ret;
> > }
--
Regards,
Laurent Pinchart
