diff mbox

drm/amdgpu: Delete an unnecessary check before the function call "kfree"

Message ID 558FB427.80103@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring June 28, 2015, 8:45 a.m. UTC 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Jun 2015 10:27:35 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Alex Deucher June 29, 2015, 3:30 p.m. UTC | #1 
On Sun, Jun 28, 2015 at 4:45 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Jun 2015 10:27:35 +0200
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

I already have the same patch from Maninder Singh.

Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index fec487d..a85cd08 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1575,8 +1575,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>         amdgpu_fence_driver_fini(adev);
>         amdgpu_fbdev_fini(adev);
>         r = amdgpu_fini(adev);
> -       if (adev->ip_block_enabled)
> -               kfree(adev->ip_block_enabled);
> +       kfree(adev->ip_block_enabled);
>         adev->ip_block_enabled = NULL;
>         adev->accel_working = false;
>         /* free i2c buses */
> --
> 2.4.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
SF Markus Elfring July 16, 2016, 2:33 p.m. UTC | #2 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 16:23:21 +0200

Further update suggestions were taken into account after patches
were applied from static source code analysis.

Markus Elfring (8):
  Delete an unnecessary check before drm_gem_object_unreference_unlocked()
  Delete unnecessary checks before the function call "kfree"
  One function call less in amdgpu_cgs_acpi_eval_object() after error detection
  Delete a variable in amdgpu_cgs_acpi_eval_object()
  Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()
  Change assignment for a variable in amdgpu_cgs_acpi_eval_object()
  Change assignment for a buffer variable in phm_dispatch_table()
  Delete an unnecessary variable initialisation in phm_dispatch_table()

 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c            | 28 ++++++++++------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c        |  4 +---
 .../gpu/drm/amd/powerplay/hwmgr/functiontables.c   | 12 ++++------
 3 files changed, 19 insertions(+), 25 deletions(-)
Christian König July 18, 2016, 7:41 a.m. UTC | #3 
Am 16.07.2016 um 16:33 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 16:23:21 +0200
>
> Further update suggestions were taken into account after patches
> were applied from static source code analysis.

Small coding style nit pick on patch #7:
> -	}
> +	} else
> +		temp_storage = NULL;
When an "if" has "{" and "}" the else should also use them even when it 
is only one line.

With that fixed the whole series is Reviewed-by: Christian König 
<christian.koenig@amd.com>, but as Walter Harms pointed out as well 
there are a couple of other things we could make more as well.

Regards,
Christian.

>
> Markus Elfring (8):
>    Delete an unnecessary check before drm_gem_object_unreference_unlocked()
>    Delete unnecessary checks before the function call "kfree"
>    One function call less in amdgpu_cgs_acpi_eval_object() after error detection
>    Delete a variable in amdgpu_cgs_acpi_eval_object()
>    Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()
>    Change assignment for a variable in amdgpu_cgs_acpi_eval_object()
>    Change assignment for a buffer variable in phm_dispatch_table()
>    Delete an unnecessary variable initialisation in phm_dispatch_table()
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c            | 28 ++++++++++------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c        |  4 +---
>   .../gpu/drm/amd/powerplay/hwmgr/functiontables.c   | 12 ++++------
>   3 files changed, 19 insertions(+), 25 deletions(-)
>
Alex Deucher July 28, 2016, 4:10 p.m. UTC | #4 
On Mon, Jul 18, 2016 at 3:41 AM, Christian König
<christian.koenig@amd.com> wrote:
> Am 16.07.2016 um 16:33 schrieb SF Markus Elfring:
>>
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sat, 16 Jul 2016 16:23:21 +0200
>>
>> Further update suggestions were taken into account after patches
>> were applied from static source code analysis.
>
>
> Small coding style nit pick on patch #7:
>>
>> -       }
>> +       } else
>> +               temp_storage = NULL;
>
> When an "if" has "{" and "}" the else should also use them even when it is
> only one line.
>
> With that fixed the whole series is Reviewed-by: Christian König
> <christian.koenig@amd.com>, but as Walter Harms pointed out as well there
> are a couple of other things we could make more as well.

Applied the series except patch 2 was which already fixed by an earlier patch.

Thanks!

Alex

>
> Regards,
> Christian.
>
>>
>> Markus Elfring (8):
>>    Delete an unnecessary check before
>> drm_gem_object_unreference_unlocked()
>>    Delete unnecessary checks before the function call "kfree"
>>    One function call less in amdgpu_cgs_acpi_eval_object() after error
>> detection
>>    Delete a variable in amdgpu_cgs_acpi_eval_object()
>>    Delete an unnecessary variable initialisation in
>> amdgpu_cgs_acpi_eval_object()
>>    Change assignment for a variable in amdgpu_cgs_acpi_eval_object()
>>    Change assignment for a buffer variable in phm_dispatch_table()
>>    Delete an unnecessary variable initialisation in phm_dispatch_table()
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c            | 28
>> ++++++++++------------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c        |  4 +---
>>   .../gpu/drm/amd/powerplay/hwmgr/functiontables.c   | 12 ++++------
>>   3 files changed, 19 insertions(+), 25 deletions(-)
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index fec487d..a85cd08 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1575,8 +1575,7 @@  void amdgpu_device_fini(struct amdgpu_device *adev)
 	amdgpu_fence_driver_fini(adev);
 	amdgpu_fbdev_fini(adev);
 	r = amdgpu_fini(adev);
-	if (adev->ip_block_enabled)
-		kfree(adev->ip_block_enabled);
+	kfree(adev->ip_block_enabled);
 	adev->ip_block_enabled = NULL;
 	adev->accel_working = false;
 	/* free i2c buses */