[Dovecot] New generic userdb lookup api
wilde at intevation.de
Mon Oct 27 12:04:21 EET 2008
Timo Sirainen <tss at iki.fi> writes:
> 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
>> > 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.
This would be really great and highly appreciated! I just didn't dare
to ask... :-)
>> > 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
> 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.
Than I don't really see the problem with the current code -- I
understand that all the memory it uses (with i_strdup and friends) is
allocated from the default pool, which I assume will be freed
If the goal of an dedicated pool is to free the memory early the code
using the auth-master API will have to allocate and free this pool, I
don't see the advantage here... But then, on a second thought I _do_
see the advantage of a consistent way to do things like this. ;-)
>> 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.
Yes, I know (already made this mistake)... ;-)
The question is, should the ioloop be an extra argument to
>> > (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. :)
Ok, I'll make this change.
Sascha Wilde OpenPGP key: 4BB86568
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
Size: 188 bytes
Desc: not available
Url : http://dovecot.org/pipermail/dovecot/attachments/20081027/ef94aa03/attachment.bin
More information about the dovecot