diff mbox series

[3/5] common/nfp: fix integer handling issues

Message ID 20231115032310.756221-4-chaoyong.he@corigine.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series fix coverity issues |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He Nov. 15, 2023, 3:23 a.m. UTC 
  CI found integer handling issues, overflow before widen.

Coverity issue: 405351
Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/common/nfp/nfp_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Nov. 22, 2023, 11:50 a.m. UTC | #1 
15/11/2023 04:23, Chaoyong He:
> CI found integer handling issues, overflow before widen.
> 
> Coverity issue: 405351
> Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> Cc: stable@dpdk.org
[...]
> -		enabled_queues |= (1 << i);
> +		enabled_queues |= (1ULL << i);

That's a very bad fix.
You should use RTE_BIT64() which is more explicit.

Please read rte_bitops.h, that's a nice set of macros and functions.
Chaoyong He Nov. 23, 2023, 1:23 a.m. UTC | #2 
> 15/11/2023 04:23, Chaoyong He:
> > CI found integer handling issues, overflow before widen.
> >
> > Coverity issue: 405351
> > Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> > Cc: stable@dpdk.org
> [...]
> > -		enabled_queues |= (1 << i);
> > +		enabled_queues |= (1ULL << i);
> 
> That's a very bad fix.
> You should use RTE_BIT64() which is more explicit.
> 
> Please read rte_bitops.h, that's a nice set of macros and functions.
> 

Okay, I got it now.
So, what should I do about it? Send out another patch to fix or something else?
Thomas Monjalon Nov. 23, 2023, 2:08 a.m. UTC | #3 
23/11/2023 02:23, Chaoyong He:
> > 15/11/2023 04:23, Chaoyong He:
> > > CI found integer handling issues, overflow before widen.
> > >
> > > Coverity issue: 405351
> > > Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> > > Cc: stable@dpdk.org
> > [...]
> > > -		enabled_queues |= (1 << i);
> > > +		enabled_queues |= (1ULL << i);
> > 
> > That's a very bad fix.
> > You should use RTE_BIT64() which is more explicit.
> > 
> > Please read rte_bitops.h, that's a nice set of macros and functions.
> 
> Okay, I got it now.
> So, what should I do about it? Send out another patch to fix or something else?

You may convert your driver to use it where appropriate
if you feel it's worth.
diff mbox series

Patch

diff --git a/drivers/common/nfp/nfp_common.c b/drivers/common/nfp/nfp_common.c
index 4c94c9d59a..40e1620c2e 100644
--- a/drivers/common/nfp/nfp_common.c
+++ b/drivers/common/nfp/nfp_common.c
@@ -187,14 +187,14 @@  nfp_enable_queues(struct nfp_hw *hw,
 	/* Enabling the required TX queues in the device */
 	enabled_queues = 0;
 	for (i = 0; i < nb_tx_queues; i++)
-		enabled_queues |= (1 << i);
+		enabled_queues |= (1ULL << i);
 
 	nn_cfg_writeq(hw, NFP_NET_CFG_TXRS_ENABLE, enabled_queues);
 
 	/* Enabling the required RX queues in the device */
 	enabled_queues = 0;
 	for (i = 0; i < nb_rx_queues; i++)
-		enabled_queues |= (1 << i);
+		enabled_queues |= (1ULL << i);
 
 	nn_cfg_writeq(hw, NFP_NET_CFG_RXRS_ENABLE, enabled_queues);
 }