[PATCH] drm/amd/display: indent an if statement

[PATCH] drm/amd/display: indent an if statement

[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]));
 	}
 
 	for (i = 0; i < pool->base.audio_count; i++) {

Re: [PATCH] drm/amd/display: indent an if statement

[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.

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
> 

Re: [PATCH] drm/amd/display: indent an if statement

[Christian König]

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
>>

[PATCH] drm/amd/display: small cleanup in destruct()

[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>
---
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])

Re: [PATCH] drm/amd/display: small cleanup in destruct()

[Christian König]

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])


--
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

Re: [PATCH] drm/amd/display: small cleanup in destruct()

[Alex Deucher]

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

Re: [PATCH] drm/amd/display: indent an if statement

[Dan Carpenter]

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

[PATCH] drm/amd/display: indent an if statement

[Dan Carpenter]

The if statement isn't indented and it makes static checkers complain.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 4ca41d6e3bcf..d82ba58c720f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -349,7 +349,7 @@ static bool is_dp_and_hdmi_sharable(
 
 	if (stream1->clamping.c_depth != COLOR_DEPTH_888 ||
 	    stream2->clamping.c_depth != COLOR_DEPTH_888)
-	return false;
+		return false;
 
 	return true;
 

Re: [PATCH] drm/amd/display: indent an if statement

[Harry Wentland]

On 2018-08-14 05:09 AM, Dan Carpenter wrote:
> The if statement isn't indented and it makes static checkers complain.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index 4ca41d6e3bcf..d82ba58c720f 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -349,7 +349,7 @@ static bool is_dp_and_hdmi_sharable(
>  
>  	if (stream1->clamping.c_depth != COLOR_DEPTH_888 ||
>  	    stream2->clamping.c_depth != COLOR_DEPTH_888)
> -	return false;
> +		return false;
>  
>  	return true;
>  
> 

[PATCH] drm/amd/display: Indent an if statement

[Dan Carpenter]

The if statement wasn't indented so it's confusing.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index ca26714c800e..c6b737dd8425 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -71,7 +71,7 @@ enum dce_version resource_parse_asic_id(struct hw_asic_id asic_id)
 		if (ASIC_REV_IS_TAHITI_P(asic_id.hw_internal_rev) ||
 		    ASIC_REV_IS_PITCAIRN_PM(asic_id.hw_internal_rev) ||
 		    ASIC_REV_IS_CAPEVERDE_M(asic_id.hw_internal_rev))
-		dc_version = DCE_VERSION_6_0;
+			dc_version = DCE_VERSION_6_0;
 		else if (ASIC_REV_IS_OLAND_M(asic_id.hw_internal_rev))
 			dc_version = DCE_VERSION_6_4;
 		else
-- 
2.27.0

Re: [PATCH] drm/amd/display: Indent an if statement

[Alex Deucher]

Applied.  Thanks!

Alex

On Mon, Aug 3, 2020 at 10:35 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The if statement wasn't indented so it's confusing.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index ca26714c800e..c6b737dd8425 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -71,7 +71,7 @@ enum dce_version resource_parse_asic_id(struct hw_asic_id asic_id)
>                 if (ASIC_REV_IS_TAHITI_P(asic_id.hw_internal_rev) ||
>                     ASIC_REV_IS_PITCAIRN_PM(asic_id.hw_internal_rev) ||
>                     ASIC_REV_IS_CAPEVERDE_M(asic_id.hw_internal_rev))
> -               dc_version = DCE_VERSION_6_0;
> +                       dc_version = DCE_VERSION_6_0;
>                 else if (ASIC_REV_IS_OLAND_M(asic_id.hw_internal_rev))
>                         dc_version = DCE_VERSION_6_4;
>                 else
> --
> 2.27.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx