diff options
author | Jim Meyering <meyering@redhat.com> | 2009-02-27 09:23:44 +0100 |
---|---|---|
committer | Jim Meyering <meyering@redhat.com> | 2009-02-27 11:44:26 +0100 |
commit | b50a4ae557d6ac479e659f685c8a8ff909e09393 (patch) | |
tree | 3e6b78b6e906050985cfc6606fd877b8da2b640a /src | |
parent | 3a914fa76dab3a4ee3dd2683866eeb664f505a00 (diff) | |
download | coreutils-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.
Diffstat (limited to 'src')
-rw-r--r-- | src/copy.c | 35 |
1 files changed, 28 insertions, 7 deletions
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. */ |