[PATCHSET] Introduce SDB - a KDC backend abstraction

Jeremy Allison jra at samba.org
Thu Jul 30 16:09:28 UTC 2015


On Thu, Jul 30, 2015 at 10:38:47AM +0200, Andreas Schneider wrote:
> On Wednesday 29 July 2015 14:18:44 Jeremy Allison wrote:
> > On Wed, Jul 29, 2015 at 01:36:43PM +0200, Andreas Schneider wrote:
> > > Hello,
> > > 
> > > attached is a patchset which brings us again a step forward. It introduces
> > > SDB a KDC backend abstraction. It implements a sdb to hdb translation
> > > layer.
> > > 
> > > I've gone through the patches with Alexander and we cleaned up the
> > > interface yesterday. It passes a full 'make test' on my machine.
> > > 
> > > I will plan to push it tomorrow.
> > 
> > Looks great work, except I *hate* :
> > 
> > +#define SDB_MALLOC(sp, dp, e, type, free_func) \
> > +do {                                           \
> > +       if (sp->e != NULL) {                   \
> > +               dp->e = malloc(sizeof(type));  \
> > +               if (dp->e == NULL) {           \
> > +                       free_func(dp);         \
> > +                       return ENOMEM;         \
> > +               }                              \
> > +               *dp->e = *sp->e;               \
> > +       } else {                               \
> > +               dp->e = NULL;                  \
> > +       }                                      \
> > +} while(0);
> > 
> > control-flow-macro :-(. I see why you've done
> > it, but for the number of cases you ended up with
> > you could have used a 'goto end' and do the free_func(dp)
> > there, and do the rest of the code explicitly
> > inline.
> > 
> > I won't NAK for this, but wanted to let you know
> > someone is watching :-) :-).
> 
> I've changed the code and did everything inline. Pushed to autobuild now.

Thanks Andreas, I wasn't going to insist on it but I'm
happy it got removed.

> Thanks for your review!

No problem !



More information about the samba-technical mailing list