On 5/9/22 5:11 AM, 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)
This is called from zynq_fpga_ops_write_init, a fpga_manager_ops
function that
uses 'const char *' as a type for its write() buf's.
I think const u8 * would be a better type for all of the fpga_manager
instances.
If folks agree, I'll make the change.
Tom
{
for (; count >= 4; buf += 4, count -= 4)
if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
