| Message ID | 5de18b71-c3db-4820-b35e-262b4cac35fc@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: remove unnecessary NULL check | expand |
Hello, On Fri, Oct 13, 2023 at 10:17:08AM +0300, Dan Carpenter wrote: > This NULL check was required when it was added, but we shuffled the code > around in commit 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation > to the _probe function") and now it's not. The inconsistent NULL > checking triggers a Smatch warning: > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c:847 mdp5_init() warn: > variable dereferenced before check 'mdp5_kms' (see line 782) > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> LGTM Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> This patch opportunity is valid since commit 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function") but applies to older trees (where it introduces a bug). On one hand it's not really a fix, but maybe still add a Fixes: line to ensure it's not backported to older stables? Hmm, I don't know. Best regards Uwe
On Fri, Oct 13, 2023 at 10:01:49AM +0200, Uwe Kleine-König wrote: > Hello, > > On Fri, Oct 13, 2023 at 10:17:08AM +0300, Dan Carpenter wrote: > > This NULL check was required when it was added, but we shuffled the code > > around in commit 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation > > to the _probe function") and now it's not. The inconsistent NULL > > checking triggers a Smatch warning: > > > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c:847 mdp5_init() warn: > > variable dereferenced before check 'mdp5_kms' (see line 782) > > > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > LGTM > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > This patch opportunity is valid since commit 1f50db2f3e1e > ("drm/msm/mdp5: move resource allocation to the _probe function") but > applies to older trees (where it introduces a bug). > On one hand it's not really a fix, but maybe still add a Fixes: line to > ensure it's not backported to older stables? Hmm, I don't know. Sure. Being extra safe is good. regards, dan carpenter
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 11d9fc2c6bf5..ec933d597e20 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -844,8 +844,7 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev) return 0; fail: - if (mdp5_kms) - mdp5_destroy(mdp5_kms); + mdp5_destroy(mdp5_kms); return ret; }
This NULL check was required when it was added, but we shuffled the code around in commit 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function") and now it's not. The inconsistent NULL checking triggers a Smatch warning: drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c:847 mdp5_init() warn: variable dereferenced before check 'mdp5_kms' (see line 782) Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)