summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJim Meyering <meyering@redhat.com>2009-02-27 09:23:44 +0100
committerJim Meyering <meyering@redhat.com>2009-02-27 11:44:26 +0100
commitb50a4ae557d6ac479e659f685c8a8ff909e09393 (patch)
tree3e6b78b6e906050985cfc6606fd877b8da2b640a
parent3a914fa76dab3a4ee3dd2683866eeb664f505a00 (diff)
downloadcoreutils-b50a4ae557d6ac479e659f685c8a8ff909e09393.tar.xz
cp: diagnose invalid "cp -rl dir dir" right away, once again
Running "mkdir dir; cp -rl dir dir" would create dir/dir/dir/... rather than diagnosing the "copy-into-self" failure. The easy fix would have been to revert this part of the change [3ece0355 2008-11-09 cp: use far less memory in some cases] that introduced the bug: - remember_copied (dst_name, dst_sb.st_ino, dst_sb.st_dev); + if (!x->hard_link) + remember_copied (dst_name, dst_sb.st_ino, dst_sb.st_dev); However, that would have induced the failure of the new cp/link-heap test, due to the added memory pressure of recording 10k dev/ino pairs. And besides, I liked that improvement and wanted to keep it. Now that it's obvious recording the just-created-directory dev/ino needn't depend on the setting of hard_link, I realized it is necessary to record the pair only for the first directory created for each source command-line argument. I made that change, then noticed the new test, cp -rl a d d, would pass when run once, yet output the into-self diagnostic twice. Also note the side effect: it creates d/a and d/d. However, running that same command a second time, now with the modified directory, would fail. That turned out to be due to the fact that although the first into-self failure was detected in copy_dir, that function would continue copying other entries regardless -- and that would make it fail (eventually) with the unwanted recursion. * src/copy.c (copy_internal): This function needed an indicator of whether, for a give command line argument, it had already created its first directory. If so, no more need to record dev/ino pairs. If this is the first, then do record its pair. Hence, the new parameter. (copy_dir, copy): Update callers. (copy_dir): Upon any into-self failure, break out of the loop. * tests/cp/into-self: Test for the above. Reported by Mikael Magnusson.
-rw-r--r--NEWS5
-rw-r--r--THANKS1
-rw-r--r--src/copy.c35
-rwxr-xr-xtests/cp/into-self21
4 files changed, 53 insertions, 9 deletions
diff --git a/NEWS b/NEWS
index 05d22cbfb..87589c4ba 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,11 @@ GNU coreutils NEWS -*- outline -*-
** Bug fixes
+ cp once again diagnoses the invalid "cp -rl dir dir" right away,
+ rather than after creating a very deep dir/dir/dir/... hierarchy.
+ The bug strikes only with both --recursive (-r, -R) and --link (-l).
+ [bug introduced in coreutils-7.1]
+
sort now handles specified key ends correctly.
Previously -k1,1b would have caused leading space from field 2 to be
included in the sort while -k2,3.0 would have not included field 3.
diff --git a/THANKS b/THANKS
index 724a9e39b..253fc4009 100644
--- a/THANKS
+++ b/THANKS
@@ -398,6 +398,7 @@ Michal Politowski mpol@charybda.icm.edu.pl
Michal Svec msvec@suse.cz
Michel Robitaille robitail@IRO.UMontreal.CA
Michiel Bacchiani bacchian@raven.bu.edu
+Mikael Magnusson mikachu@gmail.com
Mike Castle dalgoda@ix.netcom.com
Mike Coleman mkc@mathdogs.com
Mike Jetzer mjetzer@mke.catalystwms.com
diff --git a/src/copy.c b/src/copy.c
index 7a7fae449..191867126 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -104,6 +104,7 @@ static bool copy_internal (char const *src_name, char const *dst_name,
struct dir_list *ancestors,
const struct cp_options *x,
bool command_line_arg,
+ bool *first_dir_created_per_command_line_arg,
bool *copy_into_self,
bool *rename_succeeded);
static bool owner_failure_ok (struct cp_options const *x);
@@ -201,13 +202,16 @@ copy_attr_by_name (char const *src_path, char const *dst_path)
DST_NAME_IN is a directory that was created previously in the
recursion. SRC_SB and ANCESTORS describe SRC_NAME_IN.
Set *COPY_INTO_SELF if SRC_NAME_IN is a parent of
+ FIRST_DIR_CREATED_PER_COMMAND_LINE_ARG FIXME
(or the same as) DST_NAME_IN; otherwise, clear it.
Return true if successful. */
static bool
copy_dir (char const *src_name_in, char const *dst_name_in, bool new_dst,
const struct stat *src_sb, struct dir_list *ancestors,
- const struct cp_options *x, bool *copy_into_self)
+ const struct cp_options *x,
+ bool *first_dir_created_per_command_line_arg,
+ bool *copy_into_self)
{
char *name_space;
char *namep;
@@ -237,12 +241,19 @@ copy_dir (char const *src_name_in, char const *dst_name_in, bool new_dst,
ok &= copy_internal (src_name, dst_name, new_dst, src_sb->st_dev,
ancestors, &non_command_line_options, false,
+ first_dir_created_per_command_line_arg,
&local_copy_into_self, NULL);
*copy_into_self |= local_copy_into_self;
free (dst_name);
free (src_name);
+ /* If we're copying into self, there's no point in continuing,
+ and in fact, that would even infloop, now that we record only
+ the first created directory per command line argument. */
+ if (local_copy_into_self)
+ break;
+
namep += strlen (namep) + 1;
}
free (name_space);
@@ -1125,6 +1136,7 @@ restore_default_fscreatecon_or_die (void)
not known. ANCESTORS points to a linked, null terminated list of
devices and inodes of parent directories of SRC_NAME. COMMAND_LINE_ARG
is true iff SRC_NAME was specified on the command line.
+ FIRST_DIR_CREATED_PER_COMMAND_LINE_ARG is both input and output.
Set *COPY_INTO_SELF if SRC_NAME is a parent of (or the
same as) DST_NAME; otherwise, clear it.
Return true if successful. */
@@ -1135,6 +1147,7 @@ copy_internal (char const *src_name, char const *dst_name,
struct dir_list *ancestors,
const struct cp_options *x,
bool command_line_arg,
+ bool *first_dir_created_per_command_line_arg,
bool *copy_into_self,
bool *rename_succeeded)
{
@@ -1815,11 +1828,15 @@ copy_internal (char const *src_name, char const *dst_name,
}
}
- /* Insert the created directory's inode and device
- numbers into the search structure, so that we can
- avoid copying it again. */
- if (!x->hard_link)
- remember_copied (dst_name, dst_sb.st_ino, dst_sb.st_dev);
+ /* Record the created directory's inode and device numbers into
+ the search structure, so that we can avoid copying it again.
+ Do this only for the first directory that is created for each
+ source command line argument. */
+ if (!*first_dir_created_per_command_line_arg)
+ {
+ remember_copied (dst_name, dst_sb.st_ino, dst_sb.st_dev);
+ *first_dir_created_per_command_line_arg = true;
+ }
if (x->verbose)
emit_verbose (src_name, dst_name, NULL);
@@ -1838,6 +1855,7 @@ copy_internal (char const *src_name, char const *dst_name,
in a source directory would cause the containing destination
directory not to have owner/perms set properly. */
delayed_ok = copy_dir (src_name, dst_name, new_dst, &src_sb, dir, x,
+ first_dir_created_per_command_line_arg,
copy_into_self);
}
}
@@ -2185,8 +2203,11 @@ copy (char const *src_name, char const *dst_name,
top_level_src_name = src_name;
top_level_dst_name = dst_name;
+ bool first_dir_created_per_command_line_arg = false;
return copy_internal (src_name, dst_name, nonexistent_dst, 0, NULL,
- options, true, copy_into_self, rename_succeeded);
+ options, true,
+ &first_dir_created_per_command_line_arg,
+ copy_into_self, rename_succeeded);
}
/* Set *X to the default options for a value of type struct cp_options. */
diff --git a/tests/cp/into-self b/tests/cp/into-self
index ee3fcf5fd..cd8723236 100755
--- a/tests/cp/into-self
+++ b/tests/cp/into-self
@@ -1,7 +1,7 @@
#!/bin/sh
# Confirm that copying a directory into itself gets a proper diagnostic.
-# Copyright (C) 2001, 2002, 2004, 2006-2008 Free Software Foundation, Inc.
+# Copyright (C) 2001, 2002, 2004, 2006-2009 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -28,15 +28,32 @@ fi
. $srcdir/test-lib.sh
-mkdir dir || framework_failure
+mkdir a dir || framework_failure
fail=0
# This command should exit nonzero.
cp -R dir dir 2> out && fail=1
+echo 1 >> out
+
+# This should, too. However, with coreutils-7.1 it would infloop.
+cp -rl dir dir 2>> out && fail=1
+echo 2 >> out
+
+cp -rl a dir dir 2>> out && fail=1
+echo 3 >> out
+cp -rl a dir dir 2>> out && fail=1
+echo 4 >> out
cat > exp <<\EOF
cp: cannot copy a directory, `dir', into itself, `dir/dir'
+1
+cp: cannot copy a directory, `dir', into itself, `dir/dir'
+2
+cp: cannot copy a directory, `dir', into itself, `dir/dir'
+3
+cp: cannot copy a directory, `dir', into itself, `dir/dir'
+4
EOF
#'