On Mon, 2011-09-26 at 15:40 +0200, Jan Zelený wrote:
> > On Mon, 2011-09-26 at 15:10 +0200, Jan Zelený wrote:
> > > > Patch 0004: Add parser for multiple search bases
> > > > As discussed on the list, this will the ldap_*_search_base options
> > > > in the form of:
> > > > search_base[?scope?[filter][?search_base?scope?[filter]]*]
> > > > This is backwards-compatible (just use a search base)
> > >
> > > Why haven't you used the for cycle to parse search bases? Personally
> > > I'd prefer it that way, but I guess it is nothing wrong to do it
> > > this way either. Other than that the patch is fine.
> >
> > I don't understand the question. Could you explain?
>
> Sorry, I didn't notice. This was only my note without any explanation.
> What I thought is that on ldap_common.c:253, there is a block of code
> handling search bases in a cycle. I just think that we should either use
> the cycle to do both setting and parsing or neither. But as I wrote
> before, this is just cosmetic issue.
I didn't do it in the loop because it would have required an annoying
switch statement to handle which of the opts->*_search_bases values to
assign to. I figured it read better just having them in order
afterwards.
I'm not a huge fan of that loop to begin with, but I'm not sure there's
any value to dismantling it either.
Ok, as I stated before, I have no problem with this, just a cosmetic thing.
I'm recompiling SSSD to run some tests, but consider all patches to be ACKed.
Jan