From 2cce75453c2961d2e17888cbc196195ae1e70f07 Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Mon, 29 Oct 2007 20:43:34 +0000 Subject: [PATCH] My version of Matt's cleanup patch from bug 5051. This makes --copy-dest use a temp file when not in in-place mode, and has various improvments for the code. I have also "#if 0"ed the code in the receiver that makes missing directories to see if we can figure out if it is needed (and if so, what for). --- generator.c | 48 ++++++++++++++++++++++------ receiver.c | 91 ++++++++++++++++++++++++++++------------------------- util.c | 31 +++++++++--------- 3 files changed, 104 insertions(+), 66 deletions(-) diff --git a/generator.c b/generator.c index 47803c11..530c2748 100644 --- a/generator.c +++ b/generator.c @@ -317,7 +317,7 @@ static int flush_delete_delay(void) static int remember_delete(struct file_struct *file, const char *fname) { int len; - + while (1) { len = snprintf(deldelay_buf + deldelay_cnt, deldelay_size - deldelay_cnt, @@ -830,6 +830,42 @@ static int find_fuzzy(struct file_struct *file, struct file_list *dirlist) return lowest_j; } +/* Copy a file found in our --copy-dest handling. */ +static int copy_altdest_file(const char *src, const char *dest, struct file_struct *file) +{ + char buf[MAXPATHLEN]; + const char *copy_to, *partialptr; + int fd_w; + + if (inplace) { + /* Let copy_file open the destination in place. */ + fd_w = -1; + copy_to = dest; + } else { + fd_w = open_tmpfile(buf, dest, file); + if (fd_w < 0) + return -1; + copy_to = buf; + } + cleanup_set(copy_to, NULL, NULL, -1, -1); + if (copy_file(src, copy_to, fd_w, file->mode, 0) < 0) { + if (verbose) { + rsyserr(FINFO, errno, "copy_file %s => %s", + full_fname(src), copy_to); + } + /* Try to clean up. */ + unlink(copy_to); + cleanup_disable(); + return -1; + } + partialptr = partial_dir ? partial_dir_fname(dest) : NULL; + if (partialptr && *partialptr == '/') + partialptr = NULL; + finish_transfer(dest, copy_to, src, partialptr, file, 1, 0); + cleanup_disable(); + return 0; +} + /* This is only called for regular files. We return -2 if we've finished * handling the file, -1 if no dest-linking occurred, or a non-negative * value if we found an alternate basis file. */ @@ -904,14 +940,8 @@ static int try_dests_reg(struct file_struct *file, char *fname, int ndx, #ifdef SUPPORT_HARD_LINKS try_a_copy: /* Copy the file locally. */ #endif - if (!dry_run && copy_file(cmpbuf, fname, file->mode, 0) < 0) { - if (verbose) { - rsyserr(FINFO, errno, "copy_file %s => %s", - full_fname(cmpbuf), fname); - } + if (!dry_run && copy_altdest_file(cmpbuf, fname, file) < 0) return -1; - } - set_file_attrs(fname, file, NULL, cmpbuf, 0); if (itemizing) itemize(cmpbuf, file, ndx, 0, sxp, ITEM_LOCAL_CHANGE, 0, NULL); #ifdef SUPPORT_XATTRS @@ -1680,7 +1710,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, goto cleanup; if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS))) goto pretend_missing; - if (copy_file(fname, backupptr, back_file->mode, 1) < 0) { + if (copy_file(fname, backupptr, -1, back_file->mode, 1) < 0) { unmake_file(back_file); back_file = NULL; goto cleanup; diff --git a/receiver.c b/receiver.c index ed1111a4..0d46efa6 100644 --- a/receiver.c +++ b/receiver.c @@ -123,6 +123,43 @@ int get_tmpname(char *fnametmp, const char *fname) return 1; } +/* Opens a temporary file for writing. + * Success: Writes name into fnametmp, returns fd. + * Failure: Clobbers fnametmp, returns -1. + * Calling cleanup_set() is the caller's job. */ +int open_tmpfile(char *fnametmp, const char *fname, struct file_struct *file) +{ + int fd; + + if (!get_tmpname(fnametmp, fname)) + return -1; + + /* We initially set the perms without the setuid/setgid bits or group + * access to ensure that there is no race condition. They will be + * correctly updated after the right owner and group info is set. + * (Thanks to snabb@epipe.fi for pointing this out.) */ + fd = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); + +#if 0 + /* In most cases parent directories will already exist because their + * information should have been previously transferred, but that may + * not be the case with -R */ + if (fd == -1 && relative_paths && errno == ENOENT + && create_directory_path(fnametmp) == 0) { + /* Get back to name with XXXXXX in it. */ + get_tmpname(fnametmp, fname); + fd = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); + } +#endif + + if (fd == -1) { + rsyserr(FERROR, errno, "mkstemp %s failed", + full_fname(fnametmp)); + return -1; + } + + return fd; +} static int receive_data(int f_in, char *fname_r, int fd_r, OFF_T size_r, const char *fname, int fd, OFF_T total_size) @@ -606,52 +643,20 @@ int recv_files(int f_in, char *local_name) if (fd2 == -1) { rsyserr(FERROR, errno, "open %s failed", full_fname(fname)); - discard_receive_data(f_in, F_LENGTH(file)); - if (fd1 != -1) - close(fd1); - if (inc_recurse) - send_msg_int(MSG_NO_SEND, ndx); - continue; } } else { - if (!get_tmpname(fnametmp,fname)) { - discard_receive_data(f_in, F_LENGTH(file)); - if (fd1 != -1) - close(fd1); - if (inc_recurse) - send_msg_int(MSG_NO_SEND, ndx); - continue; - } - - /* we initially set the perms without the - * setuid/setgid bits to ensure that there is no race - * condition. They are then correctly updated after - * the lchown. Thanks to snabb@epipe.fi for pointing - * this out. We also set it initially without group - * access because of a similar race condition. */ - fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); - - /* in most cases parent directories will already exist - * because their information should have been previously - * transferred, but that may not be the case with -R */ - if (fd2 == -1 && relative_paths && errno == ENOENT - && create_directory_path(fnametmp) == 0) { - /* Get back to name with XXXXXX in it. */ - get_tmpname(fnametmp, fname); - fd2 = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); - } - if (fd2 == -1) { - rsyserr(FERROR, errno, "mkstemp %s failed", - full_fname(fnametmp)); - discard_receive_data(f_in, F_LENGTH(file)); - if (fd1 != -1) - close(fd1); - if (inc_recurse) - send_msg_int(MSG_NO_SEND, ndx); - continue; - } + fd2 = open_tmpfile(fnametmp, fname, file); + if (fd2 != -1) + cleanup_set(fnametmp, partialptr, file, fd1, fd2); + } - cleanup_set(fnametmp, partialptr, file, fd1, fd2); + if (fd2 == -1) { + discard_receive_data(f_in, F_LENGTH(file)); + if (fd1 != -1) + close(fd1); + if (inc_recurse) + send_msg_int(MSG_NO_SEND, ndx); + continue; } /* log the transfer */ diff --git a/util.c b/util.c index 689de61d..34e48adb 100644 --- a/util.c +++ b/util.c @@ -261,14 +261,15 @@ static int safe_read(int desc, char *ptr, size_t len) return n_chars; } -/** Copy a file. +/* Copy a file. If ofd < 0, copy_file unlinks and opens the "dest" file. + * Otherwise, it just writes to and closes the provided file descriptor. * * This is used in conjunction with the --temp-dir, --backup, and * --copy-dest options. */ -int copy_file(const char *source, const char *dest, mode_t mode, int create_bak_dir) +int copy_file(const char *source, const char *dest, int ofd, + mode_t mode, int create_bak_dir) { int ifd; - int ofd; char buf[1024 * 8]; int len; /* Number of bytes read into `buf'. */ @@ -277,17 +278,19 @@ int copy_file(const char *source, const char *dest, mode_t mode, int create_bak_ return -1; } - if (robust_unlink(dest) && errno != ENOENT) { - rsyserr(FERROR, errno, "unlink %s", full_fname(dest)); - return -1; - } + if (ofd < 0) { + if (robust_unlink(dest) && errno != ENOENT) { + rsyserr(FERROR, errno, "unlink %s", full_fname(dest)); + return -1; + } - if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0 - && (!create_bak_dir || errno != ENOENT || make_bak_dir(dest) < 0 - || (ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0)) { - rsyserr(FERROR, errno, "open %s", full_fname(dest)); - close(ifd); - return -1; + if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0 + && (!create_bak_dir || errno != ENOENT || make_bak_dir(dest) < 0 + || (ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0)) { + rsyserr(FERROR, errno, "open %s", full_fname(dest)); + close(ifd); + return -1; + } } while ((len = safe_read(ifd, buf, sizeof buf)) > 0) { @@ -407,7 +410,7 @@ int robust_rename(const char *from, const char *to, const char *partialptr, return -1; to = partialptr; } - if (copy_file(from, to, mode, 0) != 0) + if (copy_file(from, to, -1, mode, 0) != 0) return -2; do_unlink(from); return 1; -- 2.34.1