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
