[dovecot-cvs] dovecot/src/lib-storage/index/maildir maildir-save.c, 1.77, 1.78

[tss at dovecot.org]

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

Modified Files:
	maildir-save.c 
Log Message:
Use rename() instead of link() + unlink() when moving a mail from tmp/ to new/.


Index: maildir-save.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/maildir/maildir-save.c,v
retrieving revision 1.77
retrieving revision 1.78
diff -u -d -r1.77 -r1.78
--- maildir-save.c	17 Dec 2006 16:35:33 -0000	1.77
+++ maildir-save.c	28 Dec 2006 16:07:19 -0000	1.78
@@ -63,6 +63,7 @@
 			     const char *tmpname, const char *destname,
 			     bool newdir)
 {
+	struct mail_storage *storage = STORAGE(ctx->mbox->storage);
 	const char *tmp_path, *new_path;
 	int ret;
 
@@ -77,23 +78,29 @@
 		t_strconcat(ctx->newdir, "/", destname, NULL) :
 		t_strconcat(ctx->curdir, "/", destname, NULL);
 
-	if (link(tmp_path, new_path) == 0)
+	/* maildir spec says we should use link() + unlink() here. however
+	   since our filename is guaranteed to be unique, rename() works just
+	   as well, except faster. even if the filename wasn't unique, the
+	   problem could still happen if the file was already moved from
+	   new/ to cur/, so link() doesn't really provide any safety anyway.
+
+	   Besides the small temporary performance benefits, this rename() is
+	   almost required with OSX's HFS+ filesystem, since it implements
+	   hard links in a pretty ugly way, which makes the performance crawl
+	   when a lot of hard links are used. */
+	if (rename(tmp_path, new_path) == 0)
 		ret = 0;
 	else {
 		ret = -1;
 		if (ENOSPACE(errno)) {
-			mail_storage_set_error(STORAGE(ctx->mbox->storage),
+			mail_storage_set_error(storage,
 					       "Not enough disk space");
 		} else {
-			mail_storage_set_critical(STORAGE(ctx->mbox->storage),
-				"link(%s, %s) failed: %m", tmp_path, new_path);
+			mail_storage_set_critical(storage,
+				"rename(%s, %s) failed: %m",
+				tmp_path, new_path);
 		}
 	}
-
-	if (unlink(tmp_path) < 0 && errno != ENOENT) {
-		mail_storage_set_critical(STORAGE(ctx->mbox->storage),
-			"unlink(%s) failed: %m", tmp_path);
-	}
 	t_pop();
 	return ret;
 }