From: Timo Sirainen Date: Thu, 9 Feb 2017 15:14:57 +0000 (+0200) Subject: lib-storage: Remove unnecessary mail_save_context.dest_mail==NULL checks X-Git-Tag: 2.3.0.rc1~2118 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a825281071af96cc148e49c64ac36d8c5cf26f71;p=thirdparty%2Fdovecot%2Fcore.git lib-storage: Remove unnecessary mail_save_context.dest_mail==NULL checks It can never be NULL after the previous change: "lib-storage: Always create mail_save_context.dest_mail". The code removal in maildir_transaction_save_commit_pre() seemed potentially dangerous, but I don't think such code path is possible anymore. Also even if it is, it's probably fine since the mail_free() is called even earlier than before (although that itself might have been a problem). This also removes last traces of code that made it possible to save mails to mbox without assigning UID to the mail. The previous commit already caused this, so this is just removing dead code. --- diff --git a/src/lib-storage/index/cydir/cydir-save.c b/src/lib-storage/index/cydir/cydir-save.c index b939b1c9be..cf20b27140 100644 --- a/src/lib-storage/index/cydir/cydir-save.c +++ b/src/lib-storage/index/cydir/cydir-save.c @@ -28,7 +28,6 @@ struct cydir_save_context { /* updated for each appended mail: */ uint32_t seq; struct istream *input; - struct mail *mail; int fd; bool failed:1; @@ -116,11 +115,6 @@ int cydir_save_begin(struct mail_save_context *_ctx, struct istream *input) _ctx->data.min_modseq); } - if (_ctx->dest_mail == NULL) { - if (ctx->mail == NULL) - ctx->mail = mail_alloc(trans, 0, NULL); - _ctx->dest_mail = ctx->mail; - } mail_set_seq_saving(_ctx->dest_mail, ctx->seq); crlf_input = i_stream_create_crlf(input); @@ -280,9 +274,6 @@ int cydir_transaction_save_commit_pre(struct mail_save_context *_ctx) return -1; } } - - if (ctx->mail != NULL) - mail_free(&ctx->mail); return 0; } @@ -310,8 +301,6 @@ void cydir_transaction_save_rollback(struct mail_save_context *_ctx) if (ctx->sync_ctx != NULL) (void)cydir_sync_finish(&ctx->sync_ctx, FALSE); - if (ctx->mail != NULL) - mail_free(&ctx->mail); i_free(ctx->tmp_basename); i_free(ctx); } diff --git a/src/lib-storage/index/dbox-common/dbox-save.c b/src/lib-storage/index/dbox-common/dbox-save.c index abceb36376..548adb824f 100644 --- a/src/lib-storage/index/dbox-common/dbox-save.c +++ b/src/lib-storage/index/dbox-common/dbox-save.c @@ -45,11 +45,6 @@ void dbox_save_begin(struct dbox_save_context *ctx, struct istream *input) dbox_save_add_to_index(ctx); - if (_ctx->dest_mail == NULL) { - if (ctx->mail == NULL) - ctx->mail = mail_alloc(_ctx->transaction, 0, NULL); - _ctx->dest_mail = ctx->mail; - } mail_set_seq_saving(_ctx->dest_mail, ctx->seq); crlf_input = i_stream_create_lf(input); diff --git a/src/lib-storage/index/dbox-common/dbox-save.h b/src/lib-storage/index/dbox-common/dbox-save.h index 4f5e3a5e93..35818a0024 100644 --- a/src/lib-storage/index/dbox-common/dbox-save.h +++ b/src/lib-storage/index/dbox-common/dbox-save.h @@ -10,7 +10,6 @@ struct dbox_save_context { /* updated for each appended mail: */ uint32_t seq; struct istream *input; - struct mail *mail; struct ostream *dbox_output; diff --git a/src/lib-storage/index/dbox-multi/mdbox-save.c b/src/lib-storage/index/dbox-multi/mdbox-save.c index caa0ff6da9..db50757cdd 100644 --- a/src/lib-storage/index/dbox-multi/mdbox-save.c +++ b/src/lib-storage/index/dbox-multi/mdbox-save.c @@ -361,9 +361,6 @@ int mdbox_transaction_save_commit_pre(struct mail_save_context *_ctx) mail_index_sync_set_reason(ctx->sync_ctx->index_sync_ctx, "saving"); } - if (ctx->ctx.mail != NULL) - mail_free(&ctx->ctx.mail); - _t->changes->uid_validity = hdr->uid_validity; return 0; } @@ -424,8 +421,6 @@ void mdbox_transaction_save_rollback(struct mail_save_context *_ctx) if (ctx->sync_ctx != NULL) (void)mdbox_sync_finish(&ctx->sync_ctx, FALSE); - if (ctx->ctx.mail != NULL) - mail_free(&ctx->ctx.mail); array_free(&ctx->mails); i_free(ctx); } @@ -485,8 +480,7 @@ int mdbox_copy(struct mail_save_context *_ctx, struct mail *mail) save_mail = array_append_space(&ctx->mails); save_mail->seq = ctx->ctx.seq; - if (_ctx->dest_mail != NULL) - mail_set_seq_saving(_ctx->dest_mail, ctx->ctx.seq); + mail_set_seq_saving(_ctx->dest_mail, ctx->ctx.seq); index_save_context_free(_ctx); return 0; } diff --git a/src/lib-storage/index/dbox-single/sdbox-copy.c b/src/lib-storage/index/dbox-single/sdbox-copy.c index ea76b30d25..e03a0a868d 100644 --- a/src/lib-storage/index/dbox-single/sdbox-copy.c +++ b/src/lib-storage/index/dbox-single/sdbox-copy.c @@ -155,8 +155,7 @@ sdbox_copy_hardlink(struct mail_save_context *_ctx, struct mail *mail) index_copy_cache_fields(_ctx, mail, ctx->seq); sdbox_save_add_file(_ctx, dest_file); - if (_ctx->dest_mail != NULL) - mail_set_seq_saving(_ctx->dest_mail, ctx->seq); + mail_set_seq_saving(_ctx->dest_mail, ctx->seq); dbox_file_unref(&src_file); return 1; } diff --git a/src/lib-storage/index/dbox-single/sdbox-save.c b/src/lib-storage/index/dbox-single/sdbox-save.c index 6f46161ece..95ff5c256c 100644 --- a/src/lib-storage/index/dbox-single/sdbox-save.c +++ b/src/lib-storage/index/dbox-single/sdbox-save.c @@ -307,8 +307,6 @@ int sdbox_transaction_save_commit_pre(struct mail_save_context *_ctx) if (array_count(&ctx->files) == 0) { /* the mail must be freed in the commit_pre() */ - if (ctx->ctx.mail != NULL) - mail_free(&ctx->ctx.mail); return 0; } @@ -340,9 +338,6 @@ int sdbox_transaction_save_commit_pre(struct mail_save_context *_ctx) } } - if (ctx->ctx.mail != NULL) - mail_free(&ctx->ctx.mail); - _t->changes->uid_validity = hdr->uid_validity; return 0; } @@ -387,8 +382,5 @@ void sdbox_transaction_save_rollback(struct mail_save_context *_ctx) if (ctx->sync_ctx != NULL) (void)sdbox_sync_finish(&ctx->sync_ctx, FALSE); - - if (ctx->ctx.mail != NULL) - mail_free(&ctx->ctx.mail); i_free(ctx); } diff --git a/src/lib-storage/index/imapc/imapc-save.c b/src/lib-storage/index/imapc/imapc-save.c index 787748471f..d6c929222f 100644 --- a/src/lib-storage/index/imapc/imapc-save.c +++ b/src/lib-storage/index/imapc/imapc-save.c @@ -135,9 +135,6 @@ imapc_save_add_to_index(struct imapc_save_context *ctx, uint32_t uid) struct index_mail *imail = (struct index_mail *)_mail; uint32_t seq; - if (_mail == NULL) - return; - /* we'll temporarily append messages and at commit time expunge them all, since we can't guarantee that no one else has saved messages to remote server during our transaction */ diff --git a/src/lib-storage/index/index-mail.c b/src/lib-storage/index/index-mail.c index ade17d9ccf..4e3395e376 100644 --- a/src/lib-storage/index/index-mail.c +++ b/src/lib-storage/index/index-mail.c @@ -2305,9 +2305,6 @@ void index_mail_save_finish(struct mail_save_context *ctx) { struct index_mail *imail = (struct index_mail *)ctx->dest_mail; - if (imail == NULL) - return; - if (ctx->data.from_envelope != NULL && imail->data.from_envelope == NULL) { imail->data.from_envelope = diff --git a/src/lib-storage/index/index-storage.c b/src/lib-storage/index/index-storage.c index 4a4459b64e..4bb3f6f4cf 100644 --- a/src/lib-storage/index/index-storage.c +++ b/src/lib-storage/index/index-storage.c @@ -879,9 +879,8 @@ mail_copy_cache_field(struct mail_save_context *ctx, struct mail *src_mail, if (mail_cache_lookup_field(src_mail->transaction->cache_view, buf, src_mail->seq, src_field_idx) <= 0) buffer_set_used_size(buf, 0); - else if (ctx->dest_mail != NULL && - (strcmp(name, "size.physical") == 0 || - strcmp(name, "size.virtual") == 0)) { + else if (strcmp(name, "size.physical") == 0 || + strcmp(name, "size.virtual") == 0) { /* FIXME: until mail_cache_lookup() can read unwritten cached data from buffer, we'll do this optimization to make quota plugin's work faster */ diff --git a/src/lib-storage/index/maildir/maildir-save.c b/src/lib-storage/index/maildir/maildir-save.c index 51cbdbee8e..9f88cd87d6 100644 --- a/src/lib-storage/index/maildir/maildir-save.c +++ b/src/lib-storage/index/maildir/maildir-save.c @@ -49,7 +49,7 @@ struct maildir_save_context { struct maildir_uidlist_sync_ctx *uidlist_sync_ctx; struct maildir_keywords_sync_ctx *keywords_sync_ctx; struct maildir_index_sync_context *sync_ctx; - struct mail *mail, *cur_dest_mail; + struct mail *cur_dest_mail; const char *tmpdir, *newdir, *curdir; struct maildir_filename *files, **files_tail, *file_last; @@ -209,11 +209,6 @@ maildir_save_add(struct mail_save_context *_ctx, const char *tmp_fname, i_assert(ctx->files->next == NULL); } - if (_ctx->dest_mail == NULL) { - if (ctx->mail == NULL) - ctx->mail = mail_alloc(_ctx->transaction, 0, NULL); - _ctx->dest_mail = ctx->mail; - } mail_set_seq_saving(_ctx->dest_mail, ctx->seq); if (ctx->input == NULL) { @@ -970,12 +965,8 @@ int maildir_transaction_save_commit_pre(struct mail_save_context *_ctx) i_assert(_ctx->data.output == NULL); i_assert(ctx->last_save_finished); - if (ctx->files_count == 0) { - /* the mail must be freed in the commit_pre() */ - if (ctx->mail != NULL) - mail_free(&ctx->mail); + if (ctx->files_count == 0) return 0; - } sync_flags = MAILDIR_UIDLIST_SYNC_PARTIAL | MAILDIR_UIDLIST_SYNC_NOREFRESH; @@ -1030,13 +1021,6 @@ int maildir_transaction_save_commit_pre(struct mail_save_context *_ctx) _t->changes->uid_validity = maildir_uidlist_get_uid_validity(ctx->mbox->uidlist); - if (ctx->mail != NULL) { - /* Mail freeing may trigger cache updates and a call to - maildir_save_file_get_path(). Do this before finishing index - sync so we still have keywords_sync_ctx. */ - mail_free(&ctx->mail); - } - if (ctx->locked) { /* It doesn't matter if index syncing fails */ ctx->keywords_sync_ctx = NULL; @@ -1098,7 +1082,5 @@ void maildir_transaction_save_rollback(struct mail_save_context *_ctx) if (ctx->locked) maildir_uidlist_unlock(ctx->mbox->uidlist); - if (ctx->mail != NULL) - mail_free(&ctx->mail); pool_unref(&ctx->pool); } diff --git a/src/lib-storage/index/mbox/mbox-save.c b/src/lib-storage/index/mbox/mbox-save.c index 1db41ecbce..f364658734 100644 --- a/src/lib-storage/index/mbox/mbox-save.c +++ b/src/lib-storage/index/mbox/mbox-save.c @@ -36,7 +36,6 @@ struct mbox_save_context { struct mbox_mailbox *mbox; struct mail_index_transaction *trans; - struct mail *mail; uoff_t append_offset, mail_offset; time_t orig_atime; @@ -269,12 +268,11 @@ mbox_save_append_keyword_headers(struct mbox_save_context *ctx, static int mbox_save_init_file(struct mbox_save_context *ctx, - struct mbox_transaction_context *t, bool want_mail) + struct mbox_transaction_context *t) { struct mailbox_transaction_context *_t = &t->t; struct mbox_mailbox *mbox = ctx->mbox; struct mail_storage *storage = &mbox->storage->storage; - bool empty = FALSE; int ret; if (mbox_is_backend_readonly(ctx->mbox)) { @@ -283,10 +281,6 @@ mbox_save_init_file(struct mbox_save_context *ctx, return -1; } - if ((_t->flags & MAILBOX_TRANSACTION_FLAG_ASSIGN_UIDS) != 0 || - ctx->ctx.data.uid != 0) - want_mail = TRUE; - if (ctx->append_offset == (uoff_t)-1) { /* first appended mail in this transaction */ if (t->write_lock_id == 0) { @@ -300,18 +294,12 @@ mbox_save_init_file(struct mbox_save_context *ctx, } /* update mbox_sync_dirty state */ - ret = mbox_sync_has_changed_full(mbox, TRUE, &empty); + ret = mbox_sync_has_changed(mbox, TRUE); if (ret < 0) return -1; - if (!want_mail && ret == 0) { - /* we're not required to assign UIDs for the appended - mails immediately. do it only if it doesn't require - syncing. */ - mbox_save_init_sync(_t); - } } - if (!ctx->synced && (want_mail || empty)) { + if (!ctx->synced) { /* we'll need to assign UID for the mail immediately. */ if (mbox_sync(mbox, 0) < 0) return -1; @@ -416,13 +404,10 @@ mbox_save_get_input_stream(struct mbox_save_context *ctx, struct istream *input) i_stream_create_crlf(filter) : i_stream_create_lf(filter); i_stream_unref(&filter); - if (ctx->ctx.dest_mail != NULL) { - /* caching creates a tee stream */ - cache_input = - index_mail_cache_parse_init(ctx->ctx.dest_mail, ret); - i_stream_unref(&ret); - ret = cache_input; - } + /* caching creates a tee stream */ + cache_input = index_mail_cache_parse_init(ctx->ctx.dest_mail, ret); + i_stream_unref(&ret); + ret = cache_input; return ret; } @@ -463,7 +448,7 @@ int mbox_save_begin(struct mail_save_context *_ctx, struct istream *input) ctx->failed = FALSE; ctx->seq = 0; - if (mbox_save_init_file(ctx, t, _ctx->dest_mail != NULL) < 0) { + if (mbox_save_init_file(ctx, t) < 0) { ctx->failed = TRUE; return -1; } @@ -506,13 +491,6 @@ int mbox_save_begin(struct mail_save_context *_ctx, struct istream *input) ctx->next_uid++; /* parse and cache the mail headers as we read it */ - if (_ctx->dest_mail == NULL) { - if (ctx->mail == NULL) { - ctx->mail = mail_alloc(_ctx->transaction, - 0, NULL); - } - _ctx->dest_mail = ctx->mail; - } mail_set_seq_saving(_ctx->dest_mail, ctx->seq); } mbox_save_append_flag_headers(ctx->headers, save_flags); @@ -556,11 +534,9 @@ static int mbox_save_body(struct mbox_save_context *ctx) while ((ret = i_stream_read(ctx->input)) != -1) { if (mbox_save_body_input(ctx) < 0) return -1; - if (ctx->ctx.dest_mail != NULL) { - /* i_stream_read() may have returned 0 at EOF - because of this parser */ - index_mail_cache_parse_continue(ctx->ctx.dest_mail); - } + /* i_stream_read() may have returned 0 at EOF + because of this parser */ + index_mail_cache_parse_continue(ctx->ctx.dest_mail); if (ret == 0) return 0; } @@ -628,8 +604,7 @@ int mbox_save_continue(struct mail_save_context *_ctx) i_assert(size > 0); ctx->last_char = data[size-1]; i_stream_skip(ctx->input, size); - if (ctx->ctx.dest_mail != NULL) - index_mail_cache_parse_continue(ctx->ctx.dest_mail); + index_mail_cache_parse_continue(ctx->ctx.dest_mail); } if (ret == 0) return 0; @@ -697,11 +672,9 @@ int mbox_save_finish(struct mail_save_context *_ctx) } T_END; } - if (ctx->ctx.dest_mail != NULL) { - index_mail_cache_parse_deinit(ctx->ctx.dest_mail, - ctx->ctx.data.received_date, - !ctx->failed); - } + index_mail_cache_parse_deinit(ctx->ctx.dest_mail, + ctx->ctx.data.received_date, + !ctx->failed); if (ctx->input != NULL) i_stream_destroy(&ctx->input); @@ -737,8 +710,6 @@ static void mbox_transaction_save_deinit(struct mbox_save_context *ctx) { if (ctx->output != NULL) o_stream_destroy(&ctx->output); - if (ctx->mail != NULL) - mail_free(&ctx->mail); str_free(&ctx->headers); } diff --git a/src/lib-storage/index/mbox/mbox-sync-private.h b/src/lib-storage/index/mbox/mbox-sync-private.h index e25f26975d..60fb952466 100644 --- a/src/lib-storage/index/mbox/mbox-sync-private.h +++ b/src/lib-storage/index/mbox/mbox-sync-private.h @@ -153,8 +153,6 @@ struct mbox_sync_context { int mbox_sync_header_refresh(struct mbox_mailbox *mbox); int mbox_sync(struct mbox_mailbox *mbox, enum mbox_sync_flags flags); int mbox_sync_has_changed(struct mbox_mailbox *mbox, bool leave_dirty); -int mbox_sync_has_changed_full(struct mbox_mailbox *mbox, bool leave_dirty, - bool *empty_r); void mbox_sync_set_critical(struct mbox_sync_context *sync_ctx, const char *fmt, ...) ATTR_FORMAT(2, 3); diff --git a/src/lib-storage/index/mbox/mbox-sync.c b/src/lib-storage/index/mbox/mbox-sync.c index c794d9fe31..13c233ff62 100644 --- a/src/lib-storage/index/mbox/mbox-sync.c +++ b/src/lib-storage/index/mbox/mbox-sync.c @@ -1732,14 +1732,6 @@ int mbox_sync_get_guid(struct mbox_mailbox *mbox) } int mbox_sync_has_changed(struct mbox_mailbox *mbox, bool leave_dirty) -{ - bool empty; - - return mbox_sync_has_changed_full(mbox, leave_dirty, &empty); -} - -int mbox_sync_has_changed_full(struct mbox_mailbox *mbox, bool leave_dirty, - bool *empty_r) { const struct stat *st; struct stat statbuf; @@ -1765,7 +1757,6 @@ int mbox_sync_has_changed_full(struct mbox_mailbox *mbox, bool leave_dirty, } st = &statbuf; } - *empty_r = st->st_size == 0; if (mbox_sync_header_refresh(mbox) < 0) return -1;