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

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

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