Re: [bug report] platform/x86: fujitsu-laptop: Clean up constants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 05, 2018 at 03:30:53PM -0800, Darren Hart wrote:
> On Sun, Mar 04, 2018 at 09:37:59PM +0100, Michał Kępień wrote:
> > > The patch 819cddae7cfa: "platform/x86: fujitsu-laptop: Clean up
> > > constants" from Feb 20, 2018, leads to the following static checker
> > > warning:
> > > 
> > >     drivers/platform/x86/fujitsu-laptop.c:763 acpi_fujitsu_laptop_leds_register()
> > >     warn: always true condition '(call_fext_func(device, ((1 << (12)) | (1 << (0))), 2, (1 << (16)), 0) != (1 << (31))) => (s32min-s32max != 2147483648)'
> > > 
> > >     drivers/platform/x86/fujitsu-laptop.c:816 acpi_fujitsu_laptop_add()
> > >     warn: impossible condition '(priv->flags_supported == (1 << (31))) => (0-2147483647,18446744071562067968-u64max == 2147483648)'
> > 
> > Thank you for catching this, Dan.  sparse did not complain and as luck
> > would have it, I am only able to test fujitsu-laptop on an x86 machine,
> > which this bug does not affect.  I have now incorporated smatch into my
> 
> Erm, Fujitsu-laptop "depends on X86" - so which machine does this affect?
> 
> I presume this is an issue on 32b builds when BIT()'s use of a shifted 1UL
> causes the problem, combined with call_fext_func casting a ULL value into an int
> for return.
> 
> The proper fix here looks fairly invasive. Please consider a minimal fix for the
> RC cycle.
> 
> > workflow to prevent similar mishaps in the future.
> > 
> > Darren, Andy, as the bug is present in a patch queued for 4.17, I am
> > tempted to incorporate a fix for it in v2 of the last patch series I
> > submitted that reworks constants in fujitsu-laptop.  Would that be okay
> > or would you rather have me send a separate fix for this bug ASAP?
> 
> Please send this as a separate fix.

Ooof, sorry. I had my head in 4.16-rc work and missed what you said above about
this being in for-next for 4.17. It would be good to do this as a fix, followed
by the other cleanups, but if it makes sense to correct it as part of the
cleanup, that's fine too.

-- 
Darren Hart
VMware Open Source Technology Center



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux