On Fri, 2008-07-18 at 19:40 +0200, Johannes Berg wrote:
Can you help me maybe? I don't see the bug.
I'll keep digging but I don't see why
return quota_check(ctx->transaction, ctx->dest_mail != NULL ? ctx->dest_mail : qt->tmp_mail);
should pass NULL in the second argument.
Ok, I think I see the issue, and I think both quota and antispam (because I copied from quota) have it. Consider quota_save_init: quota_save_init(struct mailbox_transaction_context *t, enum mail_flags flags, struct mail_keywords *keywords, time_t received_date, int timezone_offset, const char *from_envelope, struct istream *input, struct mail *dest_mail, struct mail_save_context **ctx_r) ... if (dest_mail == NULL) { /* we always want to know the mail size */ if (qt->tmp_mail == NULL) { qt->tmp_mail = mail_alloc(t, MAIL_FETCH_PHYSICAL_SIZE, NULL); } dest_mail = qt->tmp_mail; } return qbox->module_ctx.super. save_init(t, flags, keywords, received_date, timezone_offset, from_envelope, input, dest_mail, ctx_r); } As you can see, this ends up always passing a non-NULL dest_mail into super.save_init(). Now, antispam has the same code, and let's assume that super.save_init() is quota_save_init, the code above. It will always be passed a dest_mail which is non-NULL, thus qt->tmp_mail will always be NULL. Now, consider quota_save_finish. It does not get an explicit dest_mail, so it takes it from the mail_save_context. There, however, it ends up being NULL because antispam created the mail and not whatever fills the mail_save_context. Hence, _both_ ctx->dest_mail and qt->tmp_mail end up being NULL. I think the problem is caused by the explicit/implicit API difference. The quick fix would probably be to assign dest_mail also to the context in quota_save_init, like this: diff -r ffbe9f9e0376 src/plugins/quota/quota-storage.c --- a/src/plugins/quota/quota-storage.c Fri Jul 18 17:55:02 2008 +0300 +++ b/src/plugins/quota/quota-storage.c Fri Jul 18 19:50:53 2008 +0200 @@ -233,10 +233,14 @@ dest_mail = qt->tmp_mail; } - return qbox->module_ctx.super. + ret = qbox->module_ctx.super. save_init(t, flags, keywords, received_date, timezone_offset, from_envelope, input, dest_mail, ctx_r); + + (*ctx_r)->dest_mail = dest_mail; + + return ret; } static int quota_save_finish(struct mail_save_context *ctx) Except, that doesn't work, the dest_mail in the context is still NULL again although the context is the same, so whichever code sets up the context apparently only sets it up after our plugin returns, and the plugin doesn't have access to it. I think actually fixing it requires changes in the storage API. johannes