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..
--- 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;
 }
 
 
-- 
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html

Reply via email to