[patch] Make robust_rename() handle EXDEV.

[Wayne Davison]

On Tue, Feb 17, 2004 at 10:28:20AM -0500, Jason M. Felice wrote:
> All callers of robust_rename() call copy_file() if EXDEV is received.
> This patch moves the copy_file() call into robust_rename().

Seems like a good simplification.  I did a refinement pass on your
changes, and also added back a do_unlink() call that should not have
been dropped.  So, here's my version of the patch.

..wayne..
-------------- next part --------------
--- backup.c	11 Feb 2004 05:02:21 -0000	1.25
+++ backup.c	17 Feb 2004 18:38:22 -0000
@@ -130,42 +130,10 @@ failure:
 /* robustly move a file, creating new directory structures if necessary */
 static int robust_move(char *src, char *dst)
 {
-	int keep_trying = 4;
-	int keep_path_extfs = 0;
-	int failed;
-
-	while (keep_trying) {
-		if (keep_path_extfs) {
-			failed = copy_file(src, dst, 0755);
-			if (!failed)
-				do_unlink(src);
-		} else
-			failed = robust_rename(src, dst);
-
-		if (failed) {
-			if (verbose > 2) {
-				rprintf(FERROR, "robust_move failed: %s(%d)\n",
-					strerror(errno), errno);
-			}
-			switch (errno) {
-			case EXDEV:	/* external filesystem */
-				keep_path_extfs = 1;
-				keep_trying--;
-				break;
-			case ENOENT:	/* no directory to write to */
-				if (make_bak_dir(dst) < 0)
-					keep_trying = 0;
-				else
-					keep_trying--;
-				break;
-			default:
-				keep_trying = 0;
-				break;
-			}
-		} else
-			keep_trying = 0;
-	}
-	return !failed;
+	if (robust_rename(src, dst, 0755) != 0 || errno != ENOENT
+	    || make_bak_dir(dst) < 0 || robust_rename(src, dst, 0755) != 0)
+		return -1;
+	return 0;
 }
 
 
--- proto.h	10 Feb 2004 03:23:38 -0000	1.183
+++ proto.h	17 Feb 2004 18:36:50 -0000
@@ -248,7 +248,7 @@ int set_modtime(char *fname, time_t modt
 int create_directory_path(char *fname, int base_umask);
 int copy_file(char *source, char *dest, mode_t mode);
 int robust_unlink(char *fname);
-int robust_rename(char *from, char *to);
+int robust_rename(char *from, char *to, int mode);
 pid_t do_fork(void);
 void kill_all(int sig);
 int name_to_uid(char *name, uid_t *uid);
--- rsync.c	3 Feb 2004 20:00:35 -0000	1.131
+++ rsync.c	17 Feb 2004 19:10:19 -0000
@@ -231,25 +231,17 @@ void sig_int(void)
    and ownership */
 void finish_transfer(char *fname, char *fnametmp, struct file_struct *file)
 {
+	int ret;
+
 	if (make_backups && !make_backup(fname))
 		return;
 
 	/* move tmp file over real file */
-	if (robust_rename(fnametmp,fname) != 0) {
-		if (errno == EXDEV) {
-			/* rename failed on cross-filesystem link.
-			   Copy the file instead. */
-			if (copy_file(fnametmp,fname, file->mode & INITACCESSPERMS)) {
-				rprintf(FERROR, "copy %s -> \"%s\": %s\n",
-					full_fname(fnametmp), fname,
-					strerror(errno));
-			} else {
-				set_perms(fname,file,NULL,0);
-			}
-		} else {
-			rprintf(FERROR,"rename %s -> \"%s\": %s\n",
-				full_fname(fnametmp), fname, strerror(errno));
-		}
+	ret = robust_rename(fnametmp, fname, file->mode & INITACCESSPERMS);
+	if (ret != 0) {
+		rprintf(FERROR, "%s %s -> \"%s\": %s\n",
+		    ret == -2 ? "copy" : "rename",
+		    full_fname(fnametmp), fname, strerror(errno));
 		do_unlink(fnametmp);
 	} else {
 		set_perms(fname,file,NULL,0);
--- util.c	7 Feb 2004 00:12:40 -0000	1.131
+++ util.c	17 Feb 2004 19:12:33 -0000
@@ -353,18 +353,33 @@ int robust_unlink(char *fname)
 #endif
 }
 
-int robust_rename(char *from, char *to)
+/* Returns 0 on success, -1 on most errors, and -2 if we got an error
+ * trying to copy the file across file systems. */
+int robust_rename(char *from, char *to, int mode)
 {
-#ifndef ETXTBSY
-	return do_rename(from, to);
-#else
-	int rc = do_rename(from, to);
-	if (rc == 0 || errno != ETXTBSY)
-		return rc;
-	if (robust_unlink(to) != 0)
-		return -1;
-	return do_rename(from, to);
+	int tries = 4;
+
+	while (tries--) {
+		if (do_rename(from, to) == 0)
+			return 0;
+
+		switch (errno) {
+#ifdef ETXTBSY
+		case ETXTBSY:
+			if (robust_unlink(to) != 0)
+				return -1;
+			break;
 #endif
+		case EXDEV:
+			if (copy_file(from, to, mode) != 0)
+				return -2;
+			do_unlink(from);
+			return 0;
+		default:
+			return -1;
+		}
+	}
+	return -1;
 }