[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
coreutils bug with "ln x d/"
|
From: |
Paul Eggert |
|
Subject: |
coreutils bug with "ln x d/" |
|
Date: |
Fri, 25 Jun 2004 00:27:56 -0700 |
|
User-agent: |
Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux) |
While looking at cp, ln, and mv and thinking about how to implement
the -D/--no-destdir option I proposed in
<http://lists.gnu.org/archive/html/bug-coreutils/2004-06/msg00142.html>,
I noticed that "ln" has a bug. If d/x is a directory and x a file,
"ln x d/" creates a link d/x/x; it should report an error. Here's
a shell transcript of the bug:
$ mkdir d d/x
$ touch x
$ ln x d/ # <-- This command should fail.
$ ls -li x d/x/x
17783 -rw-r--r-- 2 eggert eggert 0 2004-06-25 00:12 d/x/x
17783 -rw-r--r-- 2 eggert eggert 0 2004-06-25 00:12 x
A similar bug exists with "ln -s".
Here's a patch. It adds a test case for this bug, and fixes it.
It's pretty long; sorry. I noticed the bug only after hacking for a
while to bring some order to "cp", "ln", and "mv" so that their code
is more similar and understandable. If you like, I think I can split
up the patch into two parts: a code- and diagnostic-cleanup only, and
a bug fix. (I would do this by modifying my version to have the
bug.... :-)
I also noticed what I think is a bug in POSIX, that is related to this
patch. GNU ln -f doesn't conform to POSIX either before or after this
patch. I've filed a defect report, which is mentioned in the comments
in this patch. I'd guess the POSIX committee will get to it in a few
weeks or months.
If it's any consolation this patch shortens the code and makes it
more efficient (that is, fewer syscalls) in the usual case.
2004-06-25 Paul Eggert <address@hidden>
Use more-consistent rules among cp, ln, and mv when dealing with
last operands that are (or look like) directories. This fixes a
bug: formerly, if d/x was a directry and x a file, "ln x d/"
incorrectly created a link d/x/x. It also saves some system
calls.
* NEWS: Document the fix.
* src/cp.c (target_directory_operand): New, nearly-common function,
It reports an error if the destination appears to be a directory
(e.g., because it has a trailing slash) but is not.
* src/ln.c, src/mv.c: Likewise.
* src/cp.c (do_copy): Use it.
* src/ln.c (main): Likewise.
* src/mv.c (main): Likewise.
* src/cp.c (do_copy): Don't assume argc is positive.
Don't bother to lstat dest, since copy() will do that for us.
Use "const" to avoid the need for cast.
* src/cp.c (do_copy): Don't output a usage message because of file
problems (e.g., an operand is not a directory). Use it only for
syntax. Standardize on "target %s is not a directory" for the
diagnostic.
* src/ln.c (main): Likewise.
* src/mv.c (main): Likewise.
* src/cp.c (do_copy): Remove test for trailing slash, since
target_directory_operand now does this.
* src/ln.c (main): Likewise.
* src/mv.c (movefile): Likewise.
* src/cp.c (main): Reject multiple target directories.
Check whether a specified target is a directory when parsing the
options, using stat. This gives more-accurate diagnostics.
* src/ln.c (main): Likewise.
* src/ln.c (isdir): Remove decl; no longer needed.
* src/mv.c (isdir, lstat): Likewise.
* src/ln.c (do_link): New arg dest_is_dir. All uses changed.
Don't check the destination ourself; rely on dest_is_dir.
This way we can avoid lstatting the destination in the
usual case, and in the worst case we lstat 1, not 3 times.
Don't bother to unlink unless link failed; this saves a syscall.
Remove unnecessary backup_succeeded flag;
it was identical to "dest_backup != NULL".
* src/ln.c (main): Use int to count to argc, not unsigned int.
This handles negative operand counts.
* src/mv.c (main): Likewise.
* src/mv.c (do_move): Don't call hash_init; expect the caller to
do it, for consistency with cp.c and ln.c. All callers changed.
(movefile): dest_is_dir parameter is now bool, not int.
(main): Standardize on "missing destination file operand after %s"
for the diagnostic, for consistency with cp.c.
* tests/ln/misc: See whether a trailing slash is followed too far.
* tests/mv/diag: Don't assume "mv --target=nonexistentdir"
will complain about the arg count.
Adjust to new (briefer) diagnostics.
Index: NEWS
===================================================================
RCS file: /home/meyering/coreutils/cu/NEWS,v
retrieving revision 1.219
diff -p -u -r1.219 NEWS
--- NEWS 22 Jun 2004 15:02:59 -0000 1.219
+++ NEWS 25 Jun 2004 06:56:12 -0000
@@ -42,6 +42,9 @@ GNU coreutils NEWS
when first encountering a directory, `rm -r' would mistakenly fail
to remove files under that directory.
+ If d/x is a directory and x a file, "ln x d/" now reports an error
+ instead of incorrectly creating a link to d/x/x.
+
ptx now diagnoses invalid values for its --width=N (-w)
and --gap-size=N (-g) options.
Index: src/cp.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/cp.c,v
retrieving revision 1.194
diff -p -u -r1.194 cp.c
--- src/cp.c 21 Jun 2004 15:02:28 -0000 1.194
+++ src/cp.c 25 Jun 2004 05:09:48 -0000
@@ -465,6 +465,31 @@ make_path_private (const char *const_dir
return 0;
}
+/* FILE is the last operand of this command. Return -1 if FILE is a
+ directory, 0 if not, ENOENT if FILE does not exist.
+ But report an error there is a problem accessing FILE,
+ or if FILE does not exist but would have to refer to an existing
+ directory if it referred to anything at all.
+
+ Store the file's status into *ST, and store the resulting
+ error number into *ERRP. */
+
+static int
+target_directory_operand (char const *file, struct stat *st, int *errp)
+{
+ char const *b = base_name (file);
+ size_t blen = strlen (b);
+ bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
+ int err = (stat (file, st) == 0 ? 0 : errno);
+ bool is_a_dir = !err && S_ISDIR (st->st_mode);
+ if (err && err != ENOENT)
+ error (EXIT_FAILURE, err, _("accessing %s"), quote (file));
+ if (is_a_dir < looks_like_a_dir)
+ error (EXIT_FAILURE, err, _("target %s is not a directory"), quote (file));
+ *errp = err;
+ return is_a_dir;
+}
+
/* Scan the arguments, and copy each by calling copy.
Return 0 if successful, 1 if any errors occur. */
@@ -472,15 +497,13 @@ static int
do_copy (int n_files, char **file, const char *target_directory,
struct cp_options *x)
{
- const char *dest;
struct stat sb;
int new_dst = 0;
int ret = 0;
- int dest_is_dir = 0;
if (n_files <= !target_directory)
{
- if (n_files == 0)
+ if (n_files <= 0)
error (0, 0, _("missing file operand"));
else
error (0, 0, _("missing destination file operand after %s"),
@@ -488,66 +511,17 @@ do_copy (int n_files, char **file, const
usage (EXIT_FAILURE);
}
- if (target_directory)
- dest = target_directory;
- else
- {
- dest = file[n_files - 1];
- --n_files;
- }
-
- /* Initialize these hash tables only if we'll need them.
- The problems they're used to detect can arise only if
- there are two or more files to copy. */
- if (n_files >= 2)
+ if (!target_directory)
{
- dest_info_init (x);
- src_info_init (x);
- }
-
- if (lstat (dest, &sb))
- {
- if (errno != ENOENT)
- {
- error (0, errno, _("accessing %s"), quote (dest));
- return 1;
- }
-
- new_dst = 1;
- }
- else
- {
- struct stat sbx;
-
- /* If `dest' is not a symlink to a nonexistent file, use
- the results of stat instead of lstat, so we can copy files
- into symlinks to directories. */
- if (stat (dest, &sbx) == 0)
- sb = sbx;
-
- dest_is_dir = S_ISDIR (sb.st_mode);
+ if (2 <= n_files
+ && target_directory_operand (file[n_files - 1], &sb, &new_dst))
+ target_directory = file[--n_files];
+ else if (2 < n_files)
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (file[n_files - 1]));
}
- if (!dest_is_dir)
- {
- if (target_directory || 1 < n_files)
- {
- if (new_dst)
- error (0, 0, _("%s: destination directory does not exist"),
- quotearg_colon (dest));
- else if (target_directory)
- error (0, 0, _("%s: specified target is not a directory"),
- quotearg_colon (dest));
- else /* n_files > 1 */
- error (0, 0,
- _("copying multiple files, but last argument %s is not a directory"),
- quote (dest));
-
- usage (EXIT_FAILURE);
- }
- }
-
- if (dest_is_dir)
+ if (target_directory)
{
/* cp file1...filen edir
Copy the files `file1' through `filen'
@@ -558,6 +532,15 @@ do_copy (int n_files, char **file, const
? stat
: lstat);
+ /* Initialize these hash tables only if we'll need them.
+ The problems they're used to detect can arise only if
+ there are two or more files to copy. */
+ if (2 <= n_files)
+ {
+ dest_info_init (x);
+ src_info_init (x);
+ }
+
for (i = 0; i < n_files; i++)
{
char *dst_path;
@@ -584,7 +567,7 @@ do_copy (int n_files, char **file, const
strip_trailing_slashes (arg_no_trailing_slash);
/* Append all of `arg' (minus any trailing slash) to `dest'. */
- dst_path = path_concat (dest, arg_no_trailing_slash,
+ dst_path = path_concat (target_directory, arg_no_trailing_slash,
&arg_in_concat);
if (dst_path == NULL)
xalloc_die ();
@@ -603,13 +586,13 @@ do_copy (int n_files, char **file, const
else
{
char *arg_base;
- /* Append the last component of `arg' to `dest'. */
+ /* Append the last component of `arg' to `target_directory'. */
ASSIGN_BASENAME_STRDUPA (arg_base, arg);
/* For `cp -R source/.. dest', don't copy into `dest/..'. */
dst_path = (STREQ (arg_base, "..")
- ? xstrdup (dest)
- : path_concat (dest, arg_base, NULL));
+ ? xstrdup (target_directory)
+ : path_concat (target_directory, arg_base, NULL));
}
if (!parent_exists)
@@ -633,12 +616,12 @@ do_copy (int n_files, char **file, const
}
return ret;
}
- else /* if (n_files == 1) */
+ else /* !target_directory */
{
- char *new_dest;
- char *source;
+ char const *new_dest;
+ char const *source = file[0];
+ char const *dest = file[1];
int unused;
- struct stat source_stats;
if (flag_path)
{
@@ -647,8 +630,6 @@ do_copy (int n_files, char **file, const
usage (EXIT_FAILURE);
}
- source = file[0];
-
/* When the force and backup options have been specified and
the source and destination are the same name for an existing
regular file, convert the user's command, e.g.,
@@ -675,30 +656,12 @@ do_copy (int n_files, char **file, const
if (new_dest == NULL)
xalloc_die ();
}
-
- /* When the destination is specified with a trailing slash and the
- source exists but is not a directory, convert the user's command
- `cp source dest/' to `cp source dest/basename(source)'. Doing
- this ensures that the command `cp non-directory file/' will now
- fail rather than performing the copy. COPY diagnoses the case of
- `cp directory non-directory'. */
-
- else if (dest[strlen (dest) - 1] == '/'
- && lstat (source, &source_stats) == 0
- && !S_ISDIR (source_stats.st_mode))
- {
- char *source_base;
-
- ASSIGN_BASENAME_STRDUPA (source_base, source);
- new_dest = alloca (strlen (dest) + strlen (source_base) + 1);
- stpcpy (stpcpy (new_dest, dest), source_base);
- }
else
{
- new_dest = (char *) dest;
+ new_dest = dest;
}
- return copy (source, new_dest, new_dst, x, &unused, NULL);
+ return copy (source, new_dest, 0, x, &unused, NULL);
}
/* unreachable */
@@ -969,6 +932,18 @@ main (int argc, char **argv)
break;
case TARGET_DIRECTORY_OPTION:
+ if (target_directory)
+ error (EXIT_FAILURE, 0,
+ _("multiple target directories specified"));
+ else
+ {
+ struct stat st;
+ if (stat (optarg, &st) != 0)
+ error (EXIT_FAILURE, errno, _("accessing %s"), quote (optarg));
+ if (! S_ISDIR (st.st_mode))
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (optarg));
+ }
target_directory = optarg;
break;
Index: src/ln.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/ln.c,v
retrieving revision 1.136
diff -p -u -r1.136 ln.c
--- src/ln.c 21 Jun 2004 15:03:35 -0000 1.136
+++ src/ln.c 25 Jun 2004 06:33:06 -0000
@@ -87,8 +87,6 @@ int symlink ();
}
\
while (0)
-int isdir ();
-
/* The name by which the program was run, for error messages. */
char *program_name;
@@ -140,19 +138,40 @@ static struct option const long_options[
{NULL, 0, NULL, 0}
};
+/* FILE is the last operand of this command. Return true if FILE is a
+ directory. But report an error there is a problem accessing FILE,
+ or if FILE does not exist but would have to refer to an existing
+ directory if it referred to anything at all. */
+
+static bool
+target_directory_operand (char const *file)
+{
+ char const *b = base_name (file);
+ size_t blen = strlen (b);
+ bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
+ struct stat st;
+ int err = ((dereference_dest_dir_symlinks ? stat : lstat) (file, &st) == 0
+ ? 0 : errno);
+ bool is_a_dir = !err && S_ISDIR (st.st_mode);
+ if (err && err != ENOENT)
+ error (EXIT_FAILURE, err, _("accessing %s"), quote (file));
+ if (is_a_dir < looks_like_a_dir)
+ error (EXIT_FAILURE, err, _("target %s is not a directory"), quote (file));
+ return is_a_dir;
+}
+
/* Make a link DEST to the (usually) existing file SOURCE.
Symbolic links to nonexistent files are allowed.
- If DEST is a directory, put the link to SOURCE in that directory.
+ If DEST_IS_DIR, put the link to SOURCE in the DEST directory.
Return 1 if there is an error, otherwise 0. */
static int
-do_link (const char *source, const char *dest)
+do_link (const char *source, const char *dest, bool dest_is_dir)
{
struct stat source_stats;
struct stat dest_stats;
char *dest_backup = NULL;
- int lstat_status;
- int backup_succeeded = 0;
+ int lstat_status = -1;
/* Use stat here instead of lstat.
On SVR4, link does not follow symlinks, so this check disallows
@@ -182,33 +201,16 @@ do_link (const char *source, const char
}
}
- lstat_status = lstat (dest, &dest_stats);
- if (lstat_status != 0 && errno != ENOENT)
- {
- error (0, errno, _("accessing %s"), quote (dest));
- return 1;
- }
-
- /* If the destination is a directory or (it is a symlink to a directory
- and the user has not specified --no-dereference), then form the
- actual destination name by appending base_name (source) to the
- specified destination directory. */
- if ((lstat_status == 0
- && S_ISDIR (dest_stats.st_mode))
-#ifdef S_ISLNK
- || (dereference_dest_dir_symlinks
- && (lstat_status == 0
- && S_ISLNK (dest_stats.st_mode)
- && isdir (dest)))
-#endif
- )
+ if (dest_is_dir)
{
- /* Target is a directory; build the full filename. */
+ /* Treat DEST as a directory; build the full filename. */
char *new_dest;
PATH_BASENAME_CONCAT (new_dest, dest, source);
dest = new_dest;
+ }
- /* Get stats for new DEST. */
+ if (remove_existing_files || interactive || backup_type != none)
+ {
lstat_status = lstat (dest, &dest_stats);
if (lstat_status != 0 && errno != ENOENT)
{
@@ -243,7 +245,7 @@ do_link (const char *source, const char
return 1;
}
- if (lstat_status == 0 || lstat (dest, &dest_stats) == 0)
+ if (lstat_status == 0)
{
if (S_ISDIR (dest_stats.st_mode))
{
@@ -256,11 +258,6 @@ do_link (const char *source, const char
if (!yesno ())
return 0;
}
- else if (!remove_existing_files && backup_type == none)
- {
- error (0, 0, _("%s: File exists"), quote (dest));
- return 1;
- }
if (backup_type != none)
{
@@ -282,27 +279,8 @@ do_link (const char *source, const char
else
dest_backup = NULL;
}
- else
- {
- backup_succeeded = 1;
- }
- }
-
- /* Try to unlink DEST even if we may have renamed it. In some unusual
- cases (when DEST and DEST_BACKUP are hard-links that refer to the
- same file), rename succeeds and DEST remains. If we didn't remove
- DEST in that case, the subsequent LINKFUNC call would fail. */
- if (unlink (dest) && errno != ENOENT)
- {
- error (0, errno, _("cannot remove %s"), quote (dest));
- return 1;
}
}
- else if (errno != ENOENT)
- {
- error (0, errno, _("accessing %s"), quote (dest));
- return 1;
- }
if (verbose)
{
@@ -310,7 +288,7 @@ do_link (const char *source, const char
? _("create symbolic link %s to %s")
: _("create hard link %s to %s")),
quote_n (0, dest), quote_n (1, source));
- if (backup_succeeded)
+ if (dest_backup)
printf (_(" (backup: %s)"), quote (dest_backup));
putchar ('\n');
}
@@ -320,6 +298,36 @@ do_link (const char *source, const char
return 0;
}
+ /* If the attempt to create a link failed and we are removing or
+ backing up destinations, unlink the destination and try again.
+
+ POSIX 1003.1-2004 requires that ln -f A B must unlink B even on
+ failure (e.g., when A does not exist). This is counterintuitive,
+ and we submitted a defect report
+
<http://www.opengroup.org/sophocles/show_mail.tpl?source=L&listname=austin-review-l&id=1795>
+ (2004-06-24). If the committee does not fix the standard we'll
+ have to change the behavior of ln -f, at least if POSIXLY_CORRECT
+ is set. In the meantime ln -f A B will not unlink B unless the
+ attempt to link A to B failed because B already existed.
+
+ Try to unlink DEST even if we may have backed it up successfully.
+ In some unusual cases (when DEST and DEST_BACKUP are hard-links
+ that refer to the same file), rename succeeds and DEST remains.
+ If we didn't remove DEST in that case, the subsequent LINKFUNC
+ call would fail. */
+
+ if (errno == EEXIST && (remove_existing_files || dest_backup))
+ {
+ if (unlink (dest) != 0)
+ {
+ error (0, errno, _("cannot remove %s"), quote (dest));
+ return 1;
+ }
+
+ if (linkfunc (source, dest) == 0)
+ return 0;
+ }
+
error (0, errno,
(symbolic_link
? _("creating symbolic link %s to %s")
@@ -409,10 +417,8 @@ main (int argc, char **argv)
char *backup_suffix_string;
char *version_control_string = NULL;
char *target_directory = NULL;
- int target_directory_specified;
- unsigned int n_files;
+ int n_files;
char **file;
- int dest_is_dir;
initialize_main (&argc, &argv);
program_name = argv[0];
@@ -474,6 +480,17 @@ main (int argc, char **argv)
#endif
break;
case TARGET_DIRECTORY_OPTION:
+ if (target_directory)
+ error (EXIT_FAILURE, 0, _("multiple target directories specified"));
+ else
+ {
+ struct stat st;
+ if (stat (optarg, &st) != 0)
+ error (EXIT_FAILURE, errno, _("accessing %s"), quote (optarg));
+ if (! S_ISDIR (st.st_mode))
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (optarg));
+ }
target_directory = optarg;
break;
case 'v':
@@ -491,33 +508,24 @@ main (int argc, char **argv)
}
}
- if (argc <= optind)
+ n_files = argc - optind;
+ file = argv + optind;
+
+ if (n_files <= 0)
{
error (0, 0, _("missing file operand"));
usage (EXIT_FAILURE);
}
- n_files = argc - optind;
- file = argv + optind;
-
- target_directory_specified = (target_directory != NULL);
if (!target_directory)
- target_directory = file[n_files - 1];
-
- /* If target directory is not specified, and there's only one
- file argument, then pretend `.' was given as the second argument. */
- if (!target_directory_specified && n_files == 1)
- {
- static char *dummy[2];
- dummy[0] = file[0];
- dummy[1] = ".";
- file = dummy;
- n_files = 2;
- dest_is_dir = 1;
- }
- else
{
- dest_is_dir = isdir (target_directory);
+ if (n_files < 2)
+ target_directory = ".";
+ else if (2 <= n_files && target_directory_operand (file[n_files - 1]))
+ target_directory = file[--n_files];
+ else if (2 < n_files)
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (file[n_files - 1]));
}
if (symbolic_link)
@@ -525,13 +533,6 @@ main (int argc, char **argv)
else
linkfunc = link;
- if (target_directory_specified && !dest_is_dir)
- {
- error (0, 0, _("%s: specified target directory is not a directory"),
- quote (target_directory));
- usage (EXIT_FAILURE);
- }
-
if (backup_suffix_string)
simple_backup_suffix = xstrdup (backup_suffix_string);
@@ -539,46 +540,14 @@ main (int argc, char **argv)
? xget_version (_("backup type"), version_control_string)
: none);
- if (target_directory_specified || n_files > 2)
+ if (target_directory)
{
- unsigned int i;
- unsigned int last_file_idx = (target_directory_specified
- ? n_files - 1
- : n_files - 2);
-
- if (!target_directory_specified && !dest_is_dir)
- error (EXIT_FAILURE, 0,
- _("when making multiple links, last argument must be a directory"));
- for (i = 0; i <= last_file_idx; ++i)
- errors += do_link (file[i], target_directory);
+ int i;
+ for (i = 0; i < n_files; ++i)
+ errors |= do_link (file[i], target_directory, true);
}
else
- {
- struct stat source_stats;
- const char *source;
- char *dest;
- char *new_dest;
-
- source = file[0];
- dest = file[1];
-
- /* When the destination is specified with a trailing slash and the
- source exists but is not a directory, convert the user's command
- `ln source dest/' to `ln source dest/basename(source)'. */
-
- if (dest[strlen (dest) - 1] == '/'
- && lstat (source, &source_stats) == 0
- && !S_ISDIR (source_stats.st_mode))
- {
- PATH_BASENAME_CONCAT (new_dest, dest, source);
- }
- else
- {
- new_dest = dest;
- }
-
- errors = do_link (source, new_dest);
- }
+ errors = do_link (file[0], file[1], false);
exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
Index: src/mv.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/mv.c,v
retrieving revision 1.154
diff -p -u -r1.154 mv.c
--- src/mv.c 21 Jun 2004 15:03:35 -0000 1.154
+++ src/mv.c 25 Jun 2004 05:10:28 -0000
@@ -54,9 +54,6 @@ enum
REPLY_OPTION
};
-int isdir ();
-int lstat ();
-
/* The name this program was run with. */
char *program_name;
@@ -150,36 +147,38 @@ cp_option_init (struct cp_options *x)
x->src_info = NULL;
}
-/* If PATH is an existing directory, return nonzero, else 0. */
+/* FILE is the last operand of this command. Return true if FILE is a
+ directory. But report an error there is a problem accessing FILE,
+ or if FILE does not exist but would have to refer to an existing
+ directory if it referred to anything at all. */
-static int
-is_real_dir (const char *path)
+static bool
+target_directory_operand (char const *file)
{
- struct stat stats;
-
- return lstat (path, &stats) == 0 && S_ISDIR (stats.st_mode);
+ char const *b = base_name (file);
+ size_t blen = strlen (b);
+ bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
+ struct stat st;
+ int err = (stat (file, &st) == 0 ? 0 : errno);
+ bool is_a_dir = !err && S_ISDIR (st.st_mode);
+ if (err && err != ENOENT)
+ error (EXIT_FAILURE, err, _("accessing %s"), quote (file));
+ if (is_a_dir < looks_like_a_dir)
+ error (EXIT_FAILURE, err, _("target %s is not a directory"), quote (file));
+ return is_a_dir;
}
/* Move SOURCE onto DEST. Handles cross-filesystem moves.
If SOURCE is a directory, DEST must not exist.
- Return 0 if successful, non-zero if an error occurred. */
+ Return 0 if successful, 1 if an error occurred. */
static int
do_move (const char *source, const char *dest, const struct cp_options *x)
{
- static int first = 1;
int copy_into_self;
int rename_succeeded;
int fail;
- if (first)
- {
- first = 0;
-
- /* Allocate space for remembering copied and created files. */
- hash_init ();
- }
-
fail = copy (source, dest, 0, x, ©_into_self, &rename_succeeded);
if (!fail)
@@ -253,15 +252,13 @@ do_move (const char *source, const char
}
/* Move file SOURCE onto DEST. Handles the case when DEST is a directory.
- DEST_IS_DIR must be nonzero when DEST is a directory or a symlink to a
- directory and zero otherwise.
+ Treat DEST as a directory if DEST_IS_DIR.
Return 0 if successful, non-zero if an error occurred. */
static int
-movefile (char *source, char *dest, int dest_is_dir,
+movefile (char *source, char *dest, bool dest_is_dir,
const struct cp_options *x)
{
- int dest_had_trailing_slash = strip_trailing_slashes (dest);
int fail;
/* This code was introduced to handle the ambiguity in the semantics
@@ -271,19 +268,13 @@ movefile (char *source, char *dest, int
function that ignores a trailing slash. I believe the Linux
rename semantics are POSIX and susv2 compliant. */
+ strip_trailing_slashes (dest);
if (remove_trailing_slashes)
strip_trailing_slashes (source);
- /* In addition to when DEST is a directory, if DEST has a trailing
- slash and neither SOURCE nor DEST is a directory, presume the target
- is DEST/`basename source`. This converts `mv x y/' to `mv x y/x'.
- This change means that the command `mv any file/' will now fail
- rather than performing the move. The case when SOURCE is a
- directory and DEST is not is properly diagnosed by do_move. */
-
- if (dest_is_dir || (dest_had_trailing_slash && !is_real_dir (source)))
+ if (dest_is_dir)
{
- /* DEST is a directory; build full target filename. */
+ /* Treat DEST as a directory; build the full filename. */
char const *src_basename = base_name (source);
char *new_dest = path_concat (dest, src_basename, NULL);
if (new_dest == NULL)
@@ -369,13 +360,11 @@ main (int argc, char **argv)
int c;
int errors;
int make_backups = 0;
- int dest_is_dir;
char *backup_suffix_string;
char *version_control_string = NULL;
struct cp_options x;
char *target_directory = NULL;
- int target_directory_specified;
- unsigned int n_files;
+ int n_files;
char **file;
initialize_main (&argc, &argv);
@@ -427,6 +416,17 @@ main (int argc, char **argv)
remove_trailing_slashes = 1;
break;
case TARGET_DIRECTORY_OPTION:
+ if (target_directory)
+ error (EXIT_FAILURE, 0, _("multiple target directories specified"));
+ else
+ {
+ struct stat st;
+ if (stat (optarg, &st) != 0)
+ error (EXIT_FAILURE, errno, _("accessing %s"), quote (optarg));
+ if (! S_ISDIR (st.st_mode))
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (optarg));
+ }
target_directory = optarg;
break;
case 'u':
@@ -446,39 +446,26 @@ main (int argc, char **argv)
}
}
- n_files = (optind < argc ? argc - optind : 0);
+ n_files = argc - optind;
file = argv + optind;
- target_directory_specified = (target_directory != NULL);
- if (target_directory == NULL && n_files != 0)
- target_directory = file[n_files - 1];
-
- dest_is_dir = (n_files > 0 && isdir (target_directory));
-
- if (n_files == 0 || (n_files == 1 && !target_directory_specified))
+ if (n_files <= !target_directory)
{
- if (n_files == 0)
+ if (n_files <= 0)
error (0, 0, _("missing file operand"));
else
- error (0, 0, _("missing file operand after %s"),
- quote (argv[argc - 1]));
+ error (0, 0, _("missing destination file operand after %s"),
+ quote (file[0]));
usage (EXIT_FAILURE);
}
- if (target_directory_specified)
+ if (!target_directory)
{
- if (!dest_is_dir)
- {
- error (0, 0, _("specified target, %s is not a directory"),
- quote (target_directory));
- usage (EXIT_FAILURE);
- }
- }
- else if (n_files > 2 && !dest_is_dir)
- {
- error (0, 0,
- _("when moving multiple files, last argument must be a directory"));
- usage (EXIT_FAILURE);
+ if (2 <= n_files && target_directory_operand (file[n_files - 1]))
+ target_directory = file[--n_files];
+ else if (2 < n_files)
+ error (EXIT_FAILURE, 0, _("target %s is not a directory"),
+ quote (file[n_files - 1]));
}
if (backup_suffix_string)
@@ -489,22 +476,23 @@ main (int argc, char **argv)
version_control_string)
: none);
- /* Move each arg but the last into the target_directory. */
- {
- unsigned int last_file_idx = (target_directory_specified
- ? n_files - 1
- : n_files - 2);
- unsigned int i;
-
- /* Initialize the hash table only if we'll need it.
- The problem it is used to detect can arise only if there are
- two or more files to move. */
- if (last_file_idx)
- dest_info_init (&x);
-
- for (i = 0; i <= last_file_idx; ++i)
- errors |= movefile (file[i], target_directory, dest_is_dir, &x);
- }
+ hash_init ();
+
+ if (target_directory)
+ {
+ int i;
+
+ /* Initialize the hash table only if we'll need it.
+ The problem it is used to detect can arise only if there are
+ two or more files to move. */
+ if (2 <= n_files)
+ dest_info_init (&x);
+
+ for (i = 0; i < n_files; ++i)
+ errors |= movefile (file[i], target_directory, true, &x);
+ }
+ else
+ errors = movefile (file[0], file[1], false, &x);
exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
Index: tests/cp/fail-perm
===================================================================
RCS file: /home/meyering/coreutils/cu/tests/cp/fail-perm,v
retrieving revision 1.7
diff -p -u -r1.7 fail-perm
--- tests/cp/fail-perm 23 Jun 2004 15:07:04 -0000 1.7
+++ tests/cp/fail-perm 25 Jun 2004 06:11:17 -0000
@@ -16,7 +16,7 @@ framework_failure=0
mkdir -p $tmp || framework_failure=1
cd $tmp || framework_failure=1
-mkdir D || framework_failure=1
+mkdir D D/D || framework_failure=1
touch D/a || framework_failure=1
chmod 0 D/a || framework_failure=1
chmod 500 D || framework_failure=1
@@ -36,6 +36,20 @@ cp -pR D DD > /dev/null 2>&1 && fail=1
set X `ls -ld DD`
shift
test "$1" = dr-x------ || fail=1
+
+chmod 0 D
+ln -s D/D symlink
+touch F
+cat > exp <<\EOF
+cp: accessing `symlink': Permission denied
+EOF
+
+cp F symlink 2> out && fail=1
+cmp out exp || { (diff -c out exp) 2> /dev/null; fail=1; }
+
+cp --target-directory=symlink F 2> out && fail=1
+cmp out exp || { (diff -c out exp) 2> /dev/null; fail=1; }
+
chmod 700 D
(exit $fail); exit $fail
Index: tests/ln/misc
===================================================================
RCS file: /home/meyering/coreutils/cu/tests/ln/misc,v
retrieving revision 1.13
diff -p -u -r1.13 misc
--- tests/ln/misc 25 Dec 2000 18:31:18 -0000 1.13
+++ tests/ln/misc 23 Jun 2004 23:36:37 -0000
@@ -44,6 +44,14 @@ ln -s ../$f $d || fail=1
test -f $d/$f || fail=1
rm -rf $d $f
+# See whether a trailing slash is followed too far.
+touch $f || framework_failure=1
+rm -rf $d || framework_failure=1
+mkdir $d $d/$f || framework_failure=1
+ln $f $d/ 2> /dev/null && fail=1
+ln -s $f $d/ 2> /dev/null && fail=1
+rm -rf $d $f
+
# Make sure we get a failure with existing dest without -f option
touch $t || framework_failure=1
# FIXME: don't ignore the error message but rather test
Index: tests/mv/diag
===================================================================
RCS file: /home/meyering/coreutils/cu/tests/mv/diag,v
retrieving revision 1.6
diff -p -u -r1.6 diag
--- tests/mv/diag 21 Jun 2004 15:05:03 -0000 1.6
+++ tests/mv/diag 25 Jun 2004 07:23:22 -0000
@@ -31,7 +31,7 @@ fi
# Too few args. This first one did fail, but with an incorrect diagnostic
# until fileutils-4.0u.
-mv --target=d >> out 2>&1 && fail=1
+mv --target=. >> out 2>&1 && fail=1
mv no-file >> out 2>&1 && fail=1
# Target is not a directory.
@@ -41,12 +41,10 @@ mv --target=f2 f1 >> out 2>&1 && fail=1
cat > exp <<\EOF
mv: missing file operand
Try `mv --help' for more information.
-mv: missing file operand after `no-file'
-Try `mv --help' for more information.
-mv: when moving multiple files, last argument must be a directory
-Try `mv --help' for more information.
-mv: specified target, `f2' is not a directory
+mv: missing destination file operand after `no-file'
Try `mv --help' for more information.
+mv: target `f1' is not a directory
+mv: target `f2' is not a directory
EOF
cmp out exp || fail=1
- coreutils bug with "ln x d/",
Paul Eggert <=