[dovecot-cvs] dovecot/src/lib-storage/index/maildir maildir-sync.c, 1.77.2.12, 1.77.2.13

[tss at dovecot.org]

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

Modified Files:
      Tag: branch_1_0
	maildir-sync.c 
Log Message:
If duplicate links are found, rename() one over the other to get rid of
it.



Index: maildir-sync.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-storage/index/maildir/maildir-sync.c,v
retrieving revision 1.77.2.12
retrieving revision 1.77.2.13
diff -u -d -r1.77.2.12 -r1.77.2.13
--- maildir-sync.c	23 Mar 2007 16:38:52 -0000	1.77.2.12
+++ maildir-sync.c	23 Mar 2007 17:20:20 -0000	1.77.2.13
@@ -697,7 +697,8 @@
 	struct maildir_mailbox *mbox = ctx->mbox;
 	const char *existing_fname, *existing_path;
 	const char *new_fname, *old_path, *new_path;
-	struct stat st, st2;
+	struct stat ex_st, old_st, ex2_st, old2_st;
+	bool delete_old;
 	int ret = 0;
 
 	existing_fname =
@@ -710,18 +711,69 @@
 	existing_path = t_strconcat(dir, "/", existing_fname, NULL);
 	old_path = t_strconcat(dir, "/", old_fname, NULL);
 
-	if (stat(existing_path, &st) < 0 ||
-	    stat(old_path, &st2) < 0) {
+	if (stat(existing_path, &ex_st) < 0 ||
+	    stat(old_path, &old_st) < 0) {
 		/* most likely the files just don't exist anymore.
 		   don't really care about other errors much. */
 		t_pop();
 		return 0;
 	}
-	if (st.st_ino == st2.st_ino && CMP_DEV_T(st.st_dev, st2.st_dev)) {
+	if (ex_st.st_ino == old_st.st_ino &&
+	    CMP_DEV_T(ex_st.st_dev, old_st.st_dev)) {
 		/* files are the same. this means either a race condition
-		   between stat() calls, or someone has started link()ing the
-		   files. either way there's no data loss if we just leave it
-		   there. */
+		   between stat() calls, or that the files were link()ed.
+
+		   if the files really were link()ed, we want to get rid of
+		   them. however we can't just go directly rename() them
+		   because if this was a race between stat()s, we could reverse
+		   a flag change done by another process. so, stat again
+		   and hope that there wasn't another race condition changing
+		   the flag back.. */
+		if (!(stat(existing_path, &ex2_st) == 0 &&
+		      stat(old_path, &old2_st) == 0 &&
+#ifdef HAVE_STAT_TV_NSEC
+		      ex_st.st_ctim.tv_nsec == ex2_st.st_ctim.tv_nsec &&
+		      old_st.st_ctim.tv_nsec == old2_st.st_ctim.tv_nsec &&
+#endif
+		      ex_st.st_ctime == ex2_st.st_ctime &&
+		      old_st.st_ctime == old2_st.st_ctime)) {
+			/* changed again */
+			t_pop();
+			return 0;
+		}
+
+		/* don't unlink(), if this is a race condition after
+		   all we really don't want to delete the message
+		   accidentally.
+
+		   the ctime checks are only meant to help with stat() race
+		   conditions. the inode is shared between links, so there's
+		   no way to know which filename was created later. */
+		if (old_st.st_ctime != ex_st.st_ctime)
+			delete_old = old_st.st_ctime < ex_st.st_ctime;
+		else {
+#ifdef HAVE_STAT_TV_NSEC
+			delete_old = old_st.st_ctim.tv_nsec <
+				ex_st.st_ctim.tv_nsec;
+#else
+			delete_old = TRUE;
+#endif
+		}
+		if (delete_old)
+			new_path = existing_path;
+		else {
+			new_path = old_path;
+			old_path = existing_path;
+		}
+		ret = rename(old_path, new_path);
+		if (ret == 0) {
+			i_warning("Fixed a duplicate link: %s -> %s",
+				  old_path, new_path);
+		} else if (ret < 0 && errno != ENOENT) {
+			mail_storage_set_critical(STORAGE(mbox->storage),
+						  "rename(%s, %s) failed: %m",
+						  old_path, new_path);
+		}
 		t_pop();
 		return 0;
 	}
@@ -729,10 +781,9 @@
 	new_fname = maildir_generate_tmp_filename(&ioloop_timeval);
 	new_path = t_strconcat(mbox->path, "/new/", new_fname, NULL);
 
-	if (rename(old_path, new_path) == 0) {
-		i_warning("Fixed duplicate in %s: %s -> %s",
-			  mbox->path, old_fname, new_fname);
-	} else if (errno != ENOENT) {
+	if (rename(old_path, new_path) == 0)
+		i_warning("Fixed a duplicate: %s -> %s", old_fname, new_fname);
+	else if (errno != ENOENT) {
 		mail_storage_set_critical(STORAGE(mbox->storage),
 			"rename(%s, %s) failed: %m", old_path, new_path);
 		ret = -1;