[Dovecot] New generic userdb lookup api

[Sascha Wilde]

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... 

> 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...

> 3. Would be nice to get rid of the getenv()s :) The MAIL_CHROOT handling
> could be moved to deliver (use it if reply->chroot == NULL). The debug
> could be a parameter to auth_master_init().

You are right, and as I moved/left most of the env stuff in
deliver/auth-client anyway it is only consequent to handle those two the
same -- I'll make this change.

> 4. You're leaking memory.

Um, yes. *blush* -- at least I added the free for the connection shortly
after my announcement...  ;-)

> 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].

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?

> (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...

cheers
sascha
-- 
Sascha Wilde                                          OpenPGP key: 4BB86568
http://www.intevation.de/~wilde/                  http://www.intevation.de/
Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998
Geschäftsführer:   Frank Koormann,  Bernhard Reiter,  Dr. Jan-Oliver Wagner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
Url : http://dovecot.org/pipermail/dovecot/attachments/20081026/0857268a/attachment.bin