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
This commit is contained in:
Sam Mikes
2022-03-26 10:14:10 -06:00
committed by GitHub
parent 96ed4b47b9
commit ef76d6cfa5
3 changed files with 131 additions and 20 deletions

View File

@@ -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

45
testsuite/copy-file.test Normal file
View File

@@ -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

61
util1.c
View File

@@ -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) {
if (tmpfilefd >= 0) {
ofd = tmpfilefd;
} else {
ofd = unlink_and_reopen(dest, mode);
if (ofd < 0) {
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) {
int save_errno = errno;
rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest));
close(ifd);
errno = save_errno;
return -1;