* [bug report] Maple Tree: add new data structure
@ 2022-07-19 9:36 Dan Carpenter
2022-07-19 13:37 ` Liam Howlett
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-07-19 9:36 UTC (permalink / raw)
To: Liam.Howlett; +Cc: linux-mm
Hello Liam R. Howlett,
The patch 058c2f0f755f: "Maple Tree: add new data structure" from Jul
17, 2022, leads to the following Smatch static checker warning:
lib/maple_tree.c:6969 mas_validate_limits()
warn: add some parenthesis here? '!piv & (i != 0)'
lib/maple_tree.c
6952 static void mas_validate_limits(struct ma_state *mas)
6953 {
6954 int i;
6955 unsigned long prev_piv = 0;
6956 enum maple_type type = mte_node_type(mas->node);
6957 void __rcu **slots = ma_slots(mte_to_node(mas->node), type);
6958 unsigned long *pivots = ma_pivots(mas_mn(mas), type);
6959
6960 /* all limits are fine here. */
6961 if (mte_is_root(mas->node))
6962 return;
6963
6964 for (i = 0; i < mt_slots[type]; i++) {
6965 unsigned long piv;
6966
6967 piv = mas_safe_pivot(mas, pivots, i, type);
6968
--> 6969 if (!piv & (i != 0))
Sparse and Smatch both hate this. I think this code is correct and it
should be:
if ((!piv) & (i != 0))
However the vast majority of the time this warning indicates a
precendence bug where what was intended was:
if (!(piv & (i != 0)))
I guess it's not common to use booleans for bitwise ANDs...
6970 break;
6971
6972 if (!mte_is_leaf(mas->node)) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] Maple Tree: add new data structure
2022-07-19 9:36 [bug report] Maple Tree: add new data structure Dan Carpenter
@ 2022-07-19 13:37 ` Liam Howlett
0 siblings, 0 replies; 2+ messages in thread
From: Liam Howlett @ 2022-07-19 13:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mm
* Dan Carpenter <dan.carpenter@oracle.com> [220719 05:36]:
> Hello Liam R. Howlett,
>
> The patch 058c2f0f755f: "Maple Tree: add new data structure" from Jul
> 17, 2022, leads to the following Smatch static checker warning:
>
> lib/maple_tree.c:6969 mas_validate_limits()
> warn: add some parenthesis here? '!piv & (i != 0)'
>
> lib/maple_tree.c
> 6952 static void mas_validate_limits(struct ma_state *mas)
> 6953 {
> 6954 int i;
> 6955 unsigned long prev_piv = 0;
> 6956 enum maple_type type = mte_node_type(mas->node);
> 6957 void __rcu **slots = ma_slots(mte_to_node(mas->node), type);
> 6958 unsigned long *pivots = ma_pivots(mas_mn(mas), type);
> 6959
> 6960 /* all limits are fine here. */
> 6961 if (mte_is_root(mas->node))
> 6962 return;
> 6963
> 6964 for (i = 0; i < mt_slots[type]; i++) {
> 6965 unsigned long piv;
> 6966
> 6967 piv = mas_safe_pivot(mas, pivots, i, type);
> 6968
> --> 6969 if (!piv & (i != 0))
>
> Sparse and Smatch both hate this. I think this code is correct and it
> should be:
>
> if ((!piv) & (i != 0))
>
> However the vast majority of the time this warning indicates a
> precendence bug where what was intended was:
>
> if (!(piv & (i != 0)))
Thanks! I caught this with C=1 (sparse). I've been working through
another issue with Hugh that was more pressing. I'll send out a patch
today. The double && will work here but it shouldn't actually make a
difference.
Regards,
Liam
>
>
> I guess it's not common to use booleans for bitwise ANDs...
>
> 6970 break;
> 6971
> 6972 if (!mte_is_leaf(mas->node)) {
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-19 13:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 9:36 [bug report] Maple Tree: add new data structure Dan Carpenter
2022-07-19 13:37 ` Liam Howlett
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.