changeset 12549:bbfa924bc4bc

lib-storage: mailbox_get_status() can now fail. It might have failed earlier also, but the errors were hidden from caller.
author Timo Sirainen <tss@iki.fi>
date Wed, 29 Dec 2010 11:43:01 +0200
parents 832d77536f07
children 2c299c0e3bc8
files src/doveadm/doveadm-mail-mailbox-status.c src/dsync/dsync-worker-local.c src/imap/cmd-select.c src/imap/imap-client.c src/imap/imap-status.c src/imap/imap-sync.c src/lib-storage/index/index-status.c src/lib-storage/index/index-storage.h src/lib-storage/list/index-mailbox-list-sync.c src/lib-storage/mail-storage-private.h src/lib-storage/mail-storage.c src/lib-storage/mail-storage.h src/lib-storage/test-mailbox.c
diffstat 13 files changed, 68 insertions(+), 52 deletions(-) [+]
line wrap: on
line diff
--- a/src/doveadm/doveadm-mail-mailbox-status.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/doveadm/doveadm-mail-mailbox-status.c	Wed Dec 29 11:43:01 2010 +0200
@@ -127,11 +127,11 @@
 	}
 
 	if (doveadm_mailbox_find_and_sync(ctx->ctx.cur_mail_user,
-					  str_c(mailbox_name), &box) < 0) {
+					  str_c(mailbox_name), &box) < 0 ||
+	   mailbox_get_status(box, ctx->items, &status) < 0) {
 		ctx->ctx.failed = TRUE;
 		return;
 	}
-	mailbox_get_status(box, ctx->items, &status);
 	if (ctx->guid) {
 		if (mailbox_get_guid(box, mailbox_guid) < 0)
 			memset(mailbox_guid, 0, sizeof(mailbox_guid));
--- a/src/dsync/dsync-worker-local.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/dsync/dsync-worker-local.c	Wed Dec 29 11:43:01 2010 +0200
@@ -510,8 +510,11 @@
 		(struct local_dsync_worker_mailbox_iter *)_iter;
 	struct local_dsync_worker *worker =
 		(struct local_dsync_worker *)_iter->worker;
-	enum mailbox_flags flags =
+	const enum mailbox_flags flags =
 		MAILBOX_FLAG_READONLY | MAILBOX_FLAG_KEEP_RECENT;
+	const enum mailbox_status_items status_items =
+		STATUS_UIDNEXT | STATUS_UIDVALIDITY |
+		STATUS_HIGHESTMODSEQ | STATUS_CACHE_FIELDS;
 	const struct mailbox_info *info;
 	const char *storage_name;
 	struct mailbox *box;
@@ -555,6 +558,7 @@
 
 	box = mailbox_alloc(info->ns->list, storage_name, flags);
 	if (mailbox_sync(box, 0) < 0 ||
+	    mailbox_get_status(box, status_items, &status) < 0 ||
 	    mailbox_get_guid(box, mailbox_guid) < 0) {
 		struct mail_storage *storage = mailbox_get_storage(box);
 
@@ -565,9 +569,6 @@
 		return -1;
 	}
 
-	mailbox_get_status(box, STATUS_UIDNEXT | STATUS_UIDVALIDITY |
-			   STATUS_HIGHESTMODSEQ | STATUS_CACHE_FIELDS, &status);
-
 	change = hash_table_lookup(worker->mailbox_changes_hash, mailbox_guid);
 	if (change != NULL) {
 		/* it shouldn't be marked as deleted, but drop it to be sure */
@@ -935,7 +936,8 @@
 	array_clear(&iter->expunges);
 	iter->expunges_set = TRUE;
 
-	mailbox_get_status(box, STATUS_UIDNEXT, &status);
+	if (mailbox_get_status(box, STATUS_UIDNEXT, &status) < 0)
+		i_unreached();
 	if (prev_uid + 1 >= status.uidnext) {
 		/* no expunged messages at the end of mailbox */
 		return FALSE;
--- a/src/imap/cmd-select.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/imap/cmd-select.c	Wed Dec 29 11:43:01 2010 +0200
@@ -295,10 +295,14 @@
 					  mailbox_get_storage(ctx->box));
 		return -1;
 	}
-	mailbox_get_status(ctx->box, STATUS_MESSAGES | STATUS_RECENT |
-			   STATUS_FIRST_UNSEEN_SEQ | STATUS_UIDVALIDITY |
-			   STATUS_UIDNEXT | STATUS_KEYWORDS |
-			   STATUS_HIGHESTMODSEQ, &status);
+	if (mailbox_get_status(ctx->box, STATUS_MESSAGES | STATUS_RECENT |
+			       STATUS_FIRST_UNSEEN_SEQ | STATUS_UIDVALIDITY |
+			       STATUS_UIDNEXT | STATUS_KEYWORDS |
+			       STATUS_HIGHESTMODSEQ, &status) < 0) {
+		client_send_storage_error(ctx->cmd,
+					  mailbox_get_storage(ctx->box));
+		return -1;
+	}
 
 	client->mailbox = ctx->box;
 	client->select_counter++;
--- a/src/imap/imap-client.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/imap/imap-client.c	Wed Dec 29 11:43:01 2010 +0200
@@ -926,8 +926,9 @@
 	if ((features & MAILBOX_FEATURE_CONDSTORE) != 0) {
 		/* CONDSTORE being enabled while mailbox is selected.
 		   Notify client of the latest HIGHESTMODSEQ. */
-		mailbox_get_status(client->mailbox,
-				   STATUS_HIGHESTMODSEQ, &status);
+		if (mailbox_get_status(client->mailbox,
+				       STATUS_HIGHESTMODSEQ, &status) < 0)
+			i_unreached();
 		client_send_line(client, t_strdup_printf(
 			"* OK [HIGHESTMODSEQ %llu] Highest",
 			(unsigned long long)status.highest_modseq));
--- a/src/imap/imap-status.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/imap/imap-status.c	Wed Dec 29 11:43:01 2010 +0200
@@ -82,13 +82,9 @@
 	if ((items->mailbox_items & STATUS_HIGHESTMODSEQ) != 0)
 		client_enable(client, MAILBOX_FEATURE_CONDSTORE);
 
-	ret = box == client->mailbox ? 0 : mailbox_sync(box, 0);
-	if (ret == 0) {
-		mailbox_get_status(box, items->mailbox_items,
-				   &result_r->status);
-		if (items->guid)
-			ret = mailbox_get_guid(box, result_r->mailbox_guid);
-	}
+	ret = mailbox_get_status(box, items->mailbox_items, &result_r->status);
+	if (items->guid && ret == 0)
+		ret = mailbox_get_guid(box, result_r->mailbox_guid);
 
 	if (ret < 0) {
 		struct mail_storage *storage = mailbox_get_storage(box);
--- a/src/imap/imap-sync.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/imap/imap-sync.c	Wed Dec 29 11:43:01 2010 +0200
@@ -210,6 +210,9 @@
 int imap_sync_deinit(struct imap_sync_context *ctx,
 		     struct client_command_context *sync_cmd)
 {
+	const enum mailbox_status_items status_items =
+		STATUS_UIDVALIDITY | STATUS_MESSAGES | STATUS_RECENT |
+		STATUS_HIGHESTMODSEQ;
 	struct client *client = ctx->client;
 	struct mailbox_status status;
 	struct mailbox_sync_status sync_status;
@@ -220,15 +223,13 @@
 		array_free(&ctx->expunges);
 
 	if (mailbox_sync_deinit(&ctx->sync_ctx, &sync_status) < 0 ||
+	    mailbox_get_status(ctx->box, status_items, &status) < 0 ||
 	    ctx->failed) {
 		mailbox_transaction_rollback(&ctx->t);
 		array_free(&ctx->tmp_keywords);
 		i_free(ctx);
 		return -1;
 	}
-	mailbox_get_status(ctx->box, STATUS_UIDVALIDITY |
-			   STATUS_MESSAGES | STATUS_RECENT |
-			   STATUS_HIGHESTMODSEQ, &status);
 
 	ret = mailbox_transaction_commit(&ctx->t);
 
--- a/src/lib-storage/index/index-status.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/index/index-status.c	Wed Dec 29 11:43:01 2010 +0200
@@ -29,7 +29,7 @@
 	status_r->cache_fields = cache_fields;
 }
 
-static void
+static int
 index_storage_virtual_size_add_new(struct mailbox *box,
 				   struct index_vsize_header *vsize_hdr)
 {
@@ -94,10 +94,10 @@
 	mail_index_update_header_ext(trans->itrans, ibox->vsize_hdr_ext_id,
 				     0, vsize_hdr, sizeof(*vsize_hdr));
 	(void)mailbox_transaction_commit(&trans);
-
+	return ret;
 }
 
-static void
+static int
 index_storage_get_status_virtual_size(struct mailbox *box,
 				      struct mailbox_status *status_r)
 {
@@ -105,6 +105,7 @@
 	struct index_vsize_header vsize_hdr;
 	const void *data;
 	size_t size;
+	int ret;
 
 	mail_index_get_header_ext(box->view, ibox->vsize_hdr_ext_id,
 				  &data, &size);
@@ -123,7 +124,7 @@
 	    vsize_hdr.message_count == status_r->messages) {
 		/* up to date */
 		status_r->virtual_size = vsize_hdr.vsize;
-		return;
+		return 0;
 	}
 	if (vsize_hdr.highest_uid >= status_r->uidnext) {
 		mail_storage_set_critical(box->storage,
@@ -131,20 +132,27 @@
 			vsize_hdr.highest_uid, status_r->uidnext);
 		memset(&vsize_hdr, 0, sizeof(vsize_hdr));
 	}
-	index_storage_virtual_size_add_new(box, &vsize_hdr);
+	ret = index_storage_virtual_size_add_new(box, &vsize_hdr);
 	status_r->virtual_size = vsize_hdr.vsize;
+	return ret;
 }
 
-void index_storage_get_status(struct mailbox *box,
-			      enum mailbox_status_items items,
-			      struct mailbox_status *status_r)
+int index_storage_get_status(struct mailbox *box,
+			     enum mailbox_status_items items,
+			     struct mailbox_status *status_r)
 {
 	const struct mail_index_header *hdr;
-
-	i_assert(box->opened);
+	int ret = 0;
 
 	memset(status_r, 0, sizeof(struct mailbox_status));
 
+	if (!box->opened) {
+		if (mailbox_open(box) < 0)
+			return -1;
+		if (mailbox_sync(box, 0) < 0)
+			return -1;
+	}
+
 	/* we can get most of the status items without any trouble */
 	hdr = mail_index_get_header(box->view);
 	status_r->messages = hdr->messages_count;
@@ -174,6 +182,9 @@
 		status_r->keywords = mail_index_get_keywords(box->index);
 	if ((items & STATUS_CACHE_FIELDS) != 0)
 		index_storage_get_status_cache_fields(box, status_r);
-	if ((items & STATUS_VIRTUAL_SIZE) != 0)
-		index_storage_get_status_virtual_size(box, status_r);
+	if ((items & STATUS_VIRTUAL_SIZE) != 0) {
+		if (index_storage_get_status_virtual_size(box, status_r) < 0)
+			ret = -1;
+	}
+	return ret;
 }
--- a/src/lib-storage/index/index-storage.h	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/index/index-storage.h	Wed Dec 29 11:43:01 2010 +0200
@@ -105,9 +105,9 @@
 
 int index_storage_sync(struct mailbox *box, enum mailbox_sync_flags flags);
 enum mailbox_sync_type index_sync_type_convert(enum mail_index_sync_type type);
-void index_storage_get_status(struct mailbox *box,
-			      enum mailbox_status_items items,
-			      struct mailbox_status *status_r);
+int index_storage_get_status(struct mailbox *box,
+			     enum mailbox_status_items items,
+			     struct mailbox_status *status_r);
 
 struct mail_search_context *
 index_storage_search_init(struct mailbox_transaction_context *t,
--- a/src/lib-storage/list/index-mailbox-list-sync.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/list/index-mailbox-list-sync.c	Wed Dec 29 11:43:01 2010 +0200
@@ -159,7 +159,7 @@
 	return ret;
 }
 
-static void
+static int
 index_list_get_status(struct mailbox *box, enum mailbox_status_items items,
 		      struct mailbox_status *status)
 {
@@ -167,11 +167,11 @@
 
 	if ((items & ~CACHED_STATUS_ITEMS) == 0) {
 		if (index_list_get_cached_status(box, status) > 0)
-			return;
+			return 0;
 		/* nonsynced / error, fallback to doing it the slow way */
 	}
 
-	ibox->module_ctx.super.get_status(box, items, status);
+	return ibox->module_ctx.super.get_status(box, items, status);
 }
 
 static int index_list_lookup_or_create(struct index_mailbox_list *ilist,
--- a/src/lib-storage/mail-storage-private.h	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/mail-storage-private.h	Wed Dec 29 11:43:01 2010 +0200
@@ -114,8 +114,8 @@
 	int (*rename)(struct mailbox *src, struct mailbox *dest,
 		      bool rename_children);
 
-	void (*get_status)(struct mailbox *box, enum mailbox_status_items items,
-			   struct mailbox_status *status_r);
+	int (*get_status)(struct mailbox *box, enum mailbox_status_items items,
+			  struct mailbox_status *status_r);
 	int (*get_guid)(struct mailbox *box, uint8_t guid[MAIL_GUID_128_SIZE]);
 
 	/* Lookup sync extension record and figure out if it mailbox has
--- a/src/lib-storage/mail-storage.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/mail-storage.c	Wed Dec 29 11:43:01 2010 +0200
@@ -904,11 +904,11 @@
 	return ns1 == ns2;
 }
 
-void mailbox_get_status(struct mailbox *box,
-			enum mailbox_status_items items,
-			struct mailbox_status *status_r)
+int mailbox_get_status(struct mailbox *box,
+		       enum mailbox_status_items items,
+		       struct mailbox_status *status_r)
 {
-	box->v.get_status(box, items, status_r);
+	return box->v.get_status(box, items, status_r);
 }
 
 int mailbox_get_guid(struct mailbox *box, uint8_t guid[MAIL_GUID_128_SIZE])
--- a/src/lib-storage/mail-storage.h	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/mail-storage.h	Wed Dec 29 11:43:01 2010 +0200
@@ -394,8 +394,8 @@
 bool mailbox_is_inconsistent(struct mailbox *box);
 
 /* Gets the mailbox status information. */
-void mailbox_get_status(struct mailbox *box, enum mailbox_status_items items,
-			struct mailbox_status *status_r);
+int mailbox_get_status(struct mailbox *box, enum mailbox_status_items items,
+		       struct mailbox_status *status_r);
 /* Get mailbox GUID, creating it if necessary. */
 int mailbox_get_guid(struct mailbox *box, uint8_t guid[MAIL_GUID_128_SIZE]);
 /* Returns a mask of flags that are private to user in this mailbox
--- a/src/lib-storage/test-mailbox.c	Wed Dec 29 11:39:32 2010 +0200
+++ b/src/lib-storage/test-mailbox.c	Wed Dec 29 11:43:01 2010 +0200
@@ -68,13 +68,14 @@
 	return -1;
 }
 
-static void test_mailbox_get_status(struct mailbox *box ATTR_UNUSED,
-				    enum mailbox_status_items items ATTR_UNUSED,
-				    struct mailbox_status *status_r)
+static int test_mailbox_get_status(struct mailbox *box ATTR_UNUSED,
+				   enum mailbox_status_items items ATTR_UNUSED,
+				   struct mailbox_status *status_r)
 {
 	memset(status_r, 0, sizeof(*status_r));
 	status_r->uidvalidity = TEST_UID_VALIDITY;
 	status_r->uidnext = 1;
+	return 0;
 }
 
 static struct mailbox_sync_context *