dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: indent an if statement
@ 2017-11-04  6:12 Dan Carpenter
  2017-11-04  9:16 ` walter harms
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2017-11-04  6:12 UTC (permalink / raw)
  To: Alex Deucher, Harry Wentland
  Cc: Christian König, David Airlie, Tony Cheng,
	Dmytro Laktyushkin, Hersen Wu, Yue Hin Lau, amd-gfx, dri-devel,
	kernel-janitors

The if statement wasn't indented so it makes static analysis tools and
probably very recent GCC versions complain.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I went over 80 characters because other lines do already and it seemed
like the cleanest thing here.

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index d911590d08bc..c83ac4d9ca3a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -727,7 +727,7 @@ static void destruct(struct dcn10_resource_pool *pool)
 
 	for (i = 0; i < pool->base.stream_enc_count; i++) {
 		if (pool->base.stream_enc[i] != NULL)
-		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
+			kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
 	}
 
 	for (i = 0; i < pool->base.audio_count; i++) {

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd/display: indent an if statement
  2017-11-04  6:12 [PATCH] drm/amd/display: indent an if statement Dan Carpenter
@ 2017-11-04  9:16 ` walter harms
       [not found]   ` <59FD8567.4030500-fPG8STNUNVg@public.gmane.org>
  2017-11-06  6:42   ` [PATCH] drm/amd/display: indent an if statement Dan Carpenter
  0 siblings, 2 replies; 7+ messages in thread
From: walter harms @ 2017-11-04  9:16 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, kernel-janitors, amd-gfx, Dmytro Laktyushkin,
	Hersen Wu, dri-devel, Yue Hin Lau, Alex Deucher, Tony Cheng,
	Christian König



Am 04.11.2017 07:12, schrieb Dan Carpenter:
> The if statement wasn't indented so it makes static analysis tools and
> probably very recent GCC versions complain.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I went over 80 characters because other lines do already and it seemed
> like the cleanest thing here.
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> index d911590d08bc..c83ac4d9ca3a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> @@ -727,7 +727,7 @@ static void destruct(struct dcn10_resource_pool *pool)
>  
>  	for (i = 0; i < pool->base.stream_enc_count; i++) {
>  		if (pool->base.stream_enc[i] != NULL)
> -		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
> +			kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>  	}
>  

Is that "if (pool->base.stream_enc[i] != NULL)" needed at all ?
kfree() should happily handle NULL.

re,
 wh



>  	for (i = 0; i < pool->base.audio_count; i++) {
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd/display: indent an if statement
       [not found]   ` <59FD8567.4030500-fPG8STNUNVg@public.gmane.org>
@ 2017-11-04 10:07     ` Christian König
  2017-11-06  7:07       ` [PATCH] drm/amd/display: small cleanup in destruct() Dan Carpenter
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-11-04 10:07 UTC (permalink / raw)
  To: wharms-fPG8STNUNVg, Dan Carpenter
  Cc: David Airlie, Harry Wentland,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dmytro Laktyushkin,
	Hersen Wu, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	Yue Hin Lau, Alex Deucher, Tony Cheng

Am 04.11.2017 um 10:16 schrieb walter harms:
>
> Am 04.11.2017 07:12, schrieb Dan Carpenter:
>> The if statement wasn't indented so it makes static analysis tools and
>> probably very recent GCC versions complain.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> I went over 80 characters because other lines do already and it seemed
>> like the cleanest thing here.
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> index d911590d08bc..c83ac4d9ca3a 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> @@ -727,7 +727,7 @@ static void destruct(struct dcn10_resource_pool *pool)
>>   
>>   	for (i = 0; i < pool->base.stream_enc_count; i++) {
>>   		if (pool->base.stream_enc[i] != NULL)
>> -		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>> +			kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>>   	}
>>   
> Is that "if (pool->base.stream_enc[i] != NULL)" needed at all ?
> kfree() should happily handle NULL.

Yeah, indeed. The correct solution would be to just drop the "if" as 
well as the "{}".

I would also drop the cast done by DCE110STRENC_FROM_STRENC, cause 
kfree() doesn't care about the type of the pointer.

Regards,
Christian.

>
> re,
>   wh
>
>
>
>>   	for (i = 0; i < pool->base.audio_count; i++) {
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd/display: indent an if statement
  2017-11-04  9:16 ` walter harms
       [not found]   ` <59FD8567.4030500-fPG8STNUNVg@public.gmane.org>
@ 2017-11-06  6:42   ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2017-11-06  6:42 UTC (permalink / raw)
  To: walter harms
  Cc: Alex Deucher, Harry Wentland, Christian König, David Airlie,
	Tony Cheng, Dmytro Laktyushkin, Hersen Wu, Yue Hin Lau, amd-gfx,
	dri-devel, kernel-janitors

On Sat, Nov 04, 2017 at 10:16:23AM +0100, walter harms wrote:
> 
> 
> Am 04.11.2017 07:12, schrieb Dan Carpenter:
> > The if statement wasn't indented so it makes static analysis tools and
> > probably very recent GCC versions complain.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > I went over 80 characters because other lines do already and it seemed
> > like the cleanest thing here.
> > 
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> > index d911590d08bc..c83ac4d9ca3a 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> > @@ -727,7 +727,7 @@ static void destruct(struct dcn10_resource_pool *pool)
> >  
> >  	for (i = 0; i < pool->base.stream_enc_count; i++) {
> >  		if (pool->base.stream_enc[i] != NULL)
> > -		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
> > +			kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
> >  	}
> >  
> 
> Is that "if (pool->base.stream_enc[i] != NULL)" needed at all ?
> kfree() should happily handle NULL.
> 

Good point.  I'll resend.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH] drm/amd/display: small cleanup in destruct()
  2017-11-04 10:07     ` Christian König
@ 2017-11-06  7:07       ` Dan Carpenter
  2017-11-06  8:44         ` Christian König
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2017-11-06  7:07 UTC (permalink / raw)
  To: Alex Deucher, Harry Wentland
  Cc: Christian König, David Airlie, Tony Cheng,
	Dmytro Laktyushkin, Hersen Wu, Dave Airlie, Andrey Grodzovsky,
	Bhawanpreet Lakha, amd-gfx, dri-devel, kernel-janitors

Static analysis tools get annoyed that we don't indent this if
statement.  Actually, the if statement isn't required because kfree()
can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
macro is a wrapper around container_of() but it's basically a no-op or a
cast.  Anyway, it's not really appropriate here so it should be removed
as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: in v1 I just added a tab

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index d911590d08bc..4c4bd72d4e40 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool *pool)
 		}
 	}
 
-	for (i = 0; i < pool->base.stream_enc_count; i++) {
-		if (pool->base.stream_enc[i] != NULL)
-		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
-	}
+	for (i = 0; i < pool->base.stream_enc_count; i++)
+		kfree(pool->base.stream_enc[i]);
 
 	for (i = 0; i < pool->base.audio_count; i++) {
 		if (pool->base.audios[i])

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd/display: small cleanup in destruct()
  2017-11-06  7:07       ` [PATCH] drm/amd/display: small cleanup in destruct() Dan Carpenter
@ 2017-11-06  8:44         ` Christian König
  2017-11-06 21:29           ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-11-06  8:44 UTC (permalink / raw)
  To: Dan Carpenter, Alex Deucher, Harry Wentland
  Cc: Andrey Grodzovsky, David Airlie, Bhawanpreet Lakha,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dmytro Laktyushkin,
	Hersen Wu, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	Dave Airlie, Tony Cheng

Am 06.11.2017 um 08:07 schrieb Dan Carpenter:
> Static analysis tools get annoyed that we don't indent this if
> statement.  Actually, the if statement isn't required because kfree()
> can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
> macro is a wrapper around container_of() but it's basically a no-op or a
> cast.  Anyway, it's not really appropriate here so it should be removed
> as well.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
> v2: in v1 I just added a tab
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> index d911590d08bc..4c4bd72d4e40 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool *pool)
>   		}
>   	}
>   
> -	for (i = 0; i < pool->base.stream_enc_count; i++) {
> -		if (pool->base.stream_enc[i] != NULL)
> -		kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
> -	}
> +	for (i = 0; i < pool->base.stream_enc_count; i++)
> +		kfree(pool->base.stream_enc[i]);
>   
>   	for (i = 0; i < pool->base.audio_count; i++) {
>   		if (pool->base.audios[i])


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd/display: small cleanup in destruct()
  2017-11-06  8:44         ` Christian König
@ 2017-11-06 21:29           ` Alex Deucher
  0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2017-11-06 21:29 UTC (permalink / raw)
  To: Christian König
  Cc: Tony Cheng, David Airlie, kernel-janitors, amd-gfx list,
	Dmytro Laktyushkin, Hersen Wu, Maling list - DRI developers,
	Alex Deucher, Dave Airlie, Bhawanpreet Lakha, Dan Carpenter

On Mon, Nov 6, 2017 at 3:44 AM, Christian König
<christian.koenig@amd.com> wrote:
> Am 06.11.2017 um 08:07 schrieb Dan Carpenter:
>>
>> Static analysis tools get annoyed that we don't indent this if
>> statement.  Actually, the if statement isn't required because kfree()
>> can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
>> macro is a wrapper around container_of() but it's basically a no-op or a
>> cast.  Anyway, it's not really appropriate here so it should be removed
>> as well.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>
> Acked-by: Christian König <christian.koenig@amd.com>

Applied.  thanks!

Alex


>
>> ---
>> v2: in v1 I just added a tab
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> index d911590d08bc..4c4bd72d4e40 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool
>> *pool)
>>                 }
>>         }
>>   -     for (i = 0; i < pool->base.stream_enc_count; i++) {
>> -               if (pool->base.stream_enc[i] != NULL)
>> -               kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>> -       }
>> +       for (i = 0; i < pool->base.stream_enc_count; i++)
>> +               kfree(pool->base.stream_enc[i]);
>>         for (i = 0; i < pool->base.audio_count; i++) {
>>                 if (pool->base.audios[i])
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-06 21:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-04  6:12 [PATCH] drm/amd/display: indent an if statement Dan Carpenter
2017-11-04  9:16 ` walter harms
     [not found]   ` <59FD8567.4030500-fPG8STNUNVg@public.gmane.org>
2017-11-04 10:07     ` Christian König
2017-11-06  7:07       ` [PATCH] drm/amd/display: small cleanup in destruct() Dan Carpenter
2017-11-06  8:44         ` Christian König
2017-11-06 21:29           ` Alex Deucher
2017-11-06  6:42   ` [PATCH] drm/amd/display: indent an if statement Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).