[Dovecot] New generic userdb lookup api

[Timo Sirainen]

On Sun, 2008-10-26 at 18:33 +0100, Sascha Wilde wrote:
> Timo Sirainen <tss at iki.fi> writes:
> > On Fri, 2008-10-24 at 16:19 +0200, Sascha Wilde wrote:
> >> Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in
> >> my kolab-branch at
> >> http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made
> >> deliver use it in 94b00e377a25.
> >> 
> >> I had no time for thorough testing, but in my test-setup it seems to
> >> work like before, so at least I didn't break it completely...  ;-)
> >
> > A couple of things:
> >
> > 1. It disconnects after each lookup. Not good if multiple lookups are
> > done. Then again it probably shouldn't keep the connection alive forever
> > since the imap connections can run for ages and most of the necessary
> > lookups are probably done close to each others. Maybe timeout after 1
> > minute of idling?
> 
> I agree that this is something that should be optimized, but I was under
> the impression, that the current behavior of deliver was just like that
> -- maybe I'm mistaken, I haven't double-checked that... 

deliver does always only one lookup, so it doesn't matter. But for IMAP
if you have shared mailboxes from multiple users it'll do multiple
lookups.

> > 2. conn->to is for auth request timeout. It should be removed after
> > io_loop_run() so if 1. is fixed it won't leak timeouts.
> > (The same conn->to could actually be used for the two timeouts - one
> > value when looking up, another value when idling.)
> 
> Ack.  Unfortunately I'll have to put a working prototype of the
> "%%h"-feature together before I'll have time to look into that...

Well, I could probably get these missing things done too.

> > Cleanest fix would be to add pool_t pool parameter to
> > auth_master_user_lookup() and allocate memory only from it
> 
> I think a free_auth_user_reply function might be preferable.
> 
> But I have to admit, that I didn't look deeply enough into the memory
> pool management in dovecot to really know whats The Right Thing To
> Do[tm].

The idea behind Dovecot's memory allocations is that you shouldn't have
to go through all the trouble of doing lots of memory frees. Because 1)
it's easy to cause memory leaks then, 2) it requires more code and makes
it uglier, 3) possibly increases memory fragmentation.

So with memory pools you just allocate all the memory from the pool and
finally simply free the pool. That takes care of all these 1-3) issues.
It could use slightly more memory, but especially for these kind of
short living allocations it really doesn't matter.

> Btw, on dedicated vs. default resources, I wasn't quite sure if it was a
> good idea to use the default ioloop.  Any thoughts on that?

For deliver it doesn't matter, but for imap you really should create a
new ioloop or things will probably break.

> > (also p_array_init(&reply->extra_fields) would be cleaner to do inside
> > the lookup code than require it to be done externally).
> 
> Hmm, the idea was to only fill the extra_fields array when it was
> initialized, but maybe it isn't worth the trouble...

See above - it's only a short living lookup and this makes code slightly
cleaner since the allocation is done only in one place. :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : http://dovecot.org/pipermail/dovecot/attachments/20081026/34e0b517/attachment.bin