netdev - Re: [PATCH net-next v7 05/10] net: openvswitch: add psample action

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] [day] [month] [year] [list]

Date: Tue, 2 Jul 2024 20:52:52 +0200
From: Ilya Maximets <i.maximets@....org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: i.maximets@....org, Simon Horman <horms@...nel.org>,
 Adrián Moreno <amorenoz@...hat.com>,
 Aaron Conole <aconole@...hat.com>, netdev@...r.kernel.org,
 echaudro@...hat.com, dev@...nvswitch.org,
 Donald Hunter <donald.hunter@...il.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Pravin B Shelar <pshelar@....org>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v7 05/10] net: openvswitch: add psample action

On 7/2/24 20:24, Jakub Kicinski wrote:
> On Tue, 2 Jul 2024 20:16:51 +0200 Ilya Maximets wrote:
>> On 7/2/24 20:06, Jakub Kicinski wrote:
>>> On Tue, 2 Jul 2024 11:53:01 +0200 Ilya Maximets wrote:  
>>>> Or create a simple static function and mark all the arguments as unused,
>>>> which kind of compliant to the coding style, but the least pretty.  
>>>
>>> To be clear - kernel explicitly disables warnings about unused
>>> arguments. Unused arguments are not a concern.  
>>
>> OK.  Good to know.
>>
>> Though I think, the '12) Macros, Enums and RTL' section of the
>> coding style document needs some rephrasing in that case.
> 
> Do you mean something like:
> 
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 7e768c65aa92..0516b7009688 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -828,7 +828,7 @@ Generally, inline functions are preferable to macros resembling functions.
>  		} while (0)
>  
>  Function-like macros with unused parameters should be replaced by static
> -inline functions to avoid the issue of unused variables:
> +inline functions to avoid the issue of unused variables in the caller:
>  
>  .. code-block:: c
>  
> ?

Yes, that makes the logic behind the sentence way more clear.  At least
for me.  Without this change it's hard to tell if the doc is talking about
unused function arguments or unused variables in the caller.

There is another issue though that this particular phrase directly leads
a developer to declaring 'static inline' functions in .c files.  And even
'15) The inline disease' cites this use case as appropriate.  And it is,
with the exception for a macro that is a no-op stub version of some function.
Having an example on how to write a stub version of the function in both
.h and .c files might be a good addition to '21) Conditional Compilation'
section breaking the tie for this particular use case.  This section also
discourages from conditional compilation in .c files, but it doesn't seem
reasonable to export a static function outside for that reason.  I suppose
that section is mostly concerned about use of other modules.

Best regards, Ilya Maximets.