[BusyBox 0001470]: cp says "cannot remove..." when trying to copy a file to a non-existent directory

bugs at busybox.net bugs at busybox.net
Wed Feb 13 16:52:18 UTC 2008


The following issue has been CLOSED 
====================================================================== 
http://busybox.net/bugs/view.php?id=1470 
====================================================================== 
Reported By:                kiltedknight
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   1470
Category:                   Other
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     closed
Resolution:                 reopened
Fixed in Version:           
====================================================================== 
Date Submitted:             08-23-2007 10:55 PDT
Last Modified:              02-13-2008 08:52 PST
====================================================================== 
Summary:                    cp says "cannot remove..." when trying to copy a
file to a non-existent directory
Description: 
Run this sequence of commands:

touch /tmp/foo
busybox cp /tmp/foo /tmp/nonexistent/path/foo

This is the error message you get:
cp: cannot remove '/tmp/nonexistent/path/foo': No such file or directory

What Linux's cp will say is:
cp: cannot create regular file `/tmp/nonexistent/path/foo': No such file
or directory

If you have DO_POSIX_CP enabled, you will get:
'/tmp/nonexistent/path/foo' exists

Unless you specify the -f or -i flag, in which case you get the "cannot
remove" message
====================================================================== 

---------------------------------------------------------------------- 
 vda - 08-24-07 07:22  
---------------------------------------------------------------------- 
> This is the error message you get:
> cp: cannot remove '/tmp/nonexistent/path/foo': No such file or
directory

Which is true. cp couldn't create it. cp decided to try unlink,
and creating again. unlink didn't work either. cp lets you know that.

> What Linux's cp will say is:
> cp: cannot create regular file `/tmp/nonexistent/path/foo': No such file
or directory

Read it again.

Do you really think that "I can't create 'foo', it doesn't exist" is
better?
>From naive point of view it is silly too: "of course it doesn't exist
before you create it!"

I undoubtedly can fix bbox cp to match GNU message, but code will be
bigger
and GNU message is still semi-stupid.

For the record: fix should be here:

static int ask_and_unlink(const char *dest, int flags)
{
        ...
        if (unlink(dest) < 0) {

===> if errno == ENOENT or ENOTDIR, explain that *path* is invalid, not
filename <===

                bb_perror_msg("cannot remove '%s'", dest);
                return -1; // error
        }
        return 1; // ok (to try again)
} 

---------------------------------------------------------------------- 
 kiltedknight - 08-24-07 08:58  
---------------------------------------------------------------------- 
I've read it again.  You don't seem to understand...

If the path in which you want to create the file does not exist, you
should never see EEXIST... that implies the file already exists! 

---------------------------------------------------------------------- 
 vda - 08-24-07 09:55  
---------------------------------------------------------------------- 
bbox cp says:

cp: cannot remove '/tmp/nonexistent/path/foo': No such file or directory

which sounds stupid

GNU coreutils cp says:

cp: cannot create regular file `/tmp/nonexistent/path/foo': No such file
or directory

which is also stupid.

Do you agree with me on the above?

If yes: then why we should change from one stupid message to another
stupid one?
If no: explain why do you think that coreutils message is better. 

---------------------------------------------------------------------- 
 kiltedknight - 08-24-07 10:05  
---------------------------------------------------------------------- 
It's far less stupid to say, "No such file or directory" when part or all
of the path does not exist than to say, "cannot remove..." because the
user is trying to create a file, not remove one.

Also, try compiling with DO_POSIX_CP enabled as well... instead, you get
the message that the file exists when you try to copy to a path that
doesn't exist.

In all cases, the best choice of error messages is "No such file or
directory," which will clue the user in on what is actually wrong. 

---------------------------------------------------------------------- 
 vda - 08-24-07 10:08  
---------------------------------------------------------------------- 
Re EEXIST:

        } else if (S_ISBLK(source_stat.st_mode) ||
S_ISCHR(source_stat.st_mode)
         || S_ISSOCK(source_stat.st_mode) ||
S_ISFIFO(source_stat.st_mode)
         || S_ISLNK(source_stat.st_mode)
        ) {
                // We are lazy here, a bit lax with races...
                if (dest_exists) {
==>                     errno = EEXIST;
                        ovr = ask_and_unlink(dest, flags);

It is added here purely for the sake of bb_perror_msg inside
ask_and_unlink in "#if DO_POSIX_CP" block to print correct error code. It
will happen if you try to cp a device node, fifo, or symlink over existing
file without -i and -f.

static int ask_and_unlink(const char *dest, int flags)
{
#if DO_POSIX_CP
        if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) {
                // Either it exists, or the *path* doesnt exist
==>             bb_perror_msg("cannot create '%s'", dest);
                return -1;
        }
#endif

It will say "cp: cannot create 'foo': File already exists". 

---------------------------------------------------------------------- 
 vda - 08-24-07 13:52  
---------------------------------------------------------------------- 
kiltedknight, please test attached patch. It gives good error messages for
both ENOENT and ENOTDIR:

$ ./busybox cp busybox /qwe/asd
cp: cannot create '/qwe/asd': Path does not exist
$ ./busybox cp busybox /vmlinuz/asd
cp: cannot stat '/vmlinuz/asd': Path has non-directory component

svn busybox:

$ ./busybox cp busybox /qwe/asd
cp: cannot remove '/qwe/asd': No such file or directory
$ ./busybox cp busybox /vmlinuz/asd
cp: cannot stat '/vmlinuz/asd': Not a directory

Cost:
ask_and_unlink                                        81     119     +38
cp_mv_stat2                                           79      99     +20 

---------------------------------------------------------------------- 
 kiltedknight - 08-24-07 14:30  
---------------------------------------------------------------------- 
Patch is working.  Thank you. :) 

---------------------------------------------------------------------- 
 vda - 02-13-08 08:52  
---------------------------------------------------------------------- 
Fixed in revision 21006. Thanks. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
08-23-07 10:55  kiltedknight   New Issue                                    
08-23-07 10:55  kiltedknight   Status                   new => assigned     
08-23-07 10:55  kiltedknight   Assigned To               => BusyBox         
08-23-07 10:55  kiltedknight   File Added: bb_cp_errmsg_fix.diff                
   
08-23-07 10:55  kiltedknight   Issue Monitored: kiltedknight                    
08-24-07 07:22  vda            Status                   assigned => closed  
08-24-07 07:22  vda            Note Added: 0002681                          
08-24-07 07:22  vda            Resolution               open => won't fix   
08-24-07 08:58  kiltedknight   Status                   closed => feedback  
08-24-07 08:58  kiltedknight   Resolution               won't fix => reopened
08-24-07 08:58  kiltedknight   Note Added: 0002682                          
08-24-07 09:55  vda            Note Added: 0002683                          
08-24-07 10:05  kiltedknight   Note Added: 0002684                          
08-24-07 10:08  vda            Note Added: 0002685                          
08-24-07 13:48  vda            File Added: 2.patch                          
08-24-07 13:52  vda            Note Added: 0002686                          
08-24-07 14:30  kiltedknight   Note Added: 0002687                          
02-13-08 08:52  vda            Status                   feedback => closed  
02-13-08 08:52  vda            Note Added: 0004444                          
======================================================================




More information about the busybox-cvs mailing list