[PATCH] Simplify idmap initialization for xid2sid

[Volker Lendecke]

On Mon, Aug 24, 2015 at 09:26:07AM +0200, Stefan Metzmacher wrote:
> Overall this patchset looks like a great improvement, thanks!
> I've just a few comments:
> 
> - Why the wi_ prefix for wi_scan_global_parametrics()

wi stands for "whitespace ignoring", like from "strwicmp" in
the normal get_parametric_helper. I used that name to
indicate to the caller that there should be no whitespace in
the regex passed.

>   wouldn't lp_scan_global_parametrics() make more sense?

Well, if you like that more, I'm fine.

>   As all other functions in loadparm.c start with lp_.
>   The first line in the commit message should also match the full
> function name.

Ok, true.

> - Shouldn't idmap_init fail if wi_scan_global_parametrics() returns an
> error?

Ok, I can change that.

> 
> - I don't understand how default_idmap_domain and passdb_idmap_domain
>   endup in idmap_domains[]. Looking further in the existing code this
> might be by design.
>   idmap_found_domain_backend() ignores the default
>   domain, but shouldn't it also ignore the passdb_idmap_domain.
>   Can you explain a bit?
> 
> - How do we handle the default domain range in
> idmap_backends_unixid_to_sid()
>   if default_idmap_domain is not part of idmap_domains[]?

To be honest, I don't know. I don't get any of this
pdb_is_responsible_for_everything_else business. I had to
tweak the code a few times to get it through autobuild, in
particular the AD DC was difficult to get right for me.

Possibly this is just beyond the point where we can modify
it. I can make this a private only patch for my customer who
needs the performance improvement and who does not need the
AD DC. We might be better off not touching this code anymore
if we can't explain it.

Thanks for the review though!

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de