Timur Tabi
2024-Apr-25 16:38 UTC
[PATCH] [v7] nouveau: add command-line GSP-RM registry support
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. Thanks for catching this.
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.