Hi Colin,
> while working through a backlog of older static analysis reports from
> Coverity
So ... yeah. Every time I look at Coverity (not frequently, I must
admit) I see the same thing, and get confused.
> I found an interesting use of the ~ operator that looks
> incorrect to me in function ieee80211_set_bitrate_mask():
>
> for (j = 0; j < IEEE80211_HT_MCS_MASK_LEN; j++) {
> if (~sdata->rc_rateidx_mcs_mask[i][j]) {
> sdata->rc_has_mcs_mask[i] = true;
> break;
> }
> }
>
> for (j = 0; j < NL80211_VHT_NSS_MAX; j++) {
> if (~sdata->rc_rateidx_vht_mcs_mask[i][j]) {
> sdata->rc_has_vht_mcs_mask[i] = true;
> break;
> }
> }
>
> For the ~ operator in both if stanzas, Coverity reports:
>
> Logical vs. bitwise operator (CONSTANT_EXPRESSION_RESULT)
> logical_vs_bitwise:
>
> ~sdata->rc_rateidx_mcs_mask[i][j] is always 1/true regardless of the
> values of its operand. This occurs as the logical operand of if.
> Did you intend to use ! rather than ~?
>
> I've checked the results of this and it does seem that ~ is incorrect
> and always returns true for the if expression. So it probably should be
> !, but I'm not sure if I'm missing something deeper here and wondering
> why this has always worked.
But is it really always true?
I _think_ it was intended to check that it's not 0xffffffff or
something?
https://lore.kernel.org/linux-wireless/516C0C7F.3000204@xxxxxxxxxxx/
But maybe that isn't actually quite right due to integer promotion?
OTOH, that's a u8, so it should do the ~ in u8 space, and then compare
to 0 also?
johannes
