netdev - Re: [PATCH net-next 02/12] tools: ynl-gen: factor out free_needs

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: <873623ff-ffd4-4dc9-9282-09b204066979@intel.com>
Date: Thu, 24 Apr 2025 08:42:40 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, <davem@...emloft.net>
CC: <netdev@...r.kernel.org>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<andrew+netdev@...n.ch>, <horms@...nel.org>, <donald.hunter@...il.com>,
	<sdf@...ichev.me>
Subject: Re: [PATCH net-next 02/12] tools: ynl-gen: factor out free_needs_iter
 for a struct



On 4/23/2025 7:11 PM, Jakub Kicinski wrote:
> Instead of walking the entries in the code gen add a method
> for the struct class to return if any of the members need
> an iterator.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  tools/net/ynl/pyynl/ynl_gen_c.py | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index 077aacd5f33a..90f7fe6b623b 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -809,6 +809,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
>              raise Exception("Inheriting different members not supported")
>          self.inherited = [c_lower(x) for x in sorted(self._inherited)]
>  
> +    def free_needs_iter(self):
> +        for _, attr in self.attr_list:
> +            if attr.free_needs_iter():
> +                return True
> +        return False
> +
>  
>  class EnumEntry(SpecEnumEntry):
>      def __init__(self, enum_set, yaml, prev, value_start):
> @@ -2156,11 +2162,9 @@ _C_KW = {
>  
>  
>  def _free_type_members_iter(ri, struct):
> -    for _, attr in struct.member_list():
> -        if attr.free_needs_iter():
> -            ri.cw.p('unsigned int i;')
> -            ri.cw.nl()
> -            break
> +    if struct.free_needs_iter():
> +        ri.cw.p('unsigned int i;')
> +        ri.cw.nl()
>  

I like this, its a bit more clear. It is easy to miss that the break at
a glance and think this logic applies to every member of the list
instead of just "if any member needs this, do it".

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

>  
>  def _free_type_members(ri, var, struct, ref=''):