[SSSD] [PATCH] sbus: use hard coded getters instead of generated

Jakub Hrozek jhrozek at redhat.com
Wed Jan 21 21:00:07 UTC 2015


On Wed, Jan 21, 2015 at 11:35:05AM -0500, Pavel Brezina wrote:
> 
> 
> ----- Original Message -----
> > From: "Jakub Hrozek" <jhrozek at redhat.com>
> > To: sssd-devel at lists.fedorahosted.org
> > Sent: Wednesday, January 21, 2015 3:55:24 PM
> > Subject: Re: [SSSD] [PATCH] sbus: use hard coded getters instead of generated
> > 
> > On Tue, Jan 20, 2015 at 09:54:56PM +0100, Pavel Březina wrote:
> > > On 01/05/2015 02:04 PM, Pavel Březina wrote:
> > > >Hi,
> > > >I wrote most of this patch during the Christmas break.
> > > >
> > > >It removes the codegen code for generating property getters (invokers to
> > > >be precise) and create hard coded ones. It also add a simple and general
> > > >purpose GetAll invoker. The reason and justification is in the commit
> > > >message.
> > > >
> > > >Keeping method generated invokers makes sense, since each method can
> > > >have different number of arguments with different type.
> > > >
> > > >I hope you like it.
> > > 
> > > Rebased on top of latest respin of "[PATCHES] sbus: support multiple
> > > interfaces on single object path".
> > > 
> > 
> > I agree with the general intention, especially considering the removal
> > of the reply as functions in the second patch, but I really dislike
> > removing 800 lines of tests without a proper substitute.
> 
> There is not test remove, it is just generated code that changed.

Aah, sorry, I should have read the patches better.

> 
> > Also, the 30+ lines long macros are impossible to debug, please convert
> > them to functions. Given that callers of the macros usually just call
> > the single line, you can just return the new function.
> 
> The reason it is a macro is to change the type of variable and handler. It is not possible to do it with a function.

Could we get away with a void pointer and pass the dbus_type as void* ?

Then I don't see the point in the patches except the removal of
the reply-as functions. You pretty much just substituted one generator
(the python script) for another (the macro).

The upside is being less code, sure.

The downside being the macros are much harder to debug..I really don't
like huge amounts of code generated by macros. To see the horrors this
can lead to, open the libc code, nsswitch in particular. If you change
the python generator, it's much easier to debug the resulting C code.
OTOH if you change the macro, you need to invoke the preprocessor to see
what's going on and that's...nasty.

Maybe I'm negatively influenced by reading the nsswitch code lately,
but honestly, I'd much rather keep the python codegen.



More information about the sssd-devel mailing list