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.