[PATCH v2] drm/msm: remove unnecessary NULL check

[PATCH v2] drm/msm: remove unnecessary NULL check

[Dan Carpenter]

This NULL check was required when it was added, but we shuffled the code
around 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)

Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function"
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
v2: Added a Fixes tag.  It's not really a bug fix and so adding the
fixes tag is slightly unfair but it should prevent this patch from
accidentally getting backported before the refactoring and causing an
issue.

Btw, fixes tags are often unfair like this.  People look at fixes tags
and think, "the fix introduced a bug" but actually it's really common
that the fix was just not complete.  But from a backporting perspective
it makes sense to tie them together.

Plus everyone introduces bugs.  If you're not introducing bugs, then
you're probably not writing a lot of code.

 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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;
 }
 
-- 
2.39.2

Re: [PATCH v2] drm/msm: remove unnecessary NULL check

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Fri, Oct 13, 2023 at 11:25:15AM +0300, Dan Carpenter wrote:
> This NULL check was required when it was added, but we shuffled the code
> around 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)
> 
> Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function"
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

(already provided for (implicit) v1, but wasn't picked up)

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] drm/msm: remove unnecessary NULL check

[Dmitry Baryshkov]

On 13/10/2023 11:25, Dan Carpenter wrote:
> This NULL check was required when it was added, but we shuffled the code
> around 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)
> 
> Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function"
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: Added a Fixes tag.  It's not really a bug fix and so adding the
> fixes tag is slightly unfair but it should prevent this patch from
> accidentally getting backported before the refactoring and causing an
> issue.
> 
> Btw, fixes tags are often unfair like this.  People look at fixes tags
> and think, "the fix introduced a bug" but actually it's really common
> that the fix was just not complete.  But from a backporting perspective
> it makes sense to tie them together.
> 
> Plus everyone introduces bugs.  If you're not introducing bugs, then
> you're probably not writing a lot of code.
> 
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm: remove unnecessary NULL check

[Abhinav Kumar]

On 10/13/2023 1:25 AM, Dan Carpenter wrote:
> This NULL check was required when it was added, but we shuffled the code
> around 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)
> 
> Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the _probe function"
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: Added a Fixes tag.  It's not really a bug fix and so adding the
> fixes tag is slightly unfair but it should prevent this patch from
> accidentally getting backported before the refactoring and causing an
> issue.
> 
> Btw, fixes tags are often unfair like this.  People look at fixes tags
> and think, "the fix introduced a bug" but actually it's really common
> that the fix was just not complete.  But from a backporting perspective
> it makes sense to tie them together.
> 
> Plus everyone introduces bugs.  If you're not introducing bugs, then
> you're probably not writing a lot of code.
> 
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 

LGTM,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Re: [Freedreno] [PATCH v2] drm/msm: remove unnecessary NULL check

[Abhinav Kumar]

On 11/1/2023 12:23 PM, Abhinav Kumar wrote:
> 
> 
> On 10/13/2023 1:25 AM, Dan Carpenter wrote:
>> This NULL check was required when it was added, but we shuffled the code
>> around 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)
>>
>> Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the 
>> _probe function"

A small error here. Its missing the closing brace for the Fixes tag.
Checkpatch cries without it.

I have fixed it while applying.

>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>> v2: Added a Fixes tag.  It's not really a bug fix and so adding the
>> fixes tag is slightly unfair but it should prevent this patch from
>> accidentally getting backported before the refactoring and causing an
>> issue.
>>
>> Btw, fixes tags are often unfair like this.  People look at fixes tags
>> and think, "the fix introduced a bug" but actually it's really common
>> that the fix was just not complete.  But from a backporting perspective
>> it makes sense to tie them together.
>>
>> Plus everyone introduces bugs.  If you're not introducing bugs, then
>> you're probably not writing a lot of code.
>>
>>   drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
> 
> LGTM,
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 

Re: [Freedreno] [PATCH v2] drm/msm: remove unnecessary NULL check

[Dan Carpenter]

On Thu, Nov 16, 2023 at 01:05:52PM -0800, Abhinav Kumar wrote:
> 
> 
> On 11/1/2023 12:23 PM, Abhinav Kumar wrote:
> > 
> > 
> > On 10/13/2023 1:25 AM, Dan Carpenter wrote:
> > > This NULL check was required when it was added, but we shuffled the code
> > > around 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)
> > > 
> > > Fixes: 1f50db2f3e1e ("drm/msm/mdp5: move resource allocation to the
> > > _probe function"
> 
> A small error here. Its missing the closing brace for the Fixes tag.
> Checkpatch cries without it.
> 

Sorry.  I must have accidentally deleted it after I ran checkpatch.

> I have fixed it while applying.

Thanks!

regards,
dan carpenter

Re: [PATCH v2] drm/msm: remove unnecessary NULL check

[Abhinav Kumar]

On Fri, 13 Oct 2023 11:25:15 +0300, Dan Carpenter wrote:
> This NULL check was required when it was added, but we shuffled the code
> around 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)
> 
> [...]

Applied, thanks!

[1/1] drm/msm: remove unnecessary NULL check
      https://gitlab.freedesktop.org/drm/msm/-/commit/56466f653cb5

Best regards,
-- 
Abhinav Kumar <quic_abhinavk@quicinc.com>