On Mon, May 09, 2022 at 03:11:28PM +0300, Dan Carpenter wrote:
> The type needs to be u8. The type was accidentally changed to char as
> a cleanup. Unfortunately, that meant that the zynq_fpga_has_sync()
> function never returns true. This bug was detected by Smatch and Clang:
>
> drivers/fpga/zynq-fpga.c:245 zynq_fpga_has_sync() warn: impossible condition '(buf[2] == 153) => ((-128)-127 == 153)'
> drivers/fpga/zynq-fpga.c:246 zynq_fpga_has_sync() warn: impossible condition '(buf[3] == 170) => ((-128)-127 == 170)'
>
> drivers/fpga/zynq-fpga.c:246:14: warning: result of comparison of
> constant 170 with expression of type 'const char' is always false
> [-Wtautological-constant-out-of-range-compare]
> buf[3] == 0xaa)
> ~~~~~~ ^ ~~~~
> drivers/fpga/zynq-fpga.c:245:50: warning: result of comparison of
> constant 153 with expression of type 'const char' is always false
> [-Wtautological-constant-out-of-range-compare]
> if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
> ~~~~~~ ^ ~~~~
>
> Fixes: ada14a023a64 ("fpga: zynq: Fix incorrect variable type")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> The ada14a023a64 ("fpga: zynq: Fix incorrect variable type") patch went
> through six of revisions. The kbuild bug found this bug early on
> but the author ingored kbuild-bot and kept resending the buggy patch
> anyway.
>
> After the patch was merged then I sent a separate bug report and Xu
> Yilun asked about why only the author was on the CC list for the first
> bug reports. A valid question, definitely. I will poke the kbuild
> devs about this.
>
> Hm... Actually looking through the list there have been a bunch of bug
> reports about this because both Smatch and Clang complain so kbuild
> sends duplicate warnings for this type of bug. And then kbuild
> sends another to say "This issue is still remaining" warning. And then
> Xu Yilun sent an email "Kbuild-bot is still complaining. Please don't
> forget to fix this." So that's at least four public emails about this
> and one or two private emails directly from kbuild-bot to the author.
>
> The kbuild-bot wanted to send *another* warning today, but I decided to
> send a fix instead.
>
> LOL.
>
> drivers/fpga/zynq-fpga.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 6beaba9dfe97..426aa34c6a0d 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -239,7 +239,7 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
> * the correct byte order, and be dword aligned. The input is a Xilinx .bin
> * file with every 32 bit quantity swapped.
> */
> -static bool zynq_fpga_has_sync(const char *buf, size_t count)
> +static bool zynq_fpga_has_sync(const u8 *buf, size_t count)
Hi Dan & Moritz:
Thanks for the patch. But it actually reverts Nava's patch. Since Nava's
patch is not pushed to linux-next yet, could we just drop it from
linux-fpga?
Thanks,
Yilun
> {
> for (; count >= 4; buf += 4, count -= 4)
> if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
> --
> 2.35.1
