netdev - Re: [PATCH v3 2/2] ath9k: htc: clean up *STAT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61f00e3c-06ea-20a6-4b76-2bddf986f074@gmail.com>
Date:   Tue, 8 Feb 2022 18:49:28 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Toke Høiland-Jørgensen <toke@...e.dk>,
        Jeff Johnson <quic_jjohnson@...cinc.com>,
        ath9k-devel@....qualcomm.com, kvalo@...nel.org,
        davem@...emloft.net, kuba@...nel.org, linville@...driver.com
Cc:     linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] ath9k: htc: clean up *STAT_* macros

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