Danilo Krummrich
2024-Apr-26 16:08 UTC
[PATCH] [v7] nouveau: add command-line GSP-RM registry support
On 4/25/24 18:38, Timur Tabi wrote:> On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote: >>> + size_t length; >>> + >>> + /* Remove any whitespace from the parameter string */ >>> + length = strip(p, " \t\n"); >> >> With that, I see the following warning compiling this patch. >> >> warning: variable ?length? set but not used [-Wunused-but-set-variable] >> >> Did you intend to use the length for anything? > > No, and I could have sworn I fixed that before sending out v7. I think I > originally intended 'length' to determine when I finished parsing the > string. > >> Also, looking at the warning made me aware of 'p' potentially being NULL. >> >> If you agree, I can fix the warning and add the corresponding NULL check >> when >> applying the patch. > > Yes, that would be great. You can just delete 'length'. The NULL check for > 'p' should call clean_registry() before returning -ENOMEM.Applied to drm-misc-next, thanks!> > Thanks for catching this.
Lucas De Marchi
2024-Apr-30 13:06 UTC
[PATCH] [v7] nouveau: add command-line GSP-RM registry support
On Fri, Apr 26, 2024 at 06:08:19PM GMT, Danilo Krummrich wrote:>On 4/25/24 18:38, Timur Tabi wrote: >>On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote: >>>>+ size_t length; >>>>+ >>>>+ /* Remove any whitespace from the parameter string */ >>>>+ length = strip(p, " \t\n"); >>> >>>With that, I see the following warning compiling this patch. >>> >>>warning: variable ?length? set but not used [-Wunused-but-set-variable] >>> >>>Did you intend to use the length for anything? >> >>No, and I could have sworn I fixed that before sending out v7. I think I >>originally intended 'length' to determine when I finished parsing the >>string. >> >>>Also, looking at the warning made me aware of 'p' potentially being NULL. >>> >>>If you agree, I can fix the warning and add the corresponding NULL check >>>when >>>applying the patch. >> >>Yes, that would be great. You can just delete 'length'. The NULL check for >>'p' should call clean_registry() before returning -ENOMEM. > >Applied to drm-misc-next, thanks!since this commit our CI is failing to build with the following error: CC [M] drivers/gpu/drm/i915/display/intel_display_device.o ../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c: In function ?build_registry?: ../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1266:3: error: label at end of compound statement 1266 | default: | ^~~~~~~ CC [M] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.o HDRTEST drivers/gpu/drm/xe/compat-i915-headers/i915_reg.h CC [M] drivers/gpu/drm/amd/amdgpu/imu_v11_0.o make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.o] Error 1 make[7]: *** Waiting for unfinished jobs.... you are missing a `break;` after that default.> >> >>Thanks for catching this. >