From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Tue, 1 Oct 2019 09:15:51 +0000 (+0300)
Subject: lib-storage: Simplify mail_save_context.keywords reference counting
X-Git-Tag: 2.3.8~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4a4a947501061f7d31ad0f0ed745ac8e3b96fabc;p=thirdparty%2Fdovecot%2Fcore.git

lib-storage: Simplify mail_save_context.keywords reference counting

This change also makes it possible for plugins to change the keywords
without leaking memory.
---

diff --git a/src/lib-storage/index/index-storage.c b/src/lib-storage/index/index-storage.c
index 247bb7e7f2..8281341fcd 100644
--- a/src/lib-storage/index/index-storage.c
+++ b/src/lib-storage/index/index-storage.c
@@ -937,6 +937,8 @@ bool index_storage_is_inconsistent(struct mailbox *box)
 void index_save_context_free(struct mail_save_context *ctx)
 {
 	index_mail_save_finish(ctx);
+	if (ctx->data.keywords != NULL)
+		mailbox_keywords_unref(&ctx->data.keywords);
 	i_free_and_null(ctx->data.from_envelope);
 	i_free_and_null(ctx->data.guid);
 	i_free_and_null(ctx->data.pop3_uidl);
diff --git a/src/lib-storage/mail-copy.c b/src/lib-storage/mail-copy.c
index 4c2ad4878e..82c5f9a54a 100644
--- a/src/lib-storage/mail-copy.c
+++ b/src/lib-storage/mail-copy.c
@@ -97,13 +97,6 @@ int mail_storage_copy(struct mail_save_context *ctx, struct mail *mail)
 {
 	i_assert(ctx->copying_or_moving);
 
-	if (ctx->data.keywords != NULL) {
-		/* keywords gets unreferenced twice: first in
-		   mailbox_save_cancel()/_finish() and second time in
-		   mailbox_copy(). */
-		mailbox_keywords_ref(ctx->data.keywords);
-	}
-
 	if (mail_storage_try_copy(&ctx, mail) < 0) {
 		if (ctx != NULL)
 			mailbox_save_cancel(&ctx);
diff --git a/src/lib-storage/mail-storage.c b/src/lib-storage/mail-storage.c
index 920a705f24..712a78b46a 100644
--- a/src/lib-storage/mail-storage.c
+++ b/src/lib-storage/mail-storage.c
@@ -2541,7 +2541,6 @@ int mailbox_save_finish(struct mail_save_context **_ctx)
 	struct mailbox_transaction_context *t = ctx->transaction;
 	/* we need to keep a copy of this because save_finish implementations
 	   will likely zero the data structure during cleanup */
-	struct mail_keywords *keywords = ctx->data.keywords;
 	enum mail_flags pvt_flags = ctx->data.pvt_flags;
 	bool copying_via_save = ctx->copying_via_save;
 	int ret;
@@ -2569,8 +2568,6 @@ int mailbox_save_finish(struct mail_save_context **_ctx)
 		t->save_count++;
 	}
 
-	if (keywords != NULL)
-		mailbox_keywords_unref(&keywords);
 	mailbox_save_context_reset(ctx, TRUE);
 	return ret;
 }
@@ -2578,14 +2575,11 @@ int mailbox_save_finish(struct mail_save_context **_ctx)
 void mailbox_save_cancel(struct mail_save_context **_ctx)
 {
 	struct mail_save_context *ctx = *_ctx;
-	struct mail_keywords *keywords = ctx->data.keywords;
 
 	*_ctx = NULL;
 	T_BEGIN {
 		ctx->transaction->box->v.save_cancel(ctx);
 	} T_END;
-	if (keywords != NULL && !ctx->finishing)
-		mailbox_keywords_unref(&keywords);
 
 	/* the dest_mail is no longer valid. if we're still saving
 	   more mails, the mail sequence may get reused. make sure
@@ -2605,7 +2599,6 @@ static int mailbox_copy_int(struct mail_save_context **_ctx, struct mail *mail)
 {
 	struct mail_save_context *ctx = *_ctx;
 	struct mailbox_transaction_context *t = ctx->transaction;
-	struct mail_keywords *keywords = ctx->data.keywords;
 	enum mail_flags pvt_flags = ctx->data.pvt_flags;
 	struct mail *backend_mail;
 	int ret;
@@ -2639,8 +2632,6 @@ static int mailbox_copy_int(struct mail_save_context **_ctx, struct mail *mail)
 			mailbox_save_add_pvt_flags(t, pvt_flags);
 		t->save_count++;
 	}
-	if (keywords != NULL)
-		mailbox_keywords_unref(&keywords);
 	i_assert(!ctx->unfinished);
 
 	ctx->copy_src_mail = NULL;