All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Moritz Fischer <mdf@kernel.org>,
	Nava kishore Manne <nava.manne@xilinx.com>,
	Xu Yilun <yilun.xu@intel.com>
Cc: Wu Hao <hao.wu@intel.com>, Michal Simek <michal.simek@xilinx.com>,
	linux-fpga@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH] fpga: zynq: fix zynq_fpga_has_sync()
Date: Mon, 9 May 2022 15:11:28 +0300	[thread overview]
Message-ID: <YnkE8AbimDa7sfN8@kili> (raw)

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@intel.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
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)
 {
 	for (; count >= 4; buf += 4, count -= 4)
 		if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
-- 
2.35.1


             reply	other threads:[~2022-05-09 12:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 12:11 Dan Carpenter [this message]
2022-05-09 12:43 ` [PATCH] fpga: zynq: fix zynq_fpga_has_sync() Tom Rix
2022-05-09 15:15   ` Nava kishore Manne
2022-05-10 12:21     ` Tom Rix
2022-05-10 14:42       ` Xu Yilun
2022-05-09 16:55   ` Xu Yilun
2022-05-10  4:16     ` Nava kishore Manne
2022-05-09 17:00 ` Xu Yilun
2022-05-11  8:33   ` Dan Carpenter
2022-05-11  8:48     ` Xu Yilun
2022-05-11  9:17       ` Dan Carpenter
2022-05-11  9:15         ` Xu Yilun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YnkE8AbimDa7sfN8@kili \
    --to=dan.carpenter@oracle.com \
    --cc=hao.wu@intel.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-fpga@vger.kernel.org \
    --cc=mdf@kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=nava.manne@xilinx.com \
    --cc=yilun.xu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.