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

Pavel Brezina pbrezina at redhat.com
Wed Jan 21 21:31:24 UTC 2015



----- Original Message -----
> From: "Jakub Hrozek" <jhrozek at redhat.com>
> To: sssd-devel at lists.fedorahosted.org
> Sent: Wednesday, January 21, 2015 10:00:07 PM
> Subject: Re: [SSSD] [PATCH] sbus: use hard coded getters instead of generated
> 
> 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.

The point is that it would be hard and error proof to add new, complex types to the generator (such as a{sas} for extra attributes). I'll try to go with the void pointer.



More information about the sssd-devel mailing list