Re: [PATCH] net/txgbe: fix a bit with boolean operator

[Weiguo Li <liwg06@foxmail.com>]

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