* [PATCH] net/txgbe: fix a bit with boolean operator
@ 2022-03-01 6:08 Weiguo Li
2022-03-02 8:02 ` Jiawen Wu
0 siblings, 1 reply; 7+ messages in thread
From: Weiguo Li @ 2022-03-01 6:08 UTC (permalink / raw)
To: jiawenwu; +Cc: dev
Since boolean value is in 0 and 1, it's strange to combines a boolean
value with a bit operator.
Thus it's highly possible a typo error with "if (A & !B)", and more
probably to use "if (A & ~B)" instead.
Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
drivers/net/txgbe/txgbe_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index 19d4444748..f0994f028d 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
if (hw->mac.type != txgbe_mac_raptor)
return -ENOSYS;
- if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK)
+ if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK)
return -EIO;
PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat index %d",
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-01 6:08 [PATCH] net/txgbe: fix a bit with boolean operator Weiguo Li @ 2022-03-02 8:02 ` Jiawen Wu 2022-03-02 17:23 ` Ferruh Yigit 2022-03-02 17:34 ` Ferruh Yigit 0 siblings, 2 replies; 7+ messages in thread From: Jiawen Wu @ 2022-03-02 8:02 UTC (permalink / raw) To: 'Weiguo Li'; +Cc: dev On March 1, 2022 2:09 PM, Weiguo Li wrote: > Since boolean value is in 0 and 1, it's strange to combines a boolean value with > a bit operator. > > Thus it's highly possible a typo error with "if (A & !B)", and more probably to > use "if (A & ~B)" instead. > > Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") > > Signed-off-by: Weiguo Li <liwg06@foxmail.com> > --- > drivers/net/txgbe/txgbe_ethdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/txgbe/txgbe_ethdev.c > b/drivers/net/txgbe/txgbe_ethdev.c > index 19d4444748..f0994f028d 100644 > --- a/drivers/net/txgbe/txgbe_ethdev.c > +++ b/drivers/net/txgbe/txgbe_ethdev.c > @@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct > rte_eth_dev *eth_dev, > if (hw->mac.type != txgbe_mac_raptor) > return -ENOSYS; > > - if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK) > + if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK) > return -EIO; > > PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat > index %d", > -- > 2.25.1 Thanks. Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-02 8:02 ` Jiawen Wu @ 2022-03-02 17:23 ` Ferruh Yigit 2022-03-03 13:31 ` Weiguo Li 2022-03-02 17:34 ` Ferruh Yigit 1 sibling, 1 reply; 7+ messages in thread From: Ferruh Yigit @ 2022-03-02 17:23 UTC (permalink / raw) To: Jiawen Wu, 'Weiguo Li'; +Cc: dev, David Marchand, Aaron Conole On 3/2/2022 8:02 AM, Jiawen Wu wrote: > On March 1, 2022 2:09 PM, Weiguo Li wrote: >> Since boolean value is in 0 and 1, it's strange to combines a boolean > value with >> a bit operator. >> >> Thus it's highly possible a typo error with "if (A & !B)", and more > probably to >> use "if (A & ~B)" instead. >> >> Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") >> >> Signed-off-by: Weiguo Li <liwg06@foxmail.com> >> --- >> drivers/net/txgbe/txgbe_ethdev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/txgbe/txgbe_ethdev.c >> b/drivers/net/txgbe/txgbe_ethdev.c >> index 19d4444748..f0994f028d 100644 >> --- a/drivers/net/txgbe/txgbe_ethdev.c >> +++ b/drivers/net/txgbe/txgbe_ethdev.c >> @@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct >> rte_eth_dev *eth_dev, >> if (hw->mac.type != txgbe_mac_raptor) >> return -ENOSYS; >> >> - if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK) >> + if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK) >> return -EIO; >> >> PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat >> index %d", >> -- >> 2.25.1 > > Thanks. > > Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> > Hi Weiguo, Good catch, I wonder how did you detect this? If there is an automated way, maybe we can consider using it in our CI. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-02 17:23 ` Ferruh Yigit @ 2022-03-03 13:31 ` Weiguo Li 2022-03-03 15:18 ` Ferruh Yigit 2022-03-03 16:16 ` Stephen Hemminger 0 siblings, 2 replies; 7+ messages in thread From: Weiguo Li @ 2022-03-03 13:31 UTC (permalink / raw) To: ferruh.yigit; +Cc: jiawenwu, dev, david.marchand, aconole, julia.lawall On March 2, 2022, 5:23 p.m Ferruh Yigit wrote: > On 3/2/2022 8:02 AM, Jiawen Wu wrote: > > On March 1, 2022 2:09 PM, Weiguo Li wrote: > >> Since boolean value is in 0 and 1, it's strange to combines a boolean > > value with > >> a bit operator. > >> > >> Thus it's highly possible a typo error with "if (A & !B)", and more > > probably to > >> use "if (A & ~B)" instead. > >> > >> Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") > >> > >> Signed-off-by: Weiguo Li <liwg06@foxmail.com> > >> --- > >> drivers/net/txgbe/txgbe_ethdev.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/txgbe/txgbe_ethdev.c > >> b/drivers/net/txgbe/txgbe_ethdev.c > >> index 19d4444748..f0994f028d 100644 > >> --- a/drivers/net/txgbe/txgbe_ethdev.c > >> +++ b/drivers/net/txgbe/txgbe_ethdev.c > >> @@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct > >> rte_eth_dev *eth_dev, > >> if (hw->mac.type != txgbe_mac_raptor) > >> return -ENOSYS; > >> > >> - if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK) > >> + if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK) > >> return -EIO; > >> > >> PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat > >> index %d", > >> -- > >> 2.25.1 > > > > Thanks. > > > > Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> > > > > Hi Weiguo, > > Good catch, I wonder how did you detect this? > > If there is an automated way, maybe we can consider using > it in our CI. > Hi Ferruh, It's found by a coccinell script: @fix_bit_boolean @ expression E; constant C; @@ ( !E & !C | - !E & C + !(E & C) | - E & !C + E & ~C ) the idea came from a demo script in coccinelle website: (https://coccinelle.gitlabpages.inria.fr/website/rules/notand.html) @@ expression E; constant C; @@ ( !E & !C | - !E & C + !(E & C) ) The difference is in the last two lines and by which found the problem. I'd be happy if it would used in CI. Maybe better to let Julia know, since she's the original author. cc Julia -Weiguo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-03 13:31 ` Weiguo Li @ 2022-03-03 15:18 ` Ferruh Yigit 2022-03-03 16:16 ` Stephen Hemminger 1 sibling, 0 replies; 7+ messages in thread From: Ferruh Yigit @ 2022-03-03 15:18 UTC (permalink / raw) To: Weiguo Li, dpdklab Cc: jiawenwu, dev, david.marchand, aconole, julia.lawall, Ali Alnubani On 3/3/2022 1:31 PM, Weiguo Li wrote: > On March 2, 2022, 5:23 p.m Ferruh Yigit wrote: >> On 3/2/2022 8:02 AM, Jiawen Wu wrote: >>> On March 1, 2022 2:09 PM, Weiguo Li wrote: >>>> Since boolean value is in 0 and 1, it's strange to combines a boolean >>> value with >>>> a bit operator. >>>> >>>> Thus it's highly possible a typo error with "if (A & !B)", and more >>> probably to >>>> use "if (A & ~B)" instead. >>>> >>>> Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") >>>> >>>> Signed-off-by: Weiguo Li <liwg06@foxmail.com> >>>> --- >>>> drivers/net/txgbe/txgbe_ethdev.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/txgbe/txgbe_ethdev.c >>>> b/drivers/net/txgbe/txgbe_ethdev.c >>>> index 19d4444748..f0994f028d 100644 >>>> --- a/drivers/net/txgbe/txgbe_ethdev.c >>>> +++ b/drivers/net/txgbe/txgbe_ethdev.c >>>> @@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct >>>> rte_eth_dev *eth_dev, >>>> if (hw->mac.type != txgbe_mac_raptor) >>>> return -ENOSYS; >>>> >>>> - if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK) >>>> + if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK) >>>> return -EIO; >>>> >>>> PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat >>>> index %d", >>>> -- >>>> 2.25.1 >>> >>> Thanks. >>> >>> Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> >>> >> >> Hi Weiguo, >> >> Good catch, I wonder how did you detect this? >> >> If there is an automated way, maybe we can consider using >> it in our CI. >> > > Hi Ferruh, > > It's found by a coccinell script: > > @fix_bit_boolean @ expression E; constant C; @@ > ( > !E & !C > | > - !E & C > + !(E & C) > | > - E & !C > + E & ~C > ) > > the idea came from a demo script in coccinelle website: > (https://coccinelle.gitlabpages.inria.fr/website/rules/notand.html) > > @@ expression E; constant C; @@ > ( > !E & !C > | > - !E & C > + !(E & C) > ) > > The difference is in the last two lines and by which found the problem. > > I'd be happy if it would used in CI. > Maybe better to let Julia know, since she's the original author. > cc Julia > Thanks Weiguo. cc'ed lab people and Ali too. Aaron, lab, what do you think, can this be added into CI checks? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-03 13:31 ` Weiguo Li 2022-03-03 15:18 ` Ferruh Yigit @ 2022-03-03 16:16 ` Stephen Hemminger 1 sibling, 0 replies; 7+ messages in thread From: Stephen Hemminger @ 2022-03-03 16:16 UTC (permalink / raw) To: Weiguo Li Cc: ferruh.yigit, jiawenwu, dev, david.marchand, aconole, julia.lawall On Thu, 3 Mar 2022 21:31:00 +0800 Weiguo Li <liwg06@foxmail.com> wrote: > On March 2, 2022, 5:23 p.m Ferruh Yigit wrote: > > On 3/2/2022 8:02 AM, Jiawen Wu wrote: > > > On March 1, 2022 2:09 PM, Weiguo Li wrote: > > >> Since boolean value is in 0 and 1, it's strange to combines a boolean > > > value with > > >> a bit operator. > > >> > > >> Thus it's highly possible a typo error with "if (A & !B)", and more > > > probably to > > >> use "if (A & ~B)" instead. > > >> > > >> Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") > > >> > > >> Signed-off-by: Weiguo Li <liwg06@foxmail.com> > > >> --- > > >> drivers/net/txgbe/txgbe_ethdev.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/net/txgbe/txgbe_ethdev.c > > >> b/drivers/net/txgbe/txgbe_ethdev.c > > >> index 19d4444748..f0994f028d 100644 > > >> --- a/drivers/net/txgbe/txgbe_ethdev.c > > >> +++ b/drivers/net/txgbe/txgbe_ethdev.c > > >> @@ -376,7 +376,7 @@ txgbe_dev_queue_stats_mapping_set(struct > > >> rte_eth_dev *eth_dev, > > >> if (hw->mac.type != txgbe_mac_raptor) > > >> return -ENOSYS; > > >> > > >> - if (stat_idx & !QMAP_FIELD_RESERVED_BITS_MASK) > > >> + if (stat_idx & ~QMAP_FIELD_RESERVED_BITS_MASK) > > >> return -EIO; > > >> > > >> PMD_INIT_LOG(DEBUG, "Setting port %d, %s queue_id %d to stat > > >> index %d", > > >> -- > > >> 2.25.1 > > > > > > Thanks. > > > > > > Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> > > > > > > > Hi Weiguo, > > > > Good catch, I wonder how did you detect this? > > > > If there is an automated way, maybe we can consider using > > it in our CI. > > > > Hi Ferruh, > > It's found by a coccinell script: > > @fix_bit_boolean @ expression E; constant C; @@ > ( > !E & !C > | > - !E & C > + !(E & C) > | > - E & !C > + E & ~C > ) > > the idea came from a demo script in coccinelle website: > (https://coccinelle.gitlabpages.inria.fr/website/rules/notand.html) > > @@ expression E; constant C; @@ > ( > !E & !C > | > - !E & C > + !(E & C) > ) > > The difference is in the last two lines and by which found the problem. > > I'd be happy if it would used in CI. > Maybe better to let Julia know, since she's the original author. > cc Julia > > -Weiguo > > Could you add similar script to DPDK cocci scripts please? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/txgbe: fix a bit with boolean operator 2022-03-02 8:02 ` Jiawen Wu 2022-03-02 17:23 ` Ferruh Yigit @ 2022-03-02 17:34 ` Ferruh Yigit 1 sibling, 0 replies; 7+ messages in thread From: Ferruh Yigit @ 2022-03-02 17:34 UTC (permalink / raw) To: Jiawen Wu, 'Weiguo Li'; +Cc: dev On 3/2/2022 8:02 AM, Jiawen Wu wrote: > On March 1, 2022 2:09 PM, Weiguo Li wrote: >> Since boolean value is in 0 and 1, it's strange to combines a boolean > value with >> a bit operator. >> >> Thus it's highly possible a typo error with "if (A & !B)", and more > probably to >> use "if (A & ~B)" instead. >> >> Fixes: c1d4e9d37abdc6 ("net/txgbe: add queue stats mapping") >> >> Signed-off-by: Weiguo Li <liwg06@foxmail.com> > > Acked-by: Jiawen Wu <jiawenwu@trustnetic.com> > Applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-03 16:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-01 6:08 [PATCH] net/txgbe: fix a bit with boolean operator Weiguo Li 2022-03-02 8:02 ` Jiawen Wu 2022-03-02 17:23 ` Ferruh Yigit 2022-03-03 13:31 ` Weiguo Li 2022-03-03 15:18 ` Ferruh Yigit 2022-03-03 16:16 ` Stephen Hemminger 2022-03-02 17:34 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).