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

[Jakub Kicinski <kuba@kernel.org>]

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 :(