From: Sam Mikes <sam.mikes@gmail.com>
Date: Sat, 26 Mar 2022 16:14:10 +0000 (-0600)
Subject: Extract unlink_and_reopen from copy_file (#294)
X-Git-Tag: v3.2.4pre4~5
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ef76d6cfa5ff6b33974083756d3ff0921e5304be;p=thirdparty%2Frsync.git

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
index 000000000..a3d124f7f
--- /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
index 000000000..b829fcfc8
--- /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 fa3343299..7d0460ee9 100644
--- 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;