[dovecot-cvs] dovecot/src/lib-storage/index/mbox mbox-sync-parse.c, 1.37, 1.38 mbox-sync-private.h, 1.47, 1.48 mbox-sync-rewrite.c, 1.44, 1.45 mbox-sync-update.c, 1.31, 1.32 mbox-sync.c, 1.144, 1.145

cras at dovecot.org cras at dovecot.org
Fri Apr 8 01:27:29 EEST 2005


Update of /var/lib/cvs/dovecot/src/lib-storage/index/mbox
In directory talvi:/tmp/cvs-serv654

Modified Files:
	mbox-sync-parse.c mbox-sync-private.h mbox-sync-rewrite.c 
	mbox-sync-update.c mbox-sync.c 
Log Message:
Large mbox code cleanups, especially related to X-IMAP/X-IMAPbase header
handling. uid-next field is updated now every time while syncing.

If partial syncing failed, we weren't re-requesting all sync records from
index, which could have caused some changes to be lost, and possibly caused
some index corruption errors later on.

Several other more or less possible problems fixed.



Index: mbox-sync-parse.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/mbox/mbox-sync-parse.c,v
retrieving revision 1.37
retrieving revision 1.38
diff -u -d -r1.37 -r1.38
--- mbox-sync-parse.c	2 Apr 2005 21:09:07 -0000	1.37
+++ mbox-sync-parse.c	7 Apr 2005 22:27:26 -0000	1.38
@@ -4,6 +4,7 @@
    Solar Designer */
 
 #include "lib.h"
+#include "ioloop.h"
 #include "buffer.h"
 #include "istream.h"
 #include "str.h"
@@ -142,7 +143,7 @@
 
 	if (count != array_count(ctx->sync_ctx->ibox->keyword_names)) {
 		/* need to update this list */
-		ctx->update_imapbase_keywords = TRUE;
+		ctx->imapbase_rewrite = TRUE;
 		ctx->need_rewrite = TRUE;
 	}
 }
@@ -150,9 +151,7 @@
 static int parse_x_imap_base(struct mbox_sync_mail_context *ctx,
 			     struct message_header_line *hdr)
 {
-	const char *str;
-	char *end;
-	size_t pos;
+	size_t i, j, uid_last_pos;
 	uint32_t uid_validity, uid_last;
 
 	if (ctx->seq != 1 || ctx->seen_imapbase) {
@@ -160,42 +159,65 @@
 		return FALSE;
 	}
 
-	/* <uid validity> <last uid> */
-	t_push();
-	str = t_strndup(hdr->full_value, hdr->full_value_len);
-	uid_validity = strtoul(str, &end, 10);
-	uid_last = strtoul(end, &end, 10);
-	pos = end - str;
-	t_pop();
+	/* <uid-validity> 10x<uid-last> */
+	for (i = 0, uid_validity = 0; i < hdr->full_value_len; i++) {
+		if (hdr->full_value[i] < '0' || hdr->full_value[i] > '9') {
+			if (hdr->full_value[i] != ' ')
+				return FALSE;
+			break;
+		}
+		uid_validity = uid_validity * 10 + (hdr->full_value[i] - '0');
+	}
 
 	if (uid_validity == 0) {
 		/* broken */
 		return FALSE;
 	}
 
-	if (ctx->sync_ctx->base_uid_validity == 0) {
-		/* first time parsing this. save the values. */
-		ctx->sync_ctx->base_uid_validity = uid_validity;
-		ctx->sync_ctx->base_uid_last = uid_last;
+	for (; i < hdr->full_value_len; i++) {
+		if (!IS_LWSP_LF(hdr->full_value[i]))
+			break;
+	}
+	uid_last_pos = i;
 
-		if (ctx->sync_ctx->next_uid-1 <= uid_last)
-			ctx->sync_ctx->next_uid = uid_last+1;
-		else {
-			ctx->sync_ctx->update_base_uid_last =
-				ctx->sync_ctx->next_uid - 1;
-			ctx->need_rewrite = TRUE;
+	for (uid_last = 0, j = 0; i < hdr->full_value_len; i++, j++) {
+		if (hdr->full_value[i] < '0' || hdr->full_value[i] > '9') {
+			if (!IS_LWSP_LF(hdr->full_value[i]))
+				return FALSE;
+			break;
 		}
+		uid_last = uid_last * 10 + (hdr->full_value[i] - '0');
 	}
 
-	if (ctx->sync_ctx->next_uid <= ctx->sync_ctx->prev_msg_uid) {
-		/* broken, update */
-                ctx->sync_ctx->next_uid = ctx->sync_ctx->prev_msg_uid+1;
+	if (j != 10) {
+		/* uid-last field must be exactly 10 characters to make
+		   rewriting it easier. */
+		ctx->imapbase_rewrite = TRUE;
+		ctx->need_rewrite = TRUE;
+	} else {
+		ctx->last_uid_value_start_pos = uid_last_pos;
+		ctx->sync_ctx->base_uid_last_offset =
+			hdr->full_value_offset + uid_last_pos;
+	}
+
+	if (ctx->sync_ctx->next_uid-1 <= uid_last) {
+		/* new messages have been added since our last sync.
+		   just update our internal next_uid. */
+		ctx->sync_ctx->next_uid = uid_last+1;
+	}
+	i_assert(ctx->sync_ctx->next_uid > ctx->sync_ctx->prev_msg_uid);
+
+	if (ctx->sync_ctx->base_uid_validity == 0) {
+		/* first time parsing this (ie. we're not rewriting).
+		   save the values. */
+		ctx->sync_ctx->base_uid_validity = uid_validity;
+		ctx->sync_ctx->base_uid_last = uid_last;
 	}
 
 	ctx->hdr_pos[MBOX_HDR_X_IMAPBASE] = str_len(ctx->header);
 	ctx->seen_imapbase = TRUE;
 
-	parse_imap_keywords_list(ctx, hdr, pos);
+	parse_imap_keywords_list(ctx, hdr, i);
 	parse_trailing_whitespace(ctx, hdr);
 	return TRUE;
 }
@@ -247,21 +269,8 @@
 			     pos - keyword_start);
 		if (!mail_index_keyword_lookup(ctx->sync_ctx->ibox->index,
 					       str_c(keyword), FALSE, &idx)) {
-			if (ctx->sync_ctx->ibox->mbox_sync_dirty &&
-			    !ctx->sync_ctx->dest_first_mail &&
-			    !ctx->sync_ctx->seen_first_mail) {
-				/* we'll have to read the X-IMAP header to
-				   make sure we have the latest list of
-				   keywords */
-				i_assert(!ctx->sync_ctx->sync_restart);
-				ctx->sync_ctx->sync_restart = TRUE;
-				t_pop();
-				return FALSE;
-			}
-
-			/* index is fully up-to-date and the keyword wasn't
-			   found. that means the sent mail originally
-			   contained X-Keywords header. Delete it. */
+			/* keyword wasn't found. that means the sent mail
+			   originally contained X-Keywords header. Delete it. */
 			t_pop();
 			return FALSE;
 		}
@@ -309,50 +318,43 @@
 		}
 	}
 
-	if (ctx->sync_ctx != NULL) {
-		if (value >= ctx->sync_ctx->next_uid) {
-			/* UID is larger than expected. */
-			if (ctx->sync_ctx->ibox->mbox_sync_dirty &&
-			    !ctx->sync_ctx->dest_first_mail &&
-			    !ctx->sync_ctx->seen_first_mail) {
-				/* current next-uid isn't necessarily known
-				   if changes were made without updating index
-				   file. restart the sync. */
-				i_assert(!ctx->sync_ctx->sync_restart);
-				ctx->sync_ctx->sync_restart = TRUE;
-				return FALSE;
-			}
-
-			/* Don't allow it because incoming mails can contain
-			   untrusted X-UID fields, causing possibly DoS if
-			   the UIDs get large enough. */
-			ctx->uid_broken = TRUE;
-			return FALSE;
-		}
-
-		if (value <= ctx->sync_ctx->prev_msg_uid) {
-			/* broken - UIDs must be growing */
-			ctx->uid_broken = TRUE;
-			return FALSE;
-		}
-		ctx->sync_ctx->prev_msg_uid = value;
-	}
-
-	ctx->mail.uid = value;
-
 	if (ctx->sync_ctx == NULL) {
-		/* we're in mbox_sync_parse_match_mail() */
+		/* we're in mbox_sync_parse_match_mail().
+		   don't do any extra checks. */
+		ctx->mail.uid = value;
 		return TRUE;
 	}
 
 	if (ctx->sync_ctx->dest_first_mail && !ctx->seen_imapbase) {
-		/* everything was good, except we can't have X-UID before
-		   X-IMAPbase header (to keep c-client compatibility). keep
-		   the UID, but when we're rewriting this makes sure the
-		   X-UID is appended after X-IMAPbase. */
+		/* Don't bother allowing X-UID before X-IMAPbase
+		   header. c-client doesn't allow it either, and this
+		   way the UID doesn't have to be reset if X-IMAPbase
+		   header isn't what we expect it to be. */
+		return FALSE;
+	}
+
+	if (value == ctx->sync_ctx->next_uid) {
+		/* X-UID is the next expected one. allow it because
+		   we'd just use this UID anyway. X-IMAPbase header
+		   still needs to be updated for this. */
+		ctx->sync_ctx->next_uid++;
+	} else if (value > ctx->sync_ctx->next_uid) {
+		/* UID is larger than expected. Don't allow it because
+		   incoming mails can contain untrusted X-UID fields,
+		   causing possibly DoS if the UIDs get large enough. */
+		ctx->uid_broken = TRUE;
+		return FALSE;
+	}
+
+	if (value <= ctx->sync_ctx->prev_msg_uid) {
+		/* broken - UIDs must be growing */
+		ctx->uid_broken = TRUE;
 		return FALSE;
 	}
 
+	ctx->sync_ctx->prev_msg_uid = value;
+	ctx->mail.uid = value;
+
 	ctx->hdr_pos[MBOX_HDR_X_UID] = str_len(ctx->header);
 	ctx->parsed_uid = value;
 	parse_trailing_whitespace(ctx, hdr);
@@ -483,15 +485,17 @@
 
 	mbox_md5_finish(mbox_md5_ctx, ctx->hdr_md5_sum);
 
-	if ((ctx->seq == 1 && sync_ctx->base_uid_validity == 0) ||
+	if ((ctx->seq == 1 && !ctx->seen_imapbase) ||
 	    (ctx->seq > 1 && sync_ctx->dest_first_mail)) {
 		/* missing X-IMAPbase */
 		ctx->need_rewrite = TRUE;
-	}
-	if (ctx->seq == 1 && sync_ctx->update_base_uid_last != 0 &&
-	    sync_ctx->update_base_uid_last > sync_ctx->base_uid_last) {
-		/* update uid-last field in X-IMAPbase */
-		ctx->need_rewrite = TRUE;
+		if (sync_ctx->base_uid_validity == 0) {
+			/* figure out a new UIDVALIDITY for us. */
+			sync_ctx->base_uid_validity =
+				sync_ctx->hdr->uid_validity != 0 ?
+				sync_ctx->hdr->uid_validity :
+				I_MAX((uint32_t)ioloop_time, 1);
+		}
 	}
 
 	ctx->body_offset = input->v_offset;

Index: mbox-sync-private.h
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/mbox/mbox-sync-private.h,v
retrieving revision 1.47
retrieving revision 1.48
diff -u -d -r1.47 -r1.48
--- mbox-sync-private.h	2 Apr 2005 21:09:07 -0000	1.47
+++ mbox-sync-private.h	7 Apr 2005 22:27:26 -0000	1.48
@@ -76,6 +76,7 @@
 
 	size_t hdr_pos[MBOX_HDR_COUNT];
 	uint32_t parsed_uid;
+	unsigned int last_uid_value_start_pos;
 
 	unsigned int have_eoh:1;
 	unsigned int need_rewrite:1;
@@ -85,7 +86,7 @@
 	unsigned int recent:1;
 	unsigned int dirty:1;
 	unsigned int uid_broken:1;
-	unsigned int update_imapbase_keywords:1;
+	unsigned int imapbase_rewrite:1;
 };
 
 struct mbox_sync_context {
@@ -103,7 +104,7 @@
 
 	/* header state: */
 	uint32_t base_uid_validity, base_uid_last;
-	uint32_t update_base_uid_last;
+	uoff_t base_uid_last_offset;
 
 	/* mail state: */
 	array_t ARRAY_DEFINE(mails, struct mbox_sync_mail);
@@ -117,7 +118,6 @@
 	off_t expunged_space, space_diff;
 
 	unsigned int dest_first_mail:1;
-	unsigned int seen_first_mail:1;
 	unsigned int sync_restart:1;
 
 	/* global flags: */

Index: mbox-sync-rewrite.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/mbox/mbox-sync-rewrite.c,v
retrieving revision 1.44
retrieving revision 1.45
diff -u -d -r1.44 -r1.45
--- mbox-sync-rewrite.c	2 Apr 2005 21:09:07 -0000	1.44
+++ mbox-sync-rewrite.c	7 Apr 2005 22:27:26 -0000	1.45
@@ -208,11 +208,24 @@
 	/* FIXME: see if we could remove X-Keywords header completely */
 }
 
+static void mbox_sync_first_mail_written(struct mbox_sync_mail_context *ctx)
+{
+	/* we wrote the first mail. update the base_uid_last so we don't try
+	   to update it later unneededly. also update last-uid offset. */
+	i_assert(ctx->last_uid_value_start_pos != 0);
+
+	ctx->sync_ctx->base_uid_last_offset = ctx->hdr_offset +
+		ctx->hdr_pos[MBOX_HDR_X_IMAPBASE] +
+		ctx->last_uid_value_start_pos;
+	ctx->sync_ctx->base_uid_last = ctx->sync_ctx->next_uid - 1;
+}
+
 int mbox_sync_try_rewrite(struct mbox_sync_mail_context *ctx, off_t move_diff)
 {
+        struct mbox_sync_context *sync_ctx = ctx->sync_ctx;
 	size_t old_hdr_size, new_hdr_size;
 
-	i_assert(ctx->sync_ctx->ibox->mbox_lock_type == F_WRLCK);
+	i_assert(sync_ctx->ibox->mbox_lock_type == F_WRLCK);
 
 	old_hdr_size = ctx->body_offset - ctx->hdr_offset;
 	new_hdr_size = str_len(ctx->header);
@@ -235,10 +248,9 @@
 			/* moving backwards - we can use the extra space from
 			   it, just update expunged_space accordingly */
 			i_assert(ctx->mail.space == 0);
-			i_assert(ctx->sync_ctx->expunged_space >=
+			i_assert(sync_ctx->expunged_space >=
 				 (off_t)(new_hdr_size - old_hdr_size));
-			ctx->sync_ctx->expunged_space -=
-				new_hdr_size - old_hdr_size;
+			sync_ctx->expunged_space -= new_hdr_size - old_hdr_size;
 		} else {
 			/* couldn't get enough space */
 			i_assert(ctx->mail.space == 0);
@@ -266,22 +278,19 @@
 	    ctx->header_last_change != 0)
 		str_truncate(ctx->header, ctx->header_last_change);
 
-	if (pwrite_full(ctx->sync_ctx->write_fd,
+	if (pwrite_full(sync_ctx->write_fd,
 			str_data(ctx->header) + ctx->header_first_change,
 			str_len(ctx->header) - ctx->header_first_change,
 			ctx->hdr_offset + ctx->header_first_change +
 			move_diff) < 0) {
-		mbox_set_syscall_error(ctx->sync_ctx->ibox, "pwrite_full()");
+		mbox_set_syscall_error(sync_ctx->ibox, "pwrite_full()");
 		return -1;
 	}
 
-	if (ctx->sync_ctx->dest_first_mail) {
-		ctx->sync_ctx->base_uid_last =
-			ctx->sync_ctx->update_base_uid_last;
-                ctx->sync_ctx->update_base_uid_last = 0;
-	}
+	if (sync_ctx->dest_first_mail)
+		mbox_sync_first_mail_written(ctx);
 
-	i_stream_sync(ctx->sync_ctx->input);
+	i_stream_sync(sync_ctx->input);
 	return 1;
 }
 
@@ -345,7 +354,6 @@
 	}
 
 	sync_ctx->prev_msg_uid = old_prev_msg_uid;
-	sync_ctx->dest_first_mail = FALSE;
 
 	need_space = str_len(mail_ctx.header) - mail_ctx.mail.space -
 		(mail_ctx.body_offset - mail_ctx.hdr_offset);
@@ -379,16 +387,14 @@
 		return -1;
 	}
 
+	if (sync_ctx->dest_first_mail) {
+		mbox_sync_first_mail_written(&mail_ctx);
+		sync_ctx->dest_first_mail = FALSE;
+	}
+
 	mails[idx].offset = dest_offset +
 		(mail_ctx.mail.offset - mail_ctx.hdr_offset);
 	mails[idx].space = mail_ctx.mail.space;
-
-	if (mails[idx].from_offset == 0) {
-		sync_ctx->base_uid_last =
-			sync_ctx->update_base_uid_last;
-                sync_ctx->update_base_uid_last = 0;
-	}
-
 	return 0;
 }
 

Index: mbox-sync-update.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/mbox/mbox-sync-update.c,v
retrieving revision 1.31
retrieving revision 1.32
diff -u -d -r1.31 -r1.32
--- mbox-sync-update.c	7 Apr 2005 22:09:07 -0000	1.31
+++ mbox-sync-update.c	7 Apr 2005 22:27:26 -0000	1.32
@@ -1,7 +1,6 @@
 /* Copyright (C) 2004 Timo Sirainen */
 
 #include "lib.h"
-#include "ioloop.h"
 #include "buffer.h"
 #include "str.h"
 #include "message-parser.h"
@@ -14,13 +13,10 @@
 {
 	int i;
 
-	/* kludgy kludgy */
-	ctx->mail.flags ^= MBOX_NONRECENT_KLUDGE;
 	for (i = 0; flags_list[i].chr != 0; i++) {
 		if ((ctx->mail.flags & flags_list[i].flag) != 0)
 			str_append_c(ctx->header, flags_list[i].chr);
 	}
-	ctx->mail.flags ^= MBOX_NONRECENT_KLUDGE;
 }
 
 void mbox_sync_move_buffer(struct mbox_sync_mail_context *ctx,
@@ -184,18 +180,17 @@
 
 	if (ctx->sync_ctx->dest_first_mail &&
 	    ctx->hdr_pos[MBOX_HDR_X_IMAPBASE] == (size_t)-1) {
-		if (ctx->sync_ctx->base_uid_validity == 0) {
-			ctx->sync_ctx->base_uid_validity =
-				ctx->sync_ctx->hdr->uid_validity == 0 ?
-				(uint32_t)ioloop_time :
-				ctx->sync_ctx->hdr->uid_validity;
-		}
+		i_assert(ctx->sync_ctx->base_uid_validity != 0);
 
 		str_append(ctx->header, "X-IMAPbase: ");
 		ctx->hdr_pos[MBOX_HDR_X_IMAPBASE] = str_len(ctx->header);
-		str_printfa(ctx->header, "%u %010u",
-			    ctx->sync_ctx->base_uid_validity,
-			    ctx->sync_ctx->next_uid-1);
+		str_printfa(ctx->header, "%u ",
+			    ctx->sync_ctx->base_uid_validity);
+
+		ctx->last_uid_value_start_pos = str_len(ctx->header) -
+			ctx->hdr_pos[MBOX_HDR_X_IMAPBASE];
+		str_printfa(ctx->header, "%010u", ctx->sync_ctx->next_uid-1);
+
 		keywords_append_all(ctx, ctx->header);
 		str_append_c(ctx->header, '\n');
 	}
@@ -206,6 +201,8 @@
 		str_printfa(ctx->header, "%u\n", ctx->mail.uid);
 	}
 
+	ctx->mail.flags ^= MBOX_NONRECENT_KLUDGE;
+
 	if (ctx->hdr_pos[MBOX_HDR_STATUS] == (size_t)-1 &&
 	    (ctx->mail.flags & STATUS_FLAGS_MASK) != 0) {
 		str_append(ctx->header, "Status: ");
@@ -222,6 +219,8 @@
 		str_append_c(ctx->header, '\n');
 	}
 
+	ctx->mail.flags ^= MBOX_NONRECENT_KLUDGE;
+
 	if (ctx->hdr_pos[MBOX_HDR_X_KEYWORDS] == (size_t)-1 &&
 	    array_is_created(&ctx->mail.keywords) &&
 	    array_count(&ctx->mail.keywords) > 0) {
@@ -313,25 +312,28 @@
 	struct mbox_sync_context *sync_ctx = ctx->sync_ctx;
 	string_t *str;
 
+	i_assert(sync_ctx->base_uid_validity != 0);
+
 	if (!sync_ctx->dest_first_mail ||
 	    ctx->hdr_pos[MBOX_HDR_X_IMAPBASE] == (size_t)-1)
 		return;
 
-	if (sync_ctx->update_base_uid_last <= sync_ctx->base_uid_last)
-                sync_ctx->update_base_uid_last = 0;
-
-	/* see if anything changed */
-	if (!(ctx->update_imapbase_keywords ||
-	      sync_ctx->update_base_uid_last != 0))
+	if (!ctx->imapbase_rewrite) {
+		/* uid-last might need updating, but we'll do it later by
+		   writing it directly where needed. */
 		return;
+	}
 
-	/* update uid-last field in X-IMAPbase */
+	/* a) keyword list changed, b) uid-last didn't use 10 digits */
 	t_push();
 
 	str = t_str_new(200);
-	str_printfa(str, "%u %010u", sync_ctx->base_uid_validity,
-		    sync_ctx->update_base_uid_last != 0 ?
-		    sync_ctx->update_base_uid_last : sync_ctx->base_uid_last);
+	str_printfa(str, "%u ", sync_ctx->base_uid_validity);
+
+	ctx->last_uid_value_start_pos =
+		str_len(str) - ctx->hdr_pos[MBOX_HDR_X_IMAPBASE];
+	str_printfa(str, "%010u", sync_ctx->next_uid - 1);
+
 	keywords_append_all(ctx, str);
 	str_append_c(str, '\n');
 

Index: mbox-sync.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/mbox/mbox-sync.c,v
retrieving revision 1.144
retrieving revision 1.145
diff -u -d -r1.144 -r1.145
--- mbox-sync.c	2 Apr 2005 21:09:07 -0000	1.144
+++ mbox-sync.c	7 Apr 2005 22:27:26 -0000	1.145
@@ -38,6 +38,7 @@
 #include "istream.h"
 #include "file-set-size.h"
 #include "str.h"
+#include "read-full.h"
 #include "write-full.h"
 #include "message-date.h"
 #include "istream-raw-mbox.h"
@@ -73,7 +74,7 @@
 	return 0;
 }
 
-static void mbox_sync_array_delete_old(array_t *syncs_arr, uint32_t uid)
+static void mbox_sync_array_delete_to(array_t *syncs_arr, uint32_t last_uid)
 {
 	ARRAY_SET_TYPE(syncs_arr, struct mail_index_sync_rec);
 	struct mail_index_sync_rec *syncs;
@@ -82,7 +83,8 @@
 	syncs = array_get_modifyable(syncs_arr, &count);
 
 	for (src = dest = 0; src < count; src++) {
-		if (uid <= syncs[src].uid2) {
+		i_assert(last_uid >= syncs[src].uid1);
+		if (last_uid <= syncs[src].uid2) {
 			/* keep it */
 			if (src != dest)
 				syncs[dest] = syncs[src];
@@ -112,9 +114,6 @@
 	mail_ctx->mail.offset =
 		istream_raw_mbox_get_header_offset(sync_ctx->input);
 
-	if (mail_ctx->seq == 1)
-		sync_ctx->seen_first_mail = TRUE;
-
 	mbox_sync_parse_next_mail(sync_ctx->input, mail_ctx);
 	i_assert(sync_ctx->input->v_offset != mail_ctx->mail.from_offset ||
 		 sync_ctx->input->eof);
@@ -167,7 +166,7 @@
 		uid = (uint32_t)-1;
 	}
 
-	mbox_sync_array_delete_old(&sync_ctx->syncs, uid);
+	mbox_sync_array_delete_to(&sync_ctx->syncs, uid);
 	while (uid >= sync_rec->uid1) {
 		if (uid <= sync_rec->uid2 &&
 		    sync_rec->type != MAIL_INDEX_SYNC_TYPE_APPEND &&
@@ -191,10 +190,8 @@
 		}
 
 		if (sync_rec->type == MAIL_INDEX_SYNC_TYPE_APPEND) {
-			if (sync_rec->uid2 >= sync_ctx->next_uid) {
+			if (sync_rec->uid2 >= sync_ctx->next_uid)
 				sync_ctx->next_uid = sync_rec->uid2 + 1;
-                                sync_ctx->update_base_uid_last = sync_rec->uid2;
-			}
 			memset(sync_rec, 0, sizeof(*sync_rec));
 		}
 	}
@@ -511,6 +508,51 @@
 	return 0;
 }
 
+static int mbox_rewrite_base_uid_last(struct mbox_sync_context *sync_ctx)
+{
+	unsigned char buf[10];
+	const char *str;
+	uint32_t uid_last;
+	unsigned int i;
+
+	i_assert(sync_ctx->base_uid_last_offset != 0);
+
+	/* first check that the 10 bytes are there and they're exactly as
+	   expected. just an extra safety check to make sure we never write
+	   to wrong location in the mbox file. */
+	if (pread_full(sync_ctx->write_fd, buf, sizeof(buf),
+		       sync_ctx->base_uid_last_offset) <= 0) {
+		mbox_set_syscall_error(sync_ctx->ibox, "pread_full()");
+		return -1;
+	}
+
+	for (i = 0, uid_last = 0; i < sizeof(buf); i++) {
+		if (buf[i] < '0' || buf[i] > '9') {
+			uid_last = (uint32_t)-1;
+			break;
+		}
+		uid_last = uid_last * 10 + (buf[i] - '0');
+	}
+
+	if (uid_last != sync_ctx->base_uid_last) {
+		mail_storage_set_critical(sync_ctx->ibox->box.storage,
+			"X-IMAPbase uid-last unexpectedly lost in mbox file %s",
+			sync_ctx->ibox->path);
+		return -1;
+	}
+
+	/* and write it */
+	str = t_strdup_printf("%010u", sync_ctx->next_uid - 1);
+	if (pwrite_full(sync_ctx->write_fd, str, 10,
+			sync_ctx->base_uid_last_offset) < 0) {
+		mbox_set_syscall_error(sync_ctx->ibox, "pwrite_full()");
+		return -1;
+	}
+
+	sync_ctx->base_uid_last = sync_ctx->next_uid - 1;
+	return 0;
+}
+
 static int
 mbox_write_from_line(struct mbox_sync_mail_context *ctx)
 {
@@ -606,9 +648,7 @@
 			}
 		}
 	} else if (mail_ctx->need_rewrite ||
-		   array_count(&sync_ctx->syncs) != 0 ||
-		   (mail_ctx->seq == 1 &&
-		    sync_ctx->update_base_uid_last != 0)) {
+		   array_count(&sync_ctx->syncs) != 0) {
 		mbox_sync_update_header(mail_ctx);
 		if (sync_ctx->delay_writes) {
 			/* mark it dirty and do it later */
@@ -818,51 +858,71 @@
 	return mbox_sync_seek_to_seq(sync_ctx, seq1);
 }
 
-static int mbox_sync_loop(struct mbox_sync_context *sync_ctx,
-			  struct mbox_sync_mail_context *mail_ctx,
-			  uint32_t min_message_count, int partial)
+static int mbox_sync_partial_seek_next(struct mbox_sync_context *sync_ctx,
+				       uint32_t next_uid, int *partial,
+				       int *skipped_mails)
 {
-	const struct mail_index_record *rec;
-	uint32_t uid, messages_count;
-	uoff_t offset;
-	int ret, expunged;
+	uint32_t messages_count;
+	int ret;
 
-	messages_count =
-		mail_index_view_get_messages_count(sync_ctx->sync_view);
+	/* delete sync records up to next message. so if there's still
+	   something left in array, it means the next message needs modifying */
+	mbox_sync_array_delete_to(&sync_ctx->syncs, next_uid);
+	if (array_count(&sync_ctx->syncs) > 0)
+		return 1;
 
-	if (!mail_index_sync_have_more(sync_ctx->index_sync_ctx) ||
-	    (!partial && min_message_count != 0)) {
-		ret = mbox_sync_seek_to_seq(sync_ctx, partial ?
-					    messages_count : 0);
+	if (sync_ctx->sync_rec.uid1 != 0) {
+		/* we can skip forward to next record which needs updating. */
+		if (sync_ctx->sync_rec.uid1 != next_uid) {
+			*skipped_mails = TRUE;
+			next_uid = sync_ctx->sync_rec.uid1;
+		}
+		ret = mbox_sync_seek_to_uid(sync_ctx, next_uid);
 	} else {
-		/* we sync only what we need to. jump to first record that
-		   needs updating */
-		const struct mail_index_sync_rec *sync_rec;
-		unsigned int count;
-
-		if (array_count(&sync_ctx->syncs) == 0 &&
-		    sync_ctx->sync_rec.uid1 == 0) {
-			if (mbox_sync_read_index_syncs(sync_ctx, 1,
-						       &expunged) < 0)
-				return -1;
+		/* if there's no sync records left, we can stop. except if
+		   this is a dirty sync, check if there are new messages. */
+		if (!sync_ctx->ibox->mbox_sync_dirty)
+			return 0;
 
-			if (array_count(&sync_ctx->syncs) == 0 &&
-			    sync_ctx->sync_rec.uid1 == 0) {
-				/* nothing to do */
-				return 1;
-			}
+		messages_count =
+			mail_index_view_get_messages_count(sync_ctx->sync_view);
+		if (sync_ctx->seq + 1 != messages_count) {
+			ret = mbox_sync_seek_to_seq(sync_ctx, messages_count);
+			*skipped_mails = TRUE;
+		} else {
+			ret = 1;
 		}
+		*partial = FALSE;
+	}
 
-		sync_rec = array_get(&sync_ctx->syncs, &count);
-		if (count == 0)
-			sync_rec = &sync_ctx->sync_rec;
-
-		ret = mbox_sync_seek_to_uid(sync_ctx, sync_rec->uid1);
+	if (ret == 0) {
+		/* seek failed because the offset is dirty. just ignore and
+		   continue from where we are now. */
+		*partial = FALSE;
+		ret = 1;
 	}
+	return ret;
+}
 
+static int mbox_sync_loop(struct mbox_sync_context *sync_ctx,
+                          struct mbox_sync_mail_context *mail_ctx,
+			  int partial)
+{
+	const struct mail_index_record *rec;
+	uint32_t uid, messages_count;
+	uoff_t offset;
+	int ret, expunged, skipped_mails;
+
+	messages_count =
+		mail_index_view_get_messages_count(sync_ctx->sync_view);
+
+	/* always start from first message so we can read X-IMAP or
+	   X-IMAPbase header */
+	ret = mbox_sync_seek_to_seq(sync_ctx, 0);
 	if (ret <= 0)
 		return ret;
 
+	skipped_mails = FALSE;
 	while ((ret = mbox_sync_read_next_mail(sync_ctx, mail_ctx)) > 0) {
 		uid = mail_ctx->mail.uid;
 
@@ -896,6 +956,11 @@
 				return -1;
 		}
 
+		if (rec == NULL) {
+			/* from now on, don't skip anything */
+			partial = FALSE;
+		}
+
 		if (ret == 0) {
 			/* UID found but it's broken */
 			uid = 0;
@@ -977,23 +1042,14 @@
 				if (mbox_sync_seek(sync_ctx, offset) < 0)
 					return -1;
 			}
-		} else if (sync_ctx->seq >= min_message_count) {
-			/* +1 because we want to delete sync records
-			   from the current UID as well */
-			mbox_sync_array_delete_old(&sync_ctx->syncs, uid+1);
-			if (array_count(&sync_ctx->syncs) == 0) {
-				/* if there's no sync records left,
-				   we can stop */
-				if (sync_ctx->sync_rec.uid1 == 0)
-					break;
-
-				/* we can skip forward to next record which
-				   needs updating. if it fails because the
-				   offset is dirty, just ignore and continue
-				   from where we are now. */
-				uid = sync_ctx->sync_rec.uid1;
-				if (mbox_sync_seek_to_uid(sync_ctx, uid) < 0)
+		} else if (partial) {
+			ret = mbox_sync_partial_seek_next(sync_ctx, uid + 1,
+							  &partial,
+							  &skipped_mails);
+			if (ret <= 0) {
+				if (ret < 0)
 					return -1;
+				break;
 			}
 		}
 	}
@@ -1007,7 +1063,7 @@
 			mail_index_expunge(sync_ctx->t, sync_ctx->idx_seq++);
 	}
 
-	if (!partial)
+	if (!skipped_mails)
 		sync_ctx->ibox->mbox_sync_dirty = FALSE;
 
 	return 1;
@@ -1166,17 +1222,8 @@
 		return -1;
 	}
 
-	if ((sync_ctx->base_uid_validity != 0 &&
-	     sync_ctx->base_uid_validity != sync_ctx->hdr->uid_validity) ||
-	    sync_ctx->hdr->uid_validity == 0) {
-		if (sync_ctx->base_uid_validity == 0) {
-			/* we didn't rewrite X-IMAPbase header because
-			   a) mbox is read-only, b) we're lazy-writing,
-			   c) it's empty */
-			i_assert(sync_ctx->delay_writes ||
-				 sync_ctx->hdr->uid_validity == 0);
-                        sync_ctx->base_uid_validity = time(NULL);
-		}
+	i_assert(sync_ctx->base_uid_validity != 0);
+	if (sync_ctx->base_uid_validity != sync_ctx->hdr->uid_validity) {
 		mail_index_update_header(sync_ctx->t,
 			offsetof(struct mail_index_header, uid_validity),
 			&sync_ctx->base_uid_validity,
@@ -1219,10 +1266,13 @@
 {
 	sync_ctx->base_uid_validity = 0;
 	sync_ctx->base_uid_last = 0;
+	sync_ctx->base_uid_last_offset = 0;
 
 	array_clear(&sync_ctx->mails);
 	array_clear(&sync_ctx->syncs);
+
 	memset(&sync_ctx->sync_rec, 0, sizeof(sync_ctx->sync_rec));
+        mail_index_sync_reset(sync_ctx->index_sync_ctx);
 
 	sync_ctx->prev_msg_uid = 0;
 	sync_ctx->next_uid = sync_ctx->hdr->next_uid;
@@ -1233,7 +1283,6 @@
 	sync_ctx->space_diff = 0;
 
 	sync_ctx->dest_first_mail = TRUE;
-	sync_ctx->seen_first_mail = FALSE;
         sync_ctx->sync_restart = FALSE;
 }
 
@@ -1242,58 +1291,51 @@
 {
 	struct mbox_sync_mail_context mail_ctx;
 	const struct stat *st;
-	uint32_t min_msg_count;
 	int ret, partial;
 
-	partial = FALSE;
-
-	if ((flags & MBOX_SYNC_HEADER) != 0)
-		min_msg_count = 1;
-	else {
-		st = i_stream_stat(sync_ctx->file_input);
-		if (st == NULL) {
-			mbox_set_syscall_error(sync_ctx->ibox,
-					       "i_stream_stat()");
-			return -1;
-		}
+	st = i_stream_stat(sync_ctx->file_input);
+	if (st == NULL) {
+		mbox_set_syscall_error(sync_ctx->ibox,
+				       "i_stream_stat()");
+		return -1;
+	}
 
-		if ((uint32_t)st->st_mtime == sync_ctx->hdr->sync_stamp &&
-		    (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
-			/* file is fully synced */
-			sync_ctx->ibox->mbox_sync_dirty = FALSE;
-			min_msg_count = 0;
-		} else if ((flags & MBOX_SYNC_UNDIRTY) != 0 ||
-			   (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
-			/* we want to do full syncing. always do this if
-			   file size hasn't changed but timestamp has. it most
-			   likely means that someone had modified some header
-			   and we probably want to know about it */
-			min_msg_count = (uint32_t)-1;
-			sync_ctx->ibox->mbox_sync_dirty = TRUE;
-		} else {
-			/* see if we can delay syncing the whole file.
-			   normally we only notice expunges and appends
-			   in partial syncing. */
-			partial = TRUE;
-			min_msg_count = (uint32_t)-1;
-			sync_ctx->ibox->mbox_sync_dirty = TRUE;
-		}
+	if ((uint32_t)st->st_mtime == sync_ctx->hdr->sync_stamp &&
+	    (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
+		/* file is fully synced */
+		partial = TRUE;
+		sync_ctx->ibox->mbox_sync_dirty = FALSE;
+	} else if ((flags & MBOX_SYNC_UNDIRTY) != 0 ||
+		   (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
+		/* we want to do full syncing. always do this if
+		   file size hasn't changed but timestamp has. it most
+		   likely means that someone had modified some header
+		   and we probably want to know about it */
+		partial = FALSE;
+		sync_ctx->ibox->mbox_sync_dirty = TRUE;
+	} else {
+		/* see if we can delay syncing the whole file.
+		   normally we only notice expunges and appends
+		   in partial syncing. */
+		partial = TRUE;
+		sync_ctx->ibox->mbox_sync_dirty = TRUE;
 	}
 
 	mbox_sync_restart(sync_ctx);
-	ret = mbox_sync_loop(sync_ctx, &mail_ctx, min_msg_count, partial);
+	ret = mbox_sync_loop(sync_ctx, &mail_ctx, partial);
 	if (ret <= 0) {
 		if (ret < 0)
 			return -1;
 
 		/* partial syncing didn't work, do it again */
+		i_assert(sync_ctx->ibox->mbox_sync_dirty);
 		mbox_sync_restart(sync_ctx);
 
 		mail_index_transaction_rollback(sync_ctx->t);
 		sync_ctx->t = mail_index_transaction_begin(sync_ctx->sync_view,
 							   FALSE, TRUE);
 
-		ret = mbox_sync_loop(sync_ctx, &mail_ctx, (uint32_t)-1, FALSE);
+		ret = mbox_sync_loop(sync_ctx, &mail_ctx, FALSE);
 		if (ret <= 0) {
 			i_assert(ret != 0);
 			return -1;
@@ -1352,27 +1394,6 @@
 		st->st_size != ibox->mbox_dirty_size;
 }
 
-static int mbox_sync_update_imap_base(struct mbox_sync_context *sync_ctx)
-{
-	struct mbox_sync_mail_context mail_ctx;
-
-	sync_ctx->t = mail_index_transaction_begin(sync_ctx->sync_view,
-						   FALSE, TRUE);
-	sync_ctx->update_base_uid_last = sync_ctx->next_uid-1;
-
-	mbox_sync_restart(sync_ctx);
-	if (mbox_sync_loop(sync_ctx, &mail_ctx, 1, 0) < 0)
-		return -1;
-
-	if (mbox_sync_handle_eof_updates(sync_ctx, &mail_ctx) < 0)
-		return -1;
-
-	if (mbox_sync_update_index_header(sync_ctx) < 0)
-		return -1;
-
-	return 0;
-}
-
 int mbox_sync(struct index_mailbox *ibox, enum mbox_sync_flags flags)
 {
 	struct mail_index_sync_ctx *index_sync_ctx;
@@ -1526,37 +1547,10 @@
 		ret = -1;
 	}
 
-	if (sync_ctx.seen_first_mail &&
-	    sync_ctx.base_uid_last != sync_ctx.next_uid-1 &&
+	if (sync_ctx.base_uid_last != sync_ctx.next_uid-1 &&
 	    ret == 0 && !sync_ctx.delay_writes) {
-		/* rewrite X-IMAPbase header. do it after mail_index_sync_end()
-		   so previous transactions have been committed. */
-		/* FIXME: ugly .. */
-		ret = mail_index_sync_begin(ibox->index,
-					    &sync_ctx.index_sync_ctx,
-					    &sync_ctx.sync_view,
-					    (uint32_t)-1, (uoff_t)-1,
-					    FALSE, FALSE);
-		if (ret < 0)
-			mail_storage_set_index_error(ibox);
-		else {
-			sync_ctx.hdr =
-				mail_index_get_header(sync_ctx.sync_view);
-			if ((ret = mbox_sync_update_imap_base(&sync_ctx)) < 0)
-				mail_index_transaction_rollback(sync_ctx.t);
-			else if (mail_index_transaction_commit(sync_ctx.t,
-							       &seq,
-							       &offset) < 0) {
-				mail_storage_set_index_error(ibox);
-				ret = -1;
-			}
-
-			if (mail_index_sync_commit(sync_ctx.
-						   index_sync_ctx) < 0) {
-				mail_storage_set_index_error(ibox);
-				ret = -1;
-			}
-		}
+		/* Rewrite uid_last in X-IMAPbase header. */
+                ret = mbox_rewrite_base_uid_last(&sync_ctx);
 	}
 
 	if (ret == 0 && ibox->mbox_lock_type == F_WRLCK &&



More information about the dovecot-cvs mailing list