Hi Toke, On 2/8/22 18:32, Toke Høiland-Jørgensen wrote:
It seems that these macros (both the original and the new) aren't following the guidance from the Coding Style which tells us under "Things to avoid when using macros" that we should avoid "macros that depend on having a local variable with a magic name". Wouldn't these macros be "better" is they included the hif_dev/priv as arguments rather than being "magic"?Hmm, yeah, that's a good point; looks like the non-HTC ath9k stats macros have already been converted to take the container as a parameter, so taking this opportunity to fix these macros is not a bad idea. While we're at it, let's switch to the do{} while(0) syntax the other macros are using instead of that weird usage of ?:. And there's not really any reason for the duplication between ADD/INC either. So I'm thinking something like: #define __STAT_SAVE(_priv, _member, _n) do { if (_priv) (_priv)->_member += (_n); } while(0) #define TX_STAT_ADD(_priv, _c, _a) __STAT_SAVE(_priv, debug.tx_stats._c, _a) #define TX_STAT_INC(_priv, _c) TX_STAT_ADD(_priv, _c, 1)
Good point, thank you. Will redo these macros in v4 With regards, Pavel Skripkin
