later, the races are still present on other platforms.
[the bug dates back to the initial implementation]
- cp, install, ln, and mv now use a numbered instead of a simple
- backup if copying a backup file to what might be its original.
+ cp, install, ln, and mv no longer lose data when asked to copy a
+ backup file to its original via a differently-spelled file name.
E.g., 'rm -f a a~; : > a; echo data > a~; cp --backup=simple a~ ./a'
- now makes a numbered backup file instead of losing the data.
+ now fails instead of losing the data.
[the bug dates back to the initial implementation]
cp, install, ln, and mv now ignore nonsensical backup suffixes.
@opindex numbered @r{backup method}
Always make numbered backups.
-@item simple
-@itemx never
-@opindex simple @r{backup method}
-Make simple backups, except make a numbered backup if the source might
-be a simple backup of the destination; this avoids losing source data.
-Please note @samp{never} is not to be confused with @samp{none}.
-
@item existing
@itemx nil
@opindex existing @r{backup method}
-Make numbered backups of files that already have them, or if the source
-might be a simple backup of the destination.
-Otherwise, make simple backups.
+Make numbered backups of files that already have them, simple backups
+of the others.
+
+@item simple
+@itemx never
+@opindex simple @r{backup method}
+Always make simple backups. Please note @samp{never} is not to be
+confused with @samp{none}.
@end table
&& command_line_arg);
}
+/* Return true if the source file with basename SRCBASE and status SRC_ST
+ is likely to be the simple backup file for DST_NAME. */
+static bool
+source_is_dst_backup (char const *srcbase, struct stat const *src_st,
+ char const *dst_name)
+{
+ size_t srcbaselen = strlen (srcbase);
+ char const *dstbase = last_component (dst_name);
+ size_t dstbaselen = strlen (dstbase);
+ size_t suffixlen = strlen (simple_backup_suffix);
+ if (! (srcbaselen == dstbaselen + suffixlen
+ && memcmp (srcbase, dstbase, dstbaselen) == 0
+ && STREQ (srcbase + dstbaselen, simple_backup_suffix)))
+ return false;
+ size_t dstlen = strlen (dst_name);
+ char *dst_back = xmalloc (dstlen + suffixlen + 1);
+ strcpy (mempcpy (dst_back, dst_name, dstlen), simple_backup_suffix);
+ struct stat dst_back_sb;
+ int dst_back_status = stat (dst_back, &dst_back_sb);
+ free (dst_back);
+ return dst_back_status == 0 && SAME_INODE (*src_st, dst_back_sb);
+}
+
/* Copy the file SRC_NAME to the file DST_NAME. The files may be of
any type. NEW_DST should be true if the file DST_NAME cannot
exist because its parent directory was just created; NEW_DST should
existing hierarchy. */
&& (x->move_mode || ! S_ISDIR (dst_sb.st_mode)))
{
- /* Silently use numbered backups if creating the backup
- file might destroy the source file. Otherwise, the commands:
+ /* Fail if creating the backup file would likely destroy
+ the source file. Otherwise, the commands:
cd /tmp; rm -f a a~; : > a; echo A > a~; cp --b=simple a~ a
would leave two zero-length files: a and a~. */
- enum backup_type backup_type = x->backup_type;
- if (backup_type != numbered_backups)
+ if (x->backup_type != numbered_backups
+ && source_is_dst_backup (srcbase, &src_sb, dst_name))
{
- size_t srcbaselen = strlen (srcbase);
- char const *dstbase = last_component (dst_name);
- size_t dstbaselen = strlen (dstbase);
- if (srcbaselen == dstbaselen + strlen (simple_backup_suffix)
- && memcmp (srcbase, dstbase, dstbaselen) == 0
- && STREQ (srcbase + dstbaselen, simple_backup_suffix))
- backup_type = numbered_backups;
+ const char *fmt;
+ fmt = (x->move_mode
+ ? _("backing up %s would destroy source; %s not moved")
+ : _("backing up %s would destroy source; %s not copied"));
+ error (0, 0, fmt,
+ quoteaf_n (0, dst_name),
+ quoteaf_n (1, src_name));
+ return false;
}
- char *tmp_backup = backup_file_rename (dst_name, backup_type);
+ char *tmp_backup = backup_file_rename (dst_name, x->backup_type);
/* FIXME: use fts:
Using alloca for a file name that may be arbitrarily
print_ver_ cp
echo a > a || framework_failure_
-cp a a0 || framework_failure_
echo a-tilde > a~ || framework_failure_
-cp a~ a~0 || framework_failure_
-# This cp command should not trash the source.
-cp --b=simple a~ ./a > out 2>&1 || fail=1
+# This cp command should exit nonzero.
+cp --b=simple a~ a > out 2>&1 && fail=1
-compare a~0 a || fail=1
-compare a~0 a~ || fail=1
-compare a0 a.~1~ || fail=1
+sed "s,cp:,XXX:," out > out2
+
+cat > exp <<\EOF
+XXX: backing up 'a' would destroy source; 'a~' not copied
+EOF
+
+compare exp out2 || fail=1
Exit $fail
. "$abs_srcdir/tests/other-fs-tmpdir"
a="$other_partition_tmpdir/a"
-a2="$other_partition_tmpdir/./a~"
+a2="$other_partition_tmpdir/a~"
-rm -f "$a" "$a2" a20 || framework_failure_
+rm -f "$a" "$a2" || framework_failure_
echo a > "$a" || framework_failure_
-cp "$a" a0 || framework_failure_
echo a2 > "$a2" || framework_failure_
-cp "$a2" a20 || framework_failure_
-# This mv command should not trash the source.
-mv --b=simple "$a2" "$a" > out 2>&1 || fail=1
+# This mv command should exit nonzero.
+mv --b=simple "$a2" "$a" > out 2>&1 && fail=1
-compare a20 "$a" || fail=1
-compare a0 "$a.~1~" || fail=1
+sed \
+ -e "s,mv:,XXX:," \
+ -e "s,$a,YYY," \
+ -e "s,$a2,ZZZ," \
+ out > out2
+
+cat > exp <<\EOF
+XXX: backing up 'YYY' would destroy source; 'ZZZ' not moved
+EOF
+
+compare exp out2 || fail=1
Exit $fail