Hm, should do the false check here and above too, will update with that

style(9) has "Variables may be initialized where declared especially when they are constant for the rest of the scope." and shows as an example

struct foo one, *two;
struct baz *three = bar_get_baz(bar);
double four;
int *five, six;
char *seven, eight, nine, ten, eleven, twelve;

four = my_complicated_function(a1, f1, a4);

I prefer this style for this sort of initialization.

I don't in this case but maybe it is personal taste; for me it's because of the function argument; if you want to add debugging/tracing/KASSERT on the ni you have to possibly move the initialization.
It's probably fine for an lvalue but dereferencing a pointer before you can possibly check it (or even better as happens in some code -- not here -- check it afterwards) is questionable.

Adrian, up to you.

Correct, with bool return type tricks like !! are not necessary.

So here's to the too many layers and flags bits net80211 can be:

Given htcap_update_shortgi(), could the last two lines at least be folded into ni->ni_flags & IEEE80211_NODE_SGI20 or am I wrong?
I am *NOT* asking you to do that, Just for me to make sure I understand correctly.

That ni flag is otherwise confusingly single-use only for the ioctl (hmm and ifconfig and bsnmp use it for reporting -- and I wonder if they show what it says) and I wish we would not have 3+ options to express one thing...

I assume in times of bool a lot could be spelt more clearly, so these helpers here will do good - at least inside the kernel.

Ignoring that ht_recv_action_ht_txchwidth() may have insufficient checking (*) is this not only possible if IEEE80211_IS_CHAN_HT40(ni->ni_chan), so a check is redundant here?
I'd leave it but just saying... for for the action frame handler I supposed.

(*) if we set it to 40 there but are not on a 40 channel (or more funny an AP not [announcing] supporting 40Mhz channels) we are partially doomed for TX?

I keep wondering after all changes if this wants to call ieee80211_ht_check_tx_ht() and then only do the final check?

Yup, part of why i'm trying to pull these out of drivers is they're all doing something mostly the same but subtly not consistently. That's mainly on me, trying to bring /up/ 11n support years ago with a lack of these checks existing.

It and the other questions below are good questions. I found way too many weird corner cases in the past (HT40 STA/AP, but the channel width command/notification? Some APs claiming HT but not announcing they receive any HT rates, so you couldn't ever transmit to them - confusing the snot out of AMRR and the iwn firmware? etc, etc) that I'd really like to get sorted out here.

(And there's more to come too after we sort through this stuff - sequence number assigning, A-MPDU BAR windows and QoS NULL frames. A hilarious combination.)

Yeah that should simplify it quite a bit. Technically all of the tx_ht macros I wrote could just use tx_ht()... :p

Ignoring that ht_recv_action_ht_txchwidth() may have insufficient checking (*) is this not only possible if IEEE80211_IS_CHAN_HT40(ni->ni_chan), so a check is redundant here?
I'd leave it but just saying... for for the action frame handler I supposed.

(*) if we set it to 40 there but are not on a 40 channel (or more funny an AP not [announcing] supporting 40Mhz channels) we are partially doomed for TX?

The drivers CAN choose to TX HT20 rates on a HT40 channel if available. I think they all support that.
I just don't think most do - I know ath(4) does due to hostap support.

At least with these functions we have a chance of cleaning the drivers up and making this work consistently, especially where people want to see hostap / ibss work.