Timo Sirainen <[EMAIL PROTECTED]> 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
pgpIkkYnThP9L.pgp
Description: PGP signature