[PATCH v2] drm/amd/amdgpu: Fix an uninitialized variable

[Christian König]

Am 09.01.23 um 17:19 schrieb Limonciello, Mario:
> [Public]
>
>
>
>> -----Original Message-----
>> From: Alex Deucher <alexdeucher at gmail.com>
>> Sent: Monday, January 9, 2023 10:04
>> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
>> Cc: Wentland, Harry <Harry.Wentland at amd.com>; Deucher, Alexander
>> <Alexander.Deucher at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com>; amd-gfx at lists.freedesktop.org; Limonciello,
>> Mario <Mario.Limonciello at amd.com>
>> Subject: Re: [PATCH v2] drm/amd/amdgpu: Fix an uninitialized variable
>>
>> On Mon, Jan 9, 2023 at 10:58 AM Srinivasan Shanmugam
>> <srinivasan.shanmugam at amd.com> wrote:
>>>    CC      drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.o
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:217:71: error: variable 'i' is
>> uninitialized when used here [-Werror,-Wuninitialized]
>>>                  snprintf(fw_name, sizeof(fw_name), "amdgpu/%s%d.bin",
>> ucode_prefix, i);
>>>                                                                                      ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:207:16: note: initialize the
>> variable 'i' to silence this warning
>>>          int err = 0, i;
>>>                        ^
>>>                         = 0
>>>
>>> As suggested by Christian, buggy
>>> "snprintf(fw_name, sizeof(fw_name), "amdgpu/%s%d.bin", ucode_prefix,
>> i);"
>>> shouldn't be "i" in the first place, but rather using "instance",
>>> because for instance greater than 0, we want to have different
>>> sdma firmware for different instance we add the instance number.
>>>
>>> Remove setting err to 0 as well. This is considered very bad coding style.
>>>
>>> Cc: Christian König <christian.koenig at amd.com>
>>> Cc: Mario Limonciello <mario.limonciello at amd.com>
>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> Thanks for the fix!
>
> Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>

Reviewed-by: Christian König <christian.koenig at amd.com>

>
>>> Change-Id: I2f1180af4f37bf1efd4d47e7bf64425b0b3809fb
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> index 0e1e2521fe25a..e9b78739b9ff7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> @@ -204,7 +204,7 @@ int amdgpu_sdma_init_microcode(struct
>> amdgpu_device *adev,
>>>   {
>>>          struct amdgpu_firmware_info *info = NULL;
>>>          const struct common_firmware_header *header = NULL;
>>> -       int err = 0, i;
>>> +       int err, i;
>>>          const struct sdma_firmware_header_v2_0 *sdma_hdr;
>>>          uint16_t version_major;
>>>          char ucode_prefix[30];
>>> @@ -214,7 +214,7 @@ int amdgpu_sdma_init_microcode(struct
>> amdgpu_device *adev,
>>>          if (instance == 0)
>>>                  snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
>> ucode_prefix);
>>>          else
>>> -               snprintf(fw_name, sizeof(fw_name), "amdgpu/%s%d.bin",
>> ucode_prefix, i);
>>> +               snprintf(fw_name, sizeof(fw_name), "amdgpu/%s%d.bin",
>> ucode_prefix, instance);
>>>          err = amdgpu_ucode_request(adev, &adev-
>>> sdma.instance[instance].fw, fw_name);
>>>          if (err)
>>>                  goto out;
>>> --
>>> 2.25.1
>>>