bug#7450: cp does not check for errno=EISDIR when target is dangling link

[Jim Meyering]

Paul Eggert wrote:
> On 11/21/2010 01:49 AM, Jim Meyering wrote:
>>   ./cp: invalid destination: `no-such/'
>>   a destination with a trailing slash must refer to an existing directory
>
> In addition to the problem with mv that Alan mentions,
> this diagnostic is still confusing, at least for me.  Also, what's
> the relationship of this to the -t and -T options?  Currently we have this
> behavior when the destination exists:
>
>    $ cp a /usr/
>    cp: cannot create regular file `/usr/a': Permission denied
>    $ cp a -t /usr/
>    cp: cannot create regular file `/usr/a': Permission denied
>    $ cp -T a /usr/
>    cp: cannot overwrite directory `/usr/' with non-directory
>
> Wouldn't the change cause these diagnostics to be confusing if
> we replace /usr with /nosuch?

My patch did not change the diagnostic you'd get with -t or -T.

> How about this idea instead.  Simply replace the bogus errno value
> with a better one.  Then, we'd see this behavior:
>
>    $ cp a /nosuch/
>    cp: cannot create regular file `/nosuch/': Not a directory
>    $ cp a -t /nosuch/
>    cp: accessing `/nosuch/': No such file or directory
>    $ cp -T a /nosuch/
>    cp: cannot create regular file `/nosuch/': Not a directory
>
> These messages are perhaps not ideal, but they're good enough,
> and fit better into the existing message style.
> Here's a proposed patch to do that.  (By the way, the kernel is
> entirely within its rights to generate EISDIR: when there are
> multiple errors to report, the kernel can report any of the errno
> values.)
>
>>From 9d637cb3e052a529b5c7ec5e7ce70931436a3c45 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Sun, 21 Nov 2010 18:50:41 -0800
> Subject: [PATCH] cp: give a better diagnostic for nonexistent dest/
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This patch was written by Jim Meyering and myself.
> * src/copy.c (copy_reg): Turn EISDIR to ENOTDIR to improve the
> quality of diagnostics for commands like "cp a nosuch/".  Reported
> by 0@: >@address@hidden and Alan Curry in the thread starting at:
> http://lists.gnu.org/archive/html/bug-coreutils/2010-11/msg00178.html
> * THANKS: Update.
> * tests/mv/trailing-slash: Add a test.
> ---
...
> +
> +      /* Improve quality of diagnostic when a nonexistent dst_name
> +         ends in a slash and open fails with errno == EISDIR.  */
> +      if (dest_desc < 0 && dest_errno == EISDIR
> +          && *dst_name && dst_name[strlen (dst_name) - 1] == '/')
> +        dest_errno = ENOTDIR;

Yes, I prefer your patch.
Merely adjusting errno (and not adding a new, specialized diagnostic)
makes this small and noninvasive enough that patching copy_reg
is better than changing the caller in cp.c.

Thanks!