linux-kernel - Re: [PATCH V2 net-next 8/8] net: hns3: clear hns alarm: comparison of integer expressions of different signedness

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250619125453.GO1699@horms.kernel.org>
Date: Thu, 19 Jun 2025 13:54:53 +0100
From: Simon Horman <horms@...nel.org>
To: Jijie Shao <shaojijie@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, andrew+netdev@...n.ch, shenjian15@...wei.com,
	wangpeiyang1@...wei.com, liuyonglong@...wei.com,
	chenhao418@...wei.com, jonathan.cameron@...wei.com,
	shameerali.kolothum.thodi@...wei.com, salil.mehta@...wei.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	michal.swiatkowski@...ux.intel.com
Subject: Re: [PATCH V2 net-next 8/8] net: hns3: clear hns alarm: comparison
 of integer expressions of different signedness

On Thu, Jun 19, 2025 at 07:59:07PM +0800, Jijie Shao wrote:
> 
> on 2025/6/18 19:29, Simon Horman wrote:
> > On Tue, Jun 17, 2025 at 09:02:55AM +0800, Jijie Shao wrote:
> > > From: Peiyang Wang <wangpeiyang1@...wei.com>
> > > 
> > > A static alarm exists in the hns and needs to be cleared.
> > I'm curious to know if you used a tool to flag this.
> 
> Sorry, the last reply was not cc to netdev...
> 
> Some internal tools, and then there are the compiler options, such as -Wsign-compare.
> 
> > 
> > > The alarm is comparison of integer expressions of different
> > > signedness including 's64' and 'long unsigned int',
> > > 'int' and 'long unsigned int', 'u32' and 'int',
> > > 'int' and 'unsigned int'.
> > > 
> > > Signed-off-by: Peiyang Wang <wangpeiyang1@...wei.com>
> > > Signed-off-by: Jijie Shao <shaojijie@...wei.com>
> > > ---
> > >   .../hns3/hns3_common/hclge_comm_cmd.c         |  2 +-
> > >   .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 22 +++++++-------
> > >   .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  2 +-
> > >   .../ethernet/hisilicon/hns3/hns3_ethtool.c    |  4 +--
> > >   .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 13 ++++----
> > >   .../hisilicon/hns3/hns3pf/hclge_main.c        | 30 +++++++++----------
> > >   .../hisilicon/hns3/hns3pf/hclge_mbx.c         |  7 +++--
> > >   .../hisilicon/hns3/hns3pf/hclge_mdio.c        |  2 +-
> > >   .../hisilicon/hns3/hns3pf/hclge_ptp.h         |  2 +-
> > >   .../hisilicon/hns3/hns3vf/hclgevf_main.c      |  2 +-
> > >   .../hisilicon/hns3/hns3vf/hclgevf_mbx.c       |  2 +-
> > >   11 files changed, 44 insertions(+), 44 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_cmd.c
> > > index 4ad4e8ab2f1f..37396ca4ecfc 100644
> > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_cmd.c
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_cmd.c
> > > @@ -348,7 +348,7 @@ static int hclge_comm_cmd_csq_clean(struct hclge_comm_hw *hw)
> > >   static int hclge_comm_cmd_csq_done(struct hclge_comm_hw *hw)
> > >   {
> > >   	u32 head = hclge_comm_read_dev(hw, HCLGE_COMM_NIC_CSQ_HEAD_REG);
> > > -	return head == hw->cmq.csq.next_to_use;
> > > +	return head == (u32)hw->cmq.csq.next_to_use;
> > Can the type of next_to_use be changed to an unsigned type?
> > It would be nice to avoid casts.
> 
> Today I plan to modify the next_to_use type,
> but I found that if next_to_use is changed to u32,
> it will cause many other places to need to synchronously change the variable type.
> At a glance, there are dozens of places.
> 
> Therefore, I am considering whether this part can remain unchanged.

Thanks for checking.

Based on the above I'd say it can remain unchanged.
It improves things. And leaves room for further improvements later.