Re: [PATCH] drm/amdgpu: Fix uninitialized return value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 28.11.23 um 10:49 schrieb Lazar, Lijo:

On 11/28/2023 3:07 PM, Christian König wrote:
Am 27.11.23 um 22:55 schrieb Alex Deucher:
On Mon, Nov 27, 2023 at 2:22 PM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
Am 27.11.23 um 19:29 schrieb Lijo Lazar:
The return value is uniinitialized if ras context is NULL.

Fixes: 0f4c8faa043c (drm/amdgpu: Move mca debug mode decision to ras)

Signed-off-by: Lijo Lazar <lijo.lazar@xxxxxxx>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 1a8668a63e67..f6b47ebce9d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -3410,7 +3410,7 @@ int amdgpu_ras_reset_gpu(struct amdgpu_device *adev)    int amdgpu_ras_set_mca_debug_mode(struct amdgpu_device *adev, bool enable)
   {
       struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
-     int ret;
+     int ret = 0;
That's usually considered very bad coding style and complained about by
automated checkers.

Instead explicitly set the return value in the code paths not actually
setting it.
In this case, the function is so short, I think it makes things less
readable to do that.

Yeah, indeed but that doesn't help us with the coding style checkers.

Are these checkers for real? I see many instances of variable initialization including in core kernel code (ex: workqueue) code.

Yes, I've got multiple complains about that already.

What people basically seem to do is to search for patterns like "int ret = 0;... ret = whatever();.. return ret;" with cocci.

This then results in a note that an initialization isn't necessary and should be avoided.

Same for things like return after else, e.g. when you have something like this "if (...) { ret = whatever(); if (ret) return ret; } else { ... ret = 0;} return ret;".

Regards,
Christian.


Thanks

Lijo


We could do something like this instead:

if (!con)
    return 0;

ret = amdgpu_mca_smu_set_debug_mode(adev, enable);
...

Regards,
Christian.


Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>

Regards,
Christian.

       if (con) {
               ret = amdgpu_mca_smu_set_debug_mode(adev, enable);





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux