netdev - Re: [PATCH net-next v3 3/8] net: add basic C code generators for Netlink

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]

Date:   Thu, 19 Jan 2023 17:53:02 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, robh@...nel.org, stephen@...workplumber.org,
        ecree.xilinx@...il.com, sdf@...gle.com, f.fainelli@...il.com,
        fw@...len.de, linux-doc@...r.kernel.org, razor@...ckwall.org,
        nicolas.dichtel@...nd.com
Subject: Re: [PATCH net-next v3 3/8] net: add basic C code generators for
 Netlink

On Thu, 19 Jan 2023 21:53:12 +0100 Johannes Berg wrote:
> > +    def _attr_policy(self, policy):
> > +        mem = '{ '
> > +        if len(self.checks) == 1 and 'min-len' in self.checks:
> > +            mem += '.len = ' + str(self.checks['min-len'])  
> 
> Why does the len(self.checks) matter?

Trying to throw an exception if someone starts using checks I haven't
gotten to implementing yet.

> > +    def free_needs_iter(self):
> > +        return 'type' not in self.attr or self.attr['type'] == 'nest'
> > +
> > +    def free(self, ri, var, ref):
> > +        if 'type' not in self.attr or self.attr['type'] == 'nest':  
> 
> two more places that could use the .get trick
> 
> but it's really up to you. Just that the line like that seems rather
> long to me :-)

Better question is why attr would not have a type :S
Let me leave it be for now, I'll revisit when exercising this code 
in the future.

> > +            if has_ntf:
> > +                cw.p('// --------------- Common notification parsing --------------- //')  
>
> You said you were using /* */ comments now but this is still there.

Ugh, now I'm worried I lost something in a rebase :S

> > +                print_ntf_parse_prototype(parsed, cw)
> > +            cw.nl()
> > +        else:
> > +            cw.p('// Policies')  
>
> and here too, etc.
> 
> Whew. I think I skipped some bits ;-)

Thanks for looking!! :)

> Doesn't look that bad overall, IMHO. :)

I hope we can avoid over-focusing on the python tools :P
I'm no python expert and the code would use a lot of refactoring.
But there's only so many hours in the day and the alternative
seems that it will bit rot in my tree forever :(