Messages in this thread | | | Subject | Re: drivers/platform/surface/surface3-wmi.c:60:14: warning: variable 'status' set but not used | From | Maximilian Luz <> | Date | Thu, 4 Feb 2021 16:10:57 +0100 |
| |
On 2/4/21 12:36 PM, Hans de Goede wrote: > Hi, > > On 1/4/21 4:24 PM, Maximilian Luz wrote: >> On 1/4/21 3:52 PM, Hans de Goede wrote: >>> Hi, >>> >>> On 12/29/20 4:58 AM, kernel test robot wrote: >>>> Hi Maximilian, >>>> >>>> FYI, the error/warning still remains. >>>> >>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master >>>> head: dea8dcf2a9fa8cc540136a6cd885c3beece16ec3 >>>> commit: f23027ca3d48b6f93c5994069fb25b73539fdf34 platform/surface: Move Surface 3 WMI driver to platform/surface >>>> date: 9 weeks ago >>>> config: x86_64-randconfig-r001-20201221 (attached as .config) >>>> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 >>>> reproduce (this is a W=1 build): >>>> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f23027ca3d48b6f93c5994069fb25b73539fdf34 >>>> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >>>> git fetch --no-tags linus master >>>> git checkout f23027ca3d48b6f93c5994069fb25b73539fdf34 >>>> # save the attached .config to linux build tree >>>> make W=1 ARCH=x86_64 >>>> >>>> If you fix the issue, kindly add following tag as appropriate >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> >>>> All warnings (new ones prefixed by >>): >>>> >>>> drivers/platform/surface/surface3-wmi.c: In function 's3_wmi_query_block': >>>>>> drivers/platform/surface/surface3-wmi.c:60:14: warning: variable 'status' set but not used [-Wunused-but-set-variable] >>>> 60 | acpi_status status; >>>> | ^~~~~~ >>> >>> I guess fixing this would require something like this: >>> >>> From: Hans de Goede <hdegoede@redhat.com> >>> Subject: [PATCH] platform/surface: surface3-wmi: Fix variable 'status' set but not used compiler warning >>> >>> Explictly check the status rather then relying on output.pointer staying >>> NULL on an error. This silences the following compiler warning: >>> >>> drivers/platform/surface/surface3-wmi.c:60:14: warning: variable 'status' set but not used [-Wunused-but-set-variable] >>> >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> drivers/platform/surface/surface3-wmi.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/platform/surface/surface3-wmi.c b/drivers/platform/surface/surface3-wmi.c >>> index 130b6f52a600..4b7f79c0b78e 100644 >>> --- a/drivers/platform/surface/surface3-wmi.c >>> +++ b/drivers/platform/surface/surface3-wmi.c >>> @@ -63,6 +63,10 @@ static int s3_wmi_query_block(const char *guid, int instance, int *ret) >>> mutex_lock(&s3_wmi_lock); >>> status = wmi_query_block(guid, instance, &output); >>> + if (ACPI_FAILURE(status)) { >>> + error = -EIO; >>> + goto out_free_unlock; >>> + } >>> obj = output.pointer; >>> >>> Maximilian, can you review and/or test this fix please ? >> >> Ah, this was on my TODO list (among looking at some other things in this >> driver), sorry that I haven't gotten to it yet. I'd have proposed pretty >> much the exact same thing. >> >> One thing to note though: You should initialize obj with NULL. Keeping >> it uninitialized may mess with kfree() under out_free_unlock. >> >> Unfortunately I don't have access to a Surface 3 to test, but apart from >> not initializing obj, this patch looks good to me. You may add my >> reviewed-by tag once you've fixed that. > > Thank you, and sorry for being a bit slow with my follow up.
No worries.
> I've added the obj = NULL initialization and added your reviewed-by tag. > > I'm sending this out as an official patch submission for the archives now > and then I'll apply it to my review-hans branch right away, so no need > for you to do anything :) > >> Also note that drivers/platform/x86/msi-wmi.c suffers from the same >> problem in msi_wmi_query_block() and should receive a similar fix. > > Thanks, I'll prep a patch for that too.
Perfect, thanks!
Regards, Max
> > Regards, > > Hans >
| |