Extract unlink_and_reopen from copy_file (#294)

* add tests to exercise copy_file

* Extract new function unlink_and_reopen from copy_file

The argument `ofd` to copy_file is always set to -1 unless
`open_tmpfile()` is called at generator.c:909

This change
 * removes assignment to a function argument
 * renames argument `ofd` to `tmpfilefd` in line with existing uses
 * extracts a new function `unlink_and_reopen` which is static to util1.c
 * rewrites header comments for copy_file

diff --git a/testsuite/copy-file-inplace.test b/testsuite/copy-file-inplace.test
new file mode 100755 (executable)
index 0000000..a3d124f
--- /dev/null
+++ b/testsuite/copy-file-inplace.test
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+# Copyright (C) 2008-2022 Wayne Davison
+
+# This program is distributable under the terms of the GNU GPL (see
+# COPYING).
+
+# Test that copy_file works correctly with tmpfiles
+
+. "$suitedir/rsync.fns"
+
+SSH="$scratchdir/src/support/lsh.sh"
+
+hands_setup
+
+# Create a side dir where there is a candidate destfile of the same name as a sourcefile
+cat >"$scratchdir/from/likely" <<EOF
+This is a test file
+EOF
+
+mkdir "$scratchdir/demo"
+cat >"$scratchdir/demo/likely" <<EOF
+This is a test file
+EOF
+
+# Create a chkdir
+$RSYNC -a "$fromdir/" "$chkdir/"
+
+checkit "$RSYNC -av --inplace --copy-dest='$scratchdir/demo' '$fromdir/' '$todir/'" "$chkdir" "$todir"
+
+for filehost in '' 'localhost:'; do
+    for srchost in '' 'localhost:'; do
+       if [ -z "$srchost" ]; then
+           desthost='localhost:'
+       else
+           desthost=''
+       fi
+
+       rm -rf "$todir"
+       checkit "$RSYNC -avse '$SSH' --rsync-path='$RSYNC' --inplace --copy-dest='$desthost$scratchdir/demo' '$srchost$fromdir/' '$desthost$todir/'" "$chkdir" "$todir"
+    done
+done
+
+# The script would have aborted on error, so getting here means we've won.
+exit 0

diff --git a/testsuite/copy-file.test b/testsuite/copy-file.test
new file mode 100644 (file)
index 0000000..b829fcf
--- /dev/null
+++ b/testsuite/copy-file.test
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+# Copyright (C) 2008-2022 Wayne Davison
+
+# This program is distributable under the terms of the GNU GPL (see
+# COPYING).
+
+# Test that copy_file works correctly with tmpfiles
+
+. "$suitedir/rsync.fns"
+
+SSH="$scratchdir/src/support/lsh.sh"
+
+hands_setup
+
+# Create a side dir where there is a candidate destfile of the same name as a sourcefile
+cat >"$scratchdir/from/likely" <<EOF
+This is a test file
+EOF
+
+mkdir "$scratchdir/demo"
+cat >"$scratchdir/demo/likely" <<EOF
+This is a test file
+EOF
+
+# Create a chkdir
+$RSYNC -a "$fromdir/" "$chkdir/"
+
+checkit "$RSYNC -av --copy-dest='$scratchdir/demo' '$fromdir/' '$todir/'" "$chkdir" "$todir"
+
+for filehost in '' 'localhost:'; do
+    for srchost in '' 'localhost:'; do
+       if [ -z "$srchost" ]; then
+           desthost='localhost:'
+       else
+           desthost=''
+       fi
+
+       rm -rf "$todir"
+       checkit "$RSYNC -avse '$SSH' --rsync-path='$RSYNC' --copy-dest='$desthost$scratchdir/demo' '$srchost$fromdir/' '$desthost$todir/'" "$chkdir" "$todir"
+    done
+done
+
+# The script would have aborted on error, so getting here means we've won.
+exit 0

diff --git a/util1.c b/util1.c
index fa33432995e1e5b66ce8c71ab3107a3d95792f8a..7d0460ee95c1bb51e01808a2f58b20b12450e537 100644 (file)
--- a/util1.c
+++ b/util1.c
@@ -320,16 +320,48 @@ static int safe_read(int desc, char *ptr, size_t len)
        return n_chars;
 }
 
-/* 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.
+/* Remove existing file @dest and reopen, creating a new file with @mode */
+static int unlink_and_reopen(const char *dest, mode_t mode)
+{
+       int ofd;
+
+       if (robust_unlink(dest) && errno != ENOENT) {
+               int save_errno = errno;
+               rsyserr(FERROR_XFER, errno, "unlink %s", full_fname(dest));
+               errno = save_errno;
+               return -1;
+       }
+
+#ifdef SUPPORT_XATTRS
+       if (preserve_xattrs)
+               mode |= S_IWUSR;
+#endif
+       mode &= INITACCESSPERMS;
+       if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) {
+               int save_errno = errno;
+               rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest));
+               errno = save_errno;
+               return -1;
+       }
+       return ofd;
+}
+
+/* Copy contents of file @source to file @dest with mode @mode.
+ *
+ * If @tmpfilefd is <0, copy_file unlinks @dest and then opens a new
+ * file with name @dest.
+ *
+ * Otherwise, copy_file writes to and closes the provided file
+ * descriptor.
+ *
  * In either case, if --xattrs are being preserved, the dest file will
  * have its xattrs set from the source file.
  *
  * This is used in conjunction with the --temp-dir, --backup, and
  * --copy-dest options. */
-int copy_file(const char *source, const char *dest, int ofd, mode_t mode)
+int copy_file(const char *source, const char *dest, int tmpfilefd, mode_t mode)
 {
-       int ifd;
+       int ifd, ofd;
        char buf[1024 * 8];
        int len;   /* Number of bytes read into `buf'. */
        OFF_T prealloc_len = 0, offset = 0;
@@ -341,23 +373,12 @@ int copy_file(const char *source, const char *dest, int ofd, mode_t mode)
                return -1;
        }
 
-       if (ofd < 0) {
-               if (robust_unlink(dest) && errno != ENOENT) {
-                       int save_errno = errno;
-                       rsyserr(FERROR_XFER, errno, "unlink %s", full_fname(dest));
-                       close(ifd);
-                       errno = save_errno;
-                       return -1;
-               }
-
-#ifdef SUPPORT_XATTRS
-               if (preserve_xattrs)
-                       mode |= S_IWUSR;
-#endif
-               mode &= INITACCESSPERMS;
-               if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) {
+       if (tmpfilefd >= 0) {
+               ofd = tmpfilefd;
+       } else {
+               ofd = unlink_and_reopen(dest, mode);
+               if (ofd < 0) {
                        int save_errno = errno;
-                       rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest));
                        close(ifd);
                        errno = save_errno;
                        return -1;